On Thu, 5 Jan 2023 at 18:48, Maíra Canal <mcanal@xxxxxxxxxx> 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@xxxxxxx> > >> Signed-off-by: Maíra Canal <mcanal@xxxxxxxxxx> > > > > Acked-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > > > 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
