[PATCH RFC 0/6] AFBC fixes for MediaTek DRM

[PATCH RFC 0/6] AFBC fixes for MediaTek DRM

[Nícolas F. R. A. Prado]

This series contains a handful of fixes for AFBC support on the MediaTek
DRM driver so that it can be re-enabled.

This is sent as an RFC because there are still some issues to work out
before the series can be merged:

1. Patch 4, 'drm/mediatek: ovl: Disallow AFBC buffers with width over
   1920' did not behave well when tested with Weston, so a better
   solution probably needs to be implemented before this can be merged.

2. Remaining AFBC issues:
   
   a. The first 4 pixel rows are always skipped in the displayed output,
      that is, the first displayed pixel, on the top-left corner,
      corresponds to 4x0. And below the end of the displayed output, the
      first 4x32 pixels are displayed.

   b. On some resolutions, there are still artifacts that look like
      misalignment issues, eg 1024x1080, 1080x1080.

   c. On some resolutions, no output at all is displayed, eg 1920x1080.

Tested on the MT8195-Tomato Chromebook.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
Ariel D'Alessandro (1):
      drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

Nícolas F. R. A. Prado (5):
      drm/mediatek: plane: Remove extra block from AFBC data payload offset
      drm/mediatek: plane: Correct AFBC alignment definition to 128
      drm/mediatek: ovl: Disallow AFBC buffers with width over 1920
      drm/mediatek: ovl: Disable AFBC on MT8188
      drm/mediatek: Re-enable AFBC support on MediaTek DRM driver

 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 59 ++++++++++++++++++++++++++++++---
 drivers/gpu/drm/mediatek/mtk_plane.c    | 48 +++++++++++++++++++++++++--
 drivers/gpu/drm/mediatek/mtk_plane.h    |  6 +++-
 3 files changed, 104 insertions(+), 9 deletions(-)
---
base-commit: 6f47c4646bee47319cc0980c693ed695c4cfd395
change-id: 20251229-mtk-afbc-fixes-b656f92bc2af

Best regards,
-- 
Nícolas F. R. A. Prado <nfraprado@collabora.com>

[PATCH RFC 1/6] drm/mediatek: plane: Remove extra block from AFBC data payload offset

[Nícolas F. R. A. Prado]

The AFBC data payload is in fact not offset by 1 additional block as the
code and comment suggest, and this causes the buffer to be rendered
offset by one block. Remove this extraneous offset to get the buffer
correctly displayed.

Fixes: c410fa9b07c3 ("drm/mediatek: Add AFBC support to Mediatek DRM driver")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_plane.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
index 5043e0377270..1214f623859e 100644
--- a/drivers/gpu/drm/mediatek/mtk_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_plane.c
@@ -164,10 +164,9 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
 		 */
 		hdr_addr = addr + hdr_offset;
 
-		/* The data plane is offset by 1 additional block. */
 		offset = pitch * y_offset_in_blocks +
 			 AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT *
-			 fb->format->cpp[0] * (x_offset_in_blocks + 1);
+			 fb->format->cpp[0] * x_offset_in_blocks;
 
 		/*
 		 * Using dma_addr_t variable to calculate with multiplier of different types,

-- 
2.51.0

[PATCH RFC 2/6] drm/mediatek: plane: Correct AFBC alignment definition to 128

[Nícolas F. R. A. Prado]

The minimum alignment for both the header and data buffers in the AFBC
format for Mali GPUs with archicture version 6 and higher (which
includes MT8195's G57 (v9)) is 128, not 1024 as the MediaTek DRM driver
currently defines.

Since Mesa defines it as the correct value of 128 [1], when displaying
AFBC buffers, some resolutions will cause the OVL component to be
configured by the driver with a data address that is different from the
address that actually contains the data as set by Mesa, resulting in
corrupted output on display.

Fix the AFBC alignment definition for the MediaTek DRM driver.

[1] https://gitlab.freedesktop.org/mesa/mesa/-/blob/3848a080534a17ca075e9e95dd3a14abb80139aa/src/panfrost/lib/pan_afbc.h#L364

Fixes: c410fa9b07c3 ("drm/mediatek: Add AFBC support to Mediatek DRM driver")
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_plane.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_plane.h b/drivers/gpu/drm/mediatek/mtk_plane.h
index 95c5fa5295d8..46be4454bc92 100644
--- a/drivers/gpu/drm/mediatek/mtk_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_plane.h
@@ -13,7 +13,7 @@
 #define AFBC_DATA_BLOCK_WIDTH 32
 #define AFBC_DATA_BLOCK_HEIGHT 8
 #define AFBC_HEADER_BLOCK_SIZE 16
-#define AFBC_HEADER_ALIGNMENT 1024
+#define AFBC_HEADER_ALIGNMENT 128
 
 struct mtk_plane_pending_state {
 	bool				config;

-- 
2.51.0

[PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[Nícolas F. R. A. Prado]

From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>

In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this limitation
of the AFBC format, OVL_CLIP needs to be used to achieve the desired
output size of the layer while still meeting the alignment constraints.
Failure to do this will result in vblank timeouts and no rendered output
when the AFBC data source isn't aligned to the AFBC block (32x8).

Configure OVL_CLIP so unaligned AFBC layers can be displayed.

The following illustrates how the alignment is achieved through the clip
settings for the horizontal coordinates, the vertical coordinates are
analogous:

/------------------------------------------------\
|                                                |
|            ........................            |
|            ........................            |
|            ........................            |
|            ........................            |
|                                                |
\------------------------------------------------/
     |       |                      |       |
     |       src.x1                 src.x2  |
     |       |                      |       |
     |       |<-------------------->|       |
     |              src_width               |
     |                                      |
     N * AFBC_DATA_BLOCK_WIDTH              M * AFBC_DATA_BLOCK_WIDTH
     |                                      |
     |<----->|                      |<----->|
      clip_left                      clip_right

Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
Co-developed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 35 ++++++++++++++++++++++++++++-----
 drivers/gpu/drm/mediatek/mtk_plane.c    | 21 ++++++++++++++++++++
 drivers/gpu/drm/mediatek/mtk_plane.h    |  4 ++++
 3 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 8e20b45411fc..c6a00c2256dd 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -39,6 +39,11 @@
 #define OVL_PITCH_MSB_2ND_SUBBUF			BIT(16)
 #define DISP_REG_OVL_PITCH(n)			(0x0044 + 0x20 * (n))
 #define OVL_CONST_BLEND					BIT(28)
+#define DISP_REG_OVL_CLIP(n)			(0x004C + 0x20 * (n))
+#define OVL_CLIP_LEFT					GENMASK(7, 0)
+#define OVL_CLIP_RIGHT					GENMASK(15, 8)
+#define OVL_CLIP_TOP					GENMASK(23, 16)
+#define OVL_CLIP_BOTTOM					GENMASK(31, 24)
 #define DISP_REG_OVL_RDMA_CTRL(n)		(0x00c0 + 0x20 * (n))
 #define DISP_REG_OVL_RDMA_GMC(n)		(0x00c8 + 0x20 * (n))
 #define DISP_REG_OVL_ADDR_MT2701		0x0040
@@ -499,13 +504,14 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 	struct mtk_plane_pending_state *pending = &state->pending;
 	unsigned int addr = pending->addr;
 	unsigned int pitch_lsb = pending->pitch & GENMASK(15, 0);
+	unsigned long long modifier = pending->modifier;
 	unsigned int fmt = pending->format;
 	unsigned int rotation = pending->rotation;
 	unsigned int offset = (pending->y << 16) | pending->x;
-	unsigned int src_size = (pending->height << 16) | pending->width;
 	unsigned int blend_mode = state->base.pixel_blend_mode;
 	unsigned int ignore_pixel_alpha = 0;
-	unsigned int con;
+	unsigned int src_size, con, src_width, src_height;
+	unsigned int clip = 0;
 
 	if (!pending->enable) {
 		mtk_ovl_layer_off(dev, idx, cmdq_pkt);
@@ -550,9 +556,26 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 		addr += pending->pitch - 1;
 	}
 
-	if (ovl->data->supports_afbc)
-		mtk_ovl_set_afbc(ovl, cmdq_pkt, idx,
-				 pending->modifier != DRM_FORMAT_MOD_LINEAR);
+	if (ovl->data->supports_afbc && (modifier != DRM_FORMAT_MOD_LINEAR)) {
+		/*
+		 * In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this
+		 * limitation of the AFBC format, OVL_CLIP is used to adjust the
+		 * output size of the layer.
+		 */
+		clip = FIELD_PREP(OVL_CLIP_BOTTOM, pending->clip_bottom) |
+		       FIELD_PREP(OVL_CLIP_TOP, pending->clip_top) |
+		       FIELD_PREP(OVL_CLIP_RIGHT, pending->clip_right) |
+		       FIELD_PREP(OVL_CLIP_LEFT, pending->clip_left);
+		src_height = pending->height + pending->clip_top + pending->clip_bottom;
+		src_width = pending->width + pending->clip_left + pending->clip_right;
+		mtk_ovl_set_afbc(ovl, cmdq_pkt, idx, true);
+	} else {
+		src_height = pending->height;
+		src_width = pending->width;
+		mtk_ovl_set_afbc(ovl, cmdq_pkt, idx, false);
+	}
+
+	src_size = (src_height << 16) | src_width;
 
 	mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
 			      DISP_REG_OVL_CON(idx));
@@ -560,6 +583,8 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
 			      &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH(idx));
 	mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs,
 			      DISP_REG_OVL_SRC_SIZE(idx));
+	mtk_ddp_write_relaxed(cmdq_pkt, clip, &ovl->cmdq_reg, ovl->regs,
+			      DISP_REG_OVL_CLIP(idx));
 	mtk_ddp_write_relaxed(cmdq_pkt, offset, &ovl->cmdq_reg, ovl->regs,
 			      DISP_REG_OVL_OFFSET(idx));
 	mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg, ovl->regs,
diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
index 1214f623859e..8fb08768e8ce 100644
--- a/drivers/gpu/drm/mediatek/mtk_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_plane.c
@@ -114,6 +114,7 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
 				       struct mtk_plane_state *mtk_plane_state)
 {
 	struct drm_framebuffer *fb = new_state->fb;
+	unsigned int clip_left = 0, clip_top = 0, clip_right = 0, clip_bottom = 0;
 	struct drm_gem_object *gem;
 	struct mtk_gem_obj *mtk_gem;
 	unsigned int pitch, format;
@@ -148,6 +149,22 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
 		int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH;
 		int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT;
 		int hdr_size, hdr_offset;
+		int src_width = drm_rect_width(&new_state->src) >> 16;
+		int src_height = drm_rect_height(&new_state->src) >> 16;
+		unsigned int remainder_right, remainder_bottom;
+
+		/*
+		 * In AFBC mode, the source size configured needs to be a
+		 * multiple of the AFBC data block size. Compute and save the
+		 * necessary clips so the indeded x, y, width and height are
+		 * obtained in the output despite this constraint.
+		 */
+		clip_left = (new_state->src.x1 >> 16) % AFBC_DATA_BLOCK_WIDTH;
+		clip_top = (new_state->src.y1 >> 16) % AFBC_DATA_BLOCK_HEIGHT;
+		remainder_right = (src_width + clip_left) % AFBC_DATA_BLOCK_WIDTH;
+		clip_right = remainder_right ? AFBC_DATA_BLOCK_WIDTH - remainder_right : 0;
+		remainder_bottom = (src_height + clip_top) % AFBC_DATA_BLOCK_HEIGHT;
+		clip_bottom = remainder_bottom ? AFBC_DATA_BLOCK_HEIGHT - remainder_bottom : 0;
 
 		hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE;
 		pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH *
@@ -187,6 +204,10 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
 	mtk_plane_state->pending.y = new_state->dst.y1;
 	mtk_plane_state->pending.width = drm_rect_width(&new_state->dst);
 	mtk_plane_state->pending.height = drm_rect_height(&new_state->dst);
+	mtk_plane_state->pending.clip_left = clip_left;
+	mtk_plane_state->pending.clip_top = clip_top;
+	mtk_plane_state->pending.clip_right = clip_right;
+	mtk_plane_state->pending.clip_bottom = clip_bottom;
 	mtk_plane_state->pending.rotation = new_state->rotation;
 	mtk_plane_state->pending.color_encoding = new_state->color_encoding;
 }
diff --git a/drivers/gpu/drm/mediatek/mtk_plane.h b/drivers/gpu/drm/mediatek/mtk_plane.h
index 46be4454bc92..a9cfb2ee5859 100644
--- a/drivers/gpu/drm/mediatek/mtk_plane.h
+++ b/drivers/gpu/drm/mediatek/mtk_plane.h
@@ -28,6 +28,10 @@ struct mtk_plane_pending_state {
 	unsigned int			y;
 	unsigned int			width;
 	unsigned int			height;
+	unsigned int			clip_left;
+	unsigned int			clip_top;
+	unsigned int			clip_right;
+	unsigned int			clip_bottom;
 	unsigned int			rotation;
 	bool				dirty;
 	bool				async_dirty;

-- 
2.51.0

[PATCH RFC 4/6] drm/mediatek: ovl: Disallow AFBC buffers with width over 1920

[Nícolas F. R. A. Prado]

AFBC buffers with width over 1920 are not supported by OVL. If
attempted, the image displayed contains many artifacts.

Add this restriction to the layer check callback so such configurations
are not allowed.

NOTE: This doesn't seem to be a good way to handle this restriction, as
when tested with Weston, it simply fails to render, rather than fallback
to not using the modifier:

[19:09:03.857] atomic: couldn't commit new state: Invalid argument
[19:09:03.857] repaint-flush failed: Invalid argument

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index c6a00c2256dd..196b874057ba 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -368,6 +368,13 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
 	if (state->fb->format->is_yuv && (state->rotation & ~DRM_MODE_ROTATE_0))
 		return -EINVAL;
 
+	/*
+	 * AFBC buffers with width > 1920 are not supported and produce
+	 * artifacts, so should be disabled.
+	 */
+	if (state->fb->modifier != DRM_FORMAT_MOD_LINEAR && state->fb->width > 1920)
+		return -EINVAL;
+
 	return 0;
 }
 

-- 
2.51.0

[PATCH RFC 5/6] drm/mediatek: ovl: Disable AFBC on MT8188

[Nícolas F. R. A. Prado]

Despite MT8188's OVL being mostly the same IP as the OVL on MT8195, it
does not support AFBC, even when the same register configurations are
applied. Introduce a separate compatible for it with AFBC support
disabled.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
index 196b874057ba..97f6694772d4 100644
--- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
+++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
@@ -762,6 +762,21 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
 	.num_formats = ARRAY_SIZE(mt8173_formats),
 };
 
+static const struct mtk_disp_ovl_data mt8188_ovl_driver_data = {
+	.addr = DISP_REG_OVL_ADDR_MT8173,
+	.gmc_bits = 10,
+	.layer_nr = 4,
+	.fmt_rgb565_is_0 = true,
+	.smi_id_en = true,
+	.supports_afbc = false,
+	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
+		       BIT(DRM_MODE_BLEND_COVERAGE) |
+		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
+	.formats = mt8195_formats,
+	.num_formats = ARRAY_SIZE(mt8195_formats),
+	.supports_clrfmt_ext = true,
+};
+
 static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
 	.addr = DISP_REG_OVL_ADDR_MT8173,
 	.gmc_bits = 10,
@@ -790,6 +805,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
 	  .data = &mt8192_ovl_driver_data},
 	{ .compatible = "mediatek,mt8192-disp-ovl-2l",
 	  .data = &mt8192_ovl_2l_driver_data},
+	{ .compatible = "mediatek,mt8188-disp-ovl",
+	  .data = &mt8188_ovl_driver_data},
 	{ .compatible = "mediatek,mt8195-disp-ovl",
 	  .data = &mt8195_ovl_driver_data},
 	{},

-- 
2.51.0

[PATCH RFC 6/6] drm/mediatek: Re-enable AFBC support on MediaTek DRM driver

[Nícolas F. R. A. Prado]

Commit 9882a4064003 ("drm/mediatek: Disable AFBC support on Mediatek DRM
driver") disabled AFBC support on the MediaTek DRM driver since it was
broken.

With the bugs in the AFBC support now fixed, re-enable the support.

Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
---
 drivers/gpu/drm/mediatek/mtk_plane.c | 24 +++++++++++++++++++++++-
 1 file changed, 23 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
index 8fb08768e8ce..38dd94cdd665 100644
--- a/drivers/gpu/drm/mediatek/mtk_plane.c
+++ b/drivers/gpu/drm/mediatek/mtk_plane.c
@@ -22,6 +22,9 @@
 
 static const u64 modifiers[] = {
 	DRM_FORMAT_MOD_LINEAR,
+	DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
+				AFBC_FORMAT_MOD_SPLIT |
+				AFBC_FORMAT_MOD_SPARSE),
 	DRM_FORMAT_MOD_INVALID,
 };
 
@@ -69,7 +72,26 @@ static bool mtk_plane_format_mod_supported(struct drm_plane *plane,
 					   uint32_t format,
 					   uint64_t modifier)
 {
-	return modifier == DRM_FORMAT_MOD_LINEAR;
+	if (modifier == DRM_FORMAT_MOD_LINEAR)
+		return true;
+
+	if (modifier != DRM_FORMAT_MOD_ARM_AFBC(
+				AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
+				AFBC_FORMAT_MOD_SPLIT |
+				AFBC_FORMAT_MOD_SPARSE))
+		return false;
+
+	if (format != DRM_FORMAT_XRGB8888 &&
+	    format != DRM_FORMAT_ARGB8888 &&
+	    format != DRM_FORMAT_BGRX8888 &&
+	    format != DRM_FORMAT_BGRA8888 &&
+	    format != DRM_FORMAT_ABGR8888 &&
+	    format != DRM_FORMAT_XBGR8888 &&
+	    format != DRM_FORMAT_RGB888 &&
+	    format != DRM_FORMAT_BGR888)
+		return false;
+
+	return true;
 }
 
 static void mtk_plane_destroy_state(struct drm_plane *plane,

-- 
2.51.0

Re: [PATCH RFC 0/6] AFBC fixes for MediaTek DRM

[Nícolas F. R. A. Prado]

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> This series contains a handful of fixes for AFBC support on the
> MediaTek
> DRM driver so that it can be re-enabled.
> 
> This is sent as an RFC because there are still some issues to work
> out
> before the series can be merged:
> 
> 1. Patch 4, 'drm/mediatek: ovl: Disallow AFBC buffers with width over
>    1920' did not behave well when tested with Weston, so a better
>    solution probably needs to be implemented before this can be
> merged.
> 
> 2. Remaining AFBC issues:
>    
>    a. The first 4 pixel rows are always skipped in the displayed
> output,
>       that is, the first displayed pixel, on the top-left corner,
>       corresponds to 4x0. And below the end of the displayed output,
> the
>       first 4x32 pixels are displayed.
> 
>    b. On some resolutions, there are still artifacts that look like
>       misalignment issues, eg 1024x1080, 1080x1080.
> 
>    c. On some resolutions, no output at all is displayed, eg
> 1920x1080.

I received a suggestion that the Lx_2ND_SUBBUF bit in OVL is not needed
when cropping is not enabled, and that removing it should fix the extra
pixels after the image described in 2.a., and perhaps even fix AFBC not
working with MT8188, though I haven't had time to test it.


-- 
Thanks,

Nícolas

Re: [PATCH RFC 1/6] drm/mediatek: plane: Remove extra block from AFBC data payload offset

[CK Hu (胡俊光)]

Hi, Nicolas:

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> The AFBC data payload is in fact not offset by 1 additional block as the
> code and comment suggest, and this causes the buffer to be rendered
> offset by one block. Remove this extraneous offset to get the buffer
> correctly displayed.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Fixes: c410fa9b07c3 ("drm/mediatek: Add AFBC support to Mediatek DRM driver")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_plane.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
> index 5043e0377270..1214f623859e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.c
> @@ -164,10 +164,9 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
>  		 */
>  		hdr_addr = addr + hdr_offset;
>  
> -		/* The data plane is offset by 1 additional block. */
>  		offset = pitch * y_offset_in_blocks +
>  			 AFBC_DATA_BLOCK_WIDTH * AFBC_DATA_BLOCK_HEIGHT *
> -			 fb->format->cpp[0] * (x_offset_in_blocks + 1);
> +			 fb->format->cpp[0] * x_offset_in_blocks;
>  
>  		/*
>  		 * Using dma_addr_t variable to calculate with multiplier of different types,
> 

Re: [PATCH RFC 2/6] drm/mediatek: plane: Correct AFBC alignment definition to 128

[CK Hu (胡俊光)]

Hi, Nicolas:

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> The minimum alignment for both the header and data buffers in the AFBC
> format for Mali GPUs with archicture version 6 and higher (which
> includes MT8195's G57 (v9)) is 128, not 1024 as the MediaTek DRM driver
> currently defines.
> 
> Since Mesa defines it as the correct value of 128 [1], when displaying
> AFBC buffers, some resolutions will cause the OVL component to be
> configured by the driver with a data address that is different from the
> address that actually contains the data as set by Mesa, resulting in
> corrupted output on display.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Fix the AFBC alignment definition for the MediaTek DRM driver.
> 
> [1] https://urldefense.com/v3/__https://gitlab.freedesktop.org/mesa/mesa/-/blob/3848a080534a17ca075e9e95dd3a14abb80139aa/src/panfrost/lib/pan_afbc.h*L364__;Iw!!CTRNKA9wMg0ARbw!hx6VI6pbINcUORdlV1iTi7-tiAXUyQPRPDAvNjq5eaBKlSc8JSe-zQe7WdnWO-GSgLZ__9Kxb6VHhAL1Fxg$ 
> 
> Fixes: c410fa9b07c3 ("drm/mediatek: Add AFBC support to Mediatek DRM driver")
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_plane.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.h b/drivers/gpu/drm/mediatek/mtk_plane.h
> index 95c5fa5295d8..46be4454bc92 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.h
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.h
> @@ -13,7 +13,7 @@
>  #define AFBC_DATA_BLOCK_WIDTH 32
>  #define AFBC_DATA_BLOCK_HEIGHT 8
>  #define AFBC_HEADER_BLOCK_SIZE 16
> -#define AFBC_HEADER_ALIGNMENT 1024
> +#define AFBC_HEADER_ALIGNMENT 128
>  
>  struct mtk_plane_pending_state {
>  	bool				config;
> 

Re: [PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[CK Hu (胡俊光)]

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> 
> In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this limitation
> of the AFBC format, OVL_CLIP needs to be used to achieve the desired
> output size of the layer while still meeting the alignment constraints.
> Failure to do this will result in vblank timeouts and no rendered output
> when the AFBC data source isn't aligned to the AFBC block (32x8).
> 
> Configure OVL_CLIP so unaligned AFBC layers can be displayed.
> 
> The following illustrates how the alignment is achieved through the clip
> settings for the horizontal coordinates, the vertical coordinates are
> analogous:
> 
> /------------------------------------------------\
> >                                                |
> >            ........................            |
> >            ........................            |
> >            ........................            |
> >            ........................            |
> >                                                |
> \------------------------------------------------/
>      |       |                      |       |
>      |       src.x1                 src.x2  |
>      |       |                      |       |
>      |       |<-------------------->|       |
>      |              src_width               |

This patch looks to me.
But "In AFBC mode, OVL_SRC_SIZE must be block aligned", so this graph should show as:

     |       src.x1                 src.x2  |
     |       |                      |       |
     |       |                      |       |
     N * AFBC_DATA_BLOCK_WIDTH      |       M * AFBC_DATA_BLOCK_WIDTH
     |       |                      |       |
     |<----->|                      |<----->|
     |clip_left                      clip_right
     |                                      |
     |<------------------------------------>|
     |              src_width               |

Regards,
CK

>      |                                      |
>      N * AFBC_DATA_BLOCK_WIDTH              M * AFBC_DATA_BLOCK_WIDTH
>      |                                      |
>      |<----->|                      |<----->|
>       clip_left                      clip_right
> 
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> Co-developed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 35 ++++++++++++++++++++++++++++-----
>  drivers/gpu/drm/mediatek/mtk_plane.c    | 21 ++++++++++++++++++++
>  drivers/gpu/drm/mediatek/mtk_plane.h    |  4 ++++
>  3 files changed, 55 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 8e20b45411fc..c6a00c2256dd 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -39,6 +39,11 @@
>  #define OVL_PITCH_MSB_2ND_SUBBUF			BIT(16)
>  #define DISP_REG_OVL_PITCH(n)			(0x0044 + 0x20 * (n))
>  #define OVL_CONST_BLEND					BIT(28)
> +#define DISP_REG_OVL_CLIP(n)			(0x004C + 0x20 * (n))
> +#define OVL_CLIP_LEFT					GENMASK(7, 0)
> +#define OVL_CLIP_RIGHT					GENMASK(15, 8)
> +#define OVL_CLIP_TOP					GENMASK(23, 16)
> +#define OVL_CLIP_BOTTOM					GENMASK(31, 24)
>  #define DISP_REG_OVL_RDMA_CTRL(n)		(0x00c0 + 0x20 * (n))
>  #define DISP_REG_OVL_RDMA_GMC(n)		(0x00c8 + 0x20 * (n))
>  #define DISP_REG_OVL_ADDR_MT2701		0x0040
> @@ -499,13 +504,14 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  	struct mtk_plane_pending_state *pending = &state->pending;
>  	unsigned int addr = pending->addr;
>  	unsigned int pitch_lsb = pending->pitch & GENMASK(15, 0);
> +	unsigned long long modifier = pending->modifier;
>  	unsigned int fmt = pending->format;
>  	unsigned int rotation = pending->rotation;
>  	unsigned int offset = (pending->y << 16) | pending->x;
> -	unsigned int src_size = (pending->height << 16) | pending->width;
>  	unsigned int blend_mode = state->base.pixel_blend_mode;
>  	unsigned int ignore_pixel_alpha = 0;
> -	unsigned int con;
> +	unsigned int src_size, con, src_width, src_height;
> +	unsigned int clip = 0;
>  
>  	if (!pending->enable) {
>  		mtk_ovl_layer_off(dev, idx, cmdq_pkt);
> @@ -550,9 +556,26 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  		addr += pending->pitch - 1;
>  	}
>  
> -	if (ovl->data->supports_afbc)
> -		mtk_ovl_set_afbc(ovl, cmdq_pkt, idx,
> -				 pending->modifier != DRM_FORMAT_MOD_LINEAR);
> +	if (ovl->data->supports_afbc && (modifier != DRM_FORMAT_MOD_LINEAR)) {
> +		/*
> +		 * In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this
> +		 * limitation of the AFBC format, OVL_CLIP is used to adjust the
> +		 * output size of the layer.
> +		 */
> +		clip = FIELD_PREP(OVL_CLIP_BOTTOM, pending->clip_bottom) |
> +		       FIELD_PREP(OVL_CLIP_TOP, pending->clip_top) |
> +		       FIELD_PREP(OVL_CLIP_RIGHT, pending->clip_right) |
> +		       FIELD_PREP(OVL_CLIP_LEFT, pending->clip_left);
> +		src_height = pending->height + pending->clip_top + pending->clip_bottom;
> +		src_width = pending->width + pending->clip_left + pending->clip_right;
> +		mtk_ovl_set_afbc(ovl, cmdq_pkt, idx, true);
> +	} else {
> +		src_height = pending->height;
> +		src_width = pending->width;
> +		mtk_ovl_set_afbc(ovl, cmdq_pkt, idx, false);
> +	}
> +
> +	src_size = (src_height << 16) | src_width;
>  
>  	mtk_ddp_write_relaxed(cmdq_pkt, con, &ovl->cmdq_reg, ovl->regs,
>  			      DISP_REG_OVL_CON(idx));
> @@ -560,6 +583,8 @@ void mtk_ovl_layer_config(struct device *dev, unsigned int idx,
>  			      &ovl->cmdq_reg, ovl->regs, DISP_REG_OVL_PITCH(idx));
>  	mtk_ddp_write_relaxed(cmdq_pkt, src_size, &ovl->cmdq_reg, ovl->regs,
>  			      DISP_REG_OVL_SRC_SIZE(idx));
> +	mtk_ddp_write_relaxed(cmdq_pkt, clip, &ovl->cmdq_reg, ovl->regs,
> +			      DISP_REG_OVL_CLIP(idx));
>  	mtk_ddp_write_relaxed(cmdq_pkt, offset, &ovl->cmdq_reg, ovl->regs,
>  			      DISP_REG_OVL_OFFSET(idx));
>  	mtk_ddp_write_relaxed(cmdq_pkt, addr, &ovl->cmdq_reg, ovl->regs,
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
> index 1214f623859e..8fb08768e8ce 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.c
> @@ -114,6 +114,7 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
>  				       struct mtk_plane_state *mtk_plane_state)
>  {
>  	struct drm_framebuffer *fb = new_state->fb;
> +	unsigned int clip_left = 0, clip_top = 0, clip_right = 0, clip_bottom = 0;
>  	struct drm_gem_object *gem;
>  	struct mtk_gem_obj *mtk_gem;
>  	unsigned int pitch, format;
> @@ -148,6 +149,22 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
>  		int x_offset_in_blocks = (new_state->src.x1 >> 16) / AFBC_DATA_BLOCK_WIDTH;
>  		int y_offset_in_blocks = (new_state->src.y1 >> 16) / AFBC_DATA_BLOCK_HEIGHT;
>  		int hdr_size, hdr_offset;
> +		int src_width = drm_rect_width(&new_state->src) >> 16;
> +		int src_height = drm_rect_height(&new_state->src) >> 16;
> +		unsigned int remainder_right, remainder_bottom;
> +
> +		/*
> +		 * In AFBC mode, the source size configured needs to be a
> +		 * multiple of the AFBC data block size. Compute and save the
> +		 * necessary clips so the indeded x, y, width and height are
> +		 * obtained in the output despite this constraint.
> +		 */
> +		clip_left = (new_state->src.x1 >> 16) % AFBC_DATA_BLOCK_WIDTH;
> +		clip_top = (new_state->src.y1 >> 16) % AFBC_DATA_BLOCK_HEIGHT;
> +		remainder_right = (src_width + clip_left) % AFBC_DATA_BLOCK_WIDTH;
> +		clip_right = remainder_right ? AFBC_DATA_BLOCK_WIDTH - remainder_right : 0;
> +		remainder_bottom = (src_height + clip_top) % AFBC_DATA_BLOCK_HEIGHT;
> +		clip_bottom = remainder_bottom ? AFBC_DATA_BLOCK_HEIGHT - remainder_bottom : 0;
>  
>  		hdr_pitch = width_in_blocks * AFBC_HEADER_BLOCK_SIZE;
>  		pitch = width_in_blocks * AFBC_DATA_BLOCK_WIDTH *
> @@ -187,6 +204,10 @@ static void mtk_plane_update_new_state(struct drm_plane_state *new_state,
>  	mtk_plane_state->pending.y = new_state->dst.y1;
>  	mtk_plane_state->pending.width = drm_rect_width(&new_state->dst);
>  	mtk_plane_state->pending.height = drm_rect_height(&new_state->dst);
> +	mtk_plane_state->pending.clip_left = clip_left;
> +	mtk_plane_state->pending.clip_top = clip_top;
> +	mtk_plane_state->pending.clip_right = clip_right;
> +	mtk_plane_state->pending.clip_bottom = clip_bottom;
>  	mtk_plane_state->pending.rotation = new_state->rotation;
>  	mtk_plane_state->pending.color_encoding = new_state->color_encoding;
>  }
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.h b/drivers/gpu/drm/mediatek/mtk_plane.h
> index 46be4454bc92..a9cfb2ee5859 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.h
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.h
> @@ -28,6 +28,10 @@ struct mtk_plane_pending_state {
>  	unsigned int			y;
>  	unsigned int			width;
>  	unsigned int			height;
> +	unsigned int			clip_left;
> +	unsigned int			clip_top;
> +	unsigned int			clip_right;
> +	unsigned int			clip_bottom;
>  	unsigned int			rotation;
>  	bool				dirty;
>  	bool				async_dirty;
> 

Re: [PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[CK Hu (胡俊光)]

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> 
> In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this limitation
> of the AFBC format, OVL_CLIP needs to be used to achieve the desired
> output size of the layer while still meeting the alignment constraints.
> Failure to do this will result in vblank timeouts and no rendered output
> when the AFBC data source isn't aligned to the AFBC block (32x8).
> 
> Configure OVL_CLIP so unaligned AFBC layers can be displayed.
> 
> The following illustrates how the alignment is achieved through the clip
> settings for the horizontal coordinates, the vertical coordinates are
> analogous:
> 
> /------------------------------------------------\
> >                                                |
> >            ........................            |
> >            ........................            |
> >            ........................            |
> >            ........................            |
> >                                                |
> \------------------------------------------------/
>      |       |                      |       |
>      |       src.x1                 src.x2  |
>      |       |                      |       |
>      |       |<-------------------->|       |
>      |              src_width               |
>      |                                      |
>      N * AFBC_DATA_BLOCK_WIDTH              M * AFBC_DATA_BLOCK_WIDTH
>      |                                      |
>      |<----->|                      |<----->|
>       clip_left                      clip_right

As I know, crop is used to drop pixel data.
From the name of 'clip_left', I think it would drop the left part of this image.
But usually the image is aligned to the left (start from axis 0) and append garbage data in right part.
If so, clip_left should be zero and all the clip would be clip_right.
This is the normal behavior.
If OVL_CROP does behave as this, add comment to describe that clip_left does not drop pixel data.

Regards,
CK

> 
> Signed-off-by: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> Co-developed-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
> 

Re: [PATCH RFC 4/6] drm/mediatek: ovl: Disallow AFBC buffers with width over 1920

[CK Hu (胡俊光)]

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> AFBC buffers with width over 1920 are not supported by OVL. If
> attempted, the image displayed contains many artifacts.
> 
> Add this restriction to the layer check callback so such configurations
> are not allowed.
> 
> NOTE: This doesn't seem to be a good way to handle this restriction, as
> when tested with Weston, it simply fails to render, rather than fallback
> to not using the modifier:
> 
> [19:09:03.857] atomic: couldn't commit new state: Invalid argument
> [19:09:03.857] repaint-flush failed: Invalid argument

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index c6a00c2256dd..196b874057ba 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -368,6 +368,13 @@ int mtk_ovl_layer_check(struct device *dev, unsigned int idx,
>  	if (state->fb->format->is_yuv && (state->rotation & ~DRM_MODE_ROTATE_0))
>  		return -EINVAL;
>  
> +	/*
> +	 * AFBC buffers with width > 1920 are not supported and produce
> +	 * artifacts, so should be disabled.
> +	 */
> +	if (state->fb->modifier != DRM_FORMAT_MOD_LINEAR && state->fb->width > 1920)
> +		return -EINVAL;
> +
>  	return 0;
>  }
>  
> 

Re: [PATCH RFC 5/6] drm/mediatek: ovl: Disable AFBC on MT8188

[CK Hu (胡俊光)]

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> Despite MT8188's OVL being mostly the same IP as the OVL on MT8195, it
> does not support AFBC, even when the same register configurations are
> applied. Introduce a separate compatible for it with AFBC support
> disabled.
> 
> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 196b874057ba..97f6694772d4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -762,6 +762,21 @@ static const struct mtk_disp_ovl_data mt8192_ovl_2l_driver_data = {
>  	.num_formats = ARRAY_SIZE(mt8173_formats),
>  };
>  
> +static const struct mtk_disp_ovl_data mt8188_ovl_driver_data = {
> +	.addr = DISP_REG_OVL_ADDR_MT8173,
> +	.gmc_bits = 10,
> +	.layer_nr = 4,
> +	.fmt_rgb565_is_0 = true,
> +	.smi_id_en = true,
> +	.supports_afbc = false,

Global variable is initialized to zero, so this could be drop.

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> +	.blend_modes = BIT(DRM_MODE_BLEND_PREMULTI) |
> +		       BIT(DRM_MODE_BLEND_COVERAGE) |
> +		       BIT(DRM_MODE_BLEND_PIXEL_NONE),
> +	.formats = mt8195_formats,
> +	.num_formats = ARRAY_SIZE(mt8195_formats),
> +	.supports_clrfmt_ext = true,
> +};
> +
>  static const struct mtk_disp_ovl_data mt8195_ovl_driver_data = {
>  	.addr = DISP_REG_OVL_ADDR_MT8173,
>  	.gmc_bits = 10,
> @@ -790,6 +805,8 @@ static const struct of_device_id mtk_disp_ovl_driver_dt_match[] = {
>  	  .data = &mt8192_ovl_driver_data},
>  	{ .compatible = "mediatek,mt8192-disp-ovl-2l",
>  	  .data = &mt8192_ovl_2l_driver_data},
> +	{ .compatible = "mediatek,mt8188-disp-ovl",
> +	  .data = &mt8188_ovl_driver_data},
>  	{ .compatible = "mediatek,mt8195-disp-ovl",
>  	  .data = &mt8195_ovl_driver_data},
>  	{},
> 

Re: [PATCH RFC 6/6] drm/mediatek: Re-enable AFBC support on MediaTek DRM driver

[CK Hu (胡俊光)]

On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> Commit 9882a4064003 ("drm/mediatek: Disable AFBC support on Mediatek DRM
> driver") disabled AFBC support on the MediaTek DRM driver since it was
> broken.
> 
> With the bugs in the AFBC support now fixed, re-enable the support.
> 

Reviewed-by: CK Hu <ck.hu@mediatek.com>

> Signed-off-by: Nícolas F. R. A. Prado <nfraprado@collabora.com>
> ---
>  drivers/gpu/drm/mediatek/mtk_plane.c | 24 +++++++++++++++++++++++-
>  1 file changed, 23 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/mediatek/mtk_plane.c b/drivers/gpu/drm/mediatek/mtk_plane.c
> index 8fb08768e8ce..38dd94cdd665 100644
> --- a/drivers/gpu/drm/mediatek/mtk_plane.c
> +++ b/drivers/gpu/drm/mediatek/mtk_plane.c
> @@ -22,6 +22,9 @@
>  
>  static const u64 modifiers[] = {
>  	DRM_FORMAT_MOD_LINEAR,
> +	DRM_FORMAT_MOD_ARM_AFBC(AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> +				AFBC_FORMAT_MOD_SPLIT |
> +				AFBC_FORMAT_MOD_SPARSE),
>  	DRM_FORMAT_MOD_INVALID,
>  };
>  
> @@ -69,7 +72,26 @@ static bool mtk_plane_format_mod_supported(struct drm_plane *plane,
>  					   uint32_t format,
>  					   uint64_t modifier)
>  {
> -	return modifier == DRM_FORMAT_MOD_LINEAR;
> +	if (modifier == DRM_FORMAT_MOD_LINEAR)
> +		return true;
> +
> +	if (modifier != DRM_FORMAT_MOD_ARM_AFBC(
> +				AFBC_FORMAT_MOD_BLOCK_SIZE_32x8 |
> +				AFBC_FORMAT_MOD_SPLIT |
> +				AFBC_FORMAT_MOD_SPARSE))
> +		return false;
> +
> +	if (format != DRM_FORMAT_XRGB8888 &&
> +	    format != DRM_FORMAT_ARGB8888 &&
> +	    format != DRM_FORMAT_BGRX8888 &&
> +	    format != DRM_FORMAT_BGRA8888 &&
> +	    format != DRM_FORMAT_ABGR8888 &&
> +	    format != DRM_FORMAT_XBGR8888 &&
> +	    format != DRM_FORMAT_RGB888 &&
> +	    format != DRM_FORMAT_BGR888)
> +		return false;
> +
> +	return true;
>  }
>  
>  static void mtk_plane_destroy_state(struct drm_plane *plane,
> 

Re: [PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[Nícolas F. R. A. Prado]

On Mon, 2026-02-02 at 08:46 +0000, CK Hu (胡俊光) wrote:
> On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> > From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> > 
> > In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this
> > limitation
> > of the AFBC format, OVL_CLIP needs to be used to achieve the
> > desired
> > output size of the layer while still meeting the alignment
> > constraints.
> > Failure to do this will result in vblank timeouts and no rendered
> > output
> > when the AFBC data source isn't aligned to the AFBC block (32x8).
> > 
> > Configure OVL_CLIP so unaligned AFBC layers can be displayed.
> > 
> > The following illustrates how the alignment is achieved through the
> > clip
> > settings for the horizontal coordinates, the vertical coordinates
> > are
> > analogous:
> > 
> > /------------------------------------------------\
> > >                                                |
> > >            ........................            |
> > >            ........................            |
> > >            ........................            |
> > >            ........................            |
> > >                                                |
> > \------------------------------------------------/
> >      |       |                      |       |
> >      |       src.x1                 src.x2  |
> >      |       |                      |       |
> >      |       |<-------------------->|       |
> >      |              src_width               |
> 
> This patch looks to me.
> But "In AFBC mode, OVL_SRC_SIZE must be block aligned", so this graph
> should show as:
> 
>      |       src.x1                 src.x2  |
>      |       |                      |       |
>      |       |                      |       |
>      N * AFBC_DATA_BLOCK_WIDTH      |       M * AFBC_DATA_BLOCK_WIDTH
>      |       |                      |       |
>      |<----->|                      |<----->|
>      |clip_left                      clip_right
>      |                                      |
>      |<------------------------------------>|
>      |              src_width               |

Ah yes indeed, thanks for spotting that!

-- 
Thanks,

Nícolas

Re: [PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[Nícolas F. R. A. Prado]

On Tue, 2026-02-03 at 02:01 +0000, CK Hu (胡俊光) wrote:
> On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> > From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> > 
> > In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this
> > limitation
> > of the AFBC format, OVL_CLIP needs to be used to achieve the
> > desired
> > output size of the layer while still meeting the alignment
> > constraints.
> > Failure to do this will result in vblank timeouts and no rendered
> > output
> > when the AFBC data source isn't aligned to the AFBC block (32x8).
> > 
> > Configure OVL_CLIP so unaligned AFBC layers can be displayed.
> > 
> > The following illustrates how the alignment is achieved through the
> > clip
> > settings for the horizontal coordinates, the vertical coordinates
> > are
> > analogous:
> > 
> > /------------------------------------------------\
> > >                                                |
> > >            ........................            |
> > >            ........................            |
> > >            ........................            |
> > >            ........................            |
> > >                                                |
> > \------------------------------------------------/
> >      |       |                      |       |
> >      |       src.x1                 src.x2  |
> >      |       |                      |       |
> >      |       |<-------------------->|       |
> >      |              src_width               |
> >      |                                      |
> >      N * AFBC_DATA_BLOCK_WIDTH              M *
> > AFBC_DATA_BLOCK_WIDTH
> >      |                                      |
> >      |<----->|                      |<----->|
> >       clip_left                      clip_right
> 
> As I know, crop is used to drop pixel data.
> From the name of 'clip_left', I think it would drop the left part of
> this image.
> But usually the image is aligned to the left (start from axis 0) and
> append garbage data in right part.
> If so, clip_left should be zero and all the clip would be clip_right.
> This is the normal behavior.
> If OVL_CROP does behave as this, add comment to describe that
> clip_left does not drop pixel data.

Both clip_left and clip_right work in the same way, by discarding that
many pixels, on the left and right, respectively, of the plane's
framebuffer when compositing the plane on the final image.

In the simplest case, when the image to be displayed is left-aligned,
ie src.x1 = 0, then yes, only clip_right will be used to make sure that
the plane's width aligns with the AFBC_DATA_BLOCK_WIDTH.

However if only a sub-region of the image is to be displayed, then
src.x1 will be non-zero. If that x offset coordinate aligns with the
AFBC_DATA_BLOCK_WIDTH, then again clip_left will be 0, and we're back
at the simplest case.

But if it doesn't align, then clip_left will need to be used to ensure
only the intended sub-region is displayed, even though it starts in the
middle of an AFBC data block.

This is because not only the width and height in DISP_REG_OVL_SRC_SIZE
need to be aligned to the AFBC data block, but also the starting
address in DISP_REG_OVL_ADDR, while src.x1 and src.x2 supplied by
userspace are arbitrary and won't necessarily align, hence we use both
clips as needed to achieve the intended display outcome respecting the
hardware constraints.

-- 
Thanks,

Nícolas

Re: [PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[CK Hu (胡俊光)]

On Thu, 2026-02-05 at 14:13 -0500, Nícolas F. R. A. Prado wrote:
> On Tue, 2026-02-03 at 02:01 +0000, CK Hu (胡俊光) wrote:
> > On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> > > From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> > > 
> > > In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this
> > > limitation
> > > of the AFBC format, OVL_CLIP needs to be used to achieve the
> > > desired
> > > output size of the layer while still meeting the alignment
> > > constraints.
> > > Failure to do this will result in vblank timeouts and no rendered
> > > output
> > > when the AFBC data source isn't aligned to the AFBC block (32x8).
> > > 
> > > Configure OVL_CLIP so unaligned AFBC layers can be displayed.
> > > 
> > > The following illustrates how the alignment is achieved through the
> > > clip
> > > settings for the horizontal coordinates, the vertical coordinates
> > > are
> > > analogous:
> > > 
> > > /------------------------------------------------\
> > > >                                                |
> > > >            ........................            |
> > > >            ........................            |
> > > >            ........................            |
> > > >            ........................            |
> > > >                                                |
> > > \------------------------------------------------/
> > >      |       |                      |       |
> > >      |       src.x1                 src.x2  |
> > >      |       |                      |       |
> > >      |       |<-------------------->|       |
> > >      |              src_width               |
> > >      |                                      |
> > >      N * AFBC_DATA_BLOCK_WIDTH              M *
> > > AFBC_DATA_BLOCK_WIDTH
> > >      |                                      |
> > >      |<----->|                      |<----->|
> > >       clip_left                      clip_right
> > 
> > As I know, crop is used to drop pixel data.
> > From the name of 'clip_left', I think it would drop the left part of
> > this image.
> > But usually the image is aligned to the left (start from axis 0) and
> > append garbage data in right part.
> > If so, clip_left should be zero and all the clip would be clip_right.
> > This is the normal behavior.
> > If OVL_CROP does behave as this, add comment to describe that
> > clip_left does not drop pixel data.
> 
> Both clip_left and clip_right work in the same way, by discarding that
> many pixels, on the left and right, respectively, of the plane's
> framebuffer when compositing the plane on the final image.
> 
> In the simplest case, when the image to be displayed is left-aligned,
> ie src.x1 = 0, then yes, only clip_right will be used to make sure that
> the plane's width aligns with the AFBC_DATA_BLOCK_WIDTH.
> 
> However if only a sub-region of the image is to be displayed, then
> src.x1 will be non-zero. If that x offset coordinate aligns with the
> AFBC_DATA_BLOCK_WIDTH, then again clip_left will be 0, and we're back
> at the simplest case.
> 
> But if it doesn't align, then clip_left will need to be used to ensure
> only the intended sub-region is displayed, even though it starts in the
> middle of an AFBC data block.
> 
> This is because not only the width and height in DISP_REG_OVL_SRC_SIZE
> need to be aligned to the AFBC data block, but also the starting
> address in DISP_REG_OVL_ADDR, while src.x1 and src.x2 supplied by
> userspace are arbitrary and won't necessarily align, hence we use both
> clips as needed to achieve the intended display outcome respecting the
> hardware constraints.

OK, in mtk_plane_update_new_state(), x_offset_in_blocks has done the block-based crop.
And this patch introduce clip_left for pixel-based crop.
I think pixel-based crop could replace block-based crop.
So drop x_offset_in_blocks and let clip_left = src.x1.

In addition, the clip_right is also not necessary.
The pitch already tell hardware the buffer width.
I think hardware would not access data over src_width,
so drop clip_right and it could be simplified as

|<---------------------------------------------------->|
                    pitch
|<---------------------------------------------->|       
                 valid image data   
             |                      |       
             src.x1                 src.x2  
             |                      |       
|<---------->|                      |
   clip_left |                      |
             |                      |  
|<--------------------------------->|       
                    src_width               


But conceptually, src_width means valid image data, it's better that

|<---------------------------------------------------->|
                    pitch
|<---------------------------------------------->|       
                 valid image data  (fb->width)   |
             |                      |            |
             src.x1                 src.x2       |
             |                      |            |
|<---------->|                      |<---------->|
   clip_left |                      | clip_right |
             |                      |            |
|<---------------------------------------------->|       
                    src_width          

For the old SoC, we just tell hardware buffer start address is in x1 , and src_width is (x2 - x1).
This is because old SoC has no crop function.
When we have crop function. I would like to configure to fit the meaning.

Regards,
CK

Re: [PATCH RFC 3/6] drm/mediatek: ovl: Fix misaligned layer source size on AFBC mode

[CK Hu (胡俊光)]

On Thu, 2026-02-12 at 09:54 +0800, CK Hu wrote:
> On Thu, 2026-02-05 at 14:13 -0500, Nícolas F. R. A. Prado wrote:
> > On Tue, 2026-02-03 at 02:01 +0000, CK Hu (胡俊光) wrote:
> > > On Tue, 2025-12-30 at 11:03 -0300, Nícolas F. R. A. Prado wrote:
> > > > From: Ariel D'Alessandro <ariel.dalessandro@collabora.com>
> > > > 
> > > > In AFBC mode, OVL_SRC_SIZE must be block aligned. Due to this
> > > > limitation
> > > > of the AFBC format, OVL_CLIP needs to be used to achieve the
> > > > desired
> > > > output size of the layer while still meeting the alignment
> > > > constraints.
> > > > Failure to do this will result in vblank timeouts and no rendered
> > > > output
> > > > when the AFBC data source isn't aligned to the AFBC block (32x8).
> > > > 
> > > > Configure OVL_CLIP so unaligned AFBC layers can be displayed.
> > > > 
> > > > The following illustrates how the alignment is achieved through the
> > > > clip
> > > > settings for the horizontal coordinates, the vertical coordinates
> > > > are
> > > > analogous:
> > > > 
> > > > /------------------------------------------------\
> > > > >                                                |
> > > > >            ........................            |
> > > > >            ........................            |
> > > > >            ........................            |
> > > > >            ........................            |
> > > > >                                                |
> > > > \------------------------------------------------/
> > > >      |       |                      |       |
> > > >      |       src.x1                 src.x2  |
> > > >      |       |                      |       |
> > > >      |       |<-------------------->|       |
> > > >      |              src_width               |
> > > >      |                                      |
> > > >      N * AFBC_DATA_BLOCK_WIDTH              M *
> > > > AFBC_DATA_BLOCK_WIDTH
> > > >      |                                      |
> > > >      |<----->|                      |<----->|
> > > >       clip_left                      clip_right
> > > 
> > > As I know, crop is used to drop pixel data.
> > > From the name of 'clip_left', I think it would drop the left part of
> > > this image.
> > > But usually the image is aligned to the left (start from axis 0) and
> > > append garbage data in right part.
> > > If so, clip_left should be zero and all the clip would be clip_right.
> > > This is the normal behavior.
> > > If OVL_CROP does behave as this, add comment to describe that
> > > clip_left does not drop pixel data.
> > 
> > Both clip_left and clip_right work in the same way, by discarding that
> > many pixels, on the left and right, respectively, of the plane's
> > framebuffer when compositing the plane on the final image.
> > 
> > In the simplest case, when the image to be displayed is left-aligned,
> > ie src.x1 = 0, then yes, only clip_right will be used to make sure that
> > the plane's width aligns with the AFBC_DATA_BLOCK_WIDTH.
> > 
> > However if only a sub-region of the image is to be displayed, then
> > src.x1 will be non-zero. If that x offset coordinate aligns with the
> > AFBC_DATA_BLOCK_WIDTH, then again clip_left will be 0, and we're back
> > at the simplest case.
> > 
> > But if it doesn't align, then clip_left will need to be used to ensure
> > only the intended sub-region is displayed, even though it starts in the
> > middle of an AFBC data block.
> > 
> > This is because not only the width and height in DISP_REG_OVL_SRC_SIZE
> > need to be aligned to the AFBC data block, but also the starting
> > address in DISP_REG_OVL_ADDR, while src.x1 and src.x2 supplied by
> > userspace are arbitrary and won't necessarily align, hence we use both
> > clips as needed to achieve the intended display outcome respecting the
> > hardware constraints.
> 
> OK, in mtk_plane_update_new_state(), x_offset_in_blocks has done the block-based crop.
> And this patch introduce clip_left for pixel-based crop.
> I think pixel-based crop could replace block-based crop.
> So drop x_offset_in_blocks and let clip_left = src.x1.
> 
> In addition, the clip_right is also not necessary.
> The pitch already tell hardware the buffer width.
> I think hardware would not access data over src_width,
> so drop clip_right and it could be simplified as
> 
> > <---------------------------------------------------->|
>                     pitch
> > <---------------------------------------------->|       
>                  valid image data   
>              |                      |       
>              src.x1                 src.x2  
>              |                      |       
> > <---------->|                      |
>    clip_left |                      |
>              |                      |  
> > <--------------------------------->|       
>                     src_width               
> 
> 
> But conceptually, src_width means valid image data, it's better that
> 
> > <---------------------------------------------------->|
>                     pitch
> > <---------------------------------------------->|       
>                  valid image data  (fb->width)   |
>              |                      |            |
>              src.x1                 src.x2       |
>              |                      |            |
> > <---------->|                      |<---------->|
>    clip_left |                      | clip_right |
>              |                      |            |
> > <---------------------------------------------->|       
>                     src_width          
> 
> For the old SoC, we just tell hardware buffer start address is in x1 , and src_width is (x2 - x1).
> This is because old SoC has no crop function.
> When we have crop function. I would like to configure to fit the meaning.

Sorry, I find that crop left, right, top, bottom all has only 8 bits.
The hardware maximum crop could be 255 pixel, but the actual crop may be larger than 255.
So we still need block-based crop with this pixel-based crop.
Please add this information to describe why we need this mixed crop mechanism.

Regards,
CK

> 
> Regards,
> CK
> 
> 
>