drm/tidss: dispc: Switch to drm_fb_dma_get_gem_addr() for framebuffer addresses

[PATCH] drm/tidss: dispc: Switch to drm_fb_dma_get_gem_addr() for framebuffer addresses

Posted by Chen-Yu Tsai 3 weeks, 2 days ago

dispc_plane_state_dma_addr() and dispc_plane_state_p_uv_addr() are
basically the same as drm_fb_dma_get_addr(), without the support for
formats with block parameters. Since the driver doesn't support any of
those formats, the result is the same.

Switch to drm_fb_dma_get_gem_addr() for getting the framebuffer addresses.
Drop the const modifier on "struct drm_plane_state *state" for
dispc_plane_setup() so that the state can be passed to
drm_fb_dma_get_gem_addr().

Using the helper also future proofs the driver in case block parameters
are added for more formats, especially the common sub-sampled YUV
formats.

Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
---
 drivers/gpu/drm/tidss/tidss_dispc.c | 38 +++--------------------------
 drivers/gpu/drm/tidss/tidss_dispc.h |  2 +-
 2 files changed, 4 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
index 58d5eb033bdb..2a0ec11a6c90 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.c
+++ b/drivers/gpu/drm/tidss/tidss_dispc.c
@@ -2157,47 +2157,15 @@ int dispc_plane_check(struct dispc_device *dispc, u32 hw_plane,
 	return 0;
 }
 
-static
-dma_addr_t dispc_plane_state_dma_addr(const struct drm_plane_state *state)
-{
-	struct drm_framebuffer *fb = state->fb;
-	struct drm_gem_dma_object *gem;
-	u32 x = state->src_x >> 16;
-	u32 y = state->src_y >> 16;
-
-	gem = drm_fb_dma_get_gem_obj(state->fb, 0);
-
-	return gem->dma_addr + fb->offsets[0] + x * fb->format->cpp[0] +
-		y * fb->pitches[0];
-}
-
-static
-dma_addr_t dispc_plane_state_p_uv_addr(const struct drm_plane_state *state)
-{
-	struct drm_framebuffer *fb = state->fb;
-	struct drm_gem_dma_object *gem;
-	u32 x = state->src_x >> 16;
-	u32 y = state->src_y >> 16;
-
-	if (WARN_ON(state->fb->format->num_planes != 2))
-		return 0;
-
-	gem = drm_fb_dma_get_gem_obj(fb, 1);
-
-	return gem->dma_addr + fb->offsets[1] +
-		(x * fb->format->cpp[1] / fb->format->hsub) +
-		(y * fb->pitches[1] / fb->format->vsub);
-}
-
 void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
-		       const struct drm_plane_state *state,
+		       struct drm_plane_state *state,
 		       u32 hw_videoport)
 {
 	bool lite = dispc->feat->vid_info[hw_plane].is_lite;
 	u32 fourcc = state->fb->format->format;
 	u16 cpp = state->fb->format->cpp[0];
 	u32 fb_width = state->fb->pitches[0] / cpp;
-	dma_addr_t dma_addr = dispc_plane_state_dma_addr(state);
+	dma_addr_t dma_addr = drm_fb_dma_get_gem_addr(state->fb, state, 0);
 	struct dispc_scaling_params scale;
 
 	dispc_vid_calc_scaling(dispc, state, &scale, lite);
@@ -2229,7 +2197,7 @@ void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
 	if (state->fb->format->num_planes == 2) {
 		u16 cpp_uv = state->fb->format->cpp[1];
 		u32 fb_width_uv = state->fb->pitches[1] / cpp_uv;
-		dma_addr_t p_uv_addr = dispc_plane_state_p_uv_addr(state);
+		dma_addr_t p_uv_addr = drm_fb_dma_get_gem_addr(state->fb, state, 1);
 
 		dispc_vid_write(dispc, hw_plane,
 				DISPC_VID_BA_UV_0, p_uv_addr & 0xffffffff);
diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
index 739d211d0018..2183254f1ab7 100644
--- a/drivers/gpu/drm/tidss/tidss_dispc.h
+++ b/drivers/gpu/drm/tidss/tidss_dispc.h
@@ -140,7 +140,7 @@ int dispc_plane_check(struct dispc_device *dispc, u32 hw_plane,
 		      const struct drm_plane_state *state,
 		      u32 hw_videoport);
 void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
-		       const struct drm_plane_state *state,
+		       struct drm_plane_state *state,
 		       u32 hw_videoport);
 void dispc_plane_enable(struct dispc_device *dispc, u32 hw_plane, bool enable);
 const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len);
-- 
2.55.0.970.g62bdec98f9-goog

Re: [PATCH] drm/tidss: dispc: Switch to drm_fb_dma_get_gem_addr() for framebuffer addresses

Posted by Tomi Valkeinen 3 weeks, 1 day ago

Hi,

On 03/09/2026 09:31, Chen-Yu Tsai wrote:
> dispc_plane_state_dma_addr() and dispc_plane_state_p_uv_addr() are
> basically the same as drm_fb_dma_get_addr(), without the support for

drm_fb_dma_get_gem_addr()

> formats with block parameters. Since the driver doesn't support any of
> those formats, the result is the same.
> 
> Switch to drm_fb_dma_get_gem_addr() for getting the framebuffer addresses.
> Drop the const modifier on "struct drm_plane_state *state" for
> dispc_plane_setup() so that the state can be passed to
> drm_fb_dma_get_gem_addr().

Or add const modifier to drm_fb_dma_get_gem_addr(). But it's ok either way.

> Using the helper also future proofs the driver in case block parameters
> are added for more formats, especially the common sub-sampled YUV
> formats.
> 
> Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> ---
>   drivers/gpu/drm/tidss/tidss_dispc.c | 38 +++--------------------------
>   drivers/gpu/drm/tidss/tidss_dispc.h |  2 +-
>   2 files changed, 4 insertions(+), 36 deletions(-)

Looks good to me. I can pick this up and fix the above typo while 
applying, or wait for v2 if you want to do something about the const.

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

  Tomi

> 
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.c b/drivers/gpu/drm/tidss/tidss_dispc.c
> index 58d5eb033bdb..2a0ec11a6c90 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.c
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.c
> @@ -2157,47 +2157,15 @@ int dispc_plane_check(struct dispc_device *dispc, u32 hw_plane,
>   	return 0;
>   }
>   
> -static
> -dma_addr_t dispc_plane_state_dma_addr(const struct drm_plane_state *state)
> -{
> -	struct drm_framebuffer *fb = state->fb;
> -	struct drm_gem_dma_object *gem;
> -	u32 x = state->src_x >> 16;
> -	u32 y = state->src_y >> 16;
> -
> -	gem = drm_fb_dma_get_gem_obj(state->fb, 0);
> -
> -	return gem->dma_addr + fb->offsets[0] + x * fb->format->cpp[0] +
> -		y * fb->pitches[0];
> -}
> -
> -static
> -dma_addr_t dispc_plane_state_p_uv_addr(const struct drm_plane_state *state)
> -{
> -	struct drm_framebuffer *fb = state->fb;
> -	struct drm_gem_dma_object *gem;
> -	u32 x = state->src_x >> 16;
> -	u32 y = state->src_y >> 16;
> -
> -	if (WARN_ON(state->fb->format->num_planes != 2))
> -		return 0;
> -
> -	gem = drm_fb_dma_get_gem_obj(fb, 1);
> -
> -	return gem->dma_addr + fb->offsets[1] +
> -		(x * fb->format->cpp[1] / fb->format->hsub) +
> -		(y * fb->pitches[1] / fb->format->vsub);
> -}
> -
>   void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
> -		       const struct drm_plane_state *state,
> +		       struct drm_plane_state *state,
>   		       u32 hw_videoport)
>   {
>   	bool lite = dispc->feat->vid_info[hw_plane].is_lite;
>   	u32 fourcc = state->fb->format->format;
>   	u16 cpp = state->fb->format->cpp[0];
>   	u32 fb_width = state->fb->pitches[0] / cpp;
> -	dma_addr_t dma_addr = dispc_plane_state_dma_addr(state);
> +	dma_addr_t dma_addr = drm_fb_dma_get_gem_addr(state->fb, state, 0);
>   	struct dispc_scaling_params scale;
>   
>   	dispc_vid_calc_scaling(dispc, state, &scale, lite);
> @@ -2229,7 +2197,7 @@ void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
>   	if (state->fb->format->num_planes == 2) {
>   		u16 cpp_uv = state->fb->format->cpp[1];
>   		u32 fb_width_uv = state->fb->pitches[1] / cpp_uv;
> -		dma_addr_t p_uv_addr = dispc_plane_state_p_uv_addr(state);
> +		dma_addr_t p_uv_addr = drm_fb_dma_get_gem_addr(state->fb, state, 1);
>   
>   		dispc_vid_write(dispc, hw_plane,
>   				DISPC_VID_BA_UV_0, p_uv_addr & 0xffffffff);
> diff --git a/drivers/gpu/drm/tidss/tidss_dispc.h b/drivers/gpu/drm/tidss/tidss_dispc.h
> index 739d211d0018..2183254f1ab7 100644
> --- a/drivers/gpu/drm/tidss/tidss_dispc.h
> +++ b/drivers/gpu/drm/tidss/tidss_dispc.h
> @@ -140,7 +140,7 @@ int dispc_plane_check(struct dispc_device *dispc, u32 hw_plane,
>   		      const struct drm_plane_state *state,
>   		      u32 hw_videoport);
>   void dispc_plane_setup(struct dispc_device *dispc, u32 hw_plane,
> -		       const struct drm_plane_state *state,
> +		       struct drm_plane_state *state,
>   		       u32 hw_videoport);
>   void dispc_plane_enable(struct dispc_device *dispc, u32 hw_plane, bool enable);
>   const u32 *dispc_plane_formats(struct dispc_device *dispc, unsigned int *len);

Re: [PATCH] drm/tidss: dispc: Switch to drm_fb_dma_get_gem_addr() for framebuffer addresses

Posted by Chen-Yu Tsai 3 weeks, 1 day ago

On Thu, Sep 3, 2026 at 6:53 PM Tomi Valkeinen
<tomi.valkeinen@ideasonboard.com> wrote:
>
> Hi,
>
> On 03/09/2026 09:31, Chen-Yu Tsai wrote:
> > dispc_plane_state_dma_addr() and dispc_plane_state_p_uv_addr() are
> > basically the same as drm_fb_dma_get_addr(), without the support for
>
> drm_fb_dma_get_gem_addr()
>
> > formats with block parameters. Since the driver doesn't support any of
> > those formats, the result is the same.
> >
> > Switch to drm_fb_dma_get_gem_addr() for getting the framebuffer addresses.
> > Drop the const modifier on "struct drm_plane_state *state" for
> > dispc_plane_setup() so that the state can be passed to
> > drm_fb_dma_get_gem_addr().
>
> Or add const modifier to drm_fb_dma_get_gem_addr(). But it's ok either way.

I felt changing it here was the smaller change. Also, returning a DMA
address that one can then do anything to didn't feel completely const to
me.

> > Using the helper also future proofs the driver in case block parameters
> > are added for more formats, especially the common sub-sampled YUV
> > formats.
> >
> > Signed-off-by: Chen-Yu Tsai <wenst@chromium.org>
> > ---
> >   drivers/gpu/drm/tidss/tidss_dispc.c | 38 +++--------------------------
> >   drivers/gpu/drm/tidss/tidss_dispc.h |  2 +-
> >   2 files changed, 4 insertions(+), 36 deletions(-)
>
> Looks good to me. I can pick this up and fix the above typo while
> applying, or wait for v2 if you want to do something about the const.
>
> Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
>   Tomi

Thanks. Please apply and fix the typo.


ChenYu