Re: [PATCH v3 3/3] drm: Allow simpledrm to setup its emulated FB as firmware provided

[Javier Martinez Canillas <javierm@redhat.com>]

On 5/4/22 12:58, Daniel Vetter wrote:
> On Tue, May 03, 2022 at 09:15:40AM +0200, Javier Martinez Canillas wrote:
>> Indicate to fbdev subsystem that the registered framebuffer is provided by
>> the system firmware, so that it can handle accordingly. For example, would
>> unregister the FB devices if asked to remove the conflicting framebuffers.
>>
>> Add a new DRM_FB_FW field to drm_fbdev_generic_setup() options parameter.
>> Drivers can use this to indicate the FB helper initialization that the FB
>> registered is provided by the firmware, so it can be configured as such.
>>
>> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>
>> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
>> ---
>>
>> Changes in v3:
>> - Drop the firmware local variable (Laurent Pinchart).
>> - Use DRM_FB_OPTION() since DRM_FB_SET_OPTION() got renamed (kernel test robot).
> 
> Just for the record what I brought up on irc already:
> 
> FBINFO_MISC_FIRMWARE is purely an internal flag with no uapi impact, and
> it's only to control whether we nuke this from
> remove_conflicting_framebuffer or not. Since simpledrm only ever binds
> against sysfb I think it'd be cleaner to only rely on that, and relegate

That's not actually true. The OF subsystem also registers "simple-framebuffer"
devices when there are Device Tree nodes that contain a "simple-framebuffer"
compatible string. In that case these pdev will also bind against simpledrm.

> that entire FBINFO_MISC_FIRMWARE misc hack to the fbdev dungeons and let
> it quietly wither away there.
>
> Also I'm not a huge fan of these midlayer flags in general :-)

And while I agree with you that these midlayer flags are horrible, that is
what any other fbdev that makes use of a firmware-provided framebuffer set,
so simpledrm emulated fbdev shouldn't be the exception IMO.

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat