[PATCH v2 0/8] drm/rockchip: No more post-atomic_check fixups

I'm taking over this series to get it across the finish line. Original
cover letter from Daniel Stone on v1:

Hi,
This series is a pretty small and consistent one for VOP2. The atomic
uAPI very clearly specifies that drivers should either do what userspace
requested (on a successful commit), or fail atomic_check if it is not
for any reason possible to do what userspace requested.

VOP2 is unfortunately littered with a bunch of cases where it will apply
fixups after atomic_check - doing something different to what userspace
requested, e.g. clipping or aligning regions - or throw error messages
into the log when userspace does request a condition which can't be met.

Doing something different to what was requested is bad because it
results in unexpected visual output which can look like artifacts.
Throwing errors into the log is bad because generic userspace will
reasonably attempt to try any configuration it can. For example,
throwing an error message on a plane not being aligned to a 16 pixel
boundary can result in 15 frames' worth of error output in the log when
a window is being animated across a screen.

This series removes all post-check fixups - failing the check if the
configuration cannot be applied - and also demotes all messages about
unsupported configurations to DEBUG_KMS.

Cheers,
Daniel

Changes in v2:
- Dropped patches [1, 5] as they were already applied.
- Changed the patch subject to use prefix "drm/rockchip: vop2:" for the
  remaining ones.
- Fixed a checkpatch nag about commenting style in "Switch impossible
  pos conditional to WARN_ON".
- Reworded "eSmart" to "Esmart" for consistency, and to avoid drawing
  Tim Apple's ire.
- Make the hopefully impossible WARN_ON format conditional in
  vop2_plane_atomic_check still bubble the error up to userspace,
  instead of continuing on.
- Use dest_w instead of dsp_w in patch "Enforce scaling workaround
  in plane_check", to avoid a compiler error.
- Only reject non-multiple-of-4-pixel-wide framebuffers on
  RK3566/RK3568, as the other SoCs have no such limitation. (Thank you
  to Andy Yan for doing the research to confirm this!)
- Consequently also only WARN_ON if this condition is violated in
  atomic_update on those SoCs.
- Link to v1: https://lore.kernel.org/dri-devel/20251015110042.41273-1-daniels@collabora.com/

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
Daniel Stone (8):
      drm/rockchip: vop2: Switch impossible format conditional to WARN_ON
      drm/rockchip: vop2: Switch impossible pos conditional to WARN_ON
      drm/rockchip: vop2: Fix Esmart test condition
      drm/rockchip: vop2: Enforce scaling workaround in plane_check
      drm/rockchip: vop2: Enforce AFBC source alignment in plane_check
      drm/rockchip: vop2: Enforce AFBC transform stride align in plane_check
      drm/rockchip: vop2: Use drm_is_afbc helper function
      drm/rockchip: vop2: Simplify format_mod_supported

 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 137 ++++++++++++---------------
 1 file changed, 62 insertions(+), 75 deletions(-)
---
base-commit: 4e5a9b630580faea139e9837b4fba666db6bd728
change-id: 20251206-vop2-atomic-fixups-0c30e0980f85

Best regards,
-- 
Nicolas Frattaroli [off-list ref]

From: Daniel Stone <redacted>

We should never be able to create a framebuffer with an unsupported
format, so throw a warning if this ever happens, instead of attempting
to stagger on.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 498df0ce4680..20b49209ddcd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -1030,7 +1030,8 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
 		return 0;
 
 	format = vop2_convert_format(fb->format->format);
-	if (format < 0)
+	/* We shouldn't be able to create a fb for an unsupported format */
+	if (WARN_ON(format < 0))
 		return format;
 
 	/* Co-ordinates have now been clipped */

-- 
2.52.0

From: Daniel Stone <redacted>

We already clip the plane to the display bounds in atomic_check, and
ensure that it is sufficiently sized. Instead of trying to catch this
and adjust for it in atomic_update, just assert that atomic_check has
done its job.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 29 +++++++++-------------------
 1 file changed, 9 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 20b49209ddcd..81b3eba07095 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -1214,28 +1214,17 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 	src_w = drm_rect_width(src) >> 16;
 	src_h = drm_rect_height(src) >> 16;
 	dsp_w = drm_rect_width(dest);
-
-	if (dest->x1 + dsp_w > adjusted_mode->hdisplay) {
-		drm_dbg_kms(vop2->drm,
-			    "vp%d %s dest->x1[%d] + dsp_w[%d] exceed mode hdisplay[%d]\n",
-			    vp->id, win->data->name, dest->x1, dsp_w, adjusted_mode->hdisplay);
-		dsp_w = adjusted_mode->hdisplay - dest->x1;
-		if (dsp_w < 4)
-			dsp_w = 4;
-		src_w = dsp_w * src_w / drm_rect_width(dest);
-	}
-
 	dsp_h = drm_rect_height(dest);
 
-	if (dest->y1 + dsp_h > adjusted_mode->vdisplay) {
-		drm_dbg_kms(vop2->drm,
-			    "vp%d %s dest->y1[%d] + dsp_h[%d] exceed mode vdisplay[%d]\n",
-			    vp->id, win->data->name, dest->y1, dsp_h, adjusted_mode->vdisplay);
-		dsp_h = adjusted_mode->vdisplay - dest->y1;
-		if (dsp_h < 4)
-			dsp_h = 4;
-		src_h = dsp_h * src_h / drm_rect_height(dest);
-	}
+	/* drm_atomic_helper_check_plane_state calls drm_rect_clip_scaled for
+	 * us, which keeps our planes bounded within the CRTC active area
+	 */
+	WARN_ON(dest->x1 + dsp_w > adjusted_mode->hdisplay);
+	WARN_ON(dest->y1 + dsp_h > adjusted_mode->vdisplay);
+	WARN_ON(dsp_w < 4);
+	WARN_ON(dsp_h < 4);
+	WARN_ON(src_w < 4);
+	WARN_ON(src_h < 4);
 
 	/*
 	 * This is workaround solution for IC design:

-- 
2.52.0

From: Daniel Stone <redacted>

It seems only cluster windows are capable of applying downscaling when
the source region has an odd width. Instead of applying a workaround
inside atomic_update, fail the plane check if this is requested.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 21 +++++++++++----------
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 9d715d7659af..bc1ed0ffede0 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -998,6 +998,7 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
 	struct drm_crtc *crtc = pstate->crtc;
 	struct drm_crtc_state *cstate;
 	struct vop2_video_port *vp;
+	struct vop2_win *win = to_vop2_win(plane);
 	struct vop2 *vop2;
 	const struct vop2_data *vop2_data;
 	struct drm_rect *dest = &pstate->dst;

@@ -1065,6 +1066,16 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	/*
+	 * This is workaround solution for IC design:
+	 * esmart can't support scale down when src_w % 16 == 1.
+	 */
+	if (!vop2_cluster_window(win) && src_w > dest_w && (src_w & 1)) {
+		drm_dbg_kms(vop2->drm,
+			    "eSmart windows cannot downscale odd-width source regions\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 

@@ -1226,16 +1237,6 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 	WARN_ON(src_w < 4);
 	WARN_ON(src_h < 4);
 
-	/*
-	 * This is workaround solution for IC design:
-	 * esmart can't support scale down when src_w % 16 == 1.
-	 */
-	if (!vop2_cluster_window(win) && src_w > dsp_w && (src_w & 1)) {
-		drm_dbg_kms(vop2->drm, "vp%d %s act_w[%d] MODE 16 == 1\n",
-			    vp->id, win->data->name, src_w);
-		src_w -= 1;
-	}
-
 	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);

-- 
2.52.0

From: Daniel Stone <redacted>

Make sure we can't break the hardware by requesting an unsupported
configuration.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index e23213337104..2a308cc31632 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -1083,6 +1083,15 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
 		return -EINVAL;
 	}
 
+	if (drm_is_afbc(fb->modifier) &&
+	    pstate->rotation &
+		(DRM_MODE_REFLECT_X | DRM_MODE_ROTATE_90 | DRM_MODE_ROTATE_270) &&
+	    (fb->pitches[0] << 3) / vop2_get_bpp(fb->format) % 64) {
+		drm_dbg_kms(vop2->drm,
+			    "AFBC buffers must be 64-byte aligned for horizontal rotation or mirroring\n");
+		return -EINVAL;
+	}
+
 	return 0;
 }
 

@@ -1290,9 +1299,6 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 		 * with WIN_VIR_STRIDE.
 		 */
 		stride = (fb->pitches[0] << 3) / bpp;
-		if ((stride & 0x3f) && (xmirror || rotate_90 || rotate_270))
-			drm_dbg_kms(vop2->drm, "vp%d %s stride[%d] not 64 pixel aligned\n",
-				    vp->id, win->data->name, stride);
 
 		 /* It's for head stride, each head size is 16 byte */
 		stride = ALIGN(stride, block_w) / block_w * 16;

-- 
2.52.0

From: Daniel Stone <redacted>

If we want to find out if a window is Esmart or not, test for not being
a cluster window, rather than AFBDC presence.

No functional effect as only cluster windows support AFBC decode.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 10 ++++------
 1 file changed, 4 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 81b3eba07095..9d715d7659af 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -1230,12 +1230,10 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 	 * This is workaround solution for IC design:
 	 * esmart can't support scale down when src_w % 16 == 1.
 	 */
-	if (!(win->data->feature & WIN_FEATURE_AFBDC)) {
-		if (src_w > dsp_w && (src_w & 0xf) == 1) {
-			drm_dbg_kms(vop2->drm, "vp%d %s act_w[%d] MODE 16 == 1\n",
-				    vp->id, win->data->name, src_w);
-			src_w -= 1;
-		}
+	if (!vop2_cluster_window(win) && src_w > dsp_w && (src_w & 1)) {
+		drm_dbg_kms(vop2->drm, "vp%d %s act_w[%d] MODE 16 == 1\n",
+			    vp->id, win->data->name, src_w);
+		src_w -= 1;
 	}
 
 	if (afbc_en && src_w % 4) {

-- 
2.52.0

From: Daniel Stone <redacted>

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

From: Daniel Stone <redacted>

We don't need to do a long open-coded walk here; we can simply check the
modifier value.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 2a308cc31632..8e0727c3523f 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -1207,7 +1207,7 @@ static void vop2_plane_atomic_update(struct drm_plane *plane,
 		return;
 	}
 
-	afbc_en = rockchip_afbc(plane, fb->modifier);
+	afbc_en = drm_is_afbc(fb->modifier);
 
 	offset = (src->x1 >> 16) * fb->format->cpp[0];
 

-- 
2.52.0

From: Daniel Stone <redacted>

Make it a little less convoluted, and just directly check if the
combination of plane + format + modifier is supported.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 56 +++++++++++-----------------
 1 file changed, 22 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 8e0727c3523f..10b59487019c 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -367,59 +367,47 @@ static bool is_yuv_output(u32 bus_format)
 	}
 }
 
-static bool rockchip_afbc(struct drm_plane *plane, u64 modifier)
-{
-	int i;
-
-	if (modifier == DRM_FORMAT_MOD_LINEAR)
-		return false;
-
-	for (i = 0 ; i < plane->modifier_count; i++)
-		if (plane->modifiers[i] == modifier)
-			return true;
-
-	return false;
-}
-
 static bool rockchip_vop2_mod_supported(struct drm_plane *plane, u32 format,
 					u64 modifier)
 {
 	struct vop2_win *win = to_vop2_win(plane);
 	struct vop2 *vop2 = win->vop2;
+	int i;
 
+	/* No support for implicit modifiers */
 	if (modifier == DRM_FORMAT_MOD_INVALID)
 		return false;
 
-	if (vop2->version == VOP_VERSION_RK3568) {
-		if (vop2_cluster_window(win)) {
-			if (modifier == DRM_FORMAT_MOD_LINEAR) {
-				drm_dbg_kms(vop2->drm,
-					    "Cluster window only supports format with afbc\n");
-				return false;
-			}
-		}
+	/* The cluster window on 3568 is AFBC-only */
+	if (vop2->version == VOP_VERSION_RK3568 && vop2_cluster_window(win) &&
+	    !drm_is_afbc(modifier)) {
+		drm_dbg_kms(vop2->drm,
+			    "Cluster window only supports format with afbc\n");
+		return false;
 	}
 
-	if (format == DRM_FORMAT_XRGB2101010 || format == DRM_FORMAT_XBGR2101010) {
-		if (vop2->version == VOP_VERSION_RK3588) {
-			if (!rockchip_afbc(plane, modifier)) {
-				drm_dbg_kms(vop2->drm, "Only support 32 bpp format with afbc\n");
-				return false;
-			}
-		}
+	/* 10bpc formats on 3588 are AFBC-only */
+	if (vop2->version == VOP_VERSION_RK3588 && !drm_is_afbc(modifier) &&
+	    (format == DRM_FORMAT_XRGB2101010 || format == DRM_FORMAT_XBGR2101010)) {
+		drm_dbg_kms(vop2->drm, "Only support 10bpc format with afbc\n");
+		return false;
 	}
 
+	/* Linear is otherwise supported everywhere */
 	if (modifier == DRM_FORMAT_MOD_LINEAR)
 		return true;
 
-	if (!rockchip_afbc(plane, modifier)) {
-		drm_dbg_kms(vop2->drm, "Unsupported format modifier 0x%llx\n",
-			    modifier);
-
+	/* Not all format+modifier combinations are allowable */
+	if (vop2_convert_afbc_format(format) == VOP2_AFBC_FMT_INVALID)
 		return false;
+
+	/* Different windows have different format/modifier support */
+	for (i = 0; i < plane->modifier_count; i++) {
+		if (plane->modifiers[i] == modifier)
+			return true;
 	}
 
-	return vop2_convert_afbc_format(format) >= 0;
+	return false;
 }
 
 /*

-- 
2.52.0

Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:

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

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:

quoted

From: Daniel Stone <redacted>

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

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:

quoted

Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:

quoted

From: Daniel Stone <redacted>

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

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:

quoted

On Monday, 8 December 2025 03:48:24 Central European Standard Time Chaoyi Chen wrote:

quoted

Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:

quoted

From: Daniel Stone <redacted>

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

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:

quoted

Hi Chaoyi Chen, Andy Yan,

On Monday, 8 December 2025 08:24:52 Central European Standard Time Nicolas Frattaroli wrote:

quoted

On Monday, 8 December 2025 03:48:24 Central European Standard Time Chaoyi Chen wrote:

quoted

Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:

quoted

From: Daniel Stone <redacted>

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

quoted

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]

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:

quoted

Hello Nicolas,

On 12/9/2025 6:58 PM, Nicolas Frattaroli wrote:

quoted

Hi Chaoyi Chen, Andy Yan,

On Monday, 8 December 2025 08:24:52 Central European Standard Time Nicolas Frattaroli wrote:

quoted

On Monday, 8 December 2025 03:48:24 Central European Standard Time Chaoyi Chen wrote:

quoted

Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:

quoted

From: Daniel Stone <redacted>

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

quoted

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

quoted

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

Hi all,

On Tue, 9 Dec 2025 at 10:58, Nicolas Frattaroli
[off-list ref] wrote:

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.

That's exactly it. Changing userspace's request may result in
unpleasant visual artifacts and other unwanted effects. If userspace
wants to always hit a fast path, then it will need some kind of
hardware awareness to do something different here. The patch series
pointed out gives userspace a good way to figure this out.

With my Weston maintainer hat on, I'd take a patch to
weston-simple-egl to allow it to use a different size with
command-line arguments if you'd like that for easier testing. (Fun
fact: it was specifically made 250x250 to discover issues such as
this, which wouldn't be uncovered by something that's aligned to a
generous power of two.)

Cheers,
Daniel

Hi，
At 2025-12-12 17:59:34, "Chaoyi Chen" [off-list ref] wrote:

Hi Nicolas,

On 12/11/2025 10:16 PM, Nicolas Frattaroli wrote:

quoted

On Thursday, 11 December 2025 12:06:38 Central European Standard Time Chaoyi Chen wrote:

quoted

Hello Nicolas,

On 12/9/2025 6:58 PM, Nicolas Frattaroli wrote:

quoted

Hi Chaoyi Chen, Andy Yan,

On Monday, 8 December 2025 08:24:52 Central European Standard Time Nicolas Frattaroli wrote:

quoted

On Monday, 8 December 2025 03:48:24 Central European Standard Time Chaoyi Chen wrote:

quoted

Hello Nicolas, Daniel,

On 12/7/2025 4:45 AM, Nicolas Frattaroli wrote:

quoted

From: Daniel Stone <redacted>

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 <redacted>
[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].

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

quoted

quoted

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

quoted

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

Hi

Am 06.12.25 um 21:45 schrieb Nicolas Frattaroli:

Please use drm_WARN_ON() here.

Best regards
Thomas

  		return format;
  
  	/* Co-ordinates have now been clipped */

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Hi

Am 06.12.25 um 21:45 schrieb Nicolas Frattaroli:

This should also use drm_WARN_ON().  But maybe rather leave it out entirely? I don't remember having seen other drivers to re-test in atomic_update.

Best regards
Thomas

  
  	/*
  	 * This is workaround solution for IC design:

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

Hi

Am 06.12.25 um 21:45 schrieb Nicolas Frattaroli:

Drivers should usually be using drm_dbg(), drm_err(), drm_warn() etc. Print functions with _kms postfix are for the DRM helpers. Here and in all other patches.

Best regards
Thomas

-- 
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstr. 146, 90461 Nürnberg, Germany, www.suse.com
GF: Jochen Jaser, Andrew McDonald, Werner Knoblich, (HRB 36809, AG Nürnberg)

On Tuesday, 20 January 2026 13:49:12 Central European Standard Time Thomas Zimmermann wrote:

Hi

Am 06.12.25 um 21:45 schrieb Nicolas Frattaroli:

quoted

From: Daniel Stone <redacted>

We should never be able to create a framebuffer with an unsupported
format, so throw a warning if this ever happens, instead of attempting
to stagger on.

Signed-off-by: Daniel Stone <redacted>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
  drivers/gpu/drm/rockchip/rockchip_drm_vop2.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
index 498df0ce4680..20b49209ddcd 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop2.c

@@ -1030,7 +1030,8 @@ static int vop2_plane_atomic_check(struct drm_plane *plane,
  		return 0;
  
  	format = vop2_convert_format(fb->format->format);
-	if (format < 0)
+	/* We shouldn't be able to create a fb for an unsupported format */
+	if (WARN_ON(format < 0))

Please use drm_WARN_ON() here.

Hi Thomas,


A later revision of this series has already been applied. Feel free
to send such a patch however.


Kind regards,
Nicolas Frattaroli

Best regards
Thomas

quoted

  		return format;
  
  	/* Co-ordinates have now been clipped */