Re: [PATCH] drm/omapdrm: Pass format info to drm_helper_mode_fill_fb_struct()

[Imre Deak <imre.deak@intel.com>]

On Wed, Aug 06, 2025 at 08:12:35AM -0400, Sasha Levin wrote:
> Commit 41ab92d35ccd ("drm: Make passing of format info to
> drm_helper_mode_fill_fb_struct() mandatory") removed the fallback
> format lookup in drm_helper_mode_fill_fb_struct(), making the
> format info parameter mandatory.
> 
> The coccinelle script in commit a34cc7bf1034 ("drm: Allow the caller
> to pass in the format info to drm_helper_mode_fill_fb_struct()")
> correctly added NULL as the format parameter to omapdrm's call to
> drm_helper_mode_fill_fb_struct(). However, omapdrm was subsequently
> overlooked in the follow-up series that updated drivers to pass the
> actual format info instead of NULL (commits b4d360701b76 through
> 3f019d749671 updated other drivers like amdgpu, exynos, i915, msm,
> tegra, virtio, vmwgfx, etc., but omapdrm was not included).
> 
> This causes fb->format to be NULL, triggering a warning in
> drm_framebuffer_init() at line 870 and causing framebuffer
> initialization to fail with -EINVAL, followed by an oops when
> drm_framebuffer_remove() tries to clean up the failed initialization.
> 
> Note: Unlike other drivers that were fixed to pass format info from
> their fb_create() callbacks all the way down to avoid redundant lookups,
> we don't do that here because omap_framebuffer_init() is also called
> from the fbdev code path (omap_fbdev.c) which doesn't have the format
> info readily available. Changing the function signature to accept format
> info would require adding a format lookup in the fbdev caller, so the
> total number of lookups would remain the same - we'd just be moving the
> lookup from omap_framebuffer_init() to its fbdev caller.

With this change the format would be still looked up twice, in the
drm_internal_framebuffer_create() -> (drm_mode_config_funcs::fb_create)
omap_framebuffer_create() -> omap_framebuffer_init() call path.

This is the same wrt. the other drivers, which also have both an fbdev
and other framebuffer users (the latter calling
drm_mode_config_funcs::fb_create()) and so OMAP should look up/pass down
the format info the same way.

I posted the fix based on the above, see
https://lore.kernel.org/all/20250805175752.690504-2-imre.deak@intel.com

which is now also pushed to drm-misc-next-fixes -> drm-tip.

> Fixes: 41ab92d35ccd ("drm: Make passing of format info to drm_helper_mode_fill_fb_struct() mandatory")
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_fb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
> index 30c81e2e5d6b..42da78bcb5a6 100644
> --- a/drivers/gpu/drm/omapdrm/omap_fb.c
> +++ b/drivers/gpu/drm/omapdrm/omap_fb.c
> @@ -440,7 +440,7 @@ struct drm_framebuffer *omap_framebuffer_init(struct drm_device *dev,
>  		plane->dma_addr  = 0;
>  	}
>  
> -	drm_helper_mode_fill_fb_struct(dev, fb, NULL, mode_cmd);
> +	drm_helper_mode_fill_fb_struct(dev, fb, format, mode_cmd);
>  
>  	ret = drm_framebuffer_init(dev, fb, &omap_framebuffer_funcs);
>  	if (ret) {
> -- 
> 2.39.5
>