linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic
@ 2021-11-12 13:32 Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 1/6] drm: Don't print messages if drivers are disabled due nomodeset Javier Martinez Canillas
                   ` (6 more replies)
  0 siblings, 7 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	Javier Martinez Canillas

There is a lot of historical baggage on this parameter. It is defined in
the vgacon driver as nomodeset, but its set function is called text_mode()
and the value 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 should 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.

This is a v5 of the patches, that just renames the function used by drivers
to check to drm_firmware_drivers_only(). Other than that is the same as v4.

Patch #1 and #2 are just trivial cleanups.

Patch #3 moves the nomodeset boot option to the DRM subsystem and renames
the variables and functions names.

Patch #4 removes the relationship between the nomodeset parameter and the
CONFIG_VGA_CONSOLE Kconfig symbol.

Patch #5 adds nomodeset to the kernel parameters documentation.

Patch #6 improves the message when nomodeset is enabled to make it more
accurate and less sensational.

Changes in v5:
- Rename drm_get_modeset() to drm_firmware_drivers_only().

Changes in v4:
- Don't mention the simpledrm driver and instead explain in high level
  terms what the nomodeset option is about.
- Don't mention DRM drivers in the kernel message and instead explain
  that only the system framebuffer will be available.

Changes in v3:
- Drop the drm_drv_enabled() function and just call to drm_get_modeset().
- Make drm_get_modeset() just a getter function and don't return an error.
- Move independent cleanups in drivers as separate preparatory patches.

Changes in v2:
- Conditionally build drm_nomodeset.o if CONFIG_VGA_CONSOLE is set.
- Squash patches to move nomodeset logic to DRM and do the renaming.
- Name the function drm_check_modeset() and make it return -ENODEV.
- Squash patch to add drm_drv_enabled() and make drivers use it.
- Make the drivers changes before moving nomodeset logic to DRM.
- Make drm_drv_enabled() return an errno and -ENODEV if nomodeset.
- Remove debug and error messages in drivers.

Javier Martinez Canillas (6):
  drm: Don't print messages if drivers are disabled due nomodeset
  drm/vboxvideo: Drop CONFIG_VGA_CONSOLE guard to call
    vgacon_text_force()
  drm: Move nomodeset kernel parameter to the DRM subsystem
  drm: Decouple nomodeset from CONFIG_VGA_CONSOLE
  Documentation/admin-guide: Document nomodeset kernel parameter
  drm: Make the nomodeset message less sensational

 .../admin-guide/kernel-parameters.txt         |  7 ++++++
 drivers/gpu/drm/Kconfig                       |  6 +++++
 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_nomodeset.c               | 24 +++++++++++++++++++
 drivers/gpu/drm/i915/i915_module.c            |  4 ++--
 drivers/gpu/drm/mgag200/mgag200_drv.c         |  3 +--
 drivers/gpu/drm/nouveau/nouveau_drm.c         |  4 ++--
 drivers/gpu/drm/qxl/qxl_drv.c                 |  3 +--
 drivers/gpu/drm/radeon/radeon_drv.c           |  9 ++-----
 drivers/gpu/drm/tiny/bochs.c                  |  3 +--
 drivers/gpu/drm/tiny/cirrus.c                 |  4 ++--
 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/linux/console.h                       |  6 -----
 19 files changed, 56 insertions(+), 60 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

-- 
2.33.1


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

* [PATCH v5 1/6] drm: Don't print messages if drivers are disabled due nomodeset
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
@ 2021-11-12 13:32 ` Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 2/6] drm/vboxvideo: Drop CONFIG_VGA_CONSOLE guard to call vgacon_text_force() Javier Martinez Canillas
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	Javier Martinez Canillas

The nomodeset kernel parameter handler already prints a message that the
DRM drivers will be disabled, so there's no need for drivers to do that.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---

(no changes since v1)

 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 +---
 drivers/gpu/drm/radeon/radeon_drv.c     | 8 ++------
 2 files changed, 3 insertions(+), 9 deletions(-)

diff --git drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c718fb5f3f8a..289d04999ced 100644
--- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2514,10 +2514,8 @@ static int __init amdgpu_init(void)
 {
 	int r;
 
-	if (vgacon_text_force()) {
-		DRM_ERROR("VGACON disables amdgpu kernel modesetting.\n");
+	if (vgacon_text_force())
 		return -EINVAL;
-	}
 
 	r = amdgpu_sync_init();
 	if (r)
diff --git drivers/gpu/drm/radeon/radeon_drv.c drivers/gpu/drm/radeon/radeon_drv.c
index b74cebca1f89..380adc61e71c 100644
--- drivers/gpu/drm/radeon/radeon_drv.c
+++ drivers/gpu/drm/radeon/radeon_drv.c
@@ -637,15 +637,11 @@ static struct pci_driver radeon_kms_pci_driver = {
 
 static int __init radeon_module_init(void)
 {
-	if (vgacon_text_force() && radeon_modeset == -1) {
-		DRM_INFO("VGACON disable radeon kernel modesetting.\n");
+	if (vgacon_text_force() && radeon_modeset == -1)
 		radeon_modeset = 0;
-	}
 
-	if (radeon_modeset == 0) {
-		DRM_ERROR("No UMS support in radeon module!\n");
+	if (radeon_modeset == 0)
 		return -EINVAL;
-	}
 
 	DRM_INFO("radeon kernel modesetting enabled.\n");
 	radeon_register_atpx_handler();
-- 
2.33.1


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

* [PATCH v5 2/6] drm/vboxvideo: Drop CONFIG_VGA_CONSOLE guard to call vgacon_text_force()
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 1/6] drm: Don't print messages if drivers are disabled due nomodeset Javier Martinez Canillas
@ 2021-11-12 13:32 ` Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem Javier Martinez Canillas
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	Javier Martinez Canillas

It is already handled by the console.h macro since a stub inline function
is defined for vgacon_text_force() if CONFIG_VGA_CONSOLE is not set.

There's no need to have ifdefery in the driver when calling the function.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---

(no changes since v1)

 drivers/gpu/drm/vboxvideo/vbox_drv.c | 2 --
 1 file changed, 2 deletions(-)

diff --git drivers/gpu/drm/vboxvideo/vbox_drv.c drivers/gpu/drm/vboxvideo/vbox_drv.c
index a6c81af37345..fd7abb029c65 100644
--- drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -193,10 +193,8 @@ static const struct drm_driver driver = {
 
 static int __init vbox_init(void)
 {
-#ifdef CONFIG_VGA_CONSOLE
 	if (vgacon_text_force() && vbox_modeset == -1)
 		return -EINVAL;
-#endif
 
 	if (vbox_modeset == 0)
 		return -EINVAL;
-- 
2.33.1


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

* [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 1/6] drm: Don't print messages if drivers are disabled due nomodeset Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 2/6] drm/vboxvideo: Drop CONFIG_VGA_CONSOLE guard to call vgacon_text_force() Javier Martinez Canillas
@ 2021-11-12 13:32 ` Javier Martinez Canillas
  2021-11-19 10:20   ` Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 4/6] drm: Decouple nomodeset from CONFIG_VGA_CONSOLE Javier Martinez Canillas
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	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 the vgacon_text_force() function and related logic to the DRM
subsystem. While doing that, rename it to drm_firmware_drivers_only() and
make it return true if "nomodeset" was used and false otherwise. This is
a better description of the condition that the drivers are testing for.

Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---

Changes in v5:
- Rename drm_get_modeset() to drm_firmware_drivers_only().

Changes in v3:
- Drop the drm_drv_enabled() function and just call to drm_get_modeset().
- Make drm_get_modeset() just a getter function and don't return an error.
- Move independent cleanups in drivers as separate preparatory patches.

Changes in v2:
- Conditionally build drm_nomodeset.o if CONFIG_VGA_CONSOLE is set.
- Squash patches to move nomodeset logic to DRM and do the renaming.
- Name the function drm_check_modeset() and make it return -ENODEV.
- Squash patch to add drm_drv_enabled() and make drivers use it.
- Make the drivers changes before moving nomodeset logic to DRM.
- Make drm_drv_enabled() return an errno and -ENODEV if nomodeset.
- Remove debug and error messages in drivers.

 drivers/gpu/drm/Makefile                |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
 drivers/gpu/drm/ast/ast_drv.c           |  3 +--
 drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_module.c      |  4 ++--
 drivers/gpu/drm/mgag200/mgag200_drv.c   |  3 +--
 drivers/gpu/drm/nouveau/nouveau_drm.c   |  4 ++--
 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           |  4 ++--
 drivers/gpu/drm/vboxvideo/vbox_drv.c    |  3 +--
 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                   |  5 +++++
 include/linux/console.h                 |  6 ------
 17 files changed, 48 insertions(+), 51 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

diff --git drivers/gpu/drm/Makefile drivers/gpu/drm/Makefile
index 1c41156deb5f..c74810c285af 100644
--- drivers/gpu/drm/Makefile
+++ 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
+
 drm_cma_helper-y := drm_gem_cma_helper.o
 obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
 
diff --git drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 289d04999ced..fc761087c358 100644
--- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ 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>
@@ -2514,7 +2513,7 @@ static int __init amdgpu_init(void)
 {
 	int r;
 
-	if (vgacon_text_force())
+	if (drm_firmware_drivers_only())
 		return -EINVAL;
 
 	r = amdgpu_sync_init();
diff --git drivers/gpu/drm/ast/ast_drv.c drivers/gpu/drm/ast/ast_drv.c
index 86d5cd7b6318..6d8613f6fe1c 100644
--- drivers/gpu/drm/ast/ast_drv.c
+++ 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>
 
@@ -233,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_firmware_drivers_only() && ast_modeset == -1)
 		return -EINVAL;
 
 	if (ast_modeset == 0)
diff --git drivers/gpu/drm/drm_nomodeset.c drivers/gpu/drm/drm_nomodeset.c
new file mode 100644
index 000000000000..287edfb18b5d
--- /dev/null
+++ 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 drm_nomodeset;
+
+bool drm_firmware_drivers_only(void)
+{
+	return drm_nomodeset;
+}
+EXPORT_SYMBOL(drm_firmware_drivers_only);
+
+static int __init disable_modeset(char *str)
+{
+	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");
+	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
+
+	return 1;
+}
+
+/* Disable kernel modesetting */
+__setup("nomodeset", disable_modeset);
diff --git drivers/gpu/drm/i915/i915_module.c drivers/gpu/drm/i915/i915_module.c
index ab2295dd4500..1df66bf276ce 100644
--- drivers/gpu/drm/i915/i915_module.c
+++ drivers/gpu/drm/i915/i915_module.c
@@ -4,7 +4,7 @@
  * Copyright © 2021 Intel Corporation
  */
 
-#include <linux/console.h>
+#include <drm/drm_drv.h>
 
 #include "gem/i915_gem_context.h"
 #include "gem/i915_gem_object.h"
@@ -31,7 +31,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_firmware_drivers_only() && i915_modparams.modeset == -1)
 		use_kms = false;
 
 	if (!use_kms) {
diff --git drivers/gpu/drm/mgag200/mgag200_drv.c drivers/gpu/drm/mgag200/mgag200_drv.c
index 6b9243713b3c..740108a006ba 100644
--- drivers/gpu/drm/mgag200/mgag200_drv.c
+++ 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>
@@ -378,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_firmware_drivers_only() && mgag200_modeset == -1)
 		return -EINVAL;
 
 	if (mgag200_modeset == 0)
diff --git drivers/gpu/drm/nouveau/nouveau_drm.c drivers/gpu/drm/nouveau/nouveau_drm.c
index 1f828c9f691c..b8158064c377 100644
--- drivers/gpu/drm/nouveau/nouveau_drm.c
+++ 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>
@@ -32,6 +31,7 @@
 
 #include <drm/drm_aperture.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_drv.h>
 #include <drm/drm_gem_ttm_helper.h>
 #include <drm/drm_ioctl.h>
 #include <drm/drm_vblank.h>
@@ -1322,7 +1322,7 @@ nouveau_drm_init(void)
 	nouveau_display_options();
 
 	if (nouveau_modeset == -1) {
-		if (vgacon_text_force())
+		if (drm_firmware_drivers_only())
 			nouveau_modeset = 0;
 	}
 
diff --git drivers/gpu/drm/qxl/qxl_drv.c drivers/gpu/drm/qxl/qxl_drv.c
index fc47b0deb021..e4b16421500b 100644
--- drivers/gpu/drm/qxl/qxl_drv.c
+++ 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>
@@ -295,7 +294,7 @@ static struct drm_driver qxl_driver = {
 
 static int __init qxl_init(void)
 {
-	if (vgacon_text_force() && qxl_modeset == -1)
+	if (drm_firmware_drivers_only() && qxl_modeset == -1)
 		return -EINVAL;
 
 	if (qxl_modeset == 0)
diff --git drivers/gpu/drm/radeon/radeon_drv.c drivers/gpu/drm/radeon/radeon_drv.c
index 380adc61e71c..956c72b5aa33 100644
--- drivers/gpu/drm/radeon/radeon_drv.c
+++ 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>
@@ -637,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_firmware_drivers_only() && radeon_modeset == -1)
 		radeon_modeset = 0;
 
 	if (radeon_modeset == 0)
diff --git drivers/gpu/drm/tiny/bochs.c drivers/gpu/drm/tiny/bochs.c
index 2ce3bd903b70..fc26a1ce11ee 100644
--- drivers/gpu/drm/tiny/bochs.c
+++ 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>
@@ -719,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_firmware_drivers_only() && bochs_modeset == -1)
 		return -EINVAL;
 
 	if (bochs_modeset == 0)
diff --git drivers/gpu/drm/tiny/cirrus.c drivers/gpu/drm/tiny/cirrus.c
index 9327001d35dd..c95d9ff7d600 100644
--- drivers/gpu/drm/tiny/cirrus.c
+++ 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>
@@ -636,8 +635,9 @@ static struct pci_driver cirrus_pci_driver = {
 
 static int __init cirrus_init(void)
 {
-	if (vgacon_text_force())
+	if (drm_firmware_drivers_only())
 		return -EINVAL;
+
 	return pci_register_driver(&cirrus_pci_driver);
 }
 
diff --git drivers/gpu/drm/vboxvideo/vbox_drv.c drivers/gpu/drm/vboxvideo/vbox_drv.c
index fd7abb029c65..f35d9e44c6b7 100644
--- drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ 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>
@@ -193,7 +192,7 @@ static const struct drm_driver driver = {
 
 static int __init vbox_init(void)
 {
-	if (vgacon_text_force() && vbox_modeset == -1)
+	if (drm_firmware_drivers_only() && vbox_modeset == -1)
 		return -EINVAL;
 
 	if (vbox_modeset == 0)
diff --git drivers/gpu/drm/virtio/virtgpu_drv.c drivers/gpu/drm/virtio/virtgpu_drv.c
index 749db18dcfa2..81eab9be57ea 100644
--- drivers/gpu/drm/virtio/virtgpu_drv.c
+++ 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>
@@ -104,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_firmware_drivers_only() && virtio_gpu_modeset == -1)
 		return -EINVAL;
 
 	if (virtio_gpu_modeset == 0)
diff --git drivers/gpu/drm/vmwgfx/vmwgfx_drv.c drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index ab9a1750e1df..695b0ec725cd 100644
--- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ 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>
@@ -1651,7 +1650,7 @@ static int __init vmwgfx_init(void)
 {
 	int ret;
 
-	if (vgacon_text_force())
+	if (drm_firmware_drivers_only())
 		return -EINVAL;
 
 	ret = pci_register_driver(&vmw_pci_driver);
diff --git drivers/video/console/vgacon.c drivers/video/console/vgacon.c
index ef9c57ce0906..d4320b147956 100644
--- drivers/video/console/vgacon.c
+++ 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 include/drm/drm_drv.h include/drm/drm_drv.h
index a84eb4028e5b..89e26a732175 100644
--- include/drm/drm_drv.h
+++ include/drm/drm_drv.h
@@ -601,5 +601,10 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
 
 int drm_dev_set_unique(struct drm_device *dev, const char *name);
 
+#ifdef CONFIG_VGA_CONSOLE
+extern bool drm_firmware_drivers_only(void);
+#else
+static inline bool drm_firmware_drivers_only(void) { return false; }
+#endif
 
 #endif
diff --git include/linux/console.h include/linux/console.h
index 20874db50bc8..d4dd8384898b 100644
--- include/linux/console.h
+++ 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

* [PATCH v5 4/6] drm: Decouple nomodeset from CONFIG_VGA_CONSOLE
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
                   ` (2 preceding siblings ...)
  2021-11-12 13:32 ` [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem Javier Martinez Canillas
@ 2021-11-12 13:32 ` Javier Martinez Canillas
  2021-11-12 14:05   ` Jani Nikula
  2021-11-12 13:32 ` [PATCH v5 5/6] Documentation/admin-guide: Document nomodeset kernel parameter Javier Martinez Canillas
                   ` (2 subsequent siblings)
  6 siblings, 1 reply; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	Javier Martinez Canillas

This relationship was only for historical reasons and the nomodeset option
should be available even on platforms that don't enable CONFIG_VGA_CONSOLE.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---

(no changes since v1)

 drivers/gpu/drm/Kconfig  | 6 ++++++
 drivers/gpu/drm/Makefile | 2 +-
 include/drm/drm_drv.h    | 4 ----
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git drivers/gpu/drm/Kconfig drivers/gpu/drm/Kconfig
index fb144617055b..483d534eb074 100644
--- drivers/gpu/drm/Kconfig
+++ drivers/gpu/drm/Kconfig
@@ -8,6 +8,7 @@
 menuconfig DRM
 	tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)"
 	depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
+	select DRM_NOMODESET
 	select DRM_PANEL_ORIENTATION_QUIRKS
 	select HDMI
 	select FB_CMDLINE
@@ -493,6 +494,11 @@ config DRM_EXPORT_FOR_TESTS
 config DRM_PANEL_ORIENTATION_QUIRKS
 	tristate
 
+# Separate option because nomodeset parameter is global and expected built-in
+config DRM_NOMODESET
+	bool
+	default n
+
 config DRM_LIB_RANDOM
 	bool
 	default n
diff --git drivers/gpu/drm/Makefile drivers/gpu/drm/Makefile
index c74810c285af..fa16d3e0bbdc 100644
--- drivers/gpu/drm/Makefile
+++ drivers/gpu/drm/Makefile
@@ -33,7 +33,7 @@ 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
+obj-$(CONFIG_DRM_NOMODESET) += drm_nomodeset.o
 
 drm_cma_helper-y := drm_gem_cma_helper.o
 obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
diff --git include/drm/drm_drv.h include/drm/drm_drv.h
index 89e26a732175..da0c836fe8e1 100644
--- include/drm/drm_drv.h
+++ include/drm/drm_drv.h
@@ -601,10 +601,6 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
 
 int drm_dev_set_unique(struct drm_device *dev, const char *name);
 
-#ifdef CONFIG_VGA_CONSOLE
 extern bool drm_firmware_drivers_only(void);
-#else
-static inline bool drm_firmware_drivers_only(void) { return false; }
-#endif
 
 #endif
-- 
2.33.1


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

* [PATCH v5 5/6] Documentation/admin-guide: Document nomodeset kernel parameter
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
                   ` (3 preceding siblings ...)
  2021-11-12 13:32 ` [PATCH v5 4/6] drm: Decouple nomodeset from CONFIG_VGA_CONSOLE Javier Martinez Canillas
@ 2021-11-12 13:32 ` Javier Martinez Canillas
  2021-11-12 13:32 ` [PATCH v5 6/6] drm: Make the nomodeset message less sensational Javier Martinez Canillas
  2021-11-27 13:06 ` [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
  6 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	Javier Martinez Canillas

The nomodeset kernel command line parameter is not documented. Its name
is quite vague and is not intuitive what's the behaviour when it is set.

Document in kernel-parameters.txt what actually happens when nomodeset
is used. That way, users could know if they want to enable this option.

Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---

(no changes since v4)

Changes in v4:
- Don't mention the simpledrm driver and instead explain in high level
  terms what the nomodeset option is about.

 Documentation/admin-guide/kernel-parameters.txt | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git Documentation/admin-guide/kernel-parameters.txt Documentation/admin-guide/kernel-parameters.txt
index 91ba391f9b32..f6434aff943d 100644
--- Documentation/admin-guide/kernel-parameters.txt
+++ Documentation/admin-guide/kernel-parameters.txt
@@ -3521,6 +3521,13 @@
 			shutdown the other cpus.  Instead use the REBOOT_VECTOR
 			irq.
 
+	nomodeset	Disable kernel modesetting. DRM 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.
+
+			Useful as fallback, or for testing and debugging.
+
 	nomodule	Disable module load
 
 	nopat		[X86] Disable PAT (page attribute table extension of
-- 
2.33.1


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

* [PATCH v5 6/6] drm: Make the nomodeset message less sensational
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
                   ` (4 preceding siblings ...)
  2021-11-12 13:32 ` [PATCH v5 5/6] Documentation/admin-guide: Document nomodeset kernel parameter Javier Martinez Canillas
@ 2021-11-12 13:32 ` Javier Martinez Canillas
  2021-11-27 13:06 ` [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
  6 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-12 13:32 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen,
	Javier Martinez Canillas

The message printed when nomodeset is present in the kernel command line
makes it look as if the parameter must never be used and it's a bad idea.

But there are valid reasons to use this parameter and the message should
not imply otherwise. Change the text to be more accurate and restrained.

Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
Acked-by: Jani Nikula <jani.nikula@intel.com>
Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
---

(no changes since v4)

Changes in v4:
- Don't mention DRM drivers in the kernel message and instead explain
  that only the system framebuffer will be available.

 drivers/gpu/drm/drm_nomodeset.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git drivers/gpu/drm/drm_nomodeset.c drivers/gpu/drm/drm_nomodeset.c
index 287edfb18b5d..f3978d5bd3a1 100644
--- drivers/gpu/drm/drm_nomodeset.c
+++ drivers/gpu/drm/drm_nomodeset.c
@@ -15,9 +15,7 @@ static int __init disable_modeset(char *str)
 {
 	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");
-	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
+	pr_warn("Booted with the nomodeset parameter. Only the system framebuffer will be available\n");
 
 	return 1;
 }
-- 
2.33.1


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

* Re: [PATCH v5 4/6] drm: Decouple nomodeset from CONFIG_VGA_CONSOLE
  2021-11-12 13:32 ` [PATCH v5 4/6] drm: Decouple nomodeset from CONFIG_VGA_CONSOLE Javier Martinez Canillas
@ 2021-11-12 14:05   ` Jani Nikula
  0 siblings, 0 replies; 11+ messages in thread
From: Jani Nikula @ 2021-11-12 14:05 UTC (permalink / raw)
  To: Javier Martinez Canillas, linux-kernel
  Cc: Daniel Vetter, Peter Robinson, Michel Dänzer, dri-devel,
	Thomas Zimmermann, Pekka Paalanen, Javier Martinez Canillas

On Fri, 12 Nov 2021, Javier Martinez Canillas <javierm@redhat.com> wrote:
> This relationship was only for historical reasons and the nomodeset option
> should be available even on platforms that don't enable CONFIG_VGA_CONSOLE.
>
> Suggested-by: Thomas Zimmermann <tzimmermann@suse.de>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> ---
>
> (no changes since v1)
>
>  drivers/gpu/drm/Kconfig  | 6 ++++++
>  drivers/gpu/drm/Makefile | 2 +-
>  include/drm/drm_drv.h    | 4 ----
>  3 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git drivers/gpu/drm/Kconfig drivers/gpu/drm/Kconfig
> index fb144617055b..483d534eb074 100644
> --- drivers/gpu/drm/Kconfig
> +++ drivers/gpu/drm/Kconfig
> @@ -8,6 +8,7 @@
>  menuconfig DRM
>  	tristate "Direct Rendering Manager (XFree86 4.1.0 and higher DRI support)"
>  	depends on (AGP || AGP=n) && !EMULATED_CMPXCHG && HAS_DMA
> +	select DRM_NOMODESET
>  	select DRM_PANEL_ORIENTATION_QUIRKS
>  	select HDMI
>  	select FB_CMDLINE
> @@ -493,6 +494,11 @@ config DRM_EXPORT_FOR_TESTS
>  config DRM_PANEL_ORIENTATION_QUIRKS
>  	tristate
>  
> +# Separate option because nomodeset parameter is global and expected built-in
> +config DRM_NOMODESET
> +	bool
> +	default n
> +
>  config DRM_LIB_RANDOM
>  	bool
>  	default n
> diff --git drivers/gpu/drm/Makefile drivers/gpu/drm/Makefile
> index c74810c285af..fa16d3e0bbdc 100644
> --- drivers/gpu/drm/Makefile
> +++ drivers/gpu/drm/Makefile
> @@ -33,7 +33,7 @@ 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
> +obj-$(CONFIG_DRM_NOMODESET) += drm_nomodeset.o
>  
>  drm_cma_helper-y := drm_gem_cma_helper.o
>  obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
> diff --git include/drm/drm_drv.h include/drm/drm_drv.h
> index 89e26a732175..da0c836fe8e1 100644
> --- include/drm/drm_drv.h
> +++ include/drm/drm_drv.h
> @@ -601,10 +601,6 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
>  
>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
>  
> -#ifdef CONFIG_VGA_CONSOLE
>  extern bool drm_firmware_drivers_only(void);

I'm not bikeshedding this series anymore, but please follow up with a
patch removing the extern keyword. It's useless.

BR,
Jani.


> -#else
> -static inline bool drm_firmware_drivers_only(void) { return false; }
> -#endif
>  
>  #endif

-- 
Jani Nikula, Intel Open Source Graphics Center

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

* Re: [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem
  2021-11-12 13:32 ` [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem Javier Martinez Canillas
@ 2021-11-19 10:20   ` Javier Martinez Canillas
  2021-11-19 11:08     ` Greg Kroah-Hartman
  0 siblings, 1 reply; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-19 10:20 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen, Greg Kroah-Hartman

[ adding gregkh to Cc list for drivers/video/console ]

On 11/12/21 14:32, Javier Martinez Canillas 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 the vgacon_text_force() function and related logic to the DRM
> subsystem. While doing that, rename it to drm_firmware_drivers_only() and
> make it return true if "nomodeset" was used and false otherwise. This is
> a better description of the condition that the drivers are testing for.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> Acked-by: Jani Nikula <jani.nikula@intel.com>
> Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> ---
>

Greg, could I please get your ack for this patch ?
 
> Changes in v5:
> - Rename drm_get_modeset() to drm_firmware_drivers_only().
> 
> Changes in v3:
> - Drop the drm_drv_enabled() function and just call to drm_get_modeset().
> - Make drm_get_modeset() just a getter function and don't return an error.
> - Move independent cleanups in drivers as separate preparatory patches.
> 
> Changes in v2:
> - Conditionally build drm_nomodeset.o if CONFIG_VGA_CONSOLE is set.
> - Squash patches to move nomodeset logic to DRM and do the renaming.
> - Name the function drm_check_modeset() and make it return -ENODEV.
> - Squash patch to add drm_drv_enabled() and make drivers use it.
> - Make the drivers changes before moving nomodeset logic to DRM.
> - Make drm_drv_enabled() return an errno and -ENODEV if nomodeset.
> - Remove debug and error messages in drivers.
> 
>  drivers/gpu/drm/Makefile                |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  3 +--
>  drivers/gpu/drm/ast/ast_drv.c           |  3 +--
>  drivers/gpu/drm/drm_nomodeset.c         | 26 +++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_module.c      |  4 ++--
>  drivers/gpu/drm/mgag200/mgag200_drv.c   |  3 +--
>  drivers/gpu/drm/nouveau/nouveau_drm.c   |  4 ++--
>  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           |  4 ++--
>  drivers/gpu/drm/vboxvideo/vbox_drv.c    |  3 +--
>  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                   |  5 +++++
>  include/linux/console.h                 |  6 ------
>  17 files changed, 48 insertions(+), 51 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_nomodeset.c
> 
> diff --git drivers/gpu/drm/Makefile drivers/gpu/drm/Makefile
> index 1c41156deb5f..c74810c285af 100644
> --- drivers/gpu/drm/Makefile
> +++ 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
> +
>  drm_cma_helper-y := drm_gem_cma_helper.o
>  obj-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_cma_helper.o
>  
> diff --git drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 289d04999ced..fc761087c358 100644
> --- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ 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>
> @@ -2514,7 +2513,7 @@ static int __init amdgpu_init(void)
>  {
>  	int r;
>  
> -	if (vgacon_text_force())
> +	if (drm_firmware_drivers_only())
>  		return -EINVAL;
>  
>  	r = amdgpu_sync_init();
> diff --git drivers/gpu/drm/ast/ast_drv.c drivers/gpu/drm/ast/ast_drv.c
> index 86d5cd7b6318..6d8613f6fe1c 100644
> --- drivers/gpu/drm/ast/ast_drv.c
> +++ 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>
>  
> @@ -233,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_firmware_drivers_only() && ast_modeset == -1)
>  		return -EINVAL;
>  
>  	if (ast_modeset == 0)
> diff --git drivers/gpu/drm/drm_nomodeset.c drivers/gpu/drm/drm_nomodeset.c
> new file mode 100644
> index 000000000000..287edfb18b5d
> --- /dev/null
> +++ 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 drm_nomodeset;
> +
> +bool drm_firmware_drivers_only(void)
> +{
> +	return drm_nomodeset;
> +}
> +EXPORT_SYMBOL(drm_firmware_drivers_only);
> +
> +static int __init disable_modeset(char *str)
> +{
> +	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");
> +	pr_warn("Unless you actually understand what nomodeset does, you should reboot without enabling it\n");
> +
> +	return 1;
> +}
> +
> +/* Disable kernel modesetting */
> +__setup("nomodeset", disable_modeset);
> diff --git drivers/gpu/drm/i915/i915_module.c drivers/gpu/drm/i915/i915_module.c
> index ab2295dd4500..1df66bf276ce 100644
> --- drivers/gpu/drm/i915/i915_module.c
> +++ drivers/gpu/drm/i915/i915_module.c
> @@ -4,7 +4,7 @@
>   * Copyright © 2021 Intel Corporation
>   */
>  
> -#include <linux/console.h>
> +#include <drm/drm_drv.h>
>  
>  #include "gem/i915_gem_context.h"
>  #include "gem/i915_gem_object.h"
> @@ -31,7 +31,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_firmware_drivers_only() && i915_modparams.modeset == -1)
>  		use_kms = false;
>  
>  	if (!use_kms) {
> diff --git drivers/gpu/drm/mgag200/mgag200_drv.c drivers/gpu/drm/mgag200/mgag200_drv.c
> index 6b9243713b3c..740108a006ba 100644
> --- drivers/gpu/drm/mgag200/mgag200_drv.c
> +++ 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>
> @@ -378,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_firmware_drivers_only() && mgag200_modeset == -1)
>  		return -EINVAL;
>  
>  	if (mgag200_modeset == 0)
> diff --git drivers/gpu/drm/nouveau/nouveau_drm.c drivers/gpu/drm/nouveau/nouveau_drm.c
> index 1f828c9f691c..b8158064c377 100644
> --- drivers/gpu/drm/nouveau/nouveau_drm.c
> +++ 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>
> @@ -32,6 +31,7 @@
>  
>  #include <drm/drm_aperture.h>
>  #include <drm/drm_crtc_helper.h>
> +#include <drm/drm_drv.h>
>  #include <drm/drm_gem_ttm_helper.h>
>  #include <drm/drm_ioctl.h>
>  #include <drm/drm_vblank.h>
> @@ -1322,7 +1322,7 @@ nouveau_drm_init(void)
>  	nouveau_display_options();
>  
>  	if (nouveau_modeset == -1) {
> -		if (vgacon_text_force())
> +		if (drm_firmware_drivers_only())
>  			nouveau_modeset = 0;
>  	}
>  
> diff --git drivers/gpu/drm/qxl/qxl_drv.c drivers/gpu/drm/qxl/qxl_drv.c
> index fc47b0deb021..e4b16421500b 100644
> --- drivers/gpu/drm/qxl/qxl_drv.c
> +++ 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>
> @@ -295,7 +294,7 @@ static struct drm_driver qxl_driver = {
>  
>  static int __init qxl_init(void)
>  {
> -	if (vgacon_text_force() && qxl_modeset == -1)
> +	if (drm_firmware_drivers_only() && qxl_modeset == -1)
>  		return -EINVAL;
>  
>  	if (qxl_modeset == 0)
> diff --git drivers/gpu/drm/radeon/radeon_drv.c drivers/gpu/drm/radeon/radeon_drv.c
> index 380adc61e71c..956c72b5aa33 100644
> --- drivers/gpu/drm/radeon/radeon_drv.c
> +++ 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>
> @@ -637,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_firmware_drivers_only() && radeon_modeset == -1)
>  		radeon_modeset = 0;
>  
>  	if (radeon_modeset == 0)
> diff --git drivers/gpu/drm/tiny/bochs.c drivers/gpu/drm/tiny/bochs.c
> index 2ce3bd903b70..fc26a1ce11ee 100644
> --- drivers/gpu/drm/tiny/bochs.c
> +++ 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>
> @@ -719,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_firmware_drivers_only() && bochs_modeset == -1)
>  		return -EINVAL;
>  
>  	if (bochs_modeset == 0)
> diff --git drivers/gpu/drm/tiny/cirrus.c drivers/gpu/drm/tiny/cirrus.c
> index 9327001d35dd..c95d9ff7d600 100644
> --- drivers/gpu/drm/tiny/cirrus.c
> +++ 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>
> @@ -636,8 +635,9 @@ static struct pci_driver cirrus_pci_driver = {
>  
>  static int __init cirrus_init(void)
>  {
> -	if (vgacon_text_force())
> +	if (drm_firmware_drivers_only())
>  		return -EINVAL;
> +
>  	return pci_register_driver(&cirrus_pci_driver);
>  }
>  
> diff --git drivers/gpu/drm/vboxvideo/vbox_drv.c drivers/gpu/drm/vboxvideo/vbox_drv.c
> index fd7abb029c65..f35d9e44c6b7 100644
> --- drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ 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>
> @@ -193,7 +192,7 @@ static const struct drm_driver driver = {
>  
>  static int __init vbox_init(void)
>  {
> -	if (vgacon_text_force() && vbox_modeset == -1)
> +	if (drm_firmware_drivers_only() && vbox_modeset == -1)
>  		return -EINVAL;
>  
>  	if (vbox_modeset == 0)
> diff --git drivers/gpu/drm/virtio/virtgpu_drv.c drivers/gpu/drm/virtio/virtgpu_drv.c
> index 749db18dcfa2..81eab9be57ea 100644
> --- drivers/gpu/drm/virtio/virtgpu_drv.c
> +++ 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>
> @@ -104,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_firmware_drivers_only() && virtio_gpu_modeset == -1)
>  		return -EINVAL;
>  
>  	if (virtio_gpu_modeset == 0)
> diff --git drivers/gpu/drm/vmwgfx/vmwgfx_drv.c drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index ab9a1750e1df..695b0ec725cd 100644
> --- drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ 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>
> @@ -1651,7 +1650,7 @@ static int __init vmwgfx_init(void)
>  {
>  	int ret;
>  
> -	if (vgacon_text_force())
> +	if (drm_firmware_drivers_only())
>  		return -EINVAL;
>  
>  	ret = pci_register_driver(&vmw_pci_driver);
> diff --git drivers/video/console/vgacon.c drivers/video/console/vgacon.c
> index ef9c57ce0906..d4320b147956 100644
> --- drivers/video/console/vgacon.c
> +++ 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 include/drm/drm_drv.h include/drm/drm_drv.h
> index a84eb4028e5b..89e26a732175 100644
> --- include/drm/drm_drv.h
> +++ include/drm/drm_drv.h
> @@ -601,5 +601,10 @@ static inline bool drm_drv_uses_atomic_modeset(struct drm_device *dev)
>  
>  int drm_dev_set_unique(struct drm_device *dev, const char *name);
>  
> +#ifdef CONFIG_VGA_CONSOLE
> +extern bool drm_firmware_drivers_only(void);
> +#else
> +static inline bool drm_firmware_drivers_only(void) { return false; }
> +#endif
>  
>  #endif
> diff --git include/linux/console.h include/linux/console.h
> index 20874db50bc8..d4dd8384898b 100644
> --- include/linux/console.h
> +++ 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 */
> 

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


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

* Re: [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem
  2021-11-19 10:20   ` Javier Martinez Canillas
@ 2021-11-19 11:08     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 11+ messages in thread
From: Greg Kroah-Hartman @ 2021-11-19 11:08 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: linux-kernel, Jani Nikula, Daniel Vetter, Peter Robinson,
	Michel Dänzer, dri-devel, Thomas Zimmermann, Pekka Paalanen

On Fri, Nov 19, 2021 at 11:20:13AM +0100, Javier Martinez Canillas wrote:
> [ adding gregkh to Cc list for drivers/video/console ]
> 
> On 11/12/21 14:32, Javier Martinez Canillas 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 the vgacon_text_force() function and related logic to the DRM
> > subsystem. While doing that, rename it to drm_firmware_drivers_only() and
> > make it return true if "nomodeset" was used and false otherwise. This is
> > a better description of the condition that the drivers are testing for.
> > 
> > Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> > Acked-by: Thomas Zimmermann <tzimmermann@suse.de>
> > Acked-by: Jani Nikula <jani.nikula@intel.com>
> > Acked-by: Pekka Paalanen <pekka.paalanen@collabora.com>
> > ---
> >
> 
> Greg, could I please get your ack for this patch ?

Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

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

* Re: [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic
  2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
                   ` (5 preceding siblings ...)
  2021-11-12 13:32 ` [PATCH v5 6/6] drm: Make the nomodeset message less sensational Javier Martinez Canillas
@ 2021-11-27 13:06 ` Javier Martinez Canillas
  6 siblings, 0 replies; 11+ messages in thread
From: Javier Martinez Canillas @ 2021-11-27 13:06 UTC (permalink / raw)
  To: linux-kernel
  Cc: Jani Nikula, Daniel Vetter, Peter Robinson, Michel Dänzer,
	dri-devel, Thomas Zimmermann, Pekka Paalanen

On 11/12/21 14:32, Javier Martinez Canillas wrote:
> There is a lot of historical baggage on this parameter. It is defined in
> the vgacon driver as nomodeset, but its set function is called text_mode()
> and the value 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 should 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.
> 

I have pushed these patches to the drm-misc-next branch.

Thanks a lot everyone for the help with this series!

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-27 13:11 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-12 13:32 [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic Javier Martinez Canillas
2021-11-12 13:32 ` [PATCH v5 1/6] drm: Don't print messages if drivers are disabled due nomodeset Javier Martinez Canillas
2021-11-12 13:32 ` [PATCH v5 2/6] drm/vboxvideo: Drop CONFIG_VGA_CONSOLE guard to call vgacon_text_force() Javier Martinez Canillas
2021-11-12 13:32 ` [PATCH v5 3/6] drm: Move nomodeset kernel parameter to the DRM subsystem Javier Martinez Canillas
2021-11-19 10:20   ` Javier Martinez Canillas
2021-11-19 11:08     ` Greg Kroah-Hartman
2021-11-12 13:32 ` [PATCH v5 4/6] drm: Decouple nomodeset from CONFIG_VGA_CONSOLE Javier Martinez Canillas
2021-11-12 14:05   ` Jani Nikula
2021-11-12 13:32 ` [PATCH v5 5/6] Documentation/admin-guide: Document nomodeset kernel parameter Javier Martinez Canillas
2021-11-12 13:32 ` [PATCH v5 6/6] drm: Make the nomodeset message less sensational Javier Martinez Canillas
2021-11-27 13:06 ` [PATCH v5 0/6] Cleanups for the nomodeset kernel command line parameter logic 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).