Message-ID: <2426fd62.9ac1.19bdb71e718.Coremail.andyshrk@163.com>
Date: Tue, 20 Jan 2026 20:47:13 +0800 (CST)
From: "Andy Yan" <andyshrk@....com>
To: "Chaoyi Chen" <chaoyi.chen@...k-chips.com>
Cc: "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>, 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:Re: [PATCH v2 5/8] drm/rockchip: vop2: Enforce AFBC source
 alignment in plane_check



Hi，
At 2025-12-12 17:59:34, "Chaoyi Chen" <chaoyi.chen@...k-chips.com> wrote:
>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@...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].
>> 
>> 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].

>


For VOP2, we have a debug node to show the plane usage:
# cat /sys/kernel/debug/dri/0/vop2/summary 
Video Port0: DISABLED
Video Port1: DISABLED
Video Port2: ACTIVE
    Connector: DP-1
        bus_format[100a]: RGB888_1X24
        output_mode[f] color_space[0]
        background color (10bpc): r=0x0 g=0x0 b=0x0
    Display mode: 1920x1080p60
        clk[148500] real_clk[148500] type[48] flag[5]
        H: 1920 2008 2052 2200
        V: 1080 1084 1089 1125
    Cluster2-win0: ACTIVE
        win_id: 2
        format: XR24 little-endian (0x34325258)[AFBC] glb_alpha[0xff]
        rotate: xmirror: 0 ymirror: 0 rotate_90: 0 rotate_270: 0
        zpos: 0
        src: pos[0, 0] rect[1920 x 1080]
        dst: pos[0, 0] rect[1920 x 1080]
        buf[0]: addr: 0x0000000000000000 pitch: 7680 offset: 0
    Cluster3-win0: ACTIVE
        win_id: 3
        format: XR24 little-endian (0x34325258)[AFBC] glb_alpha[0xff]
        rotate: xmirror: 0 ymirror: 0 rotate_90: 0 rotate_270: 0
        zpos: 1
        src: pos[0, 0] rect[800 x 600]
        dst: pos[743, 358] rect[800 x 600]
        buf[0]: addr: 0x0000000001420000 pitch: 3200 offset: 0
# >[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

Powered by blists - more mailing lists