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

[Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>]

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);