Re: [PATCH v2] fbdev: Prevent probing generic drivers if a FB is already registered

[Javier Martinez Canillas]

Hello Geert,

On 11/15/21 17:20, Geert Uytterhoeven wrote:

[snip]

>> @@ -351,6 +351,17 @@ static int efifb_probe(struct platform_device *dev)
>>         char *option = NULL;
>>         efi_memory_desc_t md;
>>
>> +       /*
>> +        * Generic drivers must not be registered if a framebuffer exists.
>> +        * If a native driver was probed, the display hardware was already
>> +        * taken and attempting to use the system framebuffer is dangerous.
>> +        */
>> +       if (num_registered_fb > 0) {
> 
> Who says this registered fbdev is driving the same hardware as efifb?
> This might be e.g. a small external display connected to i2c or spi.
> 
>> +               dev_err(&dev->dev,
>> +                       "efifb: a framebuffer is already registered\n");
>> +               return -EINVAL;
>> +       }
>> +

That's true, although I wonder if the {efi,simple}fb drivers should even be
probed in that case. As I see it, these are always a best effort that are
only useful for earlycon or if there isn't another display driver supported.

Since there may be other conditions needed in order for these to work. For
example, when using the u-boot EFI stub in most cases the unused clocks and
power domains can't be gated or otherwise the firmware frame buffer could go
away (e.g: will need to boot with "clk_ignore_unused" and "pd_ignore_unused").

Same for the simplefb driver, if the DT node is missing resources that are
needed by the display controller to continue working (clocks, regulators,
power domains), the firmware setup framebuffer will go away at some point.

So this is already a fragile solution and $SUBJECT doesn't make things worse
IMO. Since not having something like this can lead to issues as reported by:

https://lore.kernel.org/all/20211110200253.rfudkt3edbd3nsyj@lahvuun/

We could probably do some smarter here by providing a function that checks
if the registered fbdev drivers matches the aperture base. But I'm unsure
if that's worth it. After all, fbdev drivers are likely to be disabled by
most distros soon now that we have the simpledrm driver.

Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat