archive mirror
 help / color / mirror / Atom feed
From: Javier Martinez Canillas <>
To: Geert Uytterhoeven <>
Cc: Linux Kernel Mailing List <>,
	Thomas Zimmermann <>,
	Sam Ravnborg <>, Peter Jones <>,
	Daniel Vetter <>,
	DRI Development <>,
	Linux Fbdev development list <>,
	Hans de Goede <>,
	Ilya Trukhanov <>,
	Thorsten Leemhuis <>,
	Borislav Petkov <>,
	Greg Kroah-Hartman <>
Subject: Re: [PATCH v2] fbdev: Prevent probing generic drivers if a FB is already registered
Date: Tue, 16 Nov 2021 10:30:23 +0100	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

Hello Geert,

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


>> @@ -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:

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

  reply	other threads:[~2021-11-16  9:30 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-11-11 11:11 [PATCH v2] fbdev: Prevent probing generic drivers if a FB is already registered Javier Martinez Canillas
2021-11-11 11:39 ` Greg Kroah-Hartman
2021-11-12 16:12   ` Daniel Vetter
2021-11-15 16:20 ` Geert Uytterhoeven
2021-11-16  9:30   ` Javier Martinez Canillas [this message]
2021-11-16  9:43     ` Geert Uytterhoeven
2021-11-16 10:01       ` Javier Martinez Canillas
2021-11-16 13:49         ` Javier Martinez Canillas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \ \ \ \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).