Re: [PATCH 2/2] fbdev: Add support for the nomodeset kernel parameter

[Helge Deller <deller@gmx.de>]

On 11/7/22 11:49, Thomas Zimmermann wrote:
> Support the kernel's nomodeset parameter for all PCI-based fbdev
> drivers that use aperture helpers to remove other, hardware-agnostic
> graphics drivers.
>
> The parameter is a simple way of using the firmware-provided scanout
> buffer if the hardware's native driver is broken.

Nah... it's probably not broken, but you want it disabled in order
to use the DRM driver instead?

> The same effect
> could be achieved with per-driver options, but the importance of the
> graphics output for many users makes a single, unified approach
> worthwhile.
>
> With nomodeset specified, the fbdev driver module will not load. This
> unifies behavior with similar DRM drivers. In DRM helpers, modules
> first check the nomodeset parameter before registering the PCI
> driver. As fbdev has no such module helpers, we have to modify each
> driver individually.

Ok.

> The name 'nomodeset' is slightly misleading, but has been chosen for
> historical reasons. Several drivers implemented it before it became a
> general option for DRM. So keeping the existing name was preferred over
> introducing a new one.

> diff --git a/drivers/video/fbdev/aty/aty128fb.c b/drivers/video/fbdev/aty/aty128fb.c
> index 57e398fe7a81c..1a26ac2865d65 100644
> --- a/drivers/video/fbdev/aty/aty128fb.c
> +++ b/drivers/video/fbdev/aty/aty128fb.c
> @@ -2503,7 +2504,12 @@ static int aty128fb_init(void)
>   {
>   #ifndef MODULE
>   	char *option = NULL;
> +#endif
> +
> +	if (video_firmware_drivers_only())
> +		return -ENODEV;

I think it makes sense to give at least some info, why a specific
driver wasn't loaded, e.g. something like this kernel message:
aty128fb: Driver disabled due to "nomodeset" kernel parameter.

If you e.g. change the function video_firmware_drivers_only()
to become video_firmware_drivers_only(const char *drivername)
then you could print such a message in video_firmware_drivers_only().

And I don't like very much the name of function video_firmware_drivers_only(),
but don't have any other better idea right now either...

Helge