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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi

Am 07.11.22 um 21:46 schrieb Helge Deller:
On 11/7/22 16:30, Thomas Zimmermann wrote:
Hi

Am 07.11.22 um 14:57 schrieb Helge Deller:
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?

No, it's really for broken native drivers or any kind of problematic
modesetting. Most DRM drivers already respect the nomodeset option
and won't load when given. All you'd get are the generic drivers,
such as simpledrm, efifb or simplefb.

There are better options of configuring video output on the kernel
command line.  But as graphics output is such a fundamental feature
to using a computer, we found that a simple and easy option to
workaround erroneous systems would benefit DRM users; hence the
nomodeset parameter.

As fbdev drivers also do modesetting, supporting the parameter simply
unifies the behavior.

Ok.

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()

Well, we do have such a message in disable_modeset() already. [1]
[1] https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/drm_nomodeset.c#L18

Yes, I saw it, but that message quite generic.
If for example my atyfb doesn't show up, I would grep dmesg for "aty" and
not "nomodeset"...

I'd like to add this to fbdev or the drivers instead of the video helper. On the DRM side, it works a a bit different and I think I have a use case for the function that does not directly involve disabling drivers. See below for a proposal.



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...

It's part of the 'video' module, hence the prefix. The 'nomodeset'
option used to be implemented in several DRM drivers. It's an awful
name, but we didn't want to remove it or introduce a new one for the
same feature. So we kept nomodeset for all of DRM.  Then we started
bikeshedding the function name that returns the setting. And
firmware-drivers-only is the best description of what is happening
here. So that's how the name happend.

video_modesetting_disabled() ?
(Just an idea)

The term modesetting is misleading and we had this problem with 'nomodeset' already. There are still plenty of drivers with mode setting, such as the USB-based ones. It's also not so easy on the DRM side, where a modesetting operation is one of many effects of loading an atomic state. Maybe let's leave the name is for now? If we ever find the perfect name, it's a simple rename with sed.

My proposal would be to add a little helper to fbdev that includes your suggestions:

  bool fb_modesetting_disabled(const char *drvname)
  {
     fwonly = video_firmware_drivers_only()
     if (fbonly && drvname)
	pr_warn("")
     return fbonly;
  }

Drivers can use that for the test.

Best regards
Thomas



Helge

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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature


[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux