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

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

[Javier Martinez Canillas]

The efifb and simplefb drivers just render to a pre-allocated frame buffer
and rely on the display hardware being initialized before the kernel boots.

But if another driver already probed correctly and registered a fbdev, the
generic drivers shouldn't be probed since an actual driver for the display
hardware is already present.

Reported-by: Ilya Trukhanov <lahvuun@gmail.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/video/fbdev/efifb.c    | 6 ++++++
 drivers/video/fbdev/simplefb.c | 6 ++++++
 2 files changed, 12 insertions(+)

diff --git drivers/video/fbdev/efifb.c drivers/video/fbdev/efifb.c
index edca3703b964..76325c07cf0c 100644
--- drivers/video/fbdev/efifb.c
+++ drivers/video/fbdev/efifb.c
@@ -351,6 +351,12 @@ static int efifb_probe(struct platform_device *dev)
 	char *option = NULL;
 	efi_memory_desc_t md;
 
+	if (num_registered_fb > 0) {
+		dev_err(&dev->dev,
+			"efifb: a framebuffer is already registered\n");
+		return -EINVAL;
+	}
+
 	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
 		return -ENODEV;
 
diff --git drivers/video/fbdev/simplefb.c drivers/video/fbdev/simplefb.c
index 62f0ded70681..55c1f54d7663 100644
--- drivers/video/fbdev/simplefb.c
+++ drivers/video/fbdev/simplefb.c
@@ -407,6 +407,12 @@ static int simplefb_probe(struct platform_device *pdev)
 	struct simplefb_par *par;
 	struct resource *mem;
 
+	if (num_registered_fb > 0) {
+		dev_err(&pdev->dev,
+			"simplefb: a framebuffer is already registered\n");
+		return -EINVAL;
+	}
+
 	if (fb_get_options("simplefb", NULL))
 		return -ENODEV;
 
-- 
2.33.1

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

[Daniel Vetter]

On Thu, Nov 11, 2021 at 10:20:53AM +0100, Javier Martinez Canillas wrote:
> The efifb and simplefb drivers just render to a pre-allocated frame buffer
> and rely on the display hardware being initialized before the kernel boots.
> 
> But if another driver already probed correctly and registered a fbdev, the
> generic drivers shouldn't be probed since an actual driver for the display
> hardware is already present.
> 
> Reported-by: Ilya Trukhanov <lahvuun@gmail.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Also Cc: stable@vger.kernel.org?

btw time to organize drm-misc commit rights so you can push stuff like
this?
-Daniel

> ---
> 
>  drivers/video/fbdev/efifb.c    | 6 ++++++
>  drivers/video/fbdev/simplefb.c | 6 ++++++
>  2 files changed, 12 insertions(+)
> 
> diff --git drivers/video/fbdev/efifb.c drivers/video/fbdev/efifb.c
> index edca3703b964..76325c07cf0c 100644
> --- drivers/video/fbdev/efifb.c
> +++ drivers/video/fbdev/efifb.c
> @@ -351,6 +351,12 @@ static int efifb_probe(struct platform_device *dev)
>  	char *option = NULL;
>  	efi_memory_desc_t md;
>  
> +	if (num_registered_fb > 0) {
> +		dev_err(&dev->dev,
> +			"efifb: a framebuffer is already registered\n");
> +		return -EINVAL;
> +	}
> +
>  	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EFI || pci_dev_disabled)
>  		return -ENODEV;
>  
> diff --git drivers/video/fbdev/simplefb.c drivers/video/fbdev/simplefb.c
> index 62f0ded70681..55c1f54d7663 100644
> --- drivers/video/fbdev/simplefb.c
> +++ drivers/video/fbdev/simplefb.c
> @@ -407,6 +407,12 @@ static int simplefb_probe(struct platform_device *pdev)
>  	struct simplefb_par *par;
>  	struct resource *mem;
>  
> +	if (num_registered_fb > 0) {
> +		dev_err(&pdev->dev,
> +			"simplefb: a framebuffer is already registered\n");
> +		return -EINVAL;
> +	}
> +
>  	if (fb_get_options("simplefb", NULL))
>  		return -ENODEV;
>  
> -- 
> 2.33.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Thorsten Leemhuis]

On 11.11.21 10:20, Javier Martinez Canillas wrote:
> The efifb and simplefb drivers just render to a pre-allocated frame buffer
> and rely on the display hardware being initialized before the kernel boots.
> 
> But if another driver already probed correctly and registered a fbdev, the
> generic drivers shouldn't be probed since an actual driver for the display
> hardware is already present.
> 
> Reported-by: Ilya Trukhanov <lahvuun@gmail.com>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---

TLDR: Javier, in case you need to send an improved patch, could you
please add this before the 'Reported-by:'

Link: https://lore.kernel.org/r/20211110200253.rfudkt3edbd3nsyj@lahvuun/

And if the patch is already good to go: could the subsystem maintainer
please add it when applying?


Long story: hi, this is your Linux kernel regression tracker speaking.
Thanks for working on a fix for a regression I'm keeping an eye on.

There is one small detail that could be improved: the commit message
would benefit from a link to the regression report, for reasons
explained in Documentation/process/submitting-patches.rst. To quote:

```
If related discussions or any other background information behind the
change can be found on the web, add 'Link:' tags pointing to it. In case
your patch fixes a bug, for example, add a tag with a URL referencing
the report in the mailing list archives or a bug tracker;
```

This concept is old, but the text was reworked recently to make this use
case for the Link: tag clearer. For details see:
https://git.kernel.org/linus/1f57bd42b77c

Yes, that "Link:" is not really crucial; but it's good to have if
someone needs to look into the backstory of this change sometime in the
future. But I care for a different reason. I'm tracking this regression
(and others) with regzbot, my Linux kernel regression tracking bot. This
bot will notice if a patch with a Link: tag to a tracked regression gets
posted and record that, which allowed anyone looking into the regression
to quickly gasp the current status from regzbot's webui
(https://linux-regtracking.leemhuis.info/regzbot ) or its reports. The
bot will also notice if a commit with a Link: tag to a regression report
is applied by Linus and then automatically mark the regression as
resolved then.

IOW: this tag makes my life a regression tracker a lot easier, as I
otherwise have to tell regzbot manually about the fix. ;-)

Ciao, Thorsten (while carrying his Linux kernel regression tracker hat)

#regzbot ^backmonitor
https://lore.kernel.org/r/20211110200253.rfudkt3edbd3nsyj@lahvuun/

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

[Daniel Vetter]

On Thu, Nov 11, 2021 at 10:58:14AM +0100, Thorsten Leemhuis wrote:
> On 11.11.21 10:20, Javier Martinez Canillas wrote:
> > The efifb and simplefb drivers just render to a pre-allocated frame buffer
> > and rely on the display hardware being initialized before the kernel boots.
> > 
> > But if another driver already probed correctly and registered a fbdev, the
> > generic drivers shouldn't be probed since an actual driver for the display
> > hardware is already present.
> > 
> > Reported-by: Ilya Trukhanov <lahvuun@gmail.com>
> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> > ---
> 
> TLDR: Javier, in case you need to send an improved patch, could you
> please add this before the 'Reported-by:'
> 
> Link: https://lore.kernel.org/r/20211110200253.rfudkt3edbd3nsyj@lahvuun/

Uh I thought Link: was for the patch submission chain, and we've used
References: for bug reports and everything else. Is the extension of Link:
a new thing?
-Daniel

> 
> And if the patch is already good to go: could the subsystem maintainer
> please add it when applying?
> 
> 
> Long story: hi, this is your Linux kernel regression tracker speaking.
> Thanks for working on a fix for a regression I'm keeping an eye on.
> 
> There is one small detail that could be improved: the commit message
> would benefit from a link to the regression report, for reasons
> explained in Documentation/process/submitting-patches.rst. To quote:
> 
> ```
> If related discussions or any other background information behind the
> change can be found on the web, add 'Link:' tags pointing to it. In case
> your patch fixes a bug, for example, add a tag with a URL referencing
> the report in the mailing list archives or a bug tracker;
> ```
> 
> This concept is old, but the text was reworked recently to make this use
> case for the Link: tag clearer. For details see:
> https://git.kernel.org/linus/1f57bd42b77c
> 
> Yes, that "Link:" is not really crucial; but it's good to have if
> someone needs to look into the backstory of this change sometime in the
> future. But I care for a different reason. I'm tracking this regression
> (and others) with regzbot, my Linux kernel regression tracking bot. This
> bot will notice if a patch with a Link: tag to a tracked regression gets
> posted and record that, which allowed anyone looking into the regression
> to quickly gasp the current status from regzbot's webui
> (https://linux-regtracking.leemhuis.info/regzbot ) or its reports. The
> bot will also notice if a commit with a Link: tag to a regression report
> is applied by Linus and then automatically mark the regression as
> resolved then.
> 
> IOW: this tag makes my life a regression tracker a lot easier, as I
> otherwise have to tell regzbot manually about the fix. ;-)
> 
> Ciao, Thorsten (while carrying his Linux kernel regression tracker hat)
> 
> #regzbot ^backmonitor
> https://lore.kernel.org/r/20211110200253.rfudkt3edbd3nsyj@lahvuun/

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

[Thorsten Leemhuis]

On 11.11.21 11:00, Daniel Vetter wrote:
> On Thu, Nov 11, 2021 at 10:58:14AM +0100, Thorsten Leemhuis wrote:
>> On 11.11.21 10:20, Javier Martinez Canillas wrote:
>>> The efifb and simplefb drivers just render to a pre-allocated frame buffer
>>> and rely on the display hardware being initialized before the kernel boots.
>>>
>>> But if another driver already probed correctly and registered a fbdev, the
>>> generic drivers shouldn't be probed since an actual driver for the display
>>> hardware is already present.
>>>
>>> Reported-by: Ilya Trukhanov <lahvuun@gmail.com>
>>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>>> ---
>>
>> TLDR: Javier, in case you need to send an improved patch, could you
>> please add this before the 'Reported-by:'
>>
>> Link: https://lore.kernel.org/r/20211110200253.rfudkt3edbd3nsyj@lahvuun/
> 
> Uh I thought Link: was for the patch submission chain, and we've used
> References: for bug reports and everything else. Is the extension of Link:
> a new thing?

Not really (afaics), but I made that clearer recently. To quote from my
own text below:

>> This concept is old, but the text was reworked recently to make this use
>> case for the Link: tag clearer. For details see:
>> https://git.kernel.org/linus/1f57bd42b77c

If you search the history, you'll find quite a few commits (some really
old) that use 'Link:' to point to bugtracker or regression reports.
'References:' is used as well, but mainly by the drm subsystem.

But yes, sadly the Link tag historically is overloaded and used for
different things afaics. I'm all for fixing this and plan to start a
discussion about this in the next few days (hopefully it doesn't become
weeks), but for now I have a few issues with regzbot I need to deal with
first.

Ciao, Thorsten

>> And if the patch is already good to go: could the subsystem maintainer
>> please add it when applying?
>>
>>
>> Long story: hi, this is your Linux kernel regression tracker speaking.
>> Thanks for working on a fix for a regression I'm keeping an eye on.
>>
>> There is one small detail that could be improved: the commit message
>> would benefit from a link to the regression report, for reasons
>> explained in Documentation/process/submitting-patches.rst. To quote:
>>
>> ```
>> If related discussions or any other background information behind the
>> change can be found on the web, add 'Link:' tags pointing to it. In case
>> your patch fixes a bug, for example, add a tag with a URL referencing
>> the report in the mailing list archives or a bug tracker;
>> ```
>>
>> This concept is old, but the text was reworked recently to make this use
>> case for the Link: tag clearer. For details see:
>> https://git.kernel.org/linus/1f57bd42b77c
>>
>> Yes, that "Link:" is not really crucial; but it's good to have if
>> someone needs to look into the backstory of this change sometime in the
>> future. But I care for a different reason. I'm tracking this regression
>> (and others) with regzbot, my Linux kernel regression tracking bot. This
>> bot will notice if a patch with a Link: tag to a tracked regression gets
>> posted and record that, which allowed anyone looking into the regression
>> to quickly gasp the current status from regzbot's webui
>> (https://linux-regtracking.leemhuis.info/regzbot ) or its reports. The
>> bot will also notice if a commit with a Link: tag to a regression report
>> is applied by Linus and then automatically mark the regression as
>> resolved then.
>>
>> IOW: this tag makes my life a regression tracker a lot easier, as I
>> otherwise have to tell regzbot manually about the fix. ;-)
>>
>> Ciao, Thorsten (while carrying his Linux kernel regression tracker hat)
>>
>> #regzbot ^backmonitor
>> https://lore.kernel.org/r/20211110200253.rfudkt3edbd3nsyj@lahvuun/
> 

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

[Javier Martinez Canillas]

Hello Daniel,

On 11/11/21 10:54, Daniel Vetter wrote:
> On Thu, Nov 11, 2021 at 10:20:53AM +0100, Javier Martinez Canillas wrote:
>> The efifb and simplefb drivers just render to a pre-allocated frame buffer
>> and rely on the display hardware being initialized before the kernel boots.
>>
>> But if another driver already probed correctly and registered a fbdev, the
>> generic drivers shouldn't be probed since an actual driver for the display
>> hardware is already present.
>>
>> Reported-by: Ilya Trukhanov <lahvuun@gmail.com>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>

Thanks for your review.
 
> Also Cc: stable@vger.kernel.org?
> 
> btw time to organize drm-misc commit rights so you can push stuff like
> this?

Yes, I'll start the process today to request that.

> -Daniel
> 

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