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

Sasha Levin posted 1 patch 1 month, 4 weeks ago
drivers/gpu/drm/omapdrm/omap_fb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] drm/omapdrm: Pass format info to drm_helper_mode_fill_fb_struct()
Posted by Sasha Levin 1 month, 4 weeks ago
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.

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
Re: [PATCH] drm/omapdrm: Pass format info to drm_helper_mode_fill_fb_struct()
Posted by Imre Deak 1 month, 4 weeks ago
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
>