On Thursday, January 5th, 2023 at 19:48, Maíra Canal <mcanal@xxxxxxxxxx> wrote: > On 1/5/23 15:36, Simon Ser wrote: > > > On Thursday, January 5th, 2023 at 19:30, Maíra Canal mcanal@xxxxxxxxxx wrote: > > > > > > > > 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. > > > > > > I'm not sure if this would correct the problem that I was trying to fix initially. > > > I was trying to make sure that the drivers pass on the > > > igt@kms_addfb_basic@addfb25-bad-modifier IGT test by making sure that the addfb > > > ioctl returns the error. > > > > > > By moving the modifier check to the drm_framebuffer_init(), the test would still > > > fail. > > > > Hm, why is that? The ADDFB2 IOCTL impl calls drm_framebuffer_init(). > > > From what I can see, ADDFB2 IOCTL calls drm_internal_framebuffer_create() [1], > then drm_internal_framebuffer_create() calls the fb_create hook [2]. I'm not sure > here ADDFB2 implicitly calls drm_framebuffer_init(), but I'm probably missing > something. Right, but then all drivers will call drm_framebuffer_init() somewhere in their fb_create hook. For instance amdgpu calls it in amdgpu_display_gem_fb_verify_and_init().
