[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aJNQvlWZNI04BU_l@ideak-desk>
Date: Wed, 6 Aug 2025 15:55:26 +0300
From: Imre Deak <imre.deak@...el.com>
To: Sasha Levin <sashal@...nel.org>
CC: <tomi.valkeinen@...asonboard.com>, <maarten.lankhorst@...ux.intel.com>,
<mripard@...nel.org>, <tzimmermann@...e.de>, <airlied@...il.com>,
<simona@...ll.ch>, <laurent.pinchart+renesas@...asonboard.com>,
<ville.syrjala@...ux.intel.com>, <dri-devel@...ts.freedesktop.org>,
<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] drm/omapdrm: Pass format info to
drm_helper_mode_fill_fb_struct()
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@...nel.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
>
Powered by blists - more mailing lists