Re: [PATCH 1/2] drm: Move nomodeset kernel parameter to drivers/video

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

Hello Thomas,

On 11/7/22 11:49, Thomas Zimmermann wrote:

[...]

> 
> diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
> index a465d5242774a..70178c5f53956 100644
> --- a/Documentation/admin-guide/kernel-parameters.txt
> +++ b/Documentation/admin-guide/kernel-parameters.txt
> @@ -3777,7 +3777,7 @@
>  			shutdown the other cpus.  Instead use the REBOOT_VECTOR
>  			irq.
>  
> -	nomodeset	Disable kernel modesetting. DRM drivers will not perform
> +	nomodeset	Disable kernel modesetting. Graphics drivers will not perform
>  			display-mode changes or accelerated rendering. Only the
>  			system framebuffer will be available for use if this was
>  			set-up by the firmware or boot loader.

Not really part of your patch but probably we should reword this a little bit.

Because as this is written, it implies that not only DRM drivers with feature
DRIVER_MODESET will not be available but also drivers with DRIVER_RENDER. But
that's not the case, render-only drivers usually just ignore this parameter
(but not all IIRC), so I wonder how we could make this comment more accurate.

Also maybe we can mention in the comment fbdev and DRM? Just to make it clear
that this will affect to both subsystems? When I first worked on this, there
were a lot of assumptions in the stack (gdm, mutter, plymouth) that nomodeset
basically meant "no DRM but fbdev".

[...]

>  
>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
>  
> -extern bool drm_firmware_drivers_only(void);
> +/* TODO: Inline drm_firmware_drivers_only() in all its callers. */

I guess you plan to do that as follow-up patches once this series land? Just
to avoid the churn to require acks for all the drivers to merge this series?

The changes looks good to me.

Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat