On 4/29/22 12:20, Thomas Zimmermann wrote: > Hi Javier [snip] >> >>> We can do this with DRIVER_FIRMWARE. Alternatively, I'd suggest to we >>> could also used the existing final parameter of >>> drm_fbdev_generic_setup() to pass a flag that designates a firmware device. >>> >> >> By existing final parameter you mean @preferred_bpp ? That doesn't seem >> correct. I also like that by using DRIVER_FIRMWARE it is completely data >> driven and transparent to the DRM driver. > > DRIVER_FIRMWARE is an indirection and only used here. (Just like > FBINFO_MISC_FIRMWARE is a bad interface for marking framebuffers that > can be unplugged.) If a driver supports hot-unplugging, it should simply > register itself with aperture helpers, regardless of whether it's a > firmware framebuffer or not. > That's fair, and if in practice will only be used by one driver (simpledrm) then that would also allow us to drop patches 1 and 2 from this series. IOW, we wouldn't really need a DRIVER_FIRMWARE capability flag. > Of preferred_bpp, we really only use the lowest byte. All other bits are > up for grabbing. The argument is a workaround for handling > mode_config.prefered_depth correctly. > Yeah, but I didn't want to abuse that argument or package data in an int. > Eventually, preferred_depth should be replaced by something like > 'preferred_format', which will hold the driver's preferred format in > 4CC. We won't need preferred_bpp then. So we could turn preferred_bpp > into a flags argument. > That's a good point, maybe we could start from there and do this cleanup as a preparatory change of this series ? Then the patches would only be 1) renaming preferred_bpp (that would be unused at this point) to flags and 2) make simpledrm to set FBDEV_FIRMWARE flag or something like that. Another option is to add a third flags param to drm_fbdev_generic_setup() and make all drivers to set 0 besides simpledrm. That way marking the fb as FBINFO_MISC_FIRMWARE won't be blocked by the preferred_depth cleanup. -- Best regards, Javier Martinez Canillas Linux Engineering Red Hat
