[PATCH] drm/gem: Check for valid formats

[Daniel Vetter]

On Thu, 5 Jan 2023 at 18:48, Maíra Canal <mcanal at igalia.com> wrote:
>
> On 1/5/23 12:26, Daniel Vetter wrote:
> > On Tue, Jan 03, 2023 at 09:53:23AM -0300, Maíra Canal wrote:
> >> Currently, drm_gem_fb_create() doesn't check if the pixel format is
> >> supported, which can lead to the acceptance of invalid pixel formats
> >> e.g. the acceptance of invalid modifiers. Therefore, add a check for
> >> valid formats on drm_gem_fb_create().
> >>
> >> Moreover, note that this check is only valid for atomic drivers,
> >> because, for non-atomic drivers, checking drm_any_plane_has_format() is
> >> not possible since the format list for the primary plane is fake, and
> >> we'd therefor reject valid formats.
> >>
> >> Suggested-by: Thomas Zimmermann <tzimmermann at suse.de>
> >> Signed-off-by: Maíra Canal <mcanal at igalia.com>
> >
> > Acked-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> >
> > I think to really make sure we have consensus it'd be good to extend this
> > to a series which removes all the callers to drm_any_plane_has_format()
> > from the various drivers, and then unexports that helper. That way your
> > series here will have more eyes on it :-)
>
> I took a look at the callers to drm_any_plane_has_format() and there are only
> 3 callers (amdgpu, i915 and vmwgfx). They all use drm_any_plane_has_format()
> before calling drm_framebuffer_init(). So, I'm not sure I could remove
> drm_any_plane_has_format() from those drivers. Maybe adding this same check
> to drm_gem_fb_init() and refactor the drivers to make them use drm_gem_fb_init(),
> but I guess this would be part of a different series.

Well vmwgfx still not yet using gem afaik, so that doesn't work.

But why can't we move the modifier check int drm_framebuffer_init()?
That's kinda where it probably should be anyway, there's nothing gem
bo specific in the code you're adding.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch