2021-11-03 10:50:49

by Javier Martinez Canillas

[permalink] [raw]
Subject: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

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 <[email protected]>
Signed-off-by: Javier Martinez Canillas <[email protected]>
---

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 drivers/gpu/drm/Makefile drivers/gpu/drm/Makefile
index 1c41156deb5f..0e2d60ea93ca 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-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 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index c718fb5f3f8a..2680a2aaa877 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>
@@ -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 drivers/gpu/drm/ast/ast_drv.c drivers/gpu/drm/ast/ast_drv.c
index 86d5cd7b6318..048be607b182 100644
--- drivers/gpu/drm/ast/ast_drv.c
+++ drivers/gpu/drm/ast/ast_drv.c
@@ -26,7 +26,6 @@
* Authors: Dave Airlie <[email protected]>
*/

-#include <linux/console.h>
#include <linux/module.h>
#include <linux/pci.h>

diff --git drivers/gpu/drm/drm_nomodeset.c drivers/gpu/drm/drm_nomodeset.c
new file mode 100644
index 000000000000..1ac9a8d5a8fe
--- /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 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 drivers/gpu/drm/i915/i915_module.c drivers/gpu/drm/i915/i915_module.c
index c7507266aa83..14a59226519d 100644
--- drivers/gpu/drm/i915/i915_module.c
+++ 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 drivers/gpu/drm/mgag200/mgag200_drv.c drivers/gpu/drm/mgag200/mgag200_drv.c
index 6b9243713b3c..685e766db6a4 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>
diff --git drivers/gpu/drm/nouveau/nouveau_drm.c drivers/gpu/drm/nouveau/nouveau_drm.c
index 1f828c9f691c..029997f50d1a 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>
diff --git drivers/gpu/drm/qxl/qxl_drv.c drivers/gpu/drm/qxl/qxl_drv.c
index fc47b0deb021..3cd6bd9f059d 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>
diff --git drivers/gpu/drm/radeon/radeon_drv.c drivers/gpu/drm/radeon/radeon_drv.c
index b74cebca1f89..9b606c1b11ec 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>
diff --git drivers/gpu/drm/tiny/bochs.c drivers/gpu/drm/tiny/bochs.c
index 2ce3bd903b70..04333f78be55 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>
diff --git drivers/gpu/drm/tiny/cirrus.c drivers/gpu/drm/tiny/cirrus.c
index 4611ec408506..8bd674f0d682 100644
--- drivers/gpu/drm/tiny/cirrus.c
+++ drivers/gpu/drm/tiny/cirrus.c
@@ -16,7 +16,6 @@
* Copyright 1999-2001 Jeff Garzik <[email protected]>
*/

-#include <linux/console.h>
#include <linux/dma-buf-map.h>
#include <linux/module.h>
#include <linux/pci.h>
diff --git drivers/gpu/drm/vboxvideo/vbox_drv.c drivers/gpu/drm/vboxvideo/vbox_drv.c
index a6c81af37345..e6d983121d0b 100644
--- drivers/gpu/drm/vboxvideo/vbox_drv.c
+++ drivers/gpu/drm/vboxvideo/vbox_drv.c
@@ -7,7 +7,6 @@
* Michael Thayer <[email protected],
* Hans de Goede <[email protected]>
*/
-#include <linux/console.h>
#include <linux/module.h>
#include <linux/pci.h>
#include <linux/vt_kern.h>
diff --git drivers/gpu/drm/virtio/virtgpu_drv.c drivers/gpu/drm/virtio/virtgpu_drv.c
index 749db18dcfa2..cd4c170236f1 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>
diff --git drivers/gpu/drm/vmwgfx/vmwgfx_drv.c drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index ab9a1750e1df..fcc4b5a7f639 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>
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_mode_config.h include/drm/drm_mode_config.h
index 48b7de80daf5..e1d2042a7b77 100644
--- include/drm/drm_mode_config.h
+++ 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 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


2021-11-03 11:11:20

by Neal Gompa

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

On Wed, Nov 3, 2021 at 6:48 AM Javier Martinez Canillas
<[email protected]> 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 <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---

Please no, I'd much rather have a better, more meaningful option
instead of "nomodeset". If anything, I would like this option to
eventually do nothing and replace it with a better named option that's
namespaced by drm on the command-line. That was part of the feedback I
gave in the original patch set, and I still stand by that.


--
真実はいつも一つ!/ Always, there's only one truth!

2021-11-03 11:25:48

by Javier Martinez Canillas

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

Hello Neal,

On 11/3/21 12:05, Neal Gompa wrote:
> On Wed, Nov 3, 2021 at 6:48 AM Javier Martinez Canillas
> <[email protected]> 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 <[email protected]>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> ---
>
> Please no, I'd much rather have a better, more meaningful option
> instead of "nomodeset". If anything, I would like this option to
> eventually do nothing and replace it with a better named option that's
> namespaced by drm on the command-line. That was part of the feedback I
> gave in the original patch set, and I still stand by that.
>

I do agree with you that a more meaningful parameter would be desirable.

But I think that's orthogonal to this series and something that could be
done as a follow-up. That is, nomodeset isn't going away anytime soon as
that's what users are get to.

This doesn't mean that a drm.disable_native_drivers, drm.safe_mode or
whatever could be added. Internally it would also toggle the same value
but it will be much more clear to users what's the expected behaviour.

By having a module parameter under the drm namespace, it would also allow
to change this from user-space through a sysfs entry. Something that's not
currently possible for the "nomodeset" paramter.

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

2021-11-03 12:35:06

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

Hi

Am 03.11.21 um 12:05 schrieb Neal Gompa:
> On Wed, Nov 3, 2021 at 6:48 AM Javier Martinez Canillas
> <[email protected]> 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 <[email protected]>
>> Signed-off-by: Javier Martinez Canillas <[email protected]>
>> ---
>
> Please no, I'd much rather have a better, more meaningful option
> instead of "nomodeset". If anything, I would like this option to
> eventually do nothing and replace it with a better named option that's
> namespaced by drm on the command-line. That was part of the feedback I
> gave in the original patch set, and I still stand by that.

This was nack'ed for now during irc chats with others. There was no
clear semantics for the new parameter and nomodeset is good enough for
now. I agree that nomodeset is badly named, though.

Best regards
Thomas

>
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2021-11-03 12:36:50

by Neal Gompa

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

On Wed, Nov 3, 2021 at 8:32 AM Thomas Zimmermann <[email protected]> wrote:
>
> Hi
>
> Am 03.11.21 um 12:05 schrieb Neal Gompa:
> > On Wed, Nov 3, 2021 at 6:48 AM Javier Martinez Canillas
> > <[email protected]> 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 <[email protected]>
> >> Signed-off-by: Javier Martinez Canillas <[email protected]>
> >> ---
> >
> > Please no, I'd much rather have a better, more meaningful option
> > instead of "nomodeset". If anything, I would like this option to
> > eventually do nothing and replace it with a better named option that's
> > namespaced by drm on the command-line. That was part of the feedback I
> > gave in the original patch set, and I still stand by that.
>
> This was nack'ed for now during irc chats with others. There was no
> clear semantics for the new parameter and nomodeset is good enough for
> now. I agree that nomodeset is badly named, though.
>

Where are these chats happening? I'm mostly talking to Javier in the
#devel:fedoraproject.org Matrix room, so I don't know about anything
else...



--
真実はいつも一つ!/ Always, there's only one truth!

2021-11-03 12:39:17

by Thomas Zimmermann

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

Hi

Am 03.11.21 um 11:48 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 <[email protected]>
> Signed-off-by: Javier Martinez Canillas <[email protected]>
> ---
>
> 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 drivers/gpu/drm/Makefile drivers/gpu/drm/Makefile
> index 1c41156deb5f..0e2d60ea93ca 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-y += drm_nomodeset.o
> +

Is this linked unconditionally?

Would it make sense to have a config define for this and have CONFIG_DRM
select it?

Best regards
Thomas

> 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 c718fb5f3f8a..2680a2aaa877 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>
> @@ -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 drivers/gpu/drm/ast/ast_drv.c drivers/gpu/drm/ast/ast_drv.c
> index 86d5cd7b6318..048be607b182 100644
> --- drivers/gpu/drm/ast/ast_drv.c
> +++ drivers/gpu/drm/ast/ast_drv.c
> @@ -26,7 +26,6 @@
> * Authors: Dave Airlie <[email protected]>
> */
>
> -#include <linux/console.h>
> #include <linux/module.h>
> #include <linux/pci.h>
>
> diff --git drivers/gpu/drm/drm_nomodeset.c drivers/gpu/drm/drm_nomodeset.c
> new file mode 100644
> index 000000000000..1ac9a8d5a8fe
> --- /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 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 drivers/gpu/drm/i915/i915_module.c drivers/gpu/drm/i915/i915_module.c
> index c7507266aa83..14a59226519d 100644
> --- drivers/gpu/drm/i915/i915_module.c
> +++ 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 drivers/gpu/drm/mgag200/mgag200_drv.c drivers/gpu/drm/mgag200/mgag200_drv.c
> index 6b9243713b3c..685e766db6a4 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>
> diff --git drivers/gpu/drm/nouveau/nouveau_drm.c drivers/gpu/drm/nouveau/nouveau_drm.c
> index 1f828c9f691c..029997f50d1a 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>
> diff --git drivers/gpu/drm/qxl/qxl_drv.c drivers/gpu/drm/qxl/qxl_drv.c
> index fc47b0deb021..3cd6bd9f059d 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>
> diff --git drivers/gpu/drm/radeon/radeon_drv.c drivers/gpu/drm/radeon/radeon_drv.c
> index b74cebca1f89..9b606c1b11ec 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>
> diff --git drivers/gpu/drm/tiny/bochs.c drivers/gpu/drm/tiny/bochs.c
> index 2ce3bd903b70..04333f78be55 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>
> diff --git drivers/gpu/drm/tiny/cirrus.c drivers/gpu/drm/tiny/cirrus.c
> index 4611ec408506..8bd674f0d682 100644
> --- drivers/gpu/drm/tiny/cirrus.c
> +++ drivers/gpu/drm/tiny/cirrus.c
> @@ -16,7 +16,6 @@
> * Copyright 1999-2001 Jeff Garzik <[email protected]>
> */
>
> -#include <linux/console.h>
> #include <linux/dma-buf-map.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> diff --git drivers/gpu/drm/vboxvideo/vbox_drv.c drivers/gpu/drm/vboxvideo/vbox_drv.c
> index a6c81af37345..e6d983121d0b 100644
> --- drivers/gpu/drm/vboxvideo/vbox_drv.c
> +++ drivers/gpu/drm/vboxvideo/vbox_drv.c
> @@ -7,7 +7,6 @@
> * Michael Thayer <[email protected],
> * Hans de Goede <[email protected]>
> */
> -#include <linux/console.h>
> #include <linux/module.h>
> #include <linux/pci.h>
> #include <linux/vt_kern.h>
> diff --git drivers/gpu/drm/virtio/virtgpu_drv.c drivers/gpu/drm/virtio/virtgpu_drv.c
> index 749db18dcfa2..cd4c170236f1 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>
> diff --git drivers/gpu/drm/vmwgfx/vmwgfx_drv.c drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index ab9a1750e1df..fcc4b5a7f639 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>
> 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_mode_config.h include/drm/drm_mode_config.h
> index 48b7de80daf5..e1d2042a7b77 100644
> --- include/drm/drm_mode_config.h
> +++ 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 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 */
>

--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev


Attachments:
OpenPGP_signature (855.00 B)
OpenPGP digital signature

2021-11-10 09:07:10

by Pekka Paalanen

[permalink] [raw]
Subject: Re: [PATCH 2/5] drm: Move nomodeset kernel parameter handler to the DRM subsystem

On Wed, 3 Nov 2021 08:34:20 -0400
Neal Gompa <[email protected]> wrote:

> On Wed, Nov 3, 2021 at 8:32 AM Thomas Zimmermann <[email protected]> wrote:
> >
> > Hi
> >
> > Am 03.11.21 um 12:05 schrieb Neal Gompa:
> > > On Wed, Nov 3, 2021 at 6:48 AM Javier Martinez Canillas
> > > <[email protected]> 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 <[email protected]>
> > >> Signed-off-by: Javier Martinez Canillas <[email protected]>
> > >> ---
> > >
> > > Please no, I'd much rather have a better, more meaningful option
> > > instead of "nomodeset". If anything, I would like this option to
> > > eventually do nothing and replace it with a better named option that's
> > > namespaced by drm on the command-line. That was part of the feedback I
> > > gave in the original patch set, and I still stand by that.
> >
> > This was nack'ed for now during irc chats with others. There was no
> > clear semantics for the new parameter and nomodeset is good enough for
> > now. I agree that nomodeset is badly named, though.
> >
>
> Where are these chats happening? I'm mostly talking to Javier in the
> #devel:fedoraproject.org Matrix room, so I don't know about anything
> else...

OFTC IRC channel #dri-devel


Thanks,
pq


Attachments:
(No filename) (849.00 B)
OpenPGP digital signature