[PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses

[PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses

[Chen-Yu Tsai]

The verisilicon driver has a custom framebuffer address calculating
helper that the common drm_fb_dma_get_addr() can substitute.

Differences from drm_fb_dma_get_addr():

- Uses drm_format_info_min_pitch() to calculate the horizontal offset;
  however the driver does not support any of the blocked formats, so
  this just ends up being the same as in drm_fb_dma_get_addr():
  "cpp[plane] * y"

- Uses clipped source coordinates instead of non-clipped coordinates
  as in drm_fb_dma_get_addr();

  For the primary plane this doesn't matter, since the primary plane
  must match the output, i.e. it cannot be clipped. Also this driver
  doesn't support scaling.

  For the cursor plane this seems wrong, as the clipping seems to be
  done by the hardware, and thus the buffer address should be unclipped.

As such, it should be fine to use the common helper and drop the custom
code.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---

This is only compile tested. I do not have the hardware.
---
 drivers/gpu/drm/verisilicon/vs_cursor_plane.c |  3 ++-
 drivers/gpu/drm/verisilicon/vs_plane.c        | 20 -------------------
 .../gpu/drm/verisilicon/vs_primary_plane.c    |  6 +++++-
 3 files changed, 7 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
index fa4f601dd0c8..ae03d415d66b 100644
--- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
@@ -176,7 +176,8 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
 		break;
 	}
 
-	dma_addr = vs_fb_get_dma_addr(fb, &state->src);
+	/* hardware handles clipping as seen below */
+	dma_addr = drm_fb_dma_get_gem_addr(fb, 0);
 
 	regmap_write(dc->regs, VSDC_CURSOR_ADDRESS(output),
 		     lower_32_bits(dma_addr));
diff --git a/drivers/gpu/drm/verisilicon/vs_plane.c b/drivers/gpu/drm/verisilicon/vs_plane.c
index d81f7b8f4c65..38b8b536eccb 100644
--- a/drivers/gpu/drm/verisilicon/vs_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_plane.c
@@ -107,26 +107,6 @@ int drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format)
 	return 0;
 }
 
-dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
-			      const struct drm_rect *src_rect)
-{
-	struct drm_gem_dma_object *gem;
-	dma_addr_t dma_addr;
-
-	/* Get the physical address of the buffer in memory */
-	gem = drm_fb_dma_get_gem_obj(fb, 0);
-
-	/* Compute the start of the displayed memory */
-	dma_addr = gem->dma_addr + fb->offsets[0];
-
-	/* Fixup framebuffer address for src coordinates */
-	dma_addr += drm_format_info_min_pitch(fb->format, 0,
-					      src_rect->x1 >> 16);
-	dma_addr += (src_rect->y1 >> 16) * fb->pitches[0];
-
-	return dma_addr;
-}
-
 struct drm_plane_state *vs_plane_duplicate_state(struct drm_plane *plane)
 {
 	struct vs_plane_state *vs_state, *vs_state_old;
diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
index 1f2be41ae496..46653469caaf 100644
--- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
+++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
@@ -126,7 +126,11 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
 			   VSDC_FB_CONFIG_UV_SWIZZLE_EN,
 			   vs_state->format.uv_swizzle);
 
-	dma_addr = vs_fb_get_dma_addr(fb, &state->src);
+	/*
+	 * Primary plane cannot be moved, no clipping is involved,
+	 * so the non-clipped framebuffer address can be used.
+	 */
+	dma_addr = drm_fb_dma_get_gem_addr(fb, 0);
 
 	regmap_write(dc->regs, VSDC_FB_ADDRESS(output),
 		     lower_32_bits(dma_addr));
-- 
2.55.0.654.g21b8a5bc05-goog

Re: [PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] The patch introduces a build failure and a potential deterministic NULL pointer dereference due to API misuse of `drm_fb_dma_get_gem_addr()`.
- [Low] A dangling function prototype for `vs_fb_get_dma_addr()` is left in `vs_plane.h`.

Pre-existing issues:
- [High] A pre-existing bug causes the hardware display controller to read out-of-bounds physical memory when the cursor is clipped, potentially leaking adjacent kernel memory to the screen.
--

--- Patch [1]: [PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses ---
commit 236e399ed29b505c46a162513a0a121f26bb52bf
Author: Chen-Yu Tsai <wenst@chromium.org>

drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses

The verisilicon driver has a custom framebuffer address calculating
helper that the common drm_fb_dma_get_addr() can substitute.

> diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> index fa4f601dd0c87..ae03d415d66b0 100644
> --- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c

[ ... ]

> @@ -176,7 +176,8 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
>  		break;
>  	}
>  
> -	dma_addr = vs_fb_get_dma_addr(fb, &state->src);
> +	/* hardware handles clipping as seen below */
> +	dma_addr = drm_fb_dma_get_gem_addr(fb, 0);

[Severity: High]
Does this compile with modern compilers? The drm_fb_dma_get_gem_addr()
function is defined in drm_fb_dma_helper.c as taking three arguments:

drm_fb_dma_helper.c:
dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
                                   struct drm_plane_state *state,
                                   unsigned int plane)

It looks like this call passes only two arguments. If the compiler permits
this due to a missing include, the 0 would map to the state pointer.

Since drm_fb_dma_get_gem_addr() unconditionally dereferences the state
pointer for sample_x and sample_y:

drm_fb_dma_helper.c:drm_fb_dma_get_gem_addr() {
        ...
        sample_x = (state->src_x >> 16) / h_div;
        sample_y = (state->src_y >> 16) / v_div;
        ...
}

Could this lead to a deterministic NULL pointer dereference when updating
the cursor plane?


[Severity: High]
This isn't a bug introduced by this patch, but does the cursor clipping
implementation properly shift the offsets when moving the cursor off the
left or top edges?

Looking at vs_cursor_plane_atomic_update() right below this address update:

vs_cursor_plane.c:vs_cursor_plane_atomic_update() {
        ...
        } else {
                regmap_update_bits(dc->regs, VSDC_CURSOR_CONFIG(output),
                                   VSDC_CURSOR_CONFIG_X_OFF_MASK,
                                   -state->crtc_x);
        ...
}

Since the -state->crtc_x value is passed directly without applying a shift
macro, could the mask clear the unshifted value and write a 0 offset to
the hardware?

If the DRM core already clipped the source coordinates, might this cause
the hardware to overshoot the end of the cursor buffer and read
out-of-bounds physical memory?

>  	regmap_write(dc->regs, VSDC_CURSOR_ADDRESS(output),
>  		     lower_32_bits(dma_addr));

[ ... ]

> diff --git a/drivers/gpu/drm/verisilicon/vs_plane.c b/drivers/gpu/drm/verisilicon/vs_plane.c
> index d81f7b8f4c650..38b8b536eccbd 100644
> --- a/drivers/gpu/drm/verisilicon/vs_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_plane.c
> @@ -107,26 +107,6 @@ int drm_format_to_vs_format(u32 drm_format, struct vs_format *vs_format)
>  	return 0;
>  }
>  
> -dma_addr_t vs_fb_get_dma_addr(struct drm_framebuffer *fb,
> -			      const struct drm_rect *src_rect)
> -{

[ ... ]

> -	return dma_addr;
> -}
> -

[Severity: Low]
Since vs_fb_get_dma_addr() is being removed here, should its function
prototype also be removed from drivers/gpu/drm/verisilicon/vs_plane.h to
avoid leaving a dangling declaration?

>  struct drm_plane_state *vs_plane_duplicate_state(struct drm_plane *plane)

[ ... ]

> diff --git a/drivers/gpu/drm/verisilicon/vs_primary_plane.c b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> index 1f2be41ae496c..46653469caaf7 100644
> --- a/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_primary_plane.c
> @@ -126,7 +126,11 @@ static void vs_primary_plane_atomic_update(struct drm_plane *plane,
>  			   VSDC_FB_CONFIG_UV_SWIZZLE_EN,
>  			   vs_state->format.uv_swizzle);
>  
> -	dma_addr = vs_fb_get_dma_addr(fb, &state->src);
> +	/*
> +	 * Primary plane cannot be moved, no clipping is involved,
> +	 * so the non-clipped framebuffer address can be used.
> +	 */
> +	dma_addr = drm_fb_dma_get_gem_addr(fb, 0);

[Severity: High]
Similar to the cursor plane, this call to drm_fb_dma_get_gem_addr() appears
to be missing the state argument, which might cause the same build failure
or runtime NULL pointer dereference.

>  
>  	regmap_write(dc->regs, VSDC_FB_ADDRESS(output),
>  		     lower_32_bits(dma_addr));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807104819.1295812-1-wenst@chromium.org?part=1

Re: [PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses

[Chen-Yu Tsai]

On Fri, Aug 7, 2026 at 6:48 PM Chen-Yu Tsai <wenst@chromium.org> wrote:
>
> The verisilicon driver has a custom framebuffer address calculating
> helper that the common drm_fb_dma_get_addr() can substitute.
>
> Differences from drm_fb_dma_get_addr():
>
> - Uses drm_format_info_min_pitch() to calculate the horizontal offset;
>   however the driver does not support any of the blocked formats, so
>   this just ends up being the same as in drm_fb_dma_get_addr():
>   "cpp[plane] * y"
>
> - Uses clipped source coordinates instead of non-clipped coordinates
>   as in drm_fb_dma_get_addr();
>
>   For the primary plane this doesn't matter, since the primary plane
>   must match the output, i.e. it cannot be clipped. Also this driver
>   doesn't support scaling.
>
>   For the cursor plane this seems wrong, as the clipping seems to be
>   done by the hardware, and thus the buffer address should be unclipped.
>
> As such, it should be fine to use the common helper and drop the custom
> code.
>
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>
> This is only compile tested. I do not have the hardware.
> ---
>  drivers/gpu/drm/verisilicon/vs_cursor_plane.c |  3 ++-
>  drivers/gpu/drm/verisilicon/vs_plane.c        | 20 -------------------
>  .../gpu/drm/verisilicon/vs_primary_plane.c    |  6 +++++-
>  3 files changed, 7 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> index fa4f601dd0c8..ae03d415d66b 100644
> --- a/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> +++ b/drivers/gpu/drm/verisilicon/vs_cursor_plane.c
> @@ -176,7 +176,8 @@ static void vs_cursor_plane_atomic_update(struct drm_plane *plane,
>                 break;
>         }
>
> -       dma_addr = vs_fb_get_dma_addr(fb, &state->src);
> +       /* hardware handles clipping as seen below */
> +       dma_addr = drm_fb_dma_get_gem_addr(fb, 0);

Blah, I don't know what happened, but this doesn't compile.

I'll send a new version.

Re: [PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses

[kernel test robot]

Hi Chen-Yu,

[This is a private test report for your RFC patch.]
kernel test robot noticed the following build errors:

[auto build test ERROR on v7.2]
[also build test ERROR on linus/master next-20260818]
[cannot apply to drm-misc/drm-misc-next]
[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/Chen-Yu-Tsai/drm-verisilicon-Switch-to-drm_fb_dma_get_addr-for-framebuffer-addresses/20260807-184818
base:   v7.2
patch link:    https://lore.kernel.org/r/20260807104819.1295812-1-wenst%40chromium.org
patch subject: [PATCH RFC] drm/verisilicon: Switch to drm_fb_dma_get_addr() for framebuffer addresses
config: alpha-allmodconfig (https://download.01.org/0day-ci/archive/20260820/202608200439.C2DMMV97-lkp@intel.com/config)
compiler: alpha-linux-gcc (GCC) 16.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260820/202608200439.C2DMMV97-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/202608200439.C2DMMV97-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/gpu/drm/verisilicon/vs_primary_plane.c: In function 'vs_primary_plane_atomic_update':
>> drivers/gpu/drm/verisilicon/vs_primary_plane.c:133:20: error: implicit declaration of function 'drm_fb_dma_get_gem_addr' [-Wimplicit-function-declaration]
     133 |         dma_addr = drm_fb_dma_get_gem_addr(fb, 0);
         |                    ^~~~~~~~~~~~~~~~~~~~~~~
--
   drivers/gpu/drm/verisilicon/vs_cursor_plane.c: In function 'vs_cursor_plane_atomic_update':
>> drivers/gpu/drm/verisilicon/vs_cursor_plane.c:180:20: error: implicit declaration of function 'drm_fb_dma_get_gem_addr' [-Wimplicit-function-declaration]
     180 |         dma_addr = drm_fb_dma_get_gem_addr(fb, 0);
         |                    ^~~~~~~~~~~~~~~~~~~~~~~


vim +/drm_fb_dma_get_gem_addr +133 drivers/gpu/drm/verisilicon/vs_primary_plane.c

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

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