Message-ID: <fdd333ac-0542-4312-8ec0-22fded3b1ce0@rock-chips.com>
Date: Thu, 11 Dec 2025 19:06:38 +0800
From: Chaoyi Chen <chaoyi.chen@...k-chips.com>
To: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>,
 Sandy Huang <hjc@...k-chips.com>, Heiko Stübner
 <heiko@...ech.de>, Andy Yan <andy.yan@...k-chips.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: kernel@...labora.com, dri-devel@...ts.freedesktop.org,
 linux-arm-kernel@...ts.infradead.org, linux-rockchip@...ts.infradead.org,
 linux-kernel@...r.kernel.org, Daniel Stone <daniels@...labora.com>
Subject: Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment
 in plane_check

Hello Nicolas,

On 12/9/2025 6:58 PM, Nicolas Frattaroli wrote:
> Hi Chaoyi Chen, Andy Yan,
> 
> On Monday, 8 December 2025 08:24:52 Central European Standard Time Nicolas Frattaroli wrote:
>> On Monday, 8 December 2025 03:48:24 Central European Standard Time Chaoyi Chen wrote:
>>> Hello Nicolas, Daniel,
>>>
>>> On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:
>>>> From: Daniel Stone <daniels@...labora.com>
>>>>
>>>> Planes can only source AFBC framebuffers at multiples of 4px wide on
>>>> RK3566/RK3568. Instead of clipping on all SoCs when the user asks for an
>>>> unaligned source rectangle, reject the configuration in the plane's
>>>> atomic check on RK3566/RK3568 only.
>>>>
>>>> Signed-off-by: Daniel Stone <daniels@...labora.com>
>>>> [Make RK3566/RK3568 specific, reword message]
>>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@...labora.com>
>>>> ---
>>>>  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 14 +++++++++-----
>>>>  1 file changed, 9 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> index bc1ed0ffede0..e23213337104 100644
>>>> --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
>>>> @@ -1076,6 +1076,13 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
>>>>  		return -EINVAL;
>>>>  	}
>>>>  
>>>> +	if (vop2->version == VOP_VERSION_RK3568 && drm_is_afbc(fb->modifier) && src_w % 4) {
>>>> +		drm_dbg_kms(vop2->drm,
>>>> +			    "AFBC source rectangles must be 4-byte aligned; is %d\n",
>>>> +			    src_w);
>>>> +		return -EINVAL;
>>>> +	}
>>>> +
>>>>  	return 0;
>>>>  }
>>>>  
>>>> @@ -1237,11 +1244,8 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
>>>>  	WARN_ON(src_w < 4);
>>>>  	WARN_ON(src_h < 4);
>>>>  
>>>> -	if (afbc_en && src_w % 4) {
>>>> -		drm_dbg_kms(vop2->drm, "vp%d %s src_w[%d] not 4 pixel aligned\n",
>>>> -			    vp->id, win->data->name, src_w);
>>>> -		src_w = ALIGN_DOWN(src_w, 4);
>>>> -	}
>>>> +	if (vop2->version == VOP_VERSION_RK3568 && drm_is_afbc(fb->modifier))
>>>> +		WARN_ON(src_w % 4);
>>>>  
>>>>  	act_info = (src_h - 1) << 16 | ((src_w - 1) & 0xffff);
>>>>  	dsp_info = (dsp_h - 1) << 16 | ((dsp_w - 1) & 0xffff);
>>>>
>>>
>>> You haven't replied to Andy's comment yet [0].
>>>
>>> [0] https://lore.kernel.org/dri-devel/7b4e26ec.75f3.19a77276b53.Coremail.andyshrk@163.com/
>>>
>>
>> Hello,
>>
>> I addressed the follow-ups where it was clarified that the 4 pixel
>> limitation was RK3566/RK3568-only. I'm not going to bring back the
>> post-atomic_check modification for a fast path, but I'm open to
>> suggestions on how to do this differently.
>>
>> One solution might be to modify the state with the ALIGN_DOWN stuff
>> in atomic_check instead, where userspace is then aware of the change
>> being done to its requested parameters. I'll need to double-check
>> whether this is in line with atomic modesetting's design.
>>
>> Kind regards,
>> Nicolas Frattaroli
> 
> Okay, so I've asked internally, and atomic_check isn't allowed to
> modify any of the parameters either. There's efforts [0] underway
> to allow error codes to be more specific, so that userspace knows
> which constraint is being violated. That would allow userspace
> applications to react by either adjusting their size or turning
> off AFBC in this case. Turning off AFBC seems more generally
> applicable here, since it means it won't need to resize the plane
> and it'll save more than enough memory bandwidth by not going
> through the GPU.
> 
> On that note: Andy, I didn't find a weston-simple-egl test in the
> Weston 14.0.2 or git test suite, and weston-simple-egl itself does
> not tell me whether GPU compositing is being used or not. Do you
> have more information on how to test for this? I'd like to know
> for when we have the necessary functionality in place to make
> userspace smart enough to pick the fast path again.
> 

I think weston-simple-egl is part of the weston client. When you build
weston from source, you should obtain it. Just run `weston-simple-egl` 
after compile and install weston.

And I guess you're using Debian... The weston package there also ships
with a weston-simple-egl binary [2].

[1]: https://gitlab.freedesktop.org/wayland/weston/-/blob/main/clients/simple-egl.c
[2]: https://packages.debian.org/sid/arm64/weston/filelist

> In either case, I think adhering to the atomic API to ensure
> artifact-free presentation is more important here than enabling
> a fast-path on RK3568. I do think in most real-world use case
> scenarios, the fallback won't degrade user experience, because
> almost everything performance intensive I can think of (video
> playback, games) will likely already use a plane geometry
> where the width is divisible by 4. 800, 1024, 1280, 1600, 1920,
> 2560, 3840 are all divisible by 4, so a window or full-screen
> playback of common content won't need to fall back to GPU
> compositing.
> 
> I'll send a v2 to fix another instance of "eSmart" left in a
> message, but beyond that I think we should be good.
> 
> Kind regards,
> Nicolas Frattaroli
> 
> https://lore.kernel.org/dri-devel/20251009-atomic-v6-0-d209709cc3ba@intel.com/ [0]
> 
> 
> 
> 

-- 
Best, 
Chaoyi

Powered by blists - more mailing lists