nouveau.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic
@ 2021-11-03 12:28 Javier Martinez Canillas
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem Javier Martinez Canillas
                   ` (2 more replies)
  0 siblings, 3 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 12:28 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, Javier Martinez Canillas,
	amd-gfx, VMware Graphics, Peter Robinson, Neal Gompa,
	Dave Airlie, Chia-I Wu, Ben Skeggs, Michel Dänzer,
	Maarten Lankhorst, Maxime Ripard, Hans de Goede, Jani Nikula,
	Rodrigo Vivi, nouveau, virtualization, Pekka Paalanen,
	Greg Kroah-Hartman, Pan, Xinhui, spice-devel, Daniel Vetter,
	Alex Deucher, intel-gfx, Christian König, Zack Rusin

[ resend with all relevant people as Cc now, sorry to others for the spam ]

There is a lot of historical baggage on this parameter. It's defined in
the vgacon driver as a "nomodeset" parameter, but it's handler function is
called text_mode() that sets a variable named vgacon_text_mode_force whose
value is queried with a function named vgacon_text_force().

All this implies that it's about forcing text mode for VGA, yet it is not
used in neither vgacon nor other console driver. The only users for these
are DRM drivers, that check for the vgacon_text_force() return value to
determine whether the driver could be loaded or not.

That makes it quite confusing to read the code, because the variables and
function names don't reflect what they actually do and also are not in the
same subsystem as the drivers that make use of them.

This patch-set attempts to cleanup the code by moving the nomodseset param
to the DRM subsystem and do some renaming to make their intention clearer.

There is also another aspect that could be improved, and is the fact that
drivers are checking for the nomodeset being set as an indication if have
to be loaded.

But there may be other reasons why this could be the case, so it is better
to encapsulate the logic in a separate function to make clear what's about.

Patch #1 is just a trivial fix for a comment that isn't referring to the
correct kernel parameter.

Patch #2 moves the nomodeset logic to the DRM subsystem.

Patch #3 renames the vgacon_text_force() function and accompaning logic as
drm_modeset_disabled(), which is what this function is really about.

Patch #4 adds a drm_drv_enabled() function that could be used by drivers
to check if could be enabled.

Patch #5 uses the drm_drv_enabled() function to check this instead of just
checking if nomodeset has been set.


Javier Martinez Canillas (5):
  drm/i915: Fix comment about modeset parameters
  drm: Move nomodeset kernel parameter handler to the DRM subsystem
  drm: Rename vgacon_text_force() function to drm_modeset_disabled()
  drm: Add a drm_drv_enabled() helper function
  drm: Use drm_drv_enabled() instead of drm_modeset_disabled()

 drivers/gpu/drm/Makefile                |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 ++---
 drivers/gpu/drm/ast/ast_drv.c           |  3 +--
 drivers/gpu/drm/drm_drv.c               | 21 ++++++++++++++++++++
 drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_module.c      | 10 +++++-----
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  3 +--
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +--
 drivers/gpu/drm/qxl/qxl_drv.c           |  3 +--
 drivers/gpu/drm/radeon/radeon_drv.c     |  3 +--
 drivers/gpu/drm/tiny/bochs.c            |  3 +--
 drivers/gpu/drm/tiny/cirrus.c           |  3 +--
 drivers/gpu/drm/vboxvideo/vbox_drv.c    |  5 +----
 drivers/gpu/drm/virtio/virtgpu_drv.c    |  3 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  3 +--
 drivers/video/console/vgacon.c          | 21 --------------------
 include/drm/drm_drv.h                   |  1 +
 include/drm/drm_mode_config.h           |  6 ++++++
 include/linux/console.h                 |  6 ------
 19 files changed, 73 insertions(+), 57 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

-- 
2.33.1


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem
  2021-11-03 12:28 [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
@ 2021-11-03 12:28 ` Javier Martinez Canillas
  2021-11-03 12:41   ` Thomas Zimmermann
  2021-11-03 12:56   ` Jani Nikula
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled() Javier Martinez Canillas
  2021-11-03 13:01 ` [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Thomas Zimmermann
  2 siblings, 2 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 12:28 UTC (permalink / raw)
  To: linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, Javier Martinez Canillas,
	amd-gfx, VMware Graphics, Peter Robinson, Neal Gompa,
	Dave Airlie, Chia-I Wu, Ben Skeggs, Michel Dänzer,
	Maarten Lankhorst, Maxime Ripard, Hans de Goede, Jani Nikula,
	Rodrigo Vivi, nouveau, virtualization, Pekka Paalanen,
	Greg Kroah-Hartman, Pan, Xinhui, spice-devel, Daniel Vetter,
	Alex Deucher, intel-gfx, Christian König, Zack Rusin

The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
but the exported vgacon_text_force() symbol is only used by DRM drivers.

It makes much more sense for the parameter logic to be in the subsystem
of the drivers that are making use of it. Let's move that to DRM.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/gpu/drm/Makefile                |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
 drivers/gpu/drm/ast/ast_drv.c           |  1 -
 drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_module.c      |  2 --
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
 drivers/gpu/drm/qxl/qxl_drv.c           |  1 -
 drivers/gpu/drm/radeon/radeon_drv.c     |  1 -
 drivers/gpu/drm/tiny/bochs.c            |  1 -
 drivers/gpu/drm/tiny/cirrus.c           |  1 -
 drivers/gpu/drm/vboxvideo/vbox_drv.c    |  1 -
 drivers/gpu/drm/virtio/virtgpu_drv.c    |  1 -
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  1 -
 drivers/video/console/vgacon.c          | 21 --------------------
 include/drm/drm_mode_config.h           |  6 ++++++
 include/linux/console.h                 |  6 ------
 17 files changed, 35 insertions(+), 41 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c41156deb5f..0e2d60ea93ca 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.
 
 obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
 
+obj-y += drm_nomodeset.o
+
 drm_cma_helper-y := drm_gem_cma_helper.o
 obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c718fb5f3f8a..2680a2aaa877 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -31,7 +31,6 @@
 #include "amdgpu_drv.h"
 
 #include <drm/drm_pciids.h>
-#include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/vga_switcheroo.h>
@@ -2515,7 +2514,7 @@ static int __init amdgpu_init(void)
 	int r;
 
 	if (vgacon_text_force()) {
-		DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
+		DRM_ERROR("amdgpu kernel modesetting disabled.\n");
 		return -EINVAL;
 	}
 
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 86d5cd7b6318..048be607b182 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -26,7 +26,6 @@
  * Authors: Dave Airlie <airlied@redhat.com>
  */
 
-#include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 
diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
new file mode 100644
index 000000000000..1ac9a8d5a8fe
--- /dev/null
+++ b/drivers/gpu/drm/drm_nomodeset.c
@@ -0,0 +1,26 @@
+// SPDX-License-Identifier: GPL-2.0
+
+#include <linux/module.h>
+#include <linux/types.h>
+
+static bool vgacon_text_mode_force;
+
+bool vgacon_text_force(void)
+{
+	return vgacon_text_mode_force;
+}
+EXPORT_SYMBOL(vgacon_text_force);
+
+static int __init text_mode(char *str)
+{
+	vgacon_text_mode_force = true;
+
+	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
+	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
+	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
+
+	return 1;
+}
+
+/* force text mode - used by kernel modesetting */
+__setup("nomodeset", text_mode);
diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
index c7507266aa83..14a59226519d 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -4,8 +4,6 @@
  * Copyright © 2021 Intel Corporation
  */
 
-#include <linux/console.h>
-
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_object.h"
 #include "i915_active.h"
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 6b9243713b3c..685e766db6a4 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -6,7 +6,6 @@
  *          Dave Airlie
  */
 
-#include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/vmalloc.h>
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 1f828c9f691c..029997f50d1a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -22,7 +22,6 @@
  * Authors: Ben Skeggs
  */
 
-#include <linux/console.h>
 #include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/pci.h>
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index fc47b0deb021..3cd6bd9f059d 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -29,7 +29,6 @@
 
 #include "qxl_drv.h"
 
-#include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/vgaarb.h>
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index b74cebca1f89..9b606c1b11ec 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -31,7 +31,6 @@
 
 
 #include <linux/compat.h>
-#include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/vga_switcheroo.h>
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 2ce3bd903b70..04333f78be55 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -1,6 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 
-#include <linux/console.h>
 #include <linux/pci.h>
 
 #include <drm/drm_aperture.h>
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 4611ec408506..8bd674f0d682 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -16,7 +16,6 @@
  * Copyright 1999-2001 Jeff Garzik <jgarzik@pobox.com>
  */
 
-#include <linux/console.h>
 #include <linux/dma-buf-map.h>
 #include <linux/module.h>
 #include <linux/pci.h>
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
index a6c81af37345..e6d983121d0b 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -7,7 +7,6 @@
  *          Michael Thayer <michael.thayer@oracle.com,
  *          Hans de Goede <hdegoede@redhat.com>
  */
-#include <linux/console.h>
 #include <linux/module.h>
 #include <linux/pci.h>
 #include <linux/vt_kern.h>
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index 749db18dcfa2..cd4c170236f1 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -27,7 +27,6 @@
  */
 
 #include <linux/module.h>
-#include <linux/console.h>
 #include <linux/pci.h>
 #include <linux/poll.h>
 #include <linux/wait.h>
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index ab9a1750e1df..fcc4b5a7f639 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -25,7 +25,6 @@
  *
  **************************************************************************/
 
-#include <linux/console.h>
 #include <linux/dma-mapping.h>
 #include <linux/module.h>
 #include <linux/pci.h>
diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
index ef9c57ce0906..d4320b147956 100644
--- a/drivers/video/console/vgacon.c
+++ b/drivers/video/console/vgacon.c
@@ -97,30 +97,9 @@ static int 		vga_video_font_height;
 static int 		vga_scan_lines		__read_mostly;
 static unsigned int 	vga_rolled_over; /* last vc_origin offset before wrap */
 
-static bool vgacon_text_mode_force;
 static bool vga_hardscroll_enabled;
 static bool vga_hardscroll_user_enable = true;
 
-bool vgacon_text_force(void)
-{
-	return vgacon_text_mode_force;
-}
-EXPORT_SYMBOL(vgacon_text_force);
-
-static int __init text_mode(char *str)
-{
-	vgacon_text_mode_force = true;
-
-	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
-	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
-	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
-
-	return 1;
-}
-
-/* force text mode - used by kernel modesetting */
-__setup("nomodeset", text_mode);
-
 static int __init no_scroll(char *str)
 {
 	/*
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index 48b7de80daf5..e1d2042a7b77 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -969,4 +969,10 @@ static inline int drm_mode_config_init(struct drm_device *dev)
 void drm_mode_config_reset(struct drm_device *dev);
 void drm_mode_config_cleanup(struct drm_device *dev);
 
+#ifdef CONFIG_VGA_CONSOLE
+extern bool vgacon_text_force(void);
+#else
+static inline bool vgacon_text_force(void) { return false; }
+#endif
+
 #endif
diff --git a/include/linux/console.h b/include/linux/console.h
index 20874db50bc8..d4dd8384898b 100644
--- a/include/linux/console.h
+++ b/include/linux/console.h
@@ -217,12 +217,6 @@ extern atomic_t ignore_console_lock_warning;
 #define VESA_HSYNC_SUSPEND      2
 #define VESA_POWERDOWN          3
 
-#ifdef CONFIG_VGA_CONSOLE
-extern bool vgacon_text_force(void);
-#else
-static inline bool vgacon_text_force(void) { return false; }
-#endif
-
 extern void console_init(void);
 
 /* For deferred console takeover */
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()
  2021-11-03 12:28 [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem Javier Martinez Canillas
@ 2021-11-03 12:28 ` Javier Martinez Canillas
  2021-11-03 12:57   ` Thomas Zimmermann
  2021-11-03 13:01 ` [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Thomas Zimmermann
  2 siblings, 1 reply; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 12:28 UTC (permalink / raw)
  To: linux-kernel
  Cc: David Airlie, Joonas Lahtinen, dri-devel, Gurchetan Singh,
	Gerd Hoffmann, Javier Martinez Canillas, amd-gfx,
	VMware Graphics, Peter Robinson, Neal Gompa, Dave Airlie,
	Chia-I Wu, Ben Skeggs, Michel Dänzer, Maarten Lankhorst,
	Maxime Ripard, Hans de Goede, Jani Nikula, Rodrigo Vivi, nouveau,
	virtualization, Pekka Paalanen, Pan, Xinhui, spice-devel,
	Daniel Vetter, Alex Deucher, intel-gfx, Christian König,
	Zack Rusin

This function is used by some DRM drivers to determine if the "nomodeset"
kernel command line parameter was set and prevent these drivers to probe.

But the function name is quite confusing and does not reflect what really
the drivers are testing when calling it. Use a better naming now that it
is part of the DRM subsystem.

Also, vgacon_text_force() is guarded by #ifdef CONFIG_VGA_CONSOLE already
so there is no need to do the same when calling the function.

Suggested-by: Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
---

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
 drivers/gpu/drm/ast/ast_drv.c           |  2 +-
 drivers/gpu/drm/drm_nomodeset.c         | 16 ++++++++--------
 drivers/gpu/drm/i915/i915_module.c      |  2 +-
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  2 +-
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +-
 drivers/gpu/drm/qxl/qxl_drv.c           |  2 +-
 drivers/gpu/drm/radeon/radeon_drv.c     |  2 +-
 drivers/gpu/drm/tiny/bochs.c            |  2 +-
 drivers/gpu/drm/tiny/cirrus.c           |  2 +-
 drivers/gpu/drm/vboxvideo/vbox_drv.c    |  4 +---
 drivers/gpu/drm/virtio/virtgpu_drv.c    |  2 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  2 +-
 include/drm/drm_mode_config.h           |  4 ++--
 14 files changed, 22 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2680a2aaa877..f7bd2616cf23 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2513,7 +2513,7 @@ static int __init amdgpu_init(void)
 {
 	int r;
 
-	if (vgacon_text_force()) {
+	if (drm_modeset_disabled()) {
 		DRM_ERROR("amdgpu kernel modesetting disabled.\n");
 		return -EINVAL;
 	}
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 048be607b182..6706050414c3 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -232,7 +232,7 @@ static struct pci_driver ast_pci_driver = {
 
 static int __init ast_init(void)
 {
-	if (vgacon_text_force() && ast_modeset == -1)
+	if (drm_modeset_disabled() && ast_modeset == -1)
 		return -EINVAL;
 
 	if (ast_modeset == 0)
diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
index 1ac9a8d5a8fe..dfc8b30f0625 100644
--- a/drivers/gpu/drm/drm_nomodeset.c
+++ b/drivers/gpu/drm/drm_nomodeset.c
@@ -3,17 +3,17 @@
 #include <linux/module.h>
 #include <linux/types.h>
 
-static bool vgacon_text_mode_force;
+static bool drm_nomodeset;
 
-bool vgacon_text_force(void)
+bool drm_modeset_disabled(void)
 {
-	return vgacon_text_mode_force;
+	return drm_nomodeset;
 }
-EXPORT_SYMBOL(vgacon_text_force);
+EXPORT_SYMBOL(drm_modeset_disabled);
 
-static int __init text_mode(char *str)
+static int __init disable_modeset(char *str)
 {
-	vgacon_text_mode_force = true;
+	drm_nomodeset = true;
 
 	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
 	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
@@ -22,5 +22,5 @@ static int __init text_mode(char *str)
 	return 1;
 }
 
-/* force text mode - used by kernel modesetting */
-__setup("nomodeset", text_mode);
+/* Disable kernel modesetting */
+__setup("nomodeset", disable_modeset);
diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
index 14a59226519d..3e5531040e4d 100644
--- a/drivers/gpu/drm/i915/i915_module.c
+++ b/drivers/gpu/drm/i915/i915_module.c
@@ -29,7 +29,7 @@ static int i915_check_nomodeset(void)
 	if (i915_modparams.modeset == 0)
 		use_kms = false;
 
-	if (vgacon_text_force() && i915_modparams.modeset == -1)
+	if (drm_modeset_disabled() && i915_modparams.modeset == -1)
 		use_kms = false;
 
 	if (!use_kms) {
diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
index 685e766db6a4..7ee87564bade 100644
--- a/drivers/gpu/drm/mgag200/mgag200_drv.c
+++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
@@ -377,7 +377,7 @@ static struct pci_driver mgag200_pci_driver = {
 
 static int __init mgag200_init(void)
 {
-	if (vgacon_text_force() && mgag200_modeset == -1)
+	if (drm_modeset_disabled() && mgag200_modeset == -1)
 		return -EINVAL;
 
 	if (mgag200_modeset == 0)
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 029997f50d1a..903d0e626954 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -1321,7 +1321,7 @@ nouveau_drm_init(void)
 	nouveau_display_options();
 
 	if (nouveau_modeset == -1) {
-		if (vgacon_text_force())
+		if (drm_modeset_disabled())
 			nouveau_modeset = 0;
 	}
 
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 3cd6bd9f059d..e4ab16837fad 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -294,7 +294,7 @@ static struct drm_driver qxl_driver = {
 
 static int __init qxl_init(void)
 {
-	if (vgacon_text_force() && qxl_modeset == -1)
+	if (drm_modeset_disabled() && qxl_modeset == -1)
 		return -EINVAL;
 
 	if (qxl_modeset == 0)
diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
index 9b606c1b11ec..36c8dac68cca 100644
--- a/drivers/gpu/drm/radeon/radeon_drv.c
+++ b/drivers/gpu/drm/radeon/radeon_drv.c
@@ -636,7 +636,7 @@ static struct pci_driver radeon_kms_pci_driver = {
 
 static int __init radeon_module_init(void)
 {
-	if (vgacon_text_force() && radeon_modeset == -1) {
+	if (drm_modeset_disabled() && radeon_modeset == -1) {
 		DRM_INFO("VGACON disable radeon kernel modesetting.\n");
 		radeon_modeset = 0;
 	}
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 04333f78be55..59189f7c1840 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -718,7 +718,7 @@ static struct pci_driver bochs_pci_driver = {
 
 static int __init bochs_init(void)
 {
-	if (vgacon_text_force() && bochs_modeset == -1)
+	if (drm_modeset_disabled() && bochs_modeset == -1)
 		return -EINVAL;
 
 	if (bochs_modeset == 0)
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 8bd674f0d682..fcf98379c641 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -635,7 +635,7 @@ static struct pci_driver cirrus_pci_driver = {
 
 static int __init cirrus_init(void)
 {
-	if (vgacon_text_force())
+	if (drm_modeset_disabled())
 		return -EINVAL;
 	return pci_register_driver(&cirrus_pci_driver);
 }
diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
index e6d983121d0b..09356dbd69b2 100644
--- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -192,10 +192,8 @@ static const struct drm_driver driver = {
 
 static int __init vbox_init(void)
 {
-#ifdef CONFIG_VGA_CONSOLE
-	if (vgacon_text_force() && vbox_modeset == -1)
+	if (drm_modeset_disabled() && vbox_modeset == -1)
 		return -EINVAL;
-#endif
 
 	if (vbox_modeset == 0)
 		return -EINVAL;
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index cd4c170236f1..d96797d70fae 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -103,7 +103,7 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
 	struct drm_device *dev;
 	int ret;
 
-	if (vgacon_text_force() && virtio_gpu_modeset == -1)
+	if (drm_modeset_disabled() && virtio_gpu_modeset == -1)
 		return -EINVAL;
 
 	if (virtio_gpu_modeset == 0)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index fcc4b5a7f639..22dab9beea03 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -1650,7 +1650,7 @@ static int __init vmwgfx_init(void)
 {
 	int ret;
 
-	if (vgacon_text_force())
+	if (drm_modeset_disabled())
 		return -EINVAL;
 
 	ret = pci_register_driver(&vmw_pci_driver);
diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
index e1d2042a7b77..a5a2dc02e892 100644
--- a/include/drm/drm_mode_config.h
+++ b/include/drm/drm_mode_config.h
@@ -970,9 +970,9 @@ void drm_mode_config_reset(struct drm_device *dev);
 void drm_mode_config_cleanup(struct drm_device *dev);
 
 #ifdef CONFIG_VGA_CONSOLE
-extern bool vgacon_text_force(void);
+extern bool drm_modeset_disabled(void);
 #else
-static inline bool vgacon_text_force(void) { return false; }
+static inline bool drm_modeset_disabled(void) { return false; }
 #endif
 
 #endif
-- 
2.33.1


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem Javier Martinez Canillas
@ 2021-11-03 12:41   ` Thomas Zimmermann
  2021-11-03 13:06     ` Javier Martinez Canillas
  2021-11-03 12:56   ` Jani Nikula
  1 sibling, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2021-11-03 12:41 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, amd-gfx, VMware Graphics,
	Peter Robinson, Neal Gompa, Dave Airlie, Chia-I Wu, Ben Skeggs,
	Michel Dänzer, Maarten Lankhorst, Maxime Ripard,
	Hans de Goede, Jani Nikula, Rodrigo Vivi, nouveau,
	virtualization, Pekka Paalanen, Greg Kroah-Hartman, Pan, Xinhui,
	spice-devel, Daniel Vetter, Alex Deucher, intel-gfx,
	Christian König, Zack Rusin


[-- Attachment #1.1: Type: text/plain, Size: 11224 bytes --]

Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
> The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
> but the exported vgacon_text_force() symbol is only used by DRM drivers.
> 
> It makes much more sense for the parameter logic to be in the subsystem
> of the drivers that are making use of it. Let's move that to DRM.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>   drivers/gpu/drm/Makefile                |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
>   drivers/gpu/drm/ast/ast_drv.c           |  1 -
>   drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
>   drivers/gpu/drm/i915/i915_module.c      |  2 --
>   drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
>   drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
>   drivers/gpu/drm/qxl/qxl_drv.c           |  1 -
>   drivers/gpu/drm/radeon/radeon_drv.c     |  1 -
>   drivers/gpu/drm/tiny/bochs.c            |  1 -
>   drivers/gpu/drm/tiny/cirrus.c           |  1 -
>   drivers/gpu/drm/vboxvideo/vbox_drv.c    |  1 -
>   drivers/gpu/drm/virtio/virtgpu_drv.c    |  1 -
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  1 -
>   drivers/video/console/vgacon.c          | 21 --------------------
>   include/drm/drm_mode_config.h           |  6 ++++++
>   include/linux/console.h                 |  6 ------
>   17 files changed, 35 insertions(+), 41 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_nomodeset.c
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1c41156deb5f..0e2d60ea93ca 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.
>   
>   obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>   
> +obj-y += drm_nomodeset.o

Repeating my other comment, should this rather be protected by a 
separate config symbol that is selected by CONFIG_DRM?

Best regards
Thomas

> +
>   drm_cma_helper-y := drm_gem_cma_helper.o
>   obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
>   
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index c718fb5f3f8a..2680a2aaa877 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -31,7 +31,6 @@
>   #include "amdgpu_drv.h"
>   
>   #include <drm/drm_pciids.h>
> -#include <linux/console.h>
>   #include <linux/module.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/vga_switcheroo.h>
> @@ -2515,7 +2514,7 @@ static int __init amdgpu_init(void)
>   	int r;
>   
>   	if (vgacon_text_force()) {
> -		DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
> +		DRM_ERROR("amdgpu kernel modesetting disabled.\n");
>   		return -EINVAL;
>   	}
>   
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 86d5cd7b6318..048be607b182 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -26,7 +26,6 @@
>    * Authors: Dave Airlie <airlied@redhat.com>
>    */
>   
> -#include <linux/console.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
>   
> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
> new file mode 100644
> index 000000000000..1ac9a8d5a8fe
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_nomodeset.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +
> +static bool vgacon_text_mode_force;
> +
> +bool vgacon_text_force(void)
> +{
> +	return vgacon_text_mode_force;
> +}
> +EXPORT_SYMBOL(vgacon_text_force);
> +
> +static int __init text_mode(char *str)
> +{
> +	vgacon_text_mode_force = true;
> +
> +	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
> +	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
> +	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
> +
> +	return 1;
> +}
> +
> +/* force text mode - used by kernel modesetting */
> +__setup("nomodeset", text_mode);
> diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
> index c7507266aa83..14a59226519d 100644
> --- a/drivers/gpu/drm/i915/i915_module.c
> +++ b/drivers/gpu/drm/i915/i915_module.c
> @@ -4,8 +4,6 @@
>    * Copyright © 2021 Intel Corporation
>    */
>   
> -#include <linux/console.h>
> -
>   #include "gem/i915_gem_context.h"
>   #include "gem/i915_gem_object.h"
>   #include "i915_active.h"
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 6b9243713b3c..685e766db6a4 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -6,7 +6,6 @@
>    *          Dave Airlie
>    */
>   
> -#include <linux/console.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
>   #include <linux/vmalloc.h>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 1f828c9f691c..029997f50d1a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -22,7 +22,6 @@
>    * Authors: Ben Skeggs
>    */
>   
> -#include <linux/console.h>
>   #include <linux/delay.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index fc47b0deb021..3cd6bd9f059d 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -29,7 +29,6 @@
>   
>   #include "qxl_drv.h"
>   
> -#include <linux/console.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
>   #include <linux/vgaarb.h>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index b74cebca1f89..9b606c1b11ec 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -31,7 +31,6 @@
>   
>   
>   #include <linux/compat.h>
> -#include <linux/console.h>
>   #include <linux/module.h>
>   #include <linux/pm_runtime.h>
>   #include <linux/vga_switcheroo.h>
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 2ce3bd903b70..04333f78be55 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -1,6 +1,5 @@
>   // SPDX-License-Identifier: GPL-2.0-or-later
>   
> -#include <linux/console.h>
>   #include <linux/pci.h>
>   
>   #include <drm/drm_aperture.h>
> diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> index 4611ec408506..8bd674f0d682 100644
> --- a/drivers/gpu/drm/tiny/cirrus.c
> +++ b/drivers/gpu/drm/tiny/cirrus.c
> @@ -16,7 +16,6 @@
>    * Copyright 1999-2001 Jeff Garzik <jgarzik@pobox.com>
>    */
>   
> -#include <linux/console.h>
>   #include <linux/dma-buf-map.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index a6c81af37345..e6d983121d0b 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -7,7 +7,6 @@
>    *          Michael Thayer <michael.thayer@oracle.com,
>    *          Hans de Goede <hdegoede@redhat.com>
>    */
> -#include <linux/console.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
>   #include <linux/vt_kern.h>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index 749db18dcfa2..cd4c170236f1 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -27,7 +27,6 @@
>    */
>   
>   #include <linux/module.h>
> -#include <linux/console.h>
>   #include <linux/pci.h>
>   #include <linux/poll.h>
>   #include <linux/wait.h>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index ab9a1750e1df..fcc4b5a7f639 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -25,7 +25,6 @@
>    *
>    **************************************************************************/
>   
> -#include <linux/console.h>
>   #include <linux/dma-mapping.h>
>   #include <linux/module.h>
>   #include <linux/pci.h>
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index ef9c57ce0906..d4320b147956 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -97,30 +97,9 @@ static int 		vga_video_font_height;
>   static int 		vga_scan_lines		__read_mostly;
>   static unsigned int 	vga_rolled_over; /* last vc_origin offset before wrap */
>   
> -static bool vgacon_text_mode_force;
>   static bool vga_hardscroll_enabled;
>   static bool vga_hardscroll_user_enable = true;
>   
> -bool vgacon_text_force(void)
> -{
> -	return vgacon_text_mode_force;
> -}
> -EXPORT_SYMBOL(vgacon_text_force);
> -
> -static int __init text_mode(char *str)
> -{
> -	vgacon_text_mode_force = true;
> -
> -	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
> -	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
> -	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
> -
> -	return 1;
> -}
> -
> -/* force text mode - used by kernel modesetting */
> -__setup("nomodeset", text_mode);
> -
>   static int __init no_scroll(char *str)
>   {
>   	/*
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 48b7de80daf5..e1d2042a7b77 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -969,4 +969,10 @@ static inline int drm_mode_config_init(struct drm_device *dev)
>   void drm_mode_config_reset(struct drm_device *dev);
>   void drm_mode_config_cleanup(struct drm_device *dev);
>   
> +#ifdef CONFIG_VGA_CONSOLE
> +extern bool vgacon_text_force(void);
> +#else
> +static inline bool vgacon_text_force(void) { return false; }
> +#endif
> +
>   #endif
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 20874db50bc8..d4dd8384898b 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -217,12 +217,6 @@ extern atomic_t ignore_console_lock_warning;
>   #define VESA_HSYNC_SUSPEND      2
>   #define VESA_POWERDOWN          3
>   
> -#ifdef CONFIG_VGA_CONSOLE
> -extern bool vgacon_text_force(void);
> -#else
> -static inline bool vgacon_text_force(void) { return false; }
> -#endif
> -
>   extern void console_init(void);
>   
>   /* For deferred console takeover */
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem Javier Martinez Canillas
  2021-11-03 12:41   ` Thomas Zimmermann
@ 2021-11-03 12:56   ` Jani Nikula
  2021-11-03 13:09     ` Javier Martinez Canillas
  1 sibling, 1 reply; 11+ messages in thread
From: Jani Nikula @ 2021-11-03 12:56 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, Javier Martinez Canillas,
	amd-gfx, VMware Graphics, Peter Robinson, Neal Gompa,
	Dave Airlie, Chia-I Wu, Ben Skeggs, Michel Dänzer,
	Maarten Lankhorst, Maxime Ripard, Hans de Goede, Rodrigo Vivi,
	nouveau, virtualization, Pekka Paalanen, Greg Kroah-Hartman, Pan,
	Xinhui, spice-devel, Daniel Vetter, Alex Deucher, intel-gfx,
	Christian König, Zack Rusin

On Wed, 03 Nov 2021, Javier Martinez Canillas <javierm@redhat.com> wrote:
> The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
> but the exported vgacon_text_force() symbol is only used by DRM drivers.
>
> It makes much more sense for the parameter logic to be in the subsystem
> of the drivers that are making use of it. Let's move that to DRM.
>
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
>
>  drivers/gpu/drm/Makefile                |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
>  drivers/gpu/drm/ast/ast_drv.c           |  1 -
>  drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_module.c      |  2 --
>  drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
>  drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
>  drivers/gpu/drm/qxl/qxl_drv.c           |  1 -
>  drivers/gpu/drm/radeon/radeon_drv.c     |  1 -
>  drivers/gpu/drm/tiny/bochs.c            |  1 -
>  drivers/gpu/drm/tiny/cirrus.c           |  1 -
>  drivers/gpu/drm/vboxvideo/vbox_drv.c    |  1 -
>  drivers/gpu/drm/virtio/virtgpu_drv.c    |  1 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  1 -
>  drivers/video/console/vgacon.c          | 21 --------------------
>  include/drm/drm_mode_config.h           |  6 ++++++
>  include/linux/console.h                 |  6 ------
>  17 files changed, 35 insertions(+), 41 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1c41156deb5f..0e2d60ea93ca 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.
>  
>  obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>  
> +obj-y += drm_nomodeset.o

This is a subtle functional change. With this, you'll always have
__setup("nomodeset", text_mode) builtin and the parameter available. And
using nomodeset will print out the pr_warn() splat from text_mode(). But
removing nomodeset will have no impact if CONFIG_VGA_CONSOLE=n as that
leads to vgacon_text_force() always returning false.

To not make functional changes, this should be:

obj-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o

Now, going with the cleanup in this series, maybe we should make the
functional change, and break the connection to CONFIG_VGA_CONSOLE
altogether, also in the header?

(Maybe we'll also need a proxy drm kconfig option to only have
drm_modeset.o builtin when CONFIG_DRM != n.)

> +
>  drm_cma_helper-y := drm_gem_cma_helper.o
>  obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
>  
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index c718fb5f3f8a..2680a2aaa877 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -31,7 +31,6 @@
>  #include "amdgpu_drv.h"
>  
>  #include <drm/drm_pciids.h>
> -#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/vga_switcheroo.h>
> @@ -2515,7 +2514,7 @@ static int __init amdgpu_init(void)
>  	int r;
>  
>  	if (vgacon_text_force()) {
> -		DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
> +		DRM_ERROR("amdgpu kernel modesetting disabled.\n");
>  		return -EINVAL;
>  	}
>  
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 86d5cd7b6318..048be607b182 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -26,7 +26,6 @@
>   * Authors: Dave Airlie <airlied@redhat.com>
>   */
>  
> -#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  
> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
> new file mode 100644
> index 000000000000..1ac9a8d5a8fe
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_nomodeset.c
> @@ -0,0 +1,26 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include <linux/module.h>
> +#include <linux/types.h>
> +
> +static bool vgacon_text_mode_force;
> +
> +bool vgacon_text_force(void)
> +{
> +	return vgacon_text_mode_force;
> +}
> +EXPORT_SYMBOL(vgacon_text_force);
> +
> +static int __init text_mode(char *str)
> +{
> +	vgacon_text_mode_force = true;
> +
> +	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
> +	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
> +	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
> +
> +	return 1;
> +}
> +
> +/* force text mode - used by kernel modesetting */
> +__setup("nomodeset", text_mode);
> diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
> index c7507266aa83..14a59226519d 100644
> --- a/drivers/gpu/drm/i915/i915_module.c
> +++ b/drivers/gpu/drm/i915/i915_module.c
> @@ -4,8 +4,6 @@
>   * Copyright © 2021 Intel Corporation
>   */
>  
> -#include <linux/console.h>
> -
>  #include "gem/i915_gem_context.h"
>  #include "gem/i915_gem_object.h"
>  #include "i915_active.h"
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 6b9243713b3c..685e766db6a4 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -6,7 +6,6 @@
>   *          Dave Airlie
>   */
>  
> -#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/vmalloc.h>
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 1f828c9f691c..029997f50d1a 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -22,7 +22,6 @@
>   * Authors: Ben Skeggs
>   */
>  
> -#include <linux/console.h>
>  #include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index fc47b0deb021..3cd6bd9f059d 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -29,7 +29,6 @@
>  
>  #include "qxl_drv.h"
>  
> -#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/vgaarb.h>
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index b74cebca1f89..9b606c1b11ec 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -31,7 +31,6 @@
>  
>  
>  #include <linux/compat.h>
> -#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/vga_switcheroo.h>
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 2ce3bd903b70..04333f78be55 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -1,6 +1,5 @@
>  // SPDX-License-Identifier: GPL-2.0-or-later
>  
> -#include <linux/console.h>
>  #include <linux/pci.h>
>  
>  #include <drm/drm_aperture.h>
> diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> index 4611ec408506..8bd674f0d682 100644
> --- a/drivers/gpu/drm/tiny/cirrus.c
> +++ b/drivers/gpu/drm/tiny/cirrus.c
> @@ -16,7 +16,6 @@
>   * Copyright 1999-2001 Jeff Garzik <jgarzik@pobox.com>
>   */
>  
> -#include <linux/console.h>
>  #include <linux/dma-buf-map.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index a6c81af37345..e6d983121d0b 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -7,7 +7,6 @@
>   *          Michael Thayer <michael.thayer@oracle.com,
>   *          Hans de Goede <hdegoede@redhat.com>
>   */
> -#include <linux/console.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
>  #include <linux/vt_kern.h>
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index 749db18dcfa2..cd4c170236f1 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -27,7 +27,6 @@
>   */
>  
>  #include <linux/module.h>
> -#include <linux/console.h>
>  #include <linux/pci.h>
>  #include <linux/poll.h>
>  #include <linux/wait.h>
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index ab9a1750e1df..fcc4b5a7f639 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -25,7 +25,6 @@
>   *
>   **************************************************************************/
>  
> -#include <linux/console.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/module.h>
>  #include <linux/pci.h>
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index ef9c57ce0906..d4320b147956 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -97,30 +97,9 @@ static int 		vga_video_font_height;
>  static int 		vga_scan_lines		__read_mostly;
>  static unsigned int 	vga_rolled_over; /* last vc_origin offset before wrap */
>  
> -static bool vgacon_text_mode_force;
>  static bool vga_hardscroll_enabled;
>  static bool vga_hardscroll_user_enable = true;
>  
> -bool vgacon_text_force(void)
> -{
> -	return vgacon_text_mode_force;
> -}
> -EXPORT_SYMBOL(vgacon_text_force);
> -
> -static int __init text_mode(char *str)
> -{
> -	vgacon_text_mode_force = true;
> -
> -	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
> -	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
> -	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
> -
> -	return 1;
> -}
> -
> -/* force text mode - used by kernel modesetting */
> -__setup("nomodeset", text_mode);
> -
>  static int __init no_scroll(char *str)
>  {
>  	/*
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 48b7de80daf5..e1d2042a7b77 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -969,4 +969,10 @@ static inline int drm_mode_config_init(struct drm_device *dev)
>  void drm_mode_config_reset(struct drm_device *dev);
>  void drm_mode_config_cleanup(struct drm_device *dev);
>  
> +#ifdef CONFIG_VGA_CONSOLE
> +extern bool vgacon_text_force(void);
> +#else
> +static inline bool vgacon_text_force(void) { return false; }
> +#endif
> +

As said, maybe the CONFIG_VGA_CONSOLE ifdef should be dropped.

Also, this seems like a completely arbitrary choice of header to place
this.


BR,
Jani.


>  #endif
> diff --git a/include/linux/console.h b/include/linux/console.h
> index 20874db50bc8..d4dd8384898b 100644
> --- a/include/linux/console.h
> +++ b/include/linux/console.h
> @@ -217,12 +217,6 @@ extern atomic_t ignore_console_lock_warning;
>  #define VESA_HSYNC_SUSPEND      2
>  #define VESA_POWERDOWN          3
>  
> -#ifdef CONFIG_VGA_CONSOLE
> -extern bool vgacon_text_force(void);
> -#else
> -static inline bool vgacon_text_force(void) { return false; }
> -#endif
> -
>  extern void console_init(void);
>  
>  /* For deferred console takeover */

-- 
Jani Nikula, Intel Open Source Graphics Center

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled() Javier Martinez Canillas
@ 2021-11-03 12:57   ` Thomas Zimmermann
  2021-11-03 14:28     ` Javier Martinez Canillas
  0 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2021-11-03 12:57 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: David Airlie, dri-devel, Gurchetan Singh, Gerd Hoffmann,
	Dave Airlie, amd-gfx, VMware Graphics, Ben Skeggs, Neal Gompa,
	spice-devel, Michel Dänzer, Peter Robinson, intel-gfx,
	Hans de Goede, Rodrigo Vivi, nouveau, virtualization,
	Pekka Paalanen, Pan, Xinhui, Alex Deucher, Christian König


[-- Attachment #1.1: Type: text/plain, Size: 10079 bytes --]

Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
> This function is used by some DRM drivers to determine if the "nomodeset"
> kernel command line parameter was set and prevent these drivers to probe.
> 
> But the function name is quite confusing and does not reflect what really
> the drivers are testing when calling it. Use a better naming now that it
> is part of the DRM subsystem.
> 
> Also, vgacon_text_force() is guarded by #ifdef CONFIG_VGA_CONSOLE already
> so there is no need to do the same when calling the function.
> 
> Suggested-by: Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  2 +-
>   drivers/gpu/drm/ast/ast_drv.c           |  2 +-
>   drivers/gpu/drm/drm_nomodeset.c         | 16 ++++++++--------
>   drivers/gpu/drm/i915/i915_module.c      |  2 +-
>   drivers/gpu/drm/mgag200/mgag200_drv.c   |  2 +-
>   drivers/gpu/drm/nouveau/nouveau_drm.c   |  2 +-
>   drivers/gpu/drm/qxl/qxl_drv.c           |  2 +-
>   drivers/gpu/drm/radeon/radeon_drv.c     |  2 +-
>   drivers/gpu/drm/tiny/bochs.c            |  2 +-
>   drivers/gpu/drm/tiny/cirrus.c           |  2 +-
>   drivers/gpu/drm/vboxvideo/vbox_drv.c    |  4 +---
>   drivers/gpu/drm/virtio/virtgpu_drv.c    |  2 +-
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  2 +-
>   include/drm/drm_mode_config.h           |  4 ++--
>   14 files changed, 22 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2680a2aaa877..f7bd2616cf23 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -2513,7 +2513,7 @@ static int __init amdgpu_init(void)
>   {
>   	int r;
>   
> -	if (vgacon_text_force()) {
> +	if (drm_modeset_disabled()) {
>   		DRM_ERROR("amdgpu kernel modesetting disabled.\n");

Please remove all such error messages from drivers. 
drm_modeset_disabled() should print a unified message instead.


>   		return -EINVAL;
>   	}
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 048be607b182..6706050414c3 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -232,7 +232,7 @@ static struct pci_driver ast_pci_driver = {
>   
>   static int __init ast_init(void)
>   {
> -	if (vgacon_text_force() && ast_modeset == -1)
> +	if (drm_modeset_disabled() && ast_modeset == -1)
>   		return -EINVAL;
>   
>   	if (ast_modeset == 0)
> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
> index 1ac9a8d5a8fe..dfc8b30f0625 100644
> --- a/drivers/gpu/drm/drm_nomodeset.c
> +++ b/drivers/gpu/drm/drm_nomodeset.c
> @@ -3,17 +3,17 @@
>   #include <linux/module.h>
>   #include <linux/types.h>
>   
> -static bool vgacon_text_mode_force;
> +static bool drm_nomodeset;
>   
> -bool vgacon_text_force(void)
> +bool drm_modeset_disabled(void)

I suggest to rename this function to drm_check_modeset() and have it 
return a negative errno code on failure. This gives maximum flexibility 
and reduces errors in drivers. Right now the drivers return something 
like -EINVAL, which seems wrong. Returning -ENODEV seems more appropriate.

>   {
> -	return vgacon_text_mode_force;
> +	return drm_nomodeset;
>   }
> -EXPORT_SYMBOL(vgacon_text_force);
> +EXPORT_SYMBOL(drm_modeset_disabled);
>   
> -static int __init text_mode(char *str)
> +static int __init disable_modeset(char *str)
>   {
> -	vgacon_text_mode_force = true;
> +	drm_nomodeset = true;
>   
>   	pr_warn("You have booted with nomodeset. This means your GPU drivers are DISABLED\n");
>   	pr_warn("Any video related functionality will be severely degraded, and you may not even be able to suspend the system properly\n");
> @@ -22,5 +22,5 @@ static int __init text_mode(char *str)
>   	return 1;
>   }
>   
> -/* force text mode - used by kernel modesetting */
> -__setup("nomodeset", text_mode);
> +/* Disable kernel modesetting */
> +__setup("nomodeset", disable_modeset);
> diff --git a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
> index 14a59226519d..3e5531040e4d 100644
> --- a/drivers/gpu/drm/i915/i915_module.c
> +++ b/drivers/gpu/drm/i915/i915_module.c
> @@ -29,7 +29,7 @@ static int i915_check_nomodeset(void)
>   	if (i915_modparams.modeset == 0)
>   		use_kms = false;
>   
> -	if (vgacon_text_force() && i915_modparams.modeset == -1)
> +	if (drm_modeset_disabled() && i915_modparams.modeset == -1)
>   		use_kms = false;
>   
>   	if (!use_kms) {
> diff --git a/drivers/gpu/drm/mgag200/mgag200_drv.c b/drivers/gpu/drm/mgag200/mgag200_drv.c
> index 685e766db6a4..7ee87564bade 100644
> --- a/drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ b/drivers/gpu/drm/mgag200/mgag200_drv.c
> @@ -377,7 +377,7 @@ static struct pci_driver mgag200_pci_driver = {
>   
>   static int __init mgag200_init(void)
>   {
> -	if (vgacon_text_force() && mgag200_modeset == -1)
> +	if (drm_modeset_disabled() && mgag200_modeset == -1)
>   		return -EINVAL;
>   
>   	if (mgag200_modeset == 0)
> diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
> index 029997f50d1a..903d0e626954 100644
> --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> @@ -1321,7 +1321,7 @@ nouveau_drm_init(void)
>   	nouveau_display_options();
>   
>   	if (nouveau_modeset == -1) {
> -		if (vgacon_text_force())
> +		if (drm_modeset_disabled())
>   			nouveau_modeset = 0;
>   	}
>   
> diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
> index 3cd6bd9f059d..e4ab16837fad 100644
> --- a/drivers/gpu/drm/qxl/qxl_drv.c
> +++ b/drivers/gpu/drm/qxl/qxl_drv.c
> @@ -294,7 +294,7 @@ static struct drm_driver qxl_driver = {
>   
>   static int __init qxl_init(void)
>   {
> -	if (vgacon_text_force() && qxl_modeset == -1)
> +	if (drm_modeset_disabled() && qxl_modeset == -1)
>   		return -EINVAL;
>   
>   	if (qxl_modeset == 0)
> diff --git a/drivers/gpu/drm/radeon/radeon_drv.c b/drivers/gpu/drm/radeon/radeon_drv.c
> index 9b606c1b11ec..36c8dac68cca 100644
> --- a/drivers/gpu/drm/radeon/radeon_drv.c
> +++ b/drivers/gpu/drm/radeon/radeon_drv.c
> @@ -636,7 +636,7 @@ static struct pci_driver radeon_kms_pci_driver = {
>   
>   static int __init radeon_module_init(void)
>   {
> -	if (vgacon_text_force() && radeon_modeset == -1) {
> +	if (drm_modeset_disabled() && radeon_modeset == -1) {
>   		DRM_INFO("VGACON disable radeon kernel modesetting.\n");
>   		radeon_modeset = 0;
>   	}
> diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
> index 04333f78be55..59189f7c1840 100644
> --- a/drivers/gpu/drm/tiny/bochs.c
> +++ b/drivers/gpu/drm/tiny/bochs.c
> @@ -718,7 +718,7 @@ static struct pci_driver bochs_pci_driver = {
>   
>   static int __init bochs_init(void)
>   {
> -	if (vgacon_text_force() && bochs_modeset == -1)
> +	if (drm_modeset_disabled() && bochs_modeset == -1)
>   		return -EINVAL;
>   
>   	if (bochs_modeset == 0)
> diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
> index 8bd674f0d682..fcf98379c641 100644
> --- a/drivers/gpu/drm/tiny/cirrus.c
> +++ b/drivers/gpu/drm/tiny/cirrus.c
> @@ -635,7 +635,7 @@ static struct pci_driver cirrus_pci_driver = {
>   
>   static int __init cirrus_init(void)
>   {
> -	if (vgacon_text_force())
> +	if (drm_modeset_disabled())
>   		return -EINVAL;
>   	return pci_register_driver(&cirrus_pci_driver);
>   }
> diff --git a/drivers/gpu/drm/vboxvideo/vbox_drv.c b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> index e6d983121d0b..09356dbd69b2 100644
> --- a/drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ b/drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -192,10 +192,8 @@ static const struct drm_driver driver = {
>   
>   static int __init vbox_init(void)
>   {
> -#ifdef CONFIG_VGA_CONSOLE
> -	if (vgacon_text_force() && vbox_modeset == -1)
> +	if (drm_modeset_disabled() && vbox_modeset == -1)
>   		return -EINVAL;
> -#endif
>   
>   	if (vbox_modeset == 0)
>   		return -EINVAL;
> diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
> index cd4c170236f1..d96797d70fae 100644
> --- a/drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
> @@ -103,7 +103,7 @@ static int virtio_gpu_probe(struct virtio_device *vdev)
>   	struct drm_device *dev;
>   	int ret;
>   
> -	if (vgacon_text_force() && virtio_gpu_modeset == -1)
> +	if (drm_modeset_disabled() && virtio_gpu_modeset == -1)
>   		return -EINVAL;
>   
>   	if (virtio_gpu_modeset == 0)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index fcc4b5a7f639..22dab9beea03 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -1650,7 +1650,7 @@ static int __init vmwgfx_init(void)
>   {
>   	int ret;
>   
> -	if (vgacon_text_force())
> +	if (drm_modeset_disabled())
>   		return -EINVAL;
>   
>   	ret = pci_register_driver(&vmw_pci_driver);
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index e1d2042a7b77..a5a2dc02e892 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -970,9 +970,9 @@ void drm_mode_config_reset(struct drm_device *dev);
>   void drm_mode_config_cleanup(struct drm_device *dev);
>   
>   #ifdef CONFIG_VGA_CONSOLE
> -extern bool vgacon_text_force(void);
> +extern bool drm_modeset_disabled(void);
>   #else
> -static inline bool vgacon_text_force(void) { return false; }
> +static inline bool drm_modeset_disabled(void) { return false; }
>   #endif
>   
>   #endif
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic
  2021-11-03 12:28 [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem Javier Martinez Canillas
  2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled() Javier Martinez Canillas
@ 2021-11-03 13:01 ` Thomas Zimmermann
  2021-11-03 15:00   ` Javier Martinez Canillas
  2 siblings, 1 reply; 11+ messages in thread
From: Thomas Zimmermann @ 2021-11-03 13:01 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, amd-gfx, VMware Graphics,
	Peter Robinson, Neal Gompa, Dave Airlie, Chia-I Wu, Ben Skeggs,
	Michel Dänzer, Maarten Lankhorst, Maxime Ripard,
	Hans de Goede, Jani Nikula, Rodrigo Vivi, nouveau,
	virtualization, Pekka Paalanen, Greg Kroah-Hartman, Pan, Xinhui,
	spice-devel, Daniel Vetter, Alex Deucher, intel-gfx,
	Christian König, Zack Rusin


[-- Attachment #1.1: Type: text/plain, Size: 3996 bytes --]

Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
> [ resend with all relevant people as Cc now, sorry to others for the spam ]
> 
> There is a lot of historical baggage on this parameter. It's defined in
> the vgacon driver as a "nomodeset" parameter, but it's handler function is
> called text_mode() that sets a variable named vgacon_text_mode_force whose
> value is queried with a function named vgacon_text_force().
> 
> All this implies that it's about forcing text mode for VGA, yet it is not
> used in neither vgacon nor other console driver. The only users for these
> are DRM drivers, that check for the vgacon_text_force() return value to
> determine whether the driver could be loaded or not.
> 
> That makes it quite confusing to read the code, because the variables and
> function names don't reflect what they actually do and also are not in the
> same subsystem as the drivers that make use of them.
> 
> This patch-set attempts to cleanup the code by moving the nomodseset param
> to the DRM subsystem and do some renaming to make their intention clearer.
> 
> There is also another aspect that could be improved, and is the fact that
> drivers are checking for the nomodeset being set as an indication if have
> to be loaded.
> 
> But there may be other reasons why this could be the case, so it is better
> to encapsulate the logic in a separate function to make clear what's about.
> 
> Patch #1 is just a trivial fix for a comment that isn't referring to the
> correct kernel parameter.
> 
> Patch #2 moves the nomodeset logic to the DRM subsystem.
> 
> Patch #3 renames the vgacon_text_force() function and accompaning logic as
> drm_modeset_disabled(), which is what this function is really about.
> 
> Patch #4 adds a drm_drv_enabled() function that could be used by drivers
> to check if could be enabled.
> 
> Patch #5 uses the drm_drv_enabled() function to check this instead of just
> checking if nomodeset has been set.
> 
> 
> Javier Martinez Canillas (5):
>    drm/i915: Fix comment about modeset parameters
>    drm: Move nomodeset kernel parameter handler to the DRM subsystem
>    drm: Rename vgacon_text_force() function to drm_modeset_disabled()
>    drm: Add a drm_drv_enabled() helper function
>    drm: Use drm_drv_enabled() instead of drm_modeset_disabled()

There's too much churn here IMHO. Please merge patches 2+3 and 4+5. And 
I'd put patch (4+5) first, so you have the drivers out of the way. After 
that patch (2+3) should only modify drm_drv_enabled().

Best regards
Thomas

> 
>   drivers/gpu/drm/Makefile                |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  5 ++---
>   drivers/gpu/drm/ast/ast_drv.c           |  3 +--
>   drivers/gpu/drm/drm_drv.c               | 21 ++++++++++++++++++++
>   drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
>   drivers/gpu/drm/i915/i915_module.c      | 10 +++++-----
>   drivers/gpu/drm/mgag200/mgag200_drv.c   |  3 +--
>   drivers/gpu/drm/nouveau/nouveau_drm.c   |  3 +--
>   drivers/gpu/drm/qxl/qxl_drv.c           |  3 +--
>   drivers/gpu/drm/radeon/radeon_drv.c     |  3 +--
>   drivers/gpu/drm/tiny/bochs.c            |  3 +--
>   drivers/gpu/drm/tiny/cirrus.c           |  3 +--
>   drivers/gpu/drm/vboxvideo/vbox_drv.c    |  5 +----
>   drivers/gpu/drm/virtio/virtgpu_drv.c    |  3 +--
>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  3 +--
>   drivers/video/console/vgacon.c          | 21 --------------------
>   include/drm/drm_drv.h                   |  1 +
>   include/drm/drm_mode_config.h           |  6 ++++++
>   include/linux/console.h                 |  6 ------
>   19 files changed, 73 insertions(+), 57 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_nomodeset.c
> 

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem
  2021-11-03 12:41   ` Thomas Zimmermann
@ 2021-11-03 13:06     ` Javier Martinez Canillas
  0 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 13:06 UTC (permalink / raw)
  To: Thomas Zimmermann, linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, amd-gfx, VMware Graphics,
	Peter Robinson, Neal Gompa, Dave Airlie, Chia-I Wu, Ben Skeggs,
	Michel Dänzer, Maarten Lankhorst, Maxime Ripard,
	Hans de Goede, Jani Nikula, Rodrigo Vivi, nouveau,
	virtualization, Pekka Paalanen, Greg Kroah-Hartman, Pan, Xinhui,
	spice-devel, Daniel Vetter, Alex Deucher, intel-gfx,
	Christian König, Zack Rusin

Hello Thomas,

On 11/3/21 13:41, Thomas Zimmermann wrote:
> Hi
> 
> Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
>> The "nomodeset" kernel cmdline parameter is handled by the vgacon driver
>> but the exported vgacon_text_force() symbol is only used by DRM drivers.
>>
>> It makes much more sense for the parameter logic to be in the subsystem
>> of the drivers that are making use of it. Let's move that to DRM.
>>
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>>
>>   drivers/gpu/drm/Makefile                |  2 ++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
>>   drivers/gpu/drm/ast/ast_drv.c           |  1 -
>>   drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
>>   drivers/gpu/drm/i915/i915_module.c      |  2 --
>>   drivers/gpu/drm/mgag200/mgag200_drv.c   |  1 -
>>   drivers/gpu/drm/nouveau/nouveau_drm.c   |  1 -
>>   drivers/gpu/drm/qxl/qxl_drv.c           |  1 -
>>   drivers/gpu/drm/radeon/radeon_drv.c     |  1 -
>>   drivers/gpu/drm/tiny/bochs.c            |  1 -
>>   drivers/gpu/drm/tiny/cirrus.c           |  1 -
>>   drivers/gpu/drm/vboxvideo/vbox_drv.c    |  1 -
>>   drivers/gpu/drm/virtio/virtgpu_drv.c    |  1 -
>>   drivers/gpu/drm/vmwgfx/vmwgfx_drv.c     |  1 -
>>   drivers/video/console/vgacon.c          | 21 --------------------
>>   include/drm/drm_mode_config.h           |  6 ++++++
>>   include/linux/console.h                 |  6 ------
>>   17 files changed, 35 insertions(+), 41 deletions(-)
>>   create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>>
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 1c41156deb5f..0e2d60ea93ca 100644
>> --- a/drivers/gpu/drm/Makefile
>> +++ b/drivers/gpu/drm/Makefile
>> @@ -33,6 +33,8 @@ drm-$(CONFIG_DRM_PRIVACY_SCREEN) += drm_privacy_screen.o drm_privacy_screen_x86.
>>   
>>   obj-$(CONFIG_DRM_DP_AUX_BUS) += drm_dp_aux_bus.o
>>   
>> +obj-y += drm_nomodeset.o
> 
> Repeating my other comment, should this rather be protected by a 
> separate config symbol that is selected by CONFIG_DRM?
>

I actually thought about that and my opinion is that obj-y reflects
what we really want here or do you envision this getting disabled
in some cases ?

Probably the problem is Kbuild descending into the drivers/gpu dir
even when CONFIG_DRM is not set. Maybe what we want is something
like the following instead?

diff --git a/drivers/gpu/Makefile b/drivers/gpu/Makefile
index 835c88318cec..ef12ee05ba6e 100644
--- a/drivers/gpu/Makefile
+++ b/drivers/gpu/Makefile
@@ -3,6 +3,7 @@
 # taken to initialize them in the correct order. Link order is the only way
 # to ensure this currently.
 obj-$(CONFIG_TEGRA_HOST1X)     += host1x/
-obj-y                  += drm/ vga/
+obj-$(CONFIG_DRM)              += drm/
+obj-y                          += vga/
 obj-$(CONFIG_IMX_IPUV3_CORE)   += ipu-v3/
 obj-$(CONFIG_TRACE_GPU_MEM)            += trace/

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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem
  2021-11-03 12:56   ` Jani Nikula
@ 2021-11-03 13:09     ` Javier Martinez Canillas
  0 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 13:09 UTC (permalink / raw)
  To: Jani Nikula, linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, amd-gfx, VMware Graphics,
	Peter Robinson, Neal Gompa, Dave Airlie, Chia-I Wu, Ben Skeggs,
	Michel Dänzer, Maarten Lankhorst, Maxime Ripard,
	Hans de Goede, Rodrigo Vivi, nouveau, virtualization,
	Pekka Paalanen, Greg Kroah-Hartman, Pan, Xinhui, spice-devel,
	Daniel Vetter, Alex Deucher, intel-gfx, Christian König,
	Zack Rusin

Hello Jani,

On 11/3/21 13:56, Jani Nikula wrote:

[snip]

>>  
>> +obj-y += drm_nomodeset.o
> 
> This is a subtle functional change. With this, you'll always have
> __setup("nomodeset", text_mode) builtin and the parameter available. And
> using nomodeset will print out the pr_warn() splat from text_mode(). But
> removing nomodeset will have no impact if CONFIG_VGA_CONSOLE=n as that
> leads to vgacon_text_force() always returning false.
>

Yes, that's what I decided at the end to make it unconditional. That
way the same behaviour is preserved (even when only DRM drivers are
using the exported symbol).
 
> To not make functional changes, this should be:
> 
> obj-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
>

Right, that should work.

> Now, going with the cleanup in this series, maybe we should make the
> functional change, and break the connection to CONFIG_VGA_CONSOLE
> altogether, also in the header?
> 
> (Maybe we'll also need a proxy drm kconfig option to only have
> drm_modeset.o builtin when CONFIG_DRM != n.)
>

See my other email. I believe the issue is drivers/gpu/drm always
being included even when CONFIG_DRM is not set.

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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()
  2021-11-03 12:57   ` Thomas Zimmermann
@ 2021-11-03 14:28     ` Javier Martinez Canillas
  0 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 14:28 UTC (permalink / raw)
  To: Thomas Zimmermann, linux-kernel
  Cc: David Airlie, dri-devel, Gurchetan Singh, Gerd Hoffmann,
	Dave Airlie, amd-gfx, VMware Graphics, Ben Skeggs, Neal Gompa,
	spice-devel, Michel Dänzer, Peter Robinson, intel-gfx,
	Hans de Goede, Rodrigo Vivi, nouveau, virtualization,
	Pekka Paalanen, Pan, Xinhui, Alex Deucher, Christian König

On 11/3/21 13:57, Thomas Zimmermann wrote:

[snip]

>>   
>> -	if (vgacon_text_force()) {
>> +	if (drm_modeset_disabled()) {
>>   		DRM_ERROR("amdgpu kernel modesetting disabled.\n");
> 
> Please remove all such error messages from drivers. 
> drm_modeset_disabled() should print a unified message instead.
>

Agreed.

>> -static bool vgacon_text_mode_force;
>> +static bool drm_nomodeset;
>>   
>> -bool vgacon_text_force(void)
>> +bool drm_modeset_disabled(void)
> 
> I suggest to rename this function to drm_check_modeset() and have it 
> return a negative errno code on failure. This gives maximum flexibility 
> and reduces errors in drivers. Right now the drivers return something 
> like -EINVAL, which seems wrong. Returning -ENODEV seems more appropriate.
>

Good idea. I'll do it in v2 as well.
 
Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic
  2021-11-03 13:01 ` [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Thomas Zimmermann
@ 2021-11-03 15:00   ` Javier Martinez Canillas
  0 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-03 15:00 UTC (permalink / raw)
  To: Thomas Zimmermann, linux-kernel
  Cc: linux-fbdev, David Airlie, Joonas Lahtinen, dri-devel,
	Gurchetan Singh, Gerd Hoffmann, amd-gfx, VMware Graphics,
	Peter Robinson, Neal Gompa, Dave Airlie, Chia-I Wu, Ben Skeggs,
	Michel Dänzer, Maarten Lankhorst, Maxime Ripard,
	Hans de Goede, Jani Nikula, Rodrigo Vivi, nouveau,
	virtualization, Pekka Paalanen, Greg Kroah-Hartman, Pan, Xinhui,
	spice-devel, Daniel Vetter, Alex Deucher, intel-gfx,
	Christian König, Zack Rusin

Hello Thomas,

On 11/3/21 14:01, Thomas Zimmermann wrote:

[snip]

>>
>>
>> Javier Martinez Canillas (5):
>>    drm/i915: Fix comment about modeset parameters
>>    drm: Move nomodeset kernel parameter handler to the DRM subsystem
>>    drm: Rename vgacon_text_force() function to drm_modeset_disabled()
>>    drm: Add a drm_drv_enabled() helper function
>>    drm: Use drm_drv_enabled() instead of drm_modeset_disabled()
> 
> There's too much churn here IMHO. Please merge patches 2+3 and 4+5. And 
> I'd put patch (4+5) first, so you have the drivers out of the way. After 
> that patch (2+3) should only modify drm_drv_enabled().
>

Sure, I'm happy with less patches.

Thanks for your feedback.
 
> Best regards
> Thomas
> 
Best regards,
-- 
Javier Martinez Canillas
Linux Engineering
Red Hat


^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2021-11-05 14:26 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-03 12:28 [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem Javier Martinez Canillas
2021-11-03 12:41   ` Thomas Zimmermann
2021-11-03 13:06     ` Javier Martinez Canillas
2021-11-03 12:56   ` Jani Nikula
2021-11-03 13:09     ` Javier Martinez Canillas
2021-11-03 12:28 ` [Nouveau] [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled() Javier Martinez Canillas
2021-11-03 12:57   ` Thomas Zimmermann
2021-11-03 14:28     ` Javier Martinez Canillas
2021-11-03 13:01 ` [Nouveau] [RESEND PATCH 0/5] Cleanups for the nomodeset kernel command line parameter logic Thomas Zimmermann
2021-11-03 15:00   ` Javier Martinez Canillas

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).