[PATCH 01/13] drm/sun4i: Fix V3s YUV scanline size

[PATCH 01/13] drm/sun4i: Fix V3s YUV scanline size

[Jernej Skrabec]

The VI scaler line buffer on V3s can hold 1024 pixels for subsampled
formats, not 2048 as currently claimed. Since coarse horizontal scaling
is engaged only once the source width exceeds that limit, YUV layers
wider than 1024 pixels are passed to the scaler unchanged and the
output is corrupted.

Use the value from the vendor driver.

Fixes: 2586de70c15c ("drm/sun4i: Add VI scaler line size quirk for DE2/DE3")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_mixer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_mixer.c b/drivers/gpu/drm/sun4i/sun8i_mixer.c
index 4e4226295010..b6cd7352e76c 100644
--- a/drivers/gpu/drm/sun4i/sun8i_mixer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_mixer.c
@@ -785,7 +785,7 @@ static const struct sun8i_mixer_cfg sun8i_v3s_mixer_cfg = {
 		.de_type	= SUN8I_MIXER_DE2,
 		.vi_scaler_num	= 2,
 		.scaler_mask	= 0x3,
-		.scanline_yuv	= 2048,
+		.scanline_yuv	= 1024,
 	},
 	.de_type	= SUN8I_MIXER_DE2,
 	.mod_rate	= 150000000,
-- 
2.43.0

Re: [PATCH 01/13] drm/sun4i: Fix V3s YUV scanline size

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> The VI scaler line buffer on V3s can hold 1024 pixels for subsampled
> formats, not 2048 as currently claimed. Since coarse horizontal scaling
> is engaged only once the source width exceeds that limit, YUV layers
> wider than 1024 pixels are passed to the scaler unchanged and the
> output is corrupted.
>
> Use the value from the vendor driver.
>
> Fixes: 2586de70c15c ("drm/sun4i: Add VI scaler line size quirk for DE2/DE3")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

Re: (subset) [PATCH 01/13] drm/sun4i: Fix V3s YUV scanline size

[Chen-Yu Tsai]

On Mon, 03 Aug 2026 18:10:39 +0200, Jernej Skrabec wrote:
> The VI scaler line buffer on V3s can hold 1024 pixels for subsampled
> formats, not 2048 as currently claimed. Since coarse horizontal scaling
> is engaged only once the source width exceeds that limit, YUV layers
> wider than 1024 pixels are passed to the scaler unchanged and the
> output is corrupted.
> 
> Use the value from the vendor driver.
> 
> [...]

Applied to drm-misc-next in drm-misc, thanks!

[01/13] drm/sun4i: Fix V3s YUV scanline size
        commit: abffce8b1eb5e119efccc51fc6a18ba38e2f6db6
[02/13] drm/sun4i: vi scaler: Fix coefficient selection
        commit: 5c31990b21f0b535732deb2b658b78b07464f56c
[03/13] drm/sun4i: vi scaler: Restore opaque alpha in video modes
        commit: 583d99165a85c3aaee924078c0f03c72e25644ac
[04/13] drm/sun4i: tcon-top: Keep mixer routes distinct
        commit: 62bac3202042a8cfd4d4969a6d057cebd8b7cc1a
[05/13] drm/sun4i: tcon: Set output mux for DSI and LVDS
        commit: 9c90199b39637ad94253c4fd8e7b1333ca1d3e0d
[06/13] drm/sun4i: tcon: Drop TCON TOP device reference
        commit: 8208832a38ff3d2560eb8a77a9d7a2f17d8ebcdc
[07/13] drm/sun4i: hdmi: Don't leak sync polarity bits into packet control
        commit: f5c3b1b0d228624786d22973a20c908c84e1a576
[08/13] drm/sun4i: crtc: Propagate layer initialization error
        commit: 7061ff05ed4a3cf16e83f7e3ad09cbd212508a32
[09/13] drm/sun4i: tcon: Drop remote endpoint reference
        commit: 3f77e4072630e2301efdbe521e7fca10311043ec
[10/13] drm/sun4i: dw-hdmi: Drop TCON TOP port reference
        commit: d2a242e5688a17b79c89cd966cd31820c5096d80
[11/13] drm/sun4i: Drop node references while building component list
        commit: 08a91f2a0664b73c2af9295ecbd441e852abe33c
[12/13] drm/sun4i: hdmi-phy: Fix H6 8-bit MPLL config at 594 MHz
        commit: 0ba6deddaae74f0539c0303bfb5f860adbe1a68b

Best regards,
-- 
Chen-Yu Tsai <wens@kernel.org>

[PATCH 02/13] drm/sun4i: vi scaler: Fix coefficient selection

[Jernej Skrabec]

Currently, vertical coefficients are selected based on horizontal
scaling, which is wrong. Additionally, chroma coefficients should be
selected based on format subsampling.

Fix all that.

Fixes: b862a648de3b ("drm/sun4i: Add support for HW scaling to DE2")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_vi_scaler.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
index 3dec4eeb1ba2..d4412a62c2f0 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
@@ -894,20 +894,26 @@ static void sun8i_vi_scaler_set_coeff(struct regmap *map, u32 base,
 			     lan3coefftab32_left[offset + i]);
 		regmap_write(map, SUN8I_SCALER_VSU_YHCOEFF1(base, i),
 			     lan3coefftab32_right[offset + i]);
+	}
+	offset = sun8i_vi_scaler_coef_index(vstep) *
+			SUN8I_VI_SCALER_COEFF_COUNT;
+	for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++)
+		regmap_write(map, SUN8I_SCALER_VSU_YVCOEFF(base, i),
+			     lan2coefftab32[offset + i]);
+
+	offset = sun8i_vi_scaler_coef_index(hstep / format->hsub) *
+			SUN8I_VI_SCALER_COEFF_COUNT;
+	for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++) {
 		regmap_write(map, SUN8I_SCALER_VSU_CHCOEFF0(base, i),
 			     ch_left[offset + i]);
 		regmap_write(map, SUN8I_SCALER_VSU_CHCOEFF1(base, i),
 			     ch_right[offset + i]);
 	}
-
-	offset = sun8i_vi_scaler_coef_index(hstep) *
+	offset = sun8i_vi_scaler_coef_index(vstep / format->vsub) *
 			SUN8I_VI_SCALER_COEFF_COUNT;
-	for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++) {
-		regmap_write(map, SUN8I_SCALER_VSU_YVCOEFF(base, i),
-			     lan2coefftab32[offset + i]);
+	for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++)
 		regmap_write(map, SUN8I_SCALER_VSU_CVCOEFF(base, i),
 			     cy[offset + i]);
-	}
 }
 
 void sun8i_vi_scaler_enable(struct sun8i_layer *layer, bool enable)
-- 
2.43.0

Re: [PATCH 02/13] drm/sun4i: vi scaler: Fix coefficient selection

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> Currently, vertical coefficients are selected based on horizontal
> scaling, which is wrong. Additionally, chroma coefficients should be
> selected based on format subsampling.
>
> Fix all that.
>
> Fixes: b862a648de3b ("drm/sun4i: Add support for HW scaling to DE2")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>


Somehow having the two fixes together made the diff much harder for me
to understand.

> ---
>  drivers/gpu/drm/sun4i/sun8i_vi_scaler.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
> index 3dec4eeb1ba2..d4412a62c2f0 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
> @@ -894,20 +894,26 @@ static void sun8i_vi_scaler_set_coeff(struct regmap *map, u32 base,
>                              lan3coefftab32_left[offset + i]);
>                 regmap_write(map, SUN8I_SCALER_VSU_YHCOEFF1(base, i),
>                              lan3coefftab32_right[offset + i]);
> +       }
> +       offset = sun8i_vi_scaler_coef_index(vstep) *
> +                       SUN8I_VI_SCALER_COEFF_COUNT;
> +       for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++)
> +               regmap_write(map, SUN8I_SCALER_VSU_YVCOEFF(base, i),
> +                            lan2coefftab32[offset + i]);
> +
> +       offset = sun8i_vi_scaler_coef_index(hstep / format->hsub) *
> +                       SUN8I_VI_SCALER_COEFF_COUNT;
> +       for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++) {
>                 regmap_write(map, SUN8I_SCALER_VSU_CHCOEFF0(base, i),
>                              ch_left[offset + i]);
>                 regmap_write(map, SUN8I_SCALER_VSU_CHCOEFF1(base, i),
>                              ch_right[offset + i]);
>         }
> -
> -       offset = sun8i_vi_scaler_coef_index(hstep) *
> +       offset = sun8i_vi_scaler_coef_index(vstep / format->vsub) *
>                         SUN8I_VI_SCALER_COEFF_COUNT;
> -       for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++) {
> -               regmap_write(map, SUN8I_SCALER_VSU_YVCOEFF(base, i),
> -                            lan2coefftab32[offset + i]);
> +       for (i = 0; i < SUN8I_VI_SCALER_COEFF_COUNT; i++)
>                 regmap_write(map, SUN8I_SCALER_VSU_CVCOEFF(base, i),
>                              cy[offset + i]);
> -       }
>  }
>
>  void sun8i_vi_scaler_enable(struct sun8i_layer *layer, bool enable)
> --
> 2.43.0
>

[PATCH 03/13] drm/sun4i: vi scaler: Restore opaque alpha in video modes

[Jernej Skrabec]

The DE3 mixer initialization clears the entire mixer register space. This
also clears VSU_GLOBAL_ALPHA, despite its hardware reset value being 0xff.

The VI scaler uses Video Normal mode for subsampled YUV formats. In this
mode, VSU_GLOBAL_ALPHA provides the scaler output alpha. Leaving the
register at zero causes the scaler to produce fully transparent output.

Set VSU_GLOBAL_ALPHA to 0xff whenever configuring a DE3 or newer VI
scaler. The register is ignored in UI scaling mode.

Fixes: c50519e6db4d ("drm/sun4i: Add basic support for DE3")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_vi_scaler.c | 2 ++
 drivers/gpu/drm/sun4i/sun8i_vi_scaler.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
index d4412a62c2f0..a5cc99981006 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
@@ -975,6 +975,8 @@ void sun8i_vi_scaler_setup(struct sun8i_layer *layer,
 
 		regmap_write(layer->regs,
 			     SUN50I_SCALER_VSU_SCALE_MODE(base), val);
+		regmap_write(layer->regs,
+			     SUN50I_SCALER_VSU_GLB_ALPHA(base), 0xff);
 	}
 
 	regmap_write(layer->regs,
diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h
index 245fe2f431c3..d89c0cbe188d 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h
@@ -36,6 +36,7 @@
 #define SUN50I_SCALER_VSU_EDSCL_CTRL(base)		((base) + 0x28)
 #define SUN50I_SCALER_VSU_ANGLE_THR(base)		((base) + 0x2c)
 #define SUN8I_SCALER_VSU_OUTSIZE(base)		((base) + 0x40)
+#define SUN50I_SCALER_VSU_GLB_ALPHA(base)	((base) + 0x44)
 #define SUN8I_SCALER_VSU_YINSIZE(base)		((base) + 0x80)
 #define SUN8I_SCALER_VSU_YHSTEP(base)		((base) + 0x88)
 #define SUN8I_SCALER_VSU_YVSTEP(base)		((base) + 0x8c)
-- 
2.43.0

Re: [PATCH 03/13] drm/sun4i: vi scaler: Restore opaque alpha in video modes

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> The DE3 mixer initialization clears the entire mixer register space. This
> also clears VSU_GLOBAL_ALPHA, despite its hardware reset value being 0xff.
>
> The VI scaler uses Video Normal mode for subsampled YUV formats. In this
> mode, VSU_GLOBAL_ALPHA provides the scaler output alpha. Leaving the
> register at zero causes the scaler to produce fully transparent output.
>
> Set VSU_GLOBAL_ALPHA to 0xff whenever configuring a DE3 or newer VI
> scaler. The register is ignored in UI scaling mode.
>
> Fixes: c50519e6db4d ("drm/sun4i: Add basic support for DE3")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun8i_vi_scaler.c | 2 ++
>  drivers/gpu/drm/sun4i/sun8i_vi_scaler.h | 1 +
>  2 files changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
> index d4412a62c2f0..a5cc99981006 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.c
> @@ -975,6 +975,8 @@ void sun8i_vi_scaler_setup(struct sun8i_layer *layer,
>
>                 regmap_write(layer->regs,
>                              SUN50I_SCALER_VSU_SCALE_MODE(base), val);
> +               regmap_write(layer->regs,
> +                            SUN50I_SCALER_VSU_GLB_ALPHA(base), 0xff);
>         }
>
>         regmap_write(layer->regs,
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h
> index 245fe2f431c3..d89c0cbe188d 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_scaler.h
> @@ -36,6 +36,7 @@
>  #define SUN50I_SCALER_VSU_EDSCL_CTRL(base)             ((base) + 0x28)
>  #define SUN50I_SCALER_VSU_ANGLE_THR(base)              ((base) + 0x2c)
>  #define SUN8I_SCALER_VSU_OUTSIZE(base)         ((base) + 0x40)
> +#define SUN50I_SCALER_VSU_GLB_ALPHA(base)      ((base) + 0x44)
>  #define SUN8I_SCALER_VSU_YINSIZE(base)         ((base) + 0x80)
>  #define SUN8I_SCALER_VSU_YHSTEP(base)          ((base) + 0x88)
>  #define SUN8I_SCALER_VSU_YVSTEP(base)          ((base) + 0x8c)
> --
> 2.43.0
>

[PATCH 04/13] drm/sun4i: tcon-top: Keep mixer routes distinct

[Jernej Skrabec]

Both mixer selectors reset to TCON 0. Selecting the same TCON for both
mixers causes black or corrupted output.

When a route would collide, park the other mixer on another described
TCON, or an unused selector if none exists.

Since the TCON index is now used as a shift, also reject negative
values.

Tested on Orange Pi 3 with TCON_LCD0 and TCON_TV0.

Link: https://lore.kernel.org/linux-sunxi/Zn8GVkpwXwhaUFno@titan/
Link: https://lore.kernel.org/linux-sunxi/20241108-tcon_fix-v1-1-616218cc0d5f@jookia.org/
Fixes: 05db311a792d ("drm/sun4i: tcon-top: Add helpers for mux switching")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 71 +++++++++++++++++++++-----
 drivers/gpu/drm/sun4i/sun8i_tcon_top.h |  6 +++
 2 files changed, 65 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
index 8adda578c51b..9cbd655518b2 100644
--- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
+++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
@@ -25,6 +25,49 @@ static bool sun8i_tcon_top_node_is_tcon_top(struct device_node *node)
 	return !!of_match_node(sun8i_tcon_top_of_table, node);
 }
 
+static unsigned int sun8i_tcon_top_get_tcon_map(struct device_node *node)
+{
+	static const u32 out_ports[] = {
+		TCON_TOP_MIXER0_OUT_PORT,
+		TCON_TOP_MIXER1_OUT_PORT,
+	};
+	unsigned int i, map = 0;
+
+	for (i = 0; i < ARRAY_SIZE(out_ports); i++) {
+		struct device_node *port;
+
+		port = of_graph_get_port_by_id(node, out_ports[i]);
+		if (!port)
+			continue;
+
+		for_each_of_graph_port_endpoint(port, ep) {
+			struct of_endpoint endpoint;
+
+			if (of_graph_parse_endpoint(ep, &endpoint))
+				continue;
+
+			if (endpoint.id < TCON_TOP_PORT_TCON_NUM)
+				map |= BIT(endpoint.id);
+		}
+
+		of_node_put(port);
+	}
+
+	return map;
+}
+
+static unsigned int sun8i_tcon_top_park_index(struct sun8i_tcon_top *tcon_top,
+					      int tcon)
+{
+	unsigned int candidates;
+
+	candidates = tcon_top->tcon_map & ~BIT(tcon);
+	if (!candidates)
+		candidates = GENMASK(TCON_TOP_PORT_TCON_NUM - 1, 0) & ~BIT(tcon);
+
+	return ffs(candidates) - 1;
+}
+
 int sun8i_tcon_top_set_hdmi_src(struct device *dev, int tcon)
 {
 	struct sun8i_tcon_top *tcon_top = dev_get_drvdata(dev);
@@ -57,6 +100,7 @@ EXPORT_SYMBOL(sun8i_tcon_top_set_hdmi_src);
 int sun8i_tcon_top_de_config(struct device *dev, int mixer, int tcon)
 {
 	struct sun8i_tcon_top *tcon_top = dev_get_drvdata(dev);
+	u32 mixer_msk, other_msk;
 	unsigned long flags;
 	u32 reg;
 
@@ -70,21 +114,27 @@ int sun8i_tcon_top_de_config(struct device *dev, int mixer, int tcon)
 		return -EINVAL;
 	}
 
-	if (tcon > 3) {
-		dev_err(dev, "TCON index is too high!\n");
+	if (tcon < 0 || tcon >= TCON_TOP_PORT_TCON_NUM) {
+		dev_err(dev, "TCON index is invalid!\n");
 		return -EINVAL;
 	}
 
+	mixer_msk = mixer ? TCON_TOP_PORT_DE1_MSK : TCON_TOP_PORT_DE0_MSK;
+	other_msk = mixer ? TCON_TOP_PORT_DE0_MSK : TCON_TOP_PORT_DE1_MSK;
+
 	spin_lock_irqsave(&tcon_top->reg_lock, flags);
 
 	reg = readl(tcon_top->regs + TCON_TOP_PORT_SEL_REG);
-	if (mixer == 0) {
-		reg &= ~TCON_TOP_PORT_DE0_MSK;
-		reg |= FIELD_PREP(TCON_TOP_PORT_DE0_MSK, tcon);
-	} else {
-		reg &= ~TCON_TOP_PORT_DE1_MSK;
-		reg |= FIELD_PREP(TCON_TOP_PORT_DE1_MSK, tcon);
+
+	reg &= ~mixer_msk;
+	reg |= field_prep(mixer_msk, tcon);
+
+	if (field_get(other_msk, reg) == tcon) {
+		reg &= ~other_msk;
+		reg |= field_prep(other_msk,
+				  sun8i_tcon_top_park_index(tcon_top, tcon));
 	}
+
 	writel(reg, tcon_top->regs + TCON_TOP_PORT_SEL_REG);
 
 	spin_unlock_irqrestore(&tcon_top->reg_lock, flags);
@@ -143,6 +193,7 @@ static int sun8i_tcon_top_bind(struct device *dev, struct device *master,
 		return -ENOMEM;
 	clk_data->num = CLK_NUM;
 	tcon_top->clk_data = clk_data;
+	tcon_top->tcon_map = sun8i_tcon_top_get_tcon_map(dev->of_node);
 
 	spin_lock_init(&tcon_top->reg_lock);
 
@@ -175,10 +226,6 @@ static int sun8i_tcon_top_bind(struct device *dev, struct device *master,
 		goto err_assert_reset;
 	}
 
-	/*
-	 * At least on H6, some registers have some bits set by default
-	 * which may cause issues. Clear them here.
-	 */
 	writel(0, regs + TCON_TOP_PORT_SEL_REG);
 	writel(0, regs + TCON_TOP_GATE_SRC_REG);
 
diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.h b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
index 0390584a330e..2b887470a49f 100644
--- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
+++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.h
@@ -14,6 +14,10 @@
 #define TCON_TOP_PORT_SEL_REG		0x1C
 #define TCON_TOP_PORT_DE0_MSK			GENMASK(1, 0)
 #define TCON_TOP_PORT_DE1_MSK			GENMASK(5, 4)
+#define TCON_TOP_PORT_TCON_NUM			4
+
+#define TCON_TOP_MIXER0_OUT_PORT		1
+#define TCON_TOP_MIXER1_OUT_PORT		3
 
 #define TCON_TOP_GATE_SRC_REG		0x20
 #define TCON_TOP_HDMI_SRC_MSK			GENMASK(29, 28)
@@ -29,6 +33,8 @@ struct sun8i_tcon_top {
 	void __iomem			*regs;
 	struct reset_control		*rst;
 
+	unsigned int			tcon_map;
+
 	/*
 	 * spinlock is used to synchronize access to same
 	 * register where multiple clock gates can be set.
-- 
2.43.0

Re: [PATCH 04/13] drm/sun4i: tcon-top: Keep mixer routes distinct

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> Both mixer selectors reset to TCON 0. Selecting the same TCON for both
> mixers causes black or corrupted output.
>
> When a route would collide, park the other mixer on another described
> TCON, or an unused selector if none exists.
>
> Since the TCON index is now used as a shift, also reject negative
> values.
>
> Tested on Orange Pi 3 with TCON_LCD0 and TCON_TV0.
>
> Link: https://lore.kernel.org/linux-sunxi/Zn8GVkpwXwhaUFno@titan/
> Link: https://lore.kernel.org/linux-sunxi/20241108-tcon_fix-v1-1-616218cc0d5f@jookia.org/
> Fixes: 05db311a792d ("drm/sun4i: tcon-top: Add helpers for mux switching")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Reviewed-by: Chen-Yu Tsai <wens@kernel.org>

Though I have a couple nits.
> ---
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.c | 71 +++++++++++++++++++++-----
>  drivers/gpu/drm/sun4i/sun8i_tcon_top.h |  6 +++
>  2 files changed, 65 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> index 8adda578c51b..9cbd655518b2 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_tcon_top.c
> @@ -25,6 +25,49 @@ static bool sun8i_tcon_top_node_is_tcon_top(struct device_node *node)
>         return !!of_match_node(sun8i_tcon_top_of_table, node);
>  }
>
> +static unsigned int sun8i_tcon_top_get_tcon_map(struct device_node *node)
> +{
> +       static const u32 out_ports[] = {
> +               TCON_TOP_MIXER0_OUT_PORT,
> +               TCON_TOP_MIXER1_OUT_PORT,
> +       };
> +       unsigned int i, map = 0;
> +
> +       for (i = 0; i < ARRAY_SIZE(out_ports); i++) {

You can declare `i` directly in the for statement.

> +               struct device_node *port;
> +
> +               port = of_graph_get_port_by_id(node, out_ports[i]);

You could probably use the __free() cleanup macro and make `port`
scoped and put the reference automatically.

> +               if (!port)
> +                       continue;
> +
> +               for_each_of_graph_port_endpoint(port, ep) {
> +                       struct of_endpoint endpoint;
> +
> +                       if (of_graph_parse_endpoint(ep, &endpoint))
> +                               continue;
> +
> +                       if (endpoint.id < TCON_TOP_PORT_TCON_NUM)
> +                               map |= BIT(endpoint.id);
> +               }
> +
> +               of_node_put(port);

And this line wouldn't be needed.

> +       }
> +
> +       return map;
> +}
> +

[...]

[PATCH 05/13] drm/sun4i: tcon: Set output mux for DSI and LVDS

[Jernej Skrabec]

DSI and LVDS skip output mux setup, so TCON TOP cannot route the selected
mixer. Configure them like other channel 0 outputs.

In practice this matters for D1, where channel 0 TCONs are fed through
TCON TOP. The remaining set_mux implementations only handle TMDS and
return an error for other encoder types, as before.

Fixes: b9b52d2f4aaf ("drm/sun4i: Add support for D1 TCONs")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index 960e83c8291d..ea7c90fef316 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -717,9 +717,11 @@ void sun4i_tcon_mode_set(struct sun4i_tcon *tcon,
 	case DRM_MODE_ENCODER_DSI:
 		/* DSI is tied to special case of CPU interface */
 		sun4i_tcon0_mode_set_cpu(tcon, encoder, mode);
+		sun4i_tcon_set_mux(tcon, 0, encoder);
 		break;
 	case DRM_MODE_ENCODER_LVDS:
 		sun4i_tcon0_mode_set_lvds(tcon, encoder, mode);
+		sun4i_tcon_set_mux(tcon, 0, encoder);
 		break;
 	case DRM_MODE_ENCODER_NONE:
 		sun4i_tcon0_mode_set_rgb(tcon, encoder, mode);
-- 
2.43.0

Re: [PATCH 05/13] drm/sun4i: tcon: Set output mux for DSI and LVDS

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> DSI and LVDS skip output mux setup, so TCON TOP cannot route the selected
> mixer. Configure them like other channel 0 outputs.
>
> In practice this matters for D1, where channel 0 TCONs are fed through
> TCON TOP. The remaining set_mux implementations only handle TMDS and
> return an error for other encoder types, as before.
>
> Fixes: b9b52d2f4aaf ("drm/sun4i: Add support for D1 TCONs")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index 960e83c8291d..ea7c90fef316 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -717,9 +717,11 @@ void sun4i_tcon_mode_set(struct sun4i_tcon *tcon,
>         case DRM_MODE_ENCODER_DSI:
>                 /* DSI is tied to special case of CPU interface */
>                 sun4i_tcon0_mode_set_cpu(tcon, encoder, mode);
> +               sun4i_tcon_set_mux(tcon, 0, encoder);
>                 break;
>         case DRM_MODE_ENCODER_LVDS:
>                 sun4i_tcon0_mode_set_lvds(tcon, encoder, mode);
> +               sun4i_tcon_set_mux(tcon, 0, encoder);
>                 break;
>         case DRM_MODE_ENCODER_NONE:
>                 sun4i_tcon0_mode_set_rgb(tcon, encoder, mode);
> --
> 2.43.0
>
>

[PATCH 06/13] drm/sun4i: tcon: Drop TCON TOP device reference

[Jernej Skrabec]

of_find_device_by_node() takes a device reference. Drop it after mux
configuration succeeds.

Fixes: 0305189afb32 ("drm/sun4i: tcon: Add support for R40 TCON")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index ea7c90fef316..de2d66623f7a 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -1409,7 +1409,7 @@ static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
 {
 	struct device_node *port, *remote;
 	struct platform_device *pdev;
-	int id, ret;
+	int id, ret = 0;
 
 	/* find TCON TOP platform device and TCON id */
 
@@ -1432,21 +1432,20 @@ static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
 	if (IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) &&
 	    encoder->encoder_type == DRM_MODE_ENCODER_TMDS) {
 		ret = sun8i_tcon_top_set_hdmi_src(&pdev->dev, id);
-		if (ret) {
-			put_device(&pdev->dev);
-			return ret;
-		}
+		if (ret)
+			goto out_put_device;
 	}
 
 	if (IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP)) {
 		ret = sun8i_tcon_top_de_config(&pdev->dev, tcon->id, id);
-		if (ret) {
-			put_device(&pdev->dev);
-			return ret;
-		}
+		if (ret)
+			goto out_put_device;
 	}
 
-	return 0;
+out_put_device:
+	put_device(&pdev->dev);
+
+	return ret;
 }
 
 static const struct sun4i_tcon_quirks sun4i_a10_quirks = {
-- 
2.43.0

Re: [PATCH 06/13] drm/sun4i: tcon: Drop TCON TOP device reference

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> of_find_device_by_node() takes a device reference. Drop it after mux
> configuration succeeds.
>
> Fixes: 0305189afb32 ("drm/sun4i: tcon: Add support for R40 TCON")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

Though I'd probably rewrite it with

    struct platform_device *pdev __free(platform_device_put) =
            of_find_device_by_node();

and then all paths can just return instead of needing the goto.

> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index ea7c90fef316..de2d66623f7a 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -1409,7 +1409,7 @@ static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
>  {
>         struct device_node *port, *remote;
>         struct platform_device *pdev;
> -       int id, ret;
> +       int id, ret = 0;
>
>         /* find TCON TOP platform device and TCON id */
>
> @@ -1432,21 +1432,20 @@ static int sun8i_r40_tcon_tv_set_mux(struct sun4i_tcon *tcon,
>         if (IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP) &&
>             encoder->encoder_type == DRM_MODE_ENCODER_TMDS) {
>                 ret = sun8i_tcon_top_set_hdmi_src(&pdev->dev, id);
> -               if (ret) {
> -                       put_device(&pdev->dev);
> -                       return ret;
> -               }
> +               if (ret)
> +                       goto out_put_device;
>         }
>
>         if (IS_ENABLED(CONFIG_DRM_SUN8I_TCON_TOP)) {
>                 ret = sun8i_tcon_top_de_config(&pdev->dev, tcon->id, id);
> -               if (ret) {
> -                       put_device(&pdev->dev);
> -                       return ret;
> -               }
> +               if (ret)
> +                       goto out_put_device;
>         }
>
> -       return 0;
> +out_put_device:
> +       put_device(&pdev->dev);
> +
> +       return ret;
>  }
>
>  static const struct sun4i_tcon_quirks sun4i_a10_quirks = {
> --
> 2.43.0
>
>

[PATCH 07/13] drm/sun4i: hdmi: Don't leak sync polarity bits into packet control

[Jernej Skrabec]

sun4i_hdmi_enable() keeps using the same variable after it programmed
the video timing polarity register with it. The leftover TX_CLK, HSYNC
and VSYNC bits are then ORed into the packet control register, where
each nibble selects the packet type sent in one slot.

As a result, slot 0 selects packet type 3 instead of the AVI infoframe
whenever the mode has positive HSYNC polarity, and the TX_CLK bits set
nibbles which the driver never programs.

Assign the packet types instead of ORing them into the stale value.

Fixes: 9ca6bc246035 ("drm/sun4i: hdmi: Move mode_set into enable")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index 74c7c3720ba8..a27d394d0570 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -161,7 +161,7 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder,
 
 	drm_atomic_helper_connector_hdmi_update_infoframes(connector, state);
 
-	val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
+	val = SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
 	val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END);
 	writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0));
 
-- 
2.43.0

Re: [PATCH 07/13] drm/sun4i: hdmi: Don't leak sync polarity bits into packet control

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> sun4i_hdmi_enable() keeps using the same variable after it programmed
> the video timing polarity register with it. The leftover TX_CLK, HSYNC
> and VSYNC bits are then ORed into the packet control register, where
> each nibble selects the packet type sent in one slot.
>
> As a result, slot 0 selects packet type 3 instead of the AVI infoframe
> whenever the mode has positive HSYNC polarity, and the TX_CLK bits set
> nibbles which the driver never programs.
>
> Assign the packet types instead of ORing them into the stale value.
>
> Fixes: 9ca6bc246035 ("drm/sun4i: hdmi: Move mode_set into enable")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> index 74c7c3720ba8..a27d394d0570 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
> @@ -161,7 +161,7 @@ static void sun4i_hdmi_enable(struct drm_encoder *encoder,
>
>         drm_atomic_helper_connector_hdmi_update_infoframes(connector, state);
>
> -       val |= SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
> +       val = SUN4I_HDMI_PKT_CTRL_TYPE(0, SUN4I_HDMI_PKT_AVI);
>         val |= SUN4I_HDMI_PKT_CTRL_TYPE(1, SUN4I_HDMI_PKT_END);
>         writel(val, hdmi->base + SUN4I_HDMI_PKT_CTRL_REG(0));
>
> --
> 2.43.0
>
>

[PATCH 08/13] drm/sun4i: crtc: Propagate layer initialization error

[Jernej Skrabec]

sun4i_crtc_init() returns plain NULL when layer initialization fails,
while all its other error paths return an error pointer. The only
caller, sun4i_tcon_bind(), checks the result with IS_ERR() and happily
continues with tcon->crtc set to NULL. sun4i_rgb_init() and
sun4i_lvds_init() then dereference it in drm_crtc_mask(), which
oopses.

Return the error pointer instead.

Fixes: dcd215801b02 ("drm/sun4i: Drop primary layer pointer from sun4i_drv")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
index c2f7f69baf75..3fa1f4fda0f0 100644
--- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
@@ -208,7 +208,7 @@ struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm,
 	planes = sunxi_engine_layers_init(drm, engine);
 	if (IS_ERR(planes)) {
 		dev_err(drm->dev, "Couldn't create the planes\n");
-		return NULL;
+		return ERR_CAST(planes);
 	}
 
 	/* find primary and cursor planes for drm_crtc_init_with_planes */
-- 
2.43.0

Re: [PATCH 08/13] drm/sun4i: crtc: Propagate layer initialization error

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> sun4i_crtc_init() returns plain NULL when layer initialization fails,
> while all its other error paths return an error pointer. The only
> caller, sun4i_tcon_bind(), checks the result with IS_ERR() and happily
> continues with tcon->crtc set to NULL. sun4i_rgb_init() and
> sun4i_lvds_init() then dereference it in drm_crtc_mask(), which
> oopses.
>
> Return the error pointer instead.
>
> Fixes: dcd215801b02 ("drm/sun4i: Drop primary layer pointer from sun4i_drv")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun4i_crtc.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_crtc.c b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> index c2f7f69baf75..3fa1f4fda0f0 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_crtc.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_crtc.c
> @@ -208,7 +208,7 @@ struct sun4i_crtc *sun4i_crtc_init(struct drm_device *drm,
>         planes = sunxi_engine_layers_init(drm, engine);
>         if (IS_ERR(planes)) {
>                 dev_err(drm->dev, "Couldn't create the planes\n");
> -               return NULL;
> +               return ERR_CAST(planes);
>         }
>
>         /* find primary and cursor planes for drm_crtc_init_with_planes */
> --
> 2.43.0
>
>

[PATCH 09/13] drm/sun4i: tcon: Drop remote endpoint reference

[Jernej Skrabec]

sun4i_tcon_of_get_id_from_port() never drops the reference taken by
of_graph_get_remote_endpoint(). The function is not only called during
bind, but also on every mode set through sun8i_r40_tcon_tv_set_mux(),
so the leak accumulates.

Fixes: e8d5bbf7f4c4 ("drm/sun4i: tcon: get TCON ID and matching engine with remote endpoint ID")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_tcon.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
index de2d66623f7a..67047e3b657f 100644
--- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
+++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
@@ -972,6 +972,7 @@ static int sun4i_tcon_of_get_id_from_port(struct device_node *port)
 			continue;
 
 		ret = of_property_read_u32(remote, "reg", &reg);
+		of_node_put(remote);
 		if (ret)
 			continue;
 
-- 
2.43.0

Re: [PATCH 09/13] drm/sun4i: tcon: Drop remote endpoint reference

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> sun4i_tcon_of_get_id_from_port() never drops the reference taken by
> of_graph_get_remote_endpoint(). The function is not only called during
> bind, but also on every mode set through sun8i_r40_tcon_tv_set_mux(),
> so the leak accumulates.
>
> Fixes: e8d5bbf7f4c4 ("drm/sun4i: tcon: get TCON ID and matching engine with remote endpoint ID")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun4i_tcon.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_tcon.c b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> index de2d66623f7a..67047e3b657f 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_tcon.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_tcon.c
> @@ -972,6 +972,7 @@ static int sun4i_tcon_of_get_id_from_port(struct device_node *port)
>                         continue;
>
>                 ret = of_property_read_u32(remote, "reg", &reg);
> +               of_node_put(remote);
>                 if (ret)
>                         continue;
>
> --
> 2.43.0
>
>

[PATCH 10/13] drm/sun4i: dw-hdmi: Drop TCON TOP port reference

[Jernej Skrabec]

When the HDMI controller is fed by TCON TOP, the port node used to
enumerate the possible CRTCs is never released.

Fixes: 57e23de02f48 ("drm/sun4i: DW HDMI: Expand algorithm for possible crtcs")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
index 96532709c2a7..51cdfa1ae3bf 100644
--- a/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
+++ b/drivers/gpu/drm/sun4i/sun8i_dw_hdmi.c
@@ -83,6 +83,8 @@ static u32 sun8i_dw_hdmi_find_possible_crtcs(struct drm_device *drm,
 				of_node_put(remote_port);
 			}
 		}
+
+		of_node_put(port);
 	} else {
 		crtcs = drm_of_find_possible_crtcs(drm, node);
 	}
-- 
2.43.0

Re: [PATCH 10/13] drm/sun4i: dw-hdmi: Drop TCON TOP port reference

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> When the HDMI controller is fed by TCON TOP, the port node used to
> enumerate the possible CRTCs is never released.
>
> Fixes: 57e23de02f48 ("drm/sun4i: DW HDMI: Expand algorithm for possible crtcs")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

[PATCH 11/13] drm/sun4i: Drop node references while building component list

[Jernej Skrabec]

Two references are leaked every time the display pipeline is walked:
the output port node in sun4i_drv_traverse_endpoints(), which was never
released since the driver was introduced, and each node taken out of
the endpoint fifo in sun4i_drv_probe(), which stopped being released
when the fifo was introduced. The latter is still safe to drop right
after processing, since drm_of_component_match_add() takes its own
reference.

Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
Fixes: 8b11aaface2b ("drm/sun4i: Implement endpoint parsing using kfifo")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun4i_drv.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index 8a409eee1dca..e9baca360160 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -289,6 +289,8 @@ static void sun4i_drv_traverse_endpoints(struct endpoint_list *list,
 
 		kfifo_put(&list->fifo, remote);
 	}
+
+	of_node_put(port);
 }
 
 static int sun4i_drv_add_endpoints(struct device *dev,
@@ -394,6 +396,7 @@ static int sun4i_drv_probe(struct platform_device *pdev)
 		/* process this endpoint */
 		ret = sun4i_drv_add_endpoints(&pdev->dev, &list, &match,
 					      endpoint);
+		of_node_put(endpoint);
 
 		/* sun4i_drv_add_endpoints can fail to allocate memory */
 		if (ret < 0)
-- 
2.43.0

Re: [PATCH 11/13] drm/sun4i: Drop node references while building component list

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> Two references are leaked every time the display pipeline is walked:
> the output port node in sun4i_drv_traverse_endpoints(), which was never
> released since the driver was introduced, and each node taken out of
> the endpoint fifo in sun4i_drv_probe(), which stopped being released
> when the fifo was introduced. The latter is still safe to drop right
> after processing, since drm_of_component_match_add() takes its own
> reference.
>
> Fixes: 9026e0d122ac ("drm: Add Allwinner A10 Display Engine support")
> Fixes: 8b11aaface2b ("drm/sun4i: Implement endpoint parsing using kfifo")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun4i_drv.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index 8a409eee1dca..e9baca360160 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -289,6 +289,8 @@ static void sun4i_drv_traverse_endpoints(struct endpoint_list *list,
>
>                 kfifo_put(&list->fifo, remote);
>         }
> +
> +       of_node_put(port);
>  }
>
>  static int sun4i_drv_add_endpoints(struct device *dev,
> @@ -394,6 +396,7 @@ static int sun4i_drv_probe(struct platform_device *pdev)
>                 /* process this endpoint */
>                 ret = sun4i_drv_add_endpoints(&pdev->dev, &list, &match,
>                                               endpoint);
> +               of_node_put(endpoint);
>
>                 /* sun4i_drv_add_endpoints can fail to allocate memory */
>                 if (ret < 0)
> --
> 2.43.0
>

[PATCH 12/13] drm/sun4i: hdmi-phy: Fix H6 8-bit MPLL config at 594 MHz

[Jernej Skrabec]

The 8-bit entry of the last MPLL row (594 MHz) doesn't lock reliably on
H6. 4K@60 RGB/YUV444, which is the mode that reaches this entry, doesn't
come up.

Align the value with the vendor driver. Other entries are left alone,
they are used by lower pixel clocks which work fine.

Tested with 4K@60 on a LG TV.

Fixes: 0fb4b858b102 ("drm/sun4i: Add support for H6 HDMI PHY")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
index 4fa69c463dc4..489ea94693ff 100644
--- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
+++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
@@ -91,7 +91,7 @@ static const struct dw_hdmi_mpll_config sun50i_h6_mpll_cfg[] = {
 		},
 	},  {
 		594000000, {
-			{ 0x1a40, 0x0003 },
+			{ 0x1a7c, 0x0003 },
 			{ 0x3b4c, 0x0003 },
 			{ 0x5a64, 0x0003 },
 		},
-- 
2.43.0

Re: [PATCH 12/13] drm/sun4i: hdmi-phy: Fix H6 8-bit MPLL config at 594 MHz

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> The 8-bit entry of the last MPLL row (594 MHz) doesn't lock reliably on
> H6. 4K@60 RGB/YUV444, which is the mode that reaches this entry, doesn't
> come up.
>
> Align the value with the vendor driver. Other entries are left alone,
> they are used by lower pixel clocks which work fine.
>
> Tested with 4K@60 on a LG TV.
>
> Fixes: 0fb4b858b102 ("drm/sun4i: Add support for H6 HDMI PHY")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>

Acked-by: Chen-Yu Tsai <wens@kernel.org>

> ---
>  drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> index 4fa69c463dc4..489ea94693ff 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_hdmi_phy.c
> @@ -91,7 +91,7 @@ static const struct dw_hdmi_mpll_config sun50i_h6_mpll_cfg[] = {
>                 },
>         },  {
>                 594000000, {
> -                       { 0x1a40, 0x0003 },
> +                       { 0x1a7c, 0x0003 },
>                         { 0x3b4c, 0x0003 },
>                         { 0x5a64, 0x0003 },
>                 },
> --
> 2.43.0
>

[PATCH 13/13] drm/sun4i: Align VI buffer addresses for subsampled formats

[Jernej Skrabec]

This is a partial revert of commit 79ac1c945ab8 ("drm/sun4i: layers:
Use drm_fb_dma_get_gem_addr() to get display memory").

Chroma must start at the beginning of a subsampling block, for example
chroma start address for NV12 must be aligned to 2 pixels.
drm_fb_dma_get_gem_addr() offsets luma by the exact source coordinates
and chroma by the coordinates divided by the subsampling factor, so for
odd offsets both planes no longer describe the same pixel, which the
Display Engine scaler can't handle.

Align source coordinates down for all planes instead. Remaining shift
of one pixel is already compensated with scaler phase shift in
sun8i_vi_layer_update_coord().

Fixes: 79ac1c945ab8 ("drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory")
Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
---
 drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
index 09f668c8af24..ad036cb9d88e 100644
--- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
+++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
@@ -197,15 +197,31 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
 	struct drm_plane_state *state = plane->state;
 	struct drm_framebuffer *fb = state->fb;
 	const struct drm_format_info *format = fb->format;
+	struct drm_gem_dma_object *gem;
+	u32 dx, dy, src_x, src_y;
 	dma_addr_t dma_addr;
 	u32 ch_base;
 	int i;
 
 	ch_base = sun8i_channel_base(layer);
 
+	/* Adjust x and y to be divisible by subsampling factor */
+	src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
+	src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
+
 	for (i = 0; i < format->num_planes; i++) {
-		/* Get the start of the displayed memory */
-		dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
+		gem = drm_fb_dma_get_gem_obj(fb, i);
+		dma_addr = gem->dma_addr + fb->offsets[i];
+
+		dx = src_x;
+		dy = src_y;
+		if (i > 0) {
+			dx /= format->hsub;
+			dy /= format->vsub;
+		}
+
+		dma_addr += dx * format->cpp[i];
+		dma_addr += dy * fb->pitches[i];
 
 		/* Set the line width */
 		DRM_DEBUG_DRIVER("Layer %d. line width: %d bytes\n",
-- 
2.43.0

Re: [PATCH 13/13] drm/sun4i: Align VI buffer addresses for subsampled formats

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
>
> This is a partial revert of commit 79ac1c945ab8 ("drm/sun4i: layers:
> Use drm_fb_dma_get_gem_addr() to get display memory").
>
> Chroma must start at the beginning of a subsampling block, for example
> chroma start address for NV12 must be aligned to 2 pixels.
> drm_fb_dma_get_gem_addr() offsets luma by the exact source coordinates
> and chroma by the coordinates divided by the subsampling factor, so for
> odd offsets both planes no longer describe the same pixel, which the
> Display Engine scaler can't handle.
>
> Align source coordinates down for all planes instead. Remaining shift
> of one pixel is already compensated with scaler phase shift in
> sun8i_vi_layer_update_coord().

Well I think this applies to the format in general, and probably should
be fixed in drm_fb_dma_get_gem_addr() instead?

> Fixes: 79ac1c945ab8 ("drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory")
> Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> ---
>  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 20 ++++++++++++++++++--
>  1 file changed, 18 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> index 09f668c8af24..ad036cb9d88e 100644
> --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> @@ -197,15 +197,31 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
>         struct drm_plane_state *state = plane->state;
>         struct drm_framebuffer *fb = state->fb;
>         const struct drm_format_info *format = fb->format;
> +       struct drm_gem_dma_object *gem;
> +       u32 dx, dy, src_x, src_y;
>         dma_addr_t dma_addr;
>         u32 ch_base;
>         int i;
>
>         ch_base = sun8i_channel_base(layer);
>
> +       /* Adjust x and y to be divisible by subsampling factor */
> +       src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
> +       src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);

AFAICT the only difference compared to drm_fb_dma_get_gem_addr()
is the masking here, i.e. round_down().

> +
>         for (i = 0; i < format->num_planes; i++) {
> -               /* Get the start of the displayed memory */
> -               dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
> +               gem = drm_fb_dma_get_gem_obj(fb, i);
> +               dma_addr = gem->dma_addr + fb->offsets[i];
> +
> +               dx = src_x;
> +               dy = src_y;
> +               if (i > 0) {
> +                       dx /= format->hsub;
> +                       dy /= format->vsub;
> +               }
> +
> +               dma_addr += dx * format->cpp[i];
> +               dma_addr += dy * fb->pitches[i];


Where as the helper has (or used to have before the blocksize stuff):

    paddr += (format->cpp[plane] * (state->src_x >> 16)) / fb->format->hsub;
    paddr += (fb->pitches[plane] * (state->src_y >> 16)) / fb->format->vsub;

Am I missing something?


ChenYu

>
>                 /* Set the line width */
>                 DRM_DEBUG_DRIVER("Layer %d. line width: %d bytes\n",
> --
> 2.43.0
>

Re: [PATCH 13/13] drm/sun4i: Align VI buffer addresses for subsampled formats

[Chen-Yu Tsai]

On Tue, Aug 4, 2026 at 1:25 AM Chen-Yu Tsai <wens@kernel.org> wrote:
>
> On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> >
> > This is a partial revert of commit 79ac1c945ab8 ("drm/sun4i: layers:
> > Use drm_fb_dma_get_gem_addr() to get display memory").
> >
> > Chroma must start at the beginning of a subsampling block, for example
> > chroma start address for NV12 must be aligned to 2 pixels.
> > drm_fb_dma_get_gem_addr() offsets luma by the exact source coordinates
> > and chroma by the coordinates divided by the subsampling factor, so for
> > odd offsets both planes no longer describe the same pixel, which the
> > Display Engine scaler can't handle.
> >
> > Align source coordinates down for all planes instead. Remaining shift
> > of one pixel is already compensated with scaler phase shift in
> > sun8i_vi_layer_update_coord().
>
> Well I think this applies to the format in general, and probably should
> be fixed in drm_fb_dma_get_gem_addr() instead?
>
> > Fixes: 79ac1c945ab8 ("drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory")
> > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > ---
> >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 20 ++++++++++++++++++--
> >  1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > index 09f668c8af24..ad036cb9d88e 100644
> > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > @@ -197,15 +197,31 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
> >         struct drm_plane_state *state = plane->state;
> >         struct drm_framebuffer *fb = state->fb;
> >         const struct drm_format_info *format = fb->format;
> > +       struct drm_gem_dma_object *gem;
> > +       u32 dx, dy, src_x, src_y;
> >         dma_addr_t dma_addr;
> >         u32 ch_base;
> >         int i;
> >
> >         ch_base = sun8i_channel_base(layer);
> >
> > +       /* Adjust x and y to be divisible by subsampling factor */
> > +       src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
> > +       src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
>
> AFAICT the only difference compared to drm_fb_dma_get_gem_addr()
> is the masking here, i.e. round_down().
>
> > +
> >         for (i = 0; i < format->num_planes; i++) {
> > -               /* Get the start of the displayed memory */
> > -               dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
> > +               gem = drm_fb_dma_get_gem_obj(fb, i);
> > +               dma_addr = gem->dma_addr + fb->offsets[i];
> > +
> > +               dx = src_x;
> > +               dy = src_y;
> > +               if (i > 0) {
> > +                       dx /= format->hsub;
> > +                       dy /= format->vsub;
> > +               }
> > +
> > +               dma_addr += dx * format->cpp[i];
> > +               dma_addr += dy * fb->pitches[i];
>
>
> Where as the helper has (or used to have before the blocksize stuff):
>
>     paddr += (format->cpp[plane] * (state->src_x >> 16)) / fb->format->hsub;
>     paddr += (fb->pitches[plane] * (state->src_y >> 16)) / fb->format->vsub;
>
> Am I missing something?

After some headbanging on my end I see that the offset for the Y plane
needs to be rounded down.

But instead of reverting the whole thing and open-coding the helper
again, could you adjust the address returned by the helper for odd
offsets?

And just a heads up, this also needs a clipped version of
drm_fb_dma_get_gem_addr() as sun8i_ui_layer_update_coord() uses the
clipped dimensions. I am currently working on this part.


ChenYu

> >
> >                 /* Set the line width */
> >                 DRM_DEBUG_DRIVER("Layer %d. line width: %d bytes\n",
> > --
> > 2.43.0
> >

Re: [PATCH 13/13] drm/sun4i: Align VI buffer addresses for subsampled formats

[Jernej Škrabec]

Dne torek, 4. avgust 2026 ob 13:14:38 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> On Tue, Aug 4, 2026 at 1:25 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> >
> > On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> > >
> > > This is a partial revert of commit 79ac1c945ab8 ("drm/sun4i: layers:
> > > Use drm_fb_dma_get_gem_addr() to get display memory").
> > >
> > > Chroma must start at the beginning of a subsampling block, for example
> > > chroma start address for NV12 must be aligned to 2 pixels.
> > > drm_fb_dma_get_gem_addr() offsets luma by the exact source coordinates
> > > and chroma by the coordinates divided by the subsampling factor, so for
> > > odd offsets both planes no longer describe the same pixel, which the
> > > Display Engine scaler can't handle.
> > >
> > > Align source coordinates down for all planes instead. Remaining shift
> > > of one pixel is already compensated with scaler phase shift in
> > > sun8i_vi_layer_update_coord().
> >
> > Well I think this applies to the format in general, and probably should
> > be fixed in drm_fb_dma_get_gem_addr() instead?
> >
> > > Fixes: 79ac1c945ab8 ("drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory")
> > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > ---
> > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 20 ++++++++++++++++++--
> > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > index 09f668c8af24..ad036cb9d88e 100644
> > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > @@ -197,15 +197,31 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
> > >         struct drm_plane_state *state = plane->state;
> > >         struct drm_framebuffer *fb = state->fb;
> > >         const struct drm_format_info *format = fb->format;
> > > +       struct drm_gem_dma_object *gem;
> > > +       u32 dx, dy, src_x, src_y;
> > >         dma_addr_t dma_addr;
> > >         u32 ch_base;
> > >         int i;
> > >
> > >         ch_base = sun8i_channel_base(layer);
> > >
> > > +       /* Adjust x and y to be divisible by subsampling factor */
> > > +       src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
> > > +       src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
> >
> > AFAICT the only difference compared to drm_fb_dma_get_gem_addr()
> > is the masking here, i.e. round_down().
> >
> > > +
> > >         for (i = 0; i < format->num_planes; i++) {
> > > -               /* Get the start of the displayed memory */
> > > -               dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
> > > +               gem = drm_fb_dma_get_gem_obj(fb, i);
> > > +               dma_addr = gem->dma_addr + fb->offsets[i];
> > > +
> > > +               dx = src_x;
> > > +               dy = src_y;
> > > +               if (i > 0) {
> > > +                       dx /= format->hsub;
> > > +                       dy /= format->vsub;
> > > +               }
> > > +
> > > +               dma_addr += dx * format->cpp[i];
> > > +               dma_addr += dy * fb->pitches[i];
> >
> >
> > Where as the helper has (or used to have before the blocksize stuff):
> >
> >     paddr += (format->cpp[plane] * (state->src_x >> 16)) / fb->format->hsub;
> >     paddr += (fb->pitches[plane] * (state->src_y >> 16)) / fb->format->vsub;
> >
> > Am I missing something?
> 
> After some headbanging on my end I see that the offset for the Y plane
> needs to be rounded down.
> 
> But instead of reverting the whole thing and open-coding the helper
> again, could you adjust the address returned by the helper for odd
> offsets?

Yes, that's also an option. I'll do it in v2.

> 
> And just a heads up, this also needs a clipped version of
> drm_fb_dma_get_gem_addr() as sun8i_ui_layer_update_coord() uses the
> clipped dimensions. I am currently working on this part.

Can you explain a bit more? I don't see why it needs any adjustement.

Best regards,
Jernej

> 
> 
> ChenYu
> 
> > >
> > >                 /* Set the line width */
> > >                 DRM_DEBUG_DRIVER("Layer %d. line width: %d bytes\n",
> > > --
> > > 2.43.0
> > >
> 

Re: [PATCH 13/13] drm/sun4i: Align VI buffer addresses for subsampled formats

[Chen-Yu Tsai]

On Wed, Aug 5, 2026 at 12:25 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
>
> Dne torek, 4. avgust 2026 ob 13:14:38 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> > On Tue, Aug 4, 2026 at 1:25 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> > >
> > > On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> > > >
> > > > This is a partial revert of commit 79ac1c945ab8 ("drm/sun4i: layers:
> > > > Use drm_fb_dma_get_gem_addr() to get display memory").
> > > >
> > > > Chroma must start at the beginning of a subsampling block, for example
> > > > chroma start address for NV12 must be aligned to 2 pixels.
> > > > drm_fb_dma_get_gem_addr() offsets luma by the exact source coordinates
> > > > and chroma by the coordinates divided by the subsampling factor, so for
> > > > odd offsets both planes no longer describe the same pixel, which the
> > > > Display Engine scaler can't handle.
> > > >
> > > > Align source coordinates down for all planes instead. Remaining shift
> > > > of one pixel is already compensated with scaler phase shift in
> > > > sun8i_vi_layer_update_coord().
> > >
> > > Well I think this applies to the format in general, and probably should
> > > be fixed in drm_fb_dma_get_gem_addr() instead?
> > >
> > > > Fixes: 79ac1c945ab8 ("drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory")
> > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > > ---
> > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 20 ++++++++++++++++++--
> > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > > index 09f668c8af24..ad036cb9d88e 100644
> > > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > > @@ -197,15 +197,31 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
> > > >         struct drm_plane_state *state = plane->state;
> > > >         struct drm_framebuffer *fb = state->fb;
> > > >         const struct drm_format_info *format = fb->format;
> > > > +       struct drm_gem_dma_object *gem;
> > > > +       u32 dx, dy, src_x, src_y;
> > > >         dma_addr_t dma_addr;
> > > >         u32 ch_base;
> > > >         int i;
> > > >
> > > >         ch_base = sun8i_channel_base(layer);
> > > >
> > > > +       /* Adjust x and y to be divisible by subsampling factor */
> > > > +       src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
> > > > +       src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
> > >
> > > AFAICT the only difference compared to drm_fb_dma_get_gem_addr()
> > > is the masking here, i.e. round_down().
> > >
> > > > +
> > > >         for (i = 0; i < format->num_planes; i++) {
> > > > -               /* Get the start of the displayed memory */
> > > > -               dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
> > > > +               gem = drm_fb_dma_get_gem_obj(fb, i);
> > > > +               dma_addr = gem->dma_addr + fb->offsets[i];
> > > > +
> > > > +               dx = src_x;
> > > > +               dy = src_y;
> > > > +               if (i > 0) {
> > > > +                       dx /= format->hsub;
> > > > +                       dy /= format->vsub;
> > > > +               }
> > > > +
> > > > +               dma_addr += dx * format->cpp[i];
> > > > +               dma_addr += dy * fb->pitches[i];
> > >
> > >
> > > Where as the helper has (or used to have before the blocksize stuff):
> > >
> > >     paddr += (format->cpp[plane] * (state->src_x >> 16)) / fb->format->hsub;
> > >     paddr += (fb->pitches[plane] * (state->src_y >> 16)) / fb->format->vsub;
> > >
> > > Am I missing something?
> >
> > After some headbanging on my end I see that the offset for the Y plane
> > needs to be rounded down.
> >
> > But instead of reverting the whole thing and open-coding the helper
> > again, could you adjust the address returned by the helper for odd
> > offsets?
>
> Yes, that's also an option. I'll do it in v2.
>
> >
> > And just a heads up, this also needs a clipped version of
> > drm_fb_dma_get_gem_addr() as sun8i_ui_layer_update_coord() uses the
> > clipped dimensions. I am currently working on this part.
>
> Can you explain a bit more? I don't see why it needs any adjustement.

My understanding is that drm_atomic_helper_check_plane_state() calculates
the "clipped" rectangles for the plane using values from userspace in
state->src_[xywh] and state->crtc_[xywh] and puts them in state->src
and state->dst, respectively. If the overlay is moved partially outside
the screen, the overlay is "clipped".


Say we have a screen of 1920x1080, with an overlay buffer that is 1280x720.
Say state->src_x and state->src_y are (-50, 0), given by userspace.
drm_atomic_helper_check_plane_state() will calculate the clipped & scaled
rectangles and put them in state->src. This latter rectangle is what is
used sun8i_ui_layer_update_coord().

So we would have:

    (src_x, src_y) = (-50, 0), (src_w, src_h) = (1280, 720)

The clipped numbers are

    (src.x1, src.y1) = (0, 0), (src.x2, src.y2) = (1230, 720)

src, not src_[xywh], is what sun8i layers uses to program the coordinates,
and prior to the drm_fb_dma_get_gem_addr() conversion, also to calculate
the buffer start address. drm_fb_dma_get_gem_addr() however uses src_[xy]
to calculate the address.


Essentially, when overlaying a clipped plane, the start address needs to
be adjusted if the source (top left) offset is outside the screen.
clipping == automatic cropping to fit the screen.

I don't know if userspace applications routinely do this, but I think this
needs to be restored to the prior behavior.


ChenYu

Re: [PATCH 13/13] drm/sun4i: Align VI buffer addresses for subsampled formats

[Jernej Škrabec]

Dne torek, 4. avgust 2026 ob 19:04:10 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> On Wed, Aug 5, 2026 at 12:25 AM Jernej Škrabec <jernej.skrabec@gmail.com> wrote:
> >
> > Dne torek, 4. avgust 2026 ob 13:14:38 Srednjeevropski poletni čas je Chen-Yu Tsai napisal(a):
> > > On Tue, Aug 4, 2026 at 1:25 AM Chen-Yu Tsai <wens@kernel.org> wrote:
> > > >
> > > > On Tue, Aug 4, 2026 at 12:11 AM Jernej Skrabec <jernej.skrabec@gmail.com> wrote:
> > > > >
> > > > > This is a partial revert of commit 79ac1c945ab8 ("drm/sun4i: layers:
> > > > > Use drm_fb_dma_get_gem_addr() to get display memory").
> > > > >
> > > > > Chroma must start at the beginning of a subsampling block, for example
> > > > > chroma start address for NV12 must be aligned to 2 pixels.
> > > > > drm_fb_dma_get_gem_addr() offsets luma by the exact source coordinates
> > > > > and chroma by the coordinates divided by the subsampling factor, so for
> > > > > odd offsets both planes no longer describe the same pixel, which the
> > > > > Display Engine scaler can't handle.
> > > > >
> > > > > Align source coordinates down for all planes instead. Remaining shift
> > > > > of one pixel is already compensated with scaler phase shift in
> > > > > sun8i_vi_layer_update_coord().
> > > >
> > > > Well I think this applies to the format in general, and probably should
> > > > be fixed in drm_fb_dma_get_gem_addr() instead?
> > > >
> > > > > Fixes: 79ac1c945ab8 ("drm/sun4i: layers: Use drm_fb_dma_get_gem_addr() to get display memory")
> > > > > Signed-off-by: Jernej Skrabec <jernej.skrabec@gmail.com>
> > > > > ---
> > > > >  drivers/gpu/drm/sun4i/sun8i_vi_layer.c | 20 ++++++++++++++++++--
> > > > >  1 file changed, 18 insertions(+), 2 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > > > index 09f668c8af24..ad036cb9d88e 100644
> > > > > --- a/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > > > +++ b/drivers/gpu/drm/sun4i/sun8i_vi_layer.c
> > > > > @@ -197,15 +197,31 @@ static void sun8i_vi_layer_update_buffer(struct sun8i_layer *layer,
> > > > >         struct drm_plane_state *state = plane->state;
> > > > >         struct drm_framebuffer *fb = state->fb;
> > > > >         const struct drm_format_info *format = fb->format;
> > > > > +       struct drm_gem_dma_object *gem;
> > > > > +       u32 dx, dy, src_x, src_y;
> > > > >         dma_addr_t dma_addr;
> > > > >         u32 ch_base;
> > > > >         int i;
> > > > >
> > > > >         ch_base = sun8i_channel_base(layer);
> > > > >
> > > > > +       /* Adjust x and y to be divisible by subsampling factor */
> > > > > +       src_x = (state->src.x1 >> 16) & ~(format->hsub - 1);
> > > > > +       src_y = (state->src.y1 >> 16) & ~(format->vsub - 1);
> > > >
> > > > AFAICT the only difference compared to drm_fb_dma_get_gem_addr()
> > > > is the masking here, i.e. round_down().
> > > >
> > > > > +
> > > > >         for (i = 0; i < format->num_planes; i++) {
> > > > > -               /* Get the start of the displayed memory */
> > > > > -               dma_addr = drm_fb_dma_get_gem_addr(fb, state, i);
> > > > > +               gem = drm_fb_dma_get_gem_obj(fb, i);
> > > > > +               dma_addr = gem->dma_addr + fb->offsets[i];
> > > > > +
> > > > > +               dx = src_x;
> > > > > +               dy = src_y;
> > > > > +               if (i > 0) {
> > > > > +                       dx /= format->hsub;
> > > > > +                       dy /= format->vsub;
> > > > > +               }
> > > > > +
> > > > > +               dma_addr += dx * format->cpp[i];
> > > > > +               dma_addr += dy * fb->pitches[i];
> > > >
> > > >
> > > > Where as the helper has (or used to have before the blocksize stuff):
> > > >
> > > >     paddr += (format->cpp[plane] * (state->src_x >> 16)) / fb->format->hsub;
> > > >     paddr += (fb->pitches[plane] * (state->src_y >> 16)) / fb->format->vsub;
> > > >
> > > > Am I missing something?
> > >
> > > After some headbanging on my end I see that the offset for the Y plane
> > > needs to be rounded down.
> > >
> > > But instead of reverting the whole thing and open-coding the helper
> > > again, could you adjust the address returned by the helper for odd
> > > offsets?
> >
> > Yes, that's also an option. I'll do it in v2.
> >
> > >
> > > And just a heads up, this also needs a clipped version of
> > > drm_fb_dma_get_gem_addr() as sun8i_ui_layer_update_coord() uses the
> > > clipped dimensions. I am currently working on this part.
> >
> > Can you explain a bit more? I don't see why it needs any adjustement.
> 
> My understanding is that drm_atomic_helper_check_plane_state() calculates
> the "clipped" rectangles for the plane using values from userspace in
> state->src_[xywh] and state->crtc_[xywh] and puts them in state->src
> and state->dst, respectively. If the overlay is moved partially outside
> the screen, the overlay is "clipped".
> 
> 
> Say we have a screen of 1920x1080, with an overlay buffer that is 1280x720.
> Say state->src_x and state->src_y are (-50, 0), given by userspace.
> drm_atomic_helper_check_plane_state() will calculate the clipped & scaled
> rectangles and put them in state->src. This latter rectangle is what is
> used sun8i_ui_layer_update_coord().
> 
> So we would have:
> 
>     (src_x, src_y) = (-50, 0), (src_w, src_h) = (1280, 720)
> 
> The clipped numbers are
> 
>     (src.x1, src.y1) = (0, 0), (src.x2, src.y2) = (1230, 720)
> 
> src, not src_[xywh], is what sun8i layers uses to program the coordinates,
> and prior to the drm_fb_dma_get_gem_addr() conversion, also to calculate
> the buffer start address. drm_fb_dma_get_gem_addr() however uses src_[xy]
> to calculate the address.
> 
> 
> Essentially, when overlaying a clipped plane, the start address needs to
> be adjusted if the source (top left) offset is outside the screen.
> clipping == automatic cropping to fit the screen.
> 
> I don't know if userspace applications routinely do this, but I think this
> needs to be restored to the prior behavior.

Uh, it would be nice if this is fixed. But I think it's not too common
for app to use negative coordinates.

Best regards,
Jernej