1. Patchew
  2. linux
  3. [PATCH v2 0/8] drm/rockchip: No more post-atomic_check fixups
  4. View patch

[PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check

Nicolas Frattaroli posted 8 patches 1 week, 5 days ago
There is a newer version of this series
[PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Nicolas Frattaroli 1 week, 5 days ago 
From: Daniel Stone <daniels@collabora.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@collabora.com>
[Make RK3566/RK3568 specific, reword message]
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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);

-- 
2.52.0
Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Chaoyi Chen 1 week, 4 days ago 
Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:
> From: Daniel Stone <daniels@collabora.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@collabora.com>
> [Make RK3566/RK3568 specific, reword message]
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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/

-- 
Best, 
Chaoyi
Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Nicolas Frattaroli 1 week, 4 days ago 
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@collabora.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@collabora.com>
> > [Make RK3566/RK3568 specific, reword message]
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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
Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Nicolas Frattaroli 1 week, 3 days ago 
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@collabora.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@collabora.com>
> > > [Make RK3566/RK3568 specific, reword message]
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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.

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]
Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Chaoyi Chen 1 week, 1 day ago 
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@collabora.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@collabora.com>
>>>> [Make RK3566/RK3568 specific, reword message]
>>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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
Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Nicolas Frattaroli 1 week ago 
On Thursday, 11 December 2025 12:06:38 Central European Standard Time Chaoyi Chen wrote:
> 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@collabora.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@collabora.com>
> >>>> [Make RK3566/RK3568 specific, reword message]
> >>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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].

Yeah, I know there's a tool called that, but I'm specifically curious
about how to determine whether it's using GPU compositing or what I
presume is fixed-function compositing.

When I enable some more logging with

  weston -l log,drm-backend,gl-renderer

and also some kms debug messages with

  echo 4 > /sys/module/drm/parameters/debug

then I see weston outputting

[atomic] drmModeAtomicCommit
        [repaint] Using mixed state composition
        [repaint] view 0xaaab18c00c10 using renderer composition
        [repaint] view 0xaaab18b68f00 using renderer composition
        [repaint] view 0xaaab18c00ec0 using renderer composition

regardless of whether the size is 250x250 or fullscreen. With
250x250, I know we're failing the plane check, because I see

  [  776.160101] rockchip-drm display-subsystem: [drm:vop2_plane_atomic_check
                 [rockchipdrm]] AFBC source rectangles must be 4-pixel
                 aligned; is 250

on the console, but with fullscreen I don't see any errors from plane-check
as the src_w is now divisible by 4, yet it's also "using renderer composition"
for all views.

Same goes for using `weston-simple-dmabuf-egl` (which is 256x256) instead of
the fullscreen simple-egl.

So basically, I need to know where a change in behaviour is actually
observed.

> 
> [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]
> > 
> > 
> > 
> > 
> 
>
Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
Posted by Chaoyi Chen 1 week ago 
Hi Nicolas,

On 12/11/2025 10:16 PM, Nicolas Frattaroli wrote:
> On Thursday, 11 December 2025 12:06:38 Central European Standard Time Chaoyi Chen wrote:
>> 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@collabora.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@collabora.com>
>>>>>> [Make RK3566/RK3568 specific, reword message]
>>>>>> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.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].
> 
> Yeah, I know there's a tool called that, but I'm specifically curious
> about how to determine whether it's using GPU compositing or what I
> presume is fixed-function compositing.
> 
> When I enable some more logging with
> 
>   weston -l log,drm-backend,gl-renderer
> 
> and also some kms debug messages with
> 
>   echo 4 > /sys/module/drm/parameters/debug
> 
> then I see weston outputting
> 
> [atomic] drmModeAtomicCommit
>         [repaint] Using mixed state composition
>         [repaint] view 0xaaab18c00c10 using renderer composition
>         [repaint] view 0xaaab18b68f00 using renderer composition
>         [repaint] view 0xaaab18c00ec0 using renderer composition
> 
> regardless of whether the size is 250x250 or fullscreen. With
> 250x250, I know we're failing the plane check, because I see
> 
>   [  776.160101] rockchip-drm display-subsystem: [drm:vop2_plane_atomic_check
>                  [rockchipdrm]] AFBC source rectangles must be 4-pixel
>                  aligned; is 250
> 
> on the console, but with fullscreen I don't see any errors from plane-check
> as the src_w is now divisible by 4, yet it's also "using renderer composition"
> for all views.
> 
> Same goes for using `weston-simple-dmabuf-egl` (which is 256x256) instead of
> the fullscreen simple-egl.
> 
> So basically, I need to know where a change in behaviour is actually
> observed.
> 

Hmm, I don't think weston logs make this obvious. We also need the drm kms logs.

Test results may vary across different platforms. But it's certain that 
fullscreen and non-fullscreen modes are identical in the following respects:

- fullscreen: Try to use only one plane (may be AFBC) .
- non fullscreen: Try to use 2 plane (may be AFBC) .

- On the RK3588, we won't get two planes without modifying the code, because
the primary plane is already assign to AFBC background plane.
- On the RK356x, we'll get a primary plane in linear format, while
weston-simple-egl will use an overlay plane with AFBC format.
- On the RK3576, we should be able to obtain an AFBC primary plane and an AFBC
overlay plane.

Try to set env `export WESTON_LIBINPUT_LOG_PRIORITY=debug` and you will see
these log in non fullscreen mode:

```
Layer 5 (pos 0x50000000):
        View 0 (role xdg_toplevel, PID 686, surface ID 14, top-level window 'simple-egl' of org.freedesktop.weston.simple-egl, 0xaaaae61a62d0):

...

                        [view] evaluating view 0xaaaae61a62d0 for plane assignment on output HDMI-A-1 (0)
                        [plane] started with zpos 18446744073709551615
                                [primary] not assigning view 0xaaaae61a62d0 on primary, plane 33 (format ARGB8888 (0x34325241) with modifier 0x800000000000051) not supported
                                [primary] not assigning view 0xaaaae61a62d0 on primary, plane 39 (format ARGB8888 (0x34325241) with modifier 0x800000000000051) not supported
                                [overlay] not assigning view 0xaaaae61a62d0 on overlay, plane 63 (format ARGB8888 (0x34325241) with modifier 0x800000000000051) not supported
                                [overlay] not assigning view 0xaaaae61a62d0 on overlay, plane 69 (format ARGB8888 (0x34325241) with modifier 0x800000000000051) not supported
                                [overlay] not assigning view 0xaaaae61a62d0 on overlay, plane 75 (format ARGB8888 (0x34325241) with modifier 0x800000000000051) not supported
                        [view] view 0xaaaae61a62d0 format: ARGB8888
                                [plane] plane 51 picked from candidate list, type: overlay
```

When vop2_plane_atomic_check() fails, in addition to the above logs, you will
also get the following log:

```
[view] not placing view 0xaaaad9ec8e40 on plane 51: atomic test failed
```

I don't think there's any more log information that can indicate the error here.
The relevant code is here [3].

[3]: https://gitlab.freedesktop.org/wayland/weston/-/blob/main/libweston/backend-drm/state-propose.c?ref_type=heads#L181

>>
>> [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

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.