[PATCH v2 0/4] drm/verisilicon: misc fixes

[PATCH v2 0/4] drm/verisilicon: misc fixes

[Icenowy Zheng]

This patchset fixes several problems. The first one fixes a bug when the
firmware did a modeset with different mode than the one picked by Linux,
and others are fixing problems pointed by Sashiko.

The fixes of atomic_disble NULL dereference is proven to be worse than
the original, and considering Sashiko points out that clearing FB_EN
would prevent showing bound-but-invisible planes, this part of fix is
redone.

Icenowy Zheng (3):
  drm/verisilicon: disable the output before enabling it
  drm/verisilicon: nuke planes' atomic_enable hooks
  drm/verisilicon: check old state CRTC in plane atomic_disable

Joey Lu (1):
  drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN

 drivers/gpu/drm/verisilicon/vs_bridge.c       |  8 ++++
 drivers/gpu/drm/verisilicon/vs_cursor_plane.c | 35 ++++++---------
 .../gpu/drm/verisilicon/vs_primary_plane.c    | 43 ++++++++-----------
 3 files changed, 40 insertions(+), 46 deletions(-)

-- 
2.56.0

[PATCH v2 1/4] drm/verisilicon: disable the output before enabling it

[Icenowy Zheng]

When either of the RUNNING bits are set, change to some timing
parameters won't take effect.

This will lead to failure of the first modeset done by the kernel, if
the bootloader sets a different mode and keeps the display running
before it.

Flip these RUNNING bits off before setting it on to fix this problem.

Fixes: dbf21777caa8 ("drm: verisilicon: add a driver for Verisilicon display controllers")
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/verisilicon/vs_bridge.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/verisilicon/vs_bridge.c b/drivers/gpu/drm/verisilicon/vs_bridge.c
index dc7c85b07fe32..a095b20e81c00 100644
--- a/drivers/gpu/drm/verisilicon/vs_bridge.c
+++ b/drivers/gpu/drm/verisilicon/vs_bridge.c
@@ -162,6 +162,14 @@ static void vs_bridge_enable_common(struct vs_crtc *crtc,
 			VSDC_DISP_PANEL_CONFIG_DE_EN |
 			VSDC_DISP_PANEL_CONFIG_DAT_EN |
 			VSDC_DISP_PANEL_CONFIG_CLK_EN);
+
+	/* Disable it first to prevent stale hardware state */
+	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
+			  VSDC_DISP_PANEL_START_MULTI_DISP_SYNC |
+			  VSDC_DISP_PANEL_START_RUNNING(output));
+	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
+			  VSDC_DISP_PANEL_CONFIG_RUNNING);
+
 	regmap_set_bits(dc->regs, VSDC_DISP_PANEL_CONFIG(output),
 			VSDC_DISP_PANEL_CONFIG_RUNNING);
 	regmap_clear_bits(dc->regs, VSDC_DISP_PANEL_START,
-- 
2.56.0

[PATCH v2 2/4] drm/verisilicon: nuke planes' atomic_enable hooks

[Icenowy Zheng]

The atomic_update hooks of both planes are just bailing out early when
the planes are invisible, so it's reasonable to just remove these hooks
and update the enabled state in atomic_update.

This fixes the problem that invisible but enabled planes won't be
shown when made visible, as pointed by Sashiko.

Fixes: dbf21777caa8 ("drm: verisilicon: add a driver for Verisilicon display controllers")
Fixes: 8c4ae2189125 ("drm: verisilicon: add support for cursor planes")
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/verisilicon/vs_cursor_plane.c | 22 +++-------------
 .../gpu/drm/verisilicon/vs_primary_plane.c    | 26 +++++--------------
 2 files changed, 10 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
index da456dced88aa..5295dc2f9ee2e 100644
--- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
@@ -98,23 +98,6 @@ static void vs_cursor_plane_commit(struct vs_dc *dc, unsigned int output)
 			VSDC_CURSOR_CONFIG_IMG_UPDATE);
 }
 
-static void vs_cursor_plane_atomic_enable(struct drm_plane *plane,
-					   struct drm_atomic_commit *atomic_state)
-{
-	struct drm_plane_state *state = drm_atomic_get_new_plane_state(atomic_state,
-								       plane);
-	struct drm_crtc *crtc = state->crtc;
-	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
-	unsigned int output = vcrtc->id;
-	struct vs_dc *dc = vcrtc->dc;
-
-	regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
-			   VSDC_CURSOR_CONFIG_FMT_MASK,
-			   VSDC_CURSOR_CONFIG_FMT_ARGB8888);
-
-	vs_cursor_plane_commit(dc, output);
-}
-
 static void vs_cursor_plane_atomic_disable(struct drm_plane *plane,
 					    struct drm_atomic_commit *atomic_state)
 {
@@ -153,6 +136,10 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
 	output = vcrtc->id;
 	dc = vcrtc->dc;
 
+	regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
+			   VSDC_CURSOR_CONFIG_FMT_MASK,
+			   VSDC_CURSOR_CONFIG_FMT_ARGB8888);
+
 	/* Other sizes should be rejected by atomic_check */
 	switch (state->crtc_w) {
 	case 32:
@@ -225,7 +212,6 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
 static const struct drm_plane_helper_funcs vs_cursor_plane_helper_funcs = {
 	.atomic_check	= vs_cursor_plane_atomic_check,
 	.atomic_update	= vs_cursor_plane_atomic_update,
-	.atomic_enable	= vs_cursor_plane_atomic_enable,
 	.atomic_disable	= vs_cursor_plane_atomic_disable,
 };
 
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index 8e94491693019..fca692f4f7d5c 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -59,25 +59,6 @@ static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int output)
 			VSDC_FB_CONFIG_EX_COMMIT);
 }
 
-static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
-					   struct drm_atomic_commit *atomic_state)
-{
-	struct drm_plane_state *state = drm_atomic_get_new_plane_state(atomic_state,
-								       plane);
-	struct drm_crtc *crtc = state->crtc;
-	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
-	unsigned int output = vcrtc->id;
-	struct vs_dc *dc = vcrtc->dc;
-
-	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
-			VSDC_FB_CONFIG_EX_FB_EN);
-	regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
-			   VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
-			   VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
-
-	vs_primary_plane_commit(dc, output);
-}
-
 static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
 					    struct drm_atomic_commit *atomic_state)
 {
@@ -144,13 +125,18 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
 	regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
 		     VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
 
+	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+			VSDC_FB_CONFIG_EX_FB_EN);
+	regmap_update_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+			   VSDC_FB_CONFIG_EX_DISPLAY_ID_MASK,
+			   VSDC_FB_CONFIG_EX_DISPLAY_ID(output));
+
 	vs_primary_plane_commit(dc, output);
 }
 
 static const struct drm_plane_helper_funcs vs_primary_plane_helper_funcs = {
 	.atomic_check	= vs_primary_plane_atomic_check,
 	.atomic_update	= vs_primary_plane_atomic_update,
-	.atomic_enable	= vs_primary_plane_atomic_enable,
 	.atomic_disable	= vs_primary_plane_atomic_disable,
 };
 
-- 
2.56.0

[PATCH v2 3/4] drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN

[Icenowy Zheng]

From: Joey Lu <a0987203069@gmail.com>

vs_primary_plane_atomic_disable() calls regmap_set_bits() on
VSDC_FB_CONFIG_EX_FB_EN instead of regmap_clear_bits(), so disabling
the primary plane on DC8200-family hardware actually leaves the
framebuffer enable bit instead of clearing it.

Fixes: dbf21777caa8 ("drm: verisilicon: add a driver for Verisilicon display controllers")
Signed-off-by: Joey Lu <a0987203069@gmail.com>
[Icenowy: rebased and reworked the commit message]
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/verisilicon/vs_primary_plane.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index fca692f4f7d5c..e687313d6f057 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -69,8 +69,8 @@ static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
 	unsigned int output = vcrtc->id;
 	struct vs_dc *dc = vcrtc->dc;
 
-	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
-			VSDC_FB_CONFIG_EX_FB_EN);
+	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+			  VSDC_FB_CONFIG_EX_FB_EN);
 
 	vs_primary_plane_commit(dc, output);
 }
-- 
2.56.0

[PATCH v2 4/4] drm/verisilicon: check old state CRTC in plane atomic_disable

[Icenowy Zheng]

As atomic_update hooks of both planes would utilize atomic_disable
functions to disable the plane when it's set to invisible (even when
technically enabled), the atomic_disable functions need to check whether
the plane is really enabled before.

Add such a check to prevent NULL dereference.

Fixes: dbf21777caa8 ("drm: verisilicon: add a driver for Verisilicon display controllers")
Fixes: 8c4ae2189125 ("drm: verisilicon: add support for cursor planes")
Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn>
---
 drivers/gpu/drm/verisilicon/vs_cursor_plane.c  | 13 ++++++++++---
 drivers/gpu/drm/verisilicon/vs_primary_plane.c | 13 ++++++++++---
 2 files changed, 20 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
index 5295dc2f9ee2e..bee19b65b9a4e 100644
--- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
@@ -104,9 +104,16 @@ static void vs_cursor_plane_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *state = drm_atomic_get_old_plane_state(atomic_state,
 								       plane);
 	struct drm_crtc *crtc = state->crtc;
-	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
-	unsigned int output = vcrtc->id;
-	struct vs_dc *dc = vcrtc->dc;
+	struct vs_crtc *vcrtc;
+	unsigned int output;
+	struct vs_dc *dc;
+
+	if (!crtc)
+		return;
+
+	vcrtc = drm_crtc_to_vs_crtc(crtc);
+	output = vcrtc->id;
+	dc = vcrtc->dc;
 
 	regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
 			   VSDC_CURSOR_CONFIG_FMT_MASK,
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index e687313d6f057..01725e60e5fbd 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -65,9 +65,16 @@ static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
 	struct drm_plane_state *state = drm_atomic_get_old_plane_state(atomic_state,
 								       plane);
 	struct drm_crtc *crtc = state->crtc;
-	struct vs_crtc *vcrtc = drm_crtc_to_vs_crtc(crtc);
-	unsigned int output = vcrtc->id;
-	struct vs_dc *dc = vcrtc->dc;
+	struct vs_crtc *vcrtc;
+	unsigned int output;
+	struct vs_dc *dc;
+
+	if (!crtc)
+		return;
+
+	vcrtc = drm_crtc_to_vs_crtc(crtc);
+	output = vcrtc->id;
+	dc = vcrtc->dc;
 
 	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
 			  VSDC_FB_CONFIG_EX_FB_EN);
-- 
2.56.0

Re: [PATCH v2 0/4] drm/verisilicon: misc fixes

[Icenowy Zheng]

在 2026-11-02一的 15:57 +0800，Icenowy Zheng写道：
> This patchset fixes several problems. The first one fixes a bug when
> the
> firmware did a modeset with different mode than the one picked by
> Linux,
> and others are fixing problems pointed by Sashiko.
> 
> The fixes of atomic_disble NULL dereference is proven to be worse
> than
> the original, and considering Sashiko points out that clearing FB_EN
> would prevent showing bound-but-invisible planes, this part of fix is
> redone.

Ping.

Could anyone try to do some review on this patchset?

Thanks,
Icenowy

> 
> Icenowy Zheng (3):
>   drm/verisilicon: disable the output before enabling it
>   drm/verisilicon: nuke planes' atomic_enable hooks
>   drm/verisilicon: check old state CRTC in plane atomic_disable
> 
> Joey Lu (1):
>   drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN
> 
>  drivers/gpu/drm/verisilicon/vs_bridge.c       |  8 ++++
>  drivers/gpu/drm/verisilicon/vs_cursor_plane.c | 35 ++++++---------
>  .../gpu/drm/verisilicon/vs_primary_plane.c    | 43 ++++++++---------
> --
>  3 files changed, 40 insertions(+), 46 deletions(-)