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 <[email protected]> >>>> >>>> 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 <[email protected]> >>>> [Make RK3566/RK3568 specific, reword message] >>>> Signed-off-by: Nicolas Frattaroli <[email protected]> >>>> --- >>>> 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/[email protected]/ >>> >> >> 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/[email protected]/ > [0] > > > > -- Best, Chaoyi
