[PATCH RESEND 0/3] drm/verisilicon: misc fixes

[PATCH RESEND 0/3] drm/verisilicon: misc fixes

[Icenowy Zheng]

This patchset fixes several problems. Two of them are pointed out by
Sashiko, and the remaining one is found when running the driver on some
weird TH1520 firmware that does mode set but fixes to 800x600.

This patchset is re-sent because of forgetting to Cc the mailing list
previously.

Icenowy Zheng (2):
  drm/verisilicon: disable the output before enabling it
  drm/verisilicon: stop to reuse atomic_disable for invisible planes

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  | 17 +++++++++++------
 drivers/gpu/drm/verisilicon/vs_primary_plane.c | 15 ++++++++++-----
 3 files changed, 29 insertions(+), 11 deletions(-)

-- 
2.56.0

[PATCH RESEND 1/3] 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 on top of torvalds/master 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 8e94491693019..d5563c3b825c3 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -88,8 +88,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

Re: [PATCH RESEND 1/3] drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/verisilicon: vs_primary_plane_atomic_disable() misuse on invisible plane
--

commit b5d25e699c72e0d03ee4c0a85f55bde0b05d2253
Author: Joey Lu <a0987203069@gmail.com>

drm/verisilicon: fix DC8200 primary plane disable clearing FB_EN

This commit changes vs_primary_plane_atomic_disable() to call
regmap_clear_bits() instead of regmap_set_bits() on VSDC_FB_CONFIG_EX_FB_EN
when disabling the primary plane.

> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index 95f4a06039e76..c12fe54096c33 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -89,8 +89,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);
>  }

[Severity: High]
Now that vs_primary_plane_atomic_disable() clears VSDC_FB_CONFIG_EX_FB_EN, can
calling it from vs_primary_plane_atomic_update() when !state->visible leave
VSDC_FB_CONFIG_EX_FB_EN permanently cleared when the plane becomes visible
again on the same CRTC?

In drivers/gpu/drm/verisilicon/vs_primary_plane.c,
vs_primary_plane_atomic_update() handles invisible planes by calling
vs_primary_plane_atomic_disable():

	if (!state->visible) {
		vs_primary_plane_atomic_disable(plane, atomic_state);
		return;
	}

When a primary plane is already attached to a CRTC (old_plane_state->crtc !=
NULL) and transitions from visible to !visible (for example, when the CRTC is
disabled while the plane stays attached, which vs_primary_plane_atomic_check()
allows via can_update_disabled = true), vs_primary_plane_atomic_update() calls
vs_primary_plane_atomic_disable() and clears VSDC_FB_CONFIG_EX_FB_EN.

When that plane later transitions back from !visible to visible on the same
CRTC (old_plane_state->crtc != NULL && new_plane_state->crtc != NULL),
drm_atomic_helper_commit_planes() calls vs_primary_plane_atomic_update() and
checks drm_atomic_plane_enabling(old_plane_state, new_plane_state), which only
tests (!old_plane_state->crtc && new_plane_state->crtc). Because
old_plane_state->crtc is non-NULL, vs_primary_plane_atomic_enable() is not
called, and vs_primary_plane_atomic_update() never sets
VSDC_FB_CONFIG_EX_FB_EN:

static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
					   struct drm_atomic_commit *atomic_state)
{
	...
	regmap_set_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
			VSDC_FB_CONFIG_EX_FB_EN);

Conversely, if a previously disabled plane (old_plane_state->crtc == NULL) is
attached to a CRTC with new_plane_state->visible == false:

1. vs_primary_plane_atomic_update() calls vs_primary_plane_atomic_disable(),
   which looks up old_plane_state via drm_atomic_get_old_plane_state() and
   dereferences state->crtc (NULL) via vcrtc->id and vcrtc->dc:

	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;

2. drm_atomic_helper_commit_planes() then sees drm_atomic_plane_enabling() ==
   true and calls vs_primary_plane_atomic_enable(), setting
   VSDC_FB_CONFIG_EX_FB_EN and committing the plane even though
   vs_primary_plane_atomic_update() returned early without programming
   VSDC_FB_ADDRESS, VSDC_FB_STRIDE, or VSDC_FB_SIZE.

Commit 3080d00108f4 ("drm/verisilicon: stop to reuse atomic_disable for
invisible planes") later in the series replaces the
vs_primary_plane_atomic_disable() call in vs_primary_plane_atomic_update()
with vs_primary_plane_disable(dc, output), but calls it before dc and output
are initialized and still leaves VSDC_FB_CONFIG_EX_FB_EN cleared after a
!visible to visible transition on the same CRTC (and enabled on an initial
!visible attach).

Should VSDC_FB_CONFIG_EX_FB_EN and VSDC_FB_CONFIG_EX_DISPLAY_ID instead be
managed in vs_primary_plane_atomic_update() based on state->visible rather
than in a separate atomic_enable callback?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030133233.197189-1-zhengxingda@iscas.ac.cn?part=1

[PATCH RESEND 2/3] 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 RESEND 3/3] drm/verisilicon: stop to reuse atomic_disable for invisible planes

[Icenowy Zheng]

When a plane is configured to be invisible, the atomic_update
implementation currently just calls the atomic_disable function.

However these two functions behave differently when disabling planes:
atomic_update disables the plane specified in the NEW state, but
atomic_disable disables the plane specified in the OLD state, so this
shortcut may lead to NULL dereference if the old state doesn't have a
CRTC bound.

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  | 17 +++++++++++------
 drivers/gpu/drm/verisilicon/vs_primary_plane.c | 15 ++++++++++-----
 2 files changed, 21 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
index da456dced88aa..725921ac25d48 100644
--- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
@@ -98,6 +98,15 @@ static void vs_cursor_plane_commit(struct vs_dc *dc, unsigned int output)
 			VSDC_CURSOR_CONFIG_IMG_UPDATE);
 }
 
+static void vs_cursor_plane_disable(struct vs_dc *dc, unsigned int output)
+{
+	regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
+			   VSDC_CURSOR_CONFIG_FMT_MASK,
+			   VSDC_CURSOR_CONFIG_FMT_OFF);
+
+	vs_cursor_plane_commit(dc, output);
+}
+
 static void vs_cursor_plane_atomic_enable(struct drm_plane *plane,
 					   struct drm_atomic_commit *atomic_state)
 {
@@ -125,11 +134,7 @@ static void vs_cursor_plane_atomic_disable(struct drm_plane *plane,
 	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_OFF);
-
-	vs_cursor_plane_commit(dc, output);
+	vs_cursor_plane_disable(dc, output);
 }
 
 static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
@@ -145,7 +150,7 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
 	dma_addr_t dma_addr;
 
 	if (!state->visible) {
-		vs_cursor_plane_atomic_disable(plane, atomic_state);
+		vs_cursor_plane_disable(dc, output);
 		return;
 	}
 
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index d5563c3b825c3..594d56bf32410 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -59,6 +59,14 @@ static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int output)
 			VSDC_FB_CONFIG_EX_COMMIT);
 }
 
+static void vs_primary_plane_disable(struct vs_dc *dc, unsigned int output)
+{
+	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
+			  VSDC_FB_CONFIG_EX_FB_EN);
+
+	vs_primary_plane_commit(dc, output);
+}
+
 static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
 					   struct drm_atomic_commit *atomic_state)
 {
@@ -88,10 +96,7 @@ static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
 	unsigned int output = vcrtc->id;
 	struct vs_dc *dc = vcrtc->dc;
 
-	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
-			  VSDC_FB_CONFIG_EX_FB_EN);
-
-	vs_primary_plane_commit(dc, output);
+	vs_primary_plane_disable(dc, output);
 }
 
 static void vs_primary_plane_atomic_update(struct drm_plane *plane,
@@ -108,7 +113,7 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
 	dma_addr_t dma_addr;
 
 	if (!state->visible) {
-		vs_primary_plane_atomic_disable(plane, atomic_state);
+		vs_primary_plane_disable(dc, output);
 		return;
 	}
 
-- 
2.56.0

Re: [PATCH RESEND 3/3] drm/verisilicon: stop to reuse atomic_disable for invisible planes

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Critical] drm/verisilicon: uninitialized dc and output in plane atomic_update()
- [High] drm/verisilicon: plane visibility state mismatch with atomic_enable()
--

commit 3080d00108f48782cd8c1c8ee016e9473b324e57
Author: Icenowy Zheng <zhengxingda@iscas.ac.cn>

drm/verisilicon: stop to reuse atomic_disable for invisible planes

This commit factors out helper functions to disable cursor and primary
planes directly from their atomic_update callbacks when state->visible is
false, avoiding a NULL CRTC dereference in the old plane state when calling
atomic_disable.

> diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> index 1fa6b7578b27e..8dd6caadb7075 100644
> --- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> @@ -99,6 +99,15 @@ static void vs_cursor_plane_commit(struct vs_dc *dc, unsigned int output)
>  			VSDC_CURSOR_CONFIG_IMG_UPDATE);
>  }
>  
> +static void vs_cursor_plane_disable(struct vs_dc *dc, unsigned int output)
> +{
> +	regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
> +			   VSDC_CURSOR_CONFIG_FMT_MASK,
> +			   VSDC_CURSOR_CONFIG_FMT_OFF);
> +
> +	vs_cursor_plane_commit(dc, output);
> +}
> +
>  static void vs_cursor_plane_atomic_enable(struct drm_plane *plane,
>  					   struct drm_atomic_commit *atomic_state)
>  {
> @@ -126,11 +135,7 @@ static void vs_cursor_plane_atomic_disable(struct drm_plane *plane,
>  	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_OFF);
> -
> -	vs_cursor_plane_commit(dc, output);
> +	vs_cursor_plane_disable(dc, output);
>  }
>  
>  static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
> @@ -146,7 +151,7 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
>  	dma_addr_t dma_addr;
>  
>  	if (!state->visible) {
> -		vs_cursor_plane_atomic_disable(plane, atomic_state);
> +		vs_cursor_plane_disable(dc, output);
>  		return;
>  	}

[Severity: Critical]
Does vs_cursor_plane_atomic_update() pass uninitialized dc and output values
to vs_cursor_plane_disable() when !state->visible is true?

In drivers/gpu/drm/verisilicon/vs_cursor_plane.c, vcrtc, output, and dc are
only initialized from state->crtc after the !state->visible early return:

drivers/gpu/drm/verisilicon/vs_cursor_plane.c:vs_cursor_plane_atomic_update() {
	struct drm_crtc *crtc = state->crtc;
	struct vs_dc *dc;
	struct vs_crtc *vcrtc;
	unsigned int output;
	dma_addr_t dma_addr;

	if (!state->visible) {
		vs_cursor_plane_disable(dc, output);
		return;
	}

	vcrtc = drm_crtc_to_vs_crtc(crtc);
	output = vcrtc->id;
	dc = vcrtc->dc;
}

When a cursor plane is updated while invisible, vs_cursor_plane_disable()
will dereference the uninitialized pointer dc->regs and index
VSDC_CURSOR_CONFIG(output) with an uninitialized output variable.

[Severity: High]
How does disabling the plane in hardware on !state->visible interact with
vs_cursor_plane_atomic_enable() and subsequent visible updates?

In drm_atomic_helper_commit_planes(), funcs->atomic_update() runs before
funcs->atomic_enable(). When a previously unbound plane (old_plane_state->crtc
is NULL) is bound to a CRTC while invisible (new_plane_state->crtc is
non-NULL and new_plane_state->visible is false):

1. vs_cursor_plane_atomic_update() calls vs_cursor_plane_disable() and
   returns early without programming VSDC_CURSOR_ADDRESS(output), size, or
   position registers.
2. drm_atomic_plane_enabling() evaluates to true because !old_plane_state->crtc
   && new_plane_state->crtc, so drm_atomic_helper_commit_planes() immediately
   calls vs_cursor_plane_atomic_enable():

drivers/gpu/drm/verisilicon/vs_cursor_plane.c:vs_cursor_plane_atomic_enable() {
	...
	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);
}

Won't vs_cursor_plane_atomic_enable() unconditionally re-enable the plane in
hardware without a valid framebuffer address or geometry configured?

Conversely, if a plane that is already bound to a CRTC moves off-screen so
state->visible becomes false, vs_cursor_plane_atomic_update() sets
VSDC_CURSOR_CONFIG_FMT_OFF via vs_cursor_plane_disable(). When a later commit
moves the plane back on-screen while remaining on the same CRTC
(old_plane_state->crtc and new_plane_state->crtc are both non-NULL, and
new_plane_state->visible becomes true), drm_atomic_plane_enabling() is false,
so vs_cursor_plane_atomic_enable() is not called and
vs_cursor_plane_atomic_update() never restores VSDC_CURSOR_CONFIG_FMT_ARGB8888.
Will the cursor plane remain disabled in hardware?

> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index c12fe54096c33..56e4c0e168871 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -60,6 +60,14 @@ static void vs_primary_plane_commit(struct vs_dc *dc, unsigned int output)
>  			VSDC_FB_CONFIG_EX_COMMIT);
>  }
>  
> +static void vs_primary_plane_disable(struct vs_dc *dc, unsigned int output)
> +{
> +	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> +			  VSDC_FB_CONFIG_EX_FB_EN);
> +
> +	vs_primary_plane_commit(dc, output);
> +}
> +
>  static void vs_primary_plane_atomic_enable(struct drm_plane *plane,
>  					   struct drm_atomic_commit *atomic_state)
>  {
> @@ -89,10 +97,7 @@ static void vs_primary_plane_atomic_disable(struct drm_plane *plane,
>  	unsigned int output = vcrtc->id;
>  	struct vs_dc *dc = vcrtc->dc;
>  
> -	regmap_clear_bits(dc->regs, VSDC_FB_CONFIG_EX(output),
> -			  VSDC_FB_CONFIG_EX_FB_EN);
> -
> -	vs_primary_plane_commit(dc, output);
> +	vs_primary_plane_disable(dc, output);
>  }
>  
>  static void vs_primary_plane_atomic_update(struct drm_plane *plane,
> @@ -109,7 +114,7 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
>  	dma_addr_t dma_addr;
>  
>  	if (!state->visible) {
> -		vs_primary_plane_atomic_disable(plane, atomic_state);
> +		vs_primary_plane_disable(dc, output);
>  		return;
>  	}

[Severity: Critical]
Similarly, in drivers/gpu/drm/verisilicon/vs_primary_plane.c, doesn't
vs_primary_plane_atomic_update() also call vs_primary_plane_disable(dc,
output) before vcrtc, output, and dc are initialized from state->crtc,
dereferencing uninitialized dc->regs and using an uninitialized output index
in VSDC_FB_CONFIG_EX(output) when !state->visible?

[Severity: High]
Does vs_primary_plane_atomic_update() have the same state mismatch with
vs_primary_plane_atomic_enable()?

When old_plane_state->crtc is NULL, new_plane_state->crtc is non-NULL, and
new_plane_state->visible is false, vs_primary_plane_atomic_update() returns
early without programming VSDC_FB_ADDRESS(output) or geometry registers, and
then vs_primary_plane_atomic_enable() sets VSDC_FB_CONFIG_EX_FB_EN and
commits the plane in hardware anyway.

And if an already-bound primary plane transitions from visible to invisible
(clearing VSDC_FB_CONFIG_EX_FB_EN in vs_primary_plane_disable()) and then
back to visible on the same CRTC, neither vs_primary_plane_atomic_enable()
nor vs_primary_plane_atomic_update() sets VSDC_FB_CONFIG_EX_FB_EN again.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030133233.197189-1-zhengxingda@iscas.ac.cn?part=3

Re: [PATCH RESEND 3/3] drm/verisilicon: stop to reuse atomic_disable for invisible planes

[kernel test robot]

Hi Icenowy,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on linus/master v7.3 next-20261030]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Icenowy-Zheng/drm-verisilicon-fix-DC8200-primary-plane-disable-clearing-FB_EN/20261030-213231
base:   https://gitlab.freedesktop.org/drm/misc/kernel.git drm-misc-next
patch link:    https://lore.kernel.org/r/20261030133233.197189-4-zhengxingda%40iscas.ac.cn
patch subject: [PATCH RESEND 3/3] drm/verisilicon: stop to reuse atomic_disable for invisible planes
config: arm64-randconfig-1001-20261031 (https://download.01.org/0day-ci/archive/20261031/202610312040.VpGybfak-lkp@intel.com/config)
compiler: clang version 24.0.0git (https://github.com/llvm/llvm-project c2ea4440d4b64c148de4fdfca1fc2c437a0701e4)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20261031/202610312040.VpGybfak-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202610312040.VpGybfak-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/verisilicon/vs_primary_plane.c:117:28: warning: variable 'dc' is uninitialized when used here [-Wuninitialized]
     117 |                 vs_primary_plane_disable(dc, output);
         |                                          ^~
   drivers/gpu/drm/verisilicon/vs_primary_plane.c:111:18: note: initialize the variable 'dc' to silence this warning
     111 |         struct vs_dc *dc;
         |                         ^
         |                          = NULL
>> drivers/gpu/drm/verisilicon/vs_primary_plane.c:117:32: warning: variable 'output' is uninitialized when used here [-Wuninitialized]
     117 |                 vs_primary_plane_disable(dc, output);
         |                                              ^~~~~~
   drivers/gpu/drm/verisilicon/vs_primary_plane.c:113:21: note: initialize the variable 'output' to silence this warning
     113 |         unsigned int output;
         |                            ^
         |                             = 0
   2 warnings generated.
--
>> drivers/gpu/drm/verisilicon/vs_cursor_plane.c:154:27: warning: variable 'dc' is uninitialized when used here [-Wuninitialized]
     154 |                 vs_cursor_plane_disable(dc, output);
         |                                         ^~
   drivers/gpu/drm/verisilicon/vs_cursor_plane.c:148:18: note: initialize the variable 'dc' to silence this warning
     148 |         struct vs_dc *dc;
         |                         ^
         |                          = NULL
>> drivers/gpu/drm/verisilicon/vs_cursor_plane.c:154:31: warning: variable 'output' is uninitialized when used here [-Wuninitialized]
     154 |                 vs_cursor_plane_disable(dc, output);
         |                                             ^~~~~~
   drivers/gpu/drm/verisilicon/vs_cursor_plane.c:150:21: note: initialize the variable 'output' to silence this warning
     150 |         unsigned int output;
         |                            ^
         |                             = 0
   2 warnings generated.


vim +/dc +117 drivers/gpu/drm/verisilicon/vs_primary_plane.c

   102	
   103	static void vs_primary_plane_atomic_update(struct drm_plane *plane,
   104						   struct drm_atomic_commit *atomic_state)
   105	{
   106		struct drm_plane_state *state = drm_atomic_get_new_plane_state(atomic_state,
   107									       plane);
   108		struct vs_plane_state *vs_state = to_vs_plane_state(state);
   109		struct drm_framebuffer *fb = state->fb;
   110		struct drm_crtc *crtc = state->crtc;
   111		struct vs_dc *dc;
   112		struct vs_crtc *vcrtc;
   113		unsigned int output;
   114		dma_addr_t dma_addr;
   115	
   116		if (!state->visible) {
 > 117			vs_primary_plane_disable(dc, output);
   118			return;
   119		}
   120	
   121		vcrtc = drm_crtc_to_vs_crtc(crtc);
   122		output = vcrtc->id;
   123		dc = vcrtc->dc;
   124	
   125		regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
   126				   VSDC_FB_CONFIG_FMT_MASK,
   127				   VSDC_FB_CONFIG_FMT(vs_state->format.color));
   128		regmap_update_bits(dc->regs, VSDC_FB_CONFIG(output),
   129				   VSDC_FB_CONFIG_SWIZZLE_MASK,
   130				   VSDC_FB_CONFIG_SWIZZLE(vs_state->format.swizzle));
   131		regmap_assign_bits(dc->regs, VSDC_FB_CONFIG(output),
   132				   VSDC_FB_CONFIG_UV_SWIZZLE_EN,
   133				   vs_state->format.uv_swizzle);
   134	
   135		/*
   136		 * Primary plane cannot be moved, no clipping is involved,
   137		 * so the non-clipped framebuffer address can be used.
   138		 */
   139		dma_addr = drm_fb_dma_get_gem_addr(fb, state, 0);
   140	
   141		regmap_write(dc->regs, VSDC_FB_ADDRESS(output),
   142			     lower_32_bits(dma_addr));
   143		regmap_write(dc->regs, VSDC_FB_STRIDE(output),
   144			     fb->pitches[0]);
   145	
   146		regmap_write(dc->regs, VSDC_FB_TOP_LEFT(output),
   147			     VSDC_MAKE_PLANE_POS(state->crtc_x, state->crtc_y));
   148		regmap_write(dc->regs, VSDC_FB_BOTTOM_RIGHT(output),
   149			     VSDC_MAKE_PLANE_POS(state->crtc_x + state->crtc_w,
   150						 state->crtc_y + state->crtc_h));
   151		regmap_write(dc->regs, VSDC_FB_SIZE(output),
   152			     VSDC_MAKE_PLANE_SIZE(state->crtc_w, state->crtc_h));
   153	
   154		regmap_write(dc->regs, VSDC_FB_BLEND_CONFIG(output),
   155			     VSDC_FB_BLEND_CONFIG_BLEND_DISABLE);
   156	
   157		vs_primary_plane_commit(dc, output);
   158	}
   159	

--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki