Re: [PATCH 4/6] drm/msm: fbdev: Calculate buffer geometry with format helpers

[Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>]

On Tue, Apr 21, 2026 at 02:51:17PM +0200, Thomas Zimmermann wrote:
> Replace the geometry and size calculation in msm's fbdev emulation
> with DRM format helpers. This consists of a 4CC lookup from the fbdev
> parameters, format lookup, pitch calculation and size calculation.
> Then allocate the GEM buffer object for the framebuffer memory from
> the calculated size.
> 
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> ---
>  drivers/gpu/drm/msm/msm_drv.h   |  7 -------
>  drivers/gpu/drm/msm/msm_fbdev.c | 26 ++++++++++++--------------
>  2 files changed, 12 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
> index b3173fa4d222..00c50d7437d6 100644
> --- a/drivers/gpu/drm/msm/msm_drv.h
> +++ b/drivers/gpu/drm/msm/msm_drv.h
> @@ -509,13 +509,6 @@ void msm_hrtimer_work_init(struct msm_hrtimer_work *work,
>  #define DBG(fmt, ...) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
>  #define VERB(fmt, ...) if (0) DRM_DEBUG_DRIVER(fmt"\n", ##__VA_ARGS__)
>  
> -static inline int align_pitch(int width, int bpp)
> -{
> -	int bytespp = (bpp + 7) / 8;
> -	/* adreno needs pitch aligned to 32 pixels: */
> -	return bytespp * ALIGN(width, 32);
> -}
> -
>  /* for the generated headers: */
>  #define INVALID_IDX(idx) ({BUG(); 0;})
>  #define fui(x)                ({BUG(); 0;})
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index 1eb0368a6d07..5532c5779f17 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -95,23 +95,25 @@ int msm_fbdev_driver_fbdev_probe(struct drm_fb_helper *helper,
>  	struct fb_info *fbi = helper->info;
>  	struct drm_mode_fb_cmd2 mode_cmd = { };
>  	struct drm_framebuffer *fb = NULL;
> +	const struct drm_format_info *format;
> +	u32 fourcc, pitch;
> +	u64 size;
>  	struct drm_gem_object *bo;
>  	uint64_t paddr;
> -	uint32_t format;
> -	int ret, pitch;
> -	int size;
> -
> -	format = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
> +	int ret;
>  
>  	DBG("create fbdev: %dx%d@%d (%dx%d)", sizes->surface_width,
>  			sizes->surface_height, sizes->surface_bpp,
>  			sizes->fb_width, sizes->fb_height);
>  
> -	pitch = align_pitch(sizes->surface_width, sizes->surface_bpp);
> +	fourcc = drm_mode_legacy_fb_format(sizes->surface_bpp, sizes->surface_depth);
> +	format = drm_get_format_info(dev, fourcc, DRM_FORMAT_MOD_LINEAR);
> +	/* adreno needs pitch aligned to 32 pixels: */
> +	pitch = drm_format_info_min_pitch(format, 0, ALIGN(sizes->surface_width, 32));
> +	size = ALIGN(pitch * sizes->surface_height, PAGE_SIZE);

Hmm, why do we need to align the size to the PAGE_SIZE? I might be
missing a point, but it would be nice to mention it in the commit
message.

Other than that, LGTM.

-- 
With best wishes
Dmitry