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

From
Thomas Zimmermann <tzimmermann@suse.de>
Date
2022-11-11 13:06:36
Also in
dri-devel
Reply to
Javier Martinez Canillas, 2022-11-11
Hi

Am 11.11.22 um 10:28 schrieb Javier Martinez Canillas:
Hello Thomas,

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

[...]
quoted
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.
I see what you mean, but it's hard to describe in simple words. The option is a bit fuzzy. It means that a driver will not replace a firmware buffer; even if that means it won't initialize at all. I guess we should spell that out.
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".
I can change the text to say 'DRM and fbdev drivers'.

Best regards
Thomas
[...]
quoted
  
  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>
-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

Attachments

Thread (15 messages) 15 messages, 3 authors, last reply 2022-11-11
Keyboard shortcuts
hback out one level
jnext message in thread / next series-diff position
kprevious message in thread / previous series-diff position
ldrill in
Escclose help / fold thread tree
?toggle this help