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.