diff mbox series

[v2,2/2] drm: Move nomodeset kernel parameter to the DRM subsystem

Message ID 20211104160707.1407052-3-javierm@redhat.com (mailing list archive)
State New, archived
Headers show
Series Cleanups for the nomodeset kernel command line parameter logic | expand

Commit Message

Javier Martinez Canillas Nov. 4, 2021, 4:07 p.m. UTC
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 the function to drm_check_modeset()
which better reflects what the function is really used to test for.

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

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.

 drivers/gpu/drm/Makefile                |  2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
 drivers/gpu/drm/ast/ast_drv.c           |  1 -
 drivers/gpu/drm/drm_drv.c               |  9 +++++----
 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 ------
 18 files changed, 39 insertions(+), 44 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_nomodeset.c

Comments

Thomas Zimmermann Nov. 5, 2021, 9 a.m. UTC | #1
Hi

Am 04.11.21 um 17:07 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 the vgacon_text_force() function and related logic to the DRM
> subsystem. While doing that, rename the function to drm_check_modeset()
> which better reflects what the function is really used to test for.
> 
> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
> ---
> 
> 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.
> 
>   drivers/gpu/drm/Makefile                |  2 ++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
>   drivers/gpu/drm/ast/ast_drv.c           |  1 -
>   drivers/gpu/drm/drm_drv.c               |  9 +++++----
>   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 ------
>   18 files changed, 39 insertions(+), 44 deletions(-)
>   create mode 100644 drivers/gpu/drm/drm_nomodeset.c
> 
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 1c41156deb5f..c74810c285af 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
> +

This now depends on the VGA textmode console. Even if you have no VGA 
console, you'd want drm_nomodeset.o. Simpledrm might be built-in and can 
provide graphics. Non-PC systems don't even have a VGA device.

I think we really want a separate boolean config option that gets 
selected by CONFIG_DRM.


>   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 7fde40d06181..b4b6993861e6 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>
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 802063279b86..6222082c3082 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_drv.c b/drivers/gpu/drm/drm_drv.c
> index 3fb567d62881..80b85b8ea776 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -986,12 +986,13 @@ EXPORT_SYMBOL(drm_dev_set_unique);
>    */
>   int drm_drv_enabled(const struct drm_driver *driver)
>   {
> -	if (vgacon_text_force()) {
> +	int ret;
> +
> +	ret = drm_check_modeset();
> +	if (ret)
>   		DRM_INFO("%s driver is disabled\n", driver->name);
> -		return -ENODEV;
> -	}
>   
> -	return 0;
> +	return ret;
>   }
>   EXPORT_SYMBOL(drm_drv_enabled);
>   
> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
> new file mode 100644
> index 000000000000..6683e396d2c5
> --- /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 drm_nomodeset;
> +
> +int drm_check_modeset(void)
> +{
> +	return drm_nomodeset ? -ENODEV : 0;
> +}
> +EXPORT_SYMBOL(drm_check_modeset);
> +
> +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");

I'd update this text to be less sensational.

> +
> +	return 1;
> +}
> +
> +/* 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 45cb3e540eff..c890c1ca20c4 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>
> -

These changes should be in patch 1?

>   #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 2a581094ba2b..8e000cac11ba 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 8844d3602d87..bd1456521b7c 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 3ac2ef2bf545..ff070ac76111 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 56d688c04346..f59cc971ec95 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 ee6b1ff9128b..6e9a31f1a0f3 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 4706c5bc3067..659208d5aef9 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 e4377c37cf33..b1e63fd543bb 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 28200dfba2d1..ba9c0c2f8ae6 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 05e9949293d5..115ec9518277 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..18982d3507e4 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 int drm_check_modeset(void);
> +#else
> +static inline int drm_check_modeset(void) { return 0; }
> +#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 */
>
Jani Nikula Nov. 5, 2021, 9:22 a.m. UTC | #2
On Fri, 05 Nov 2021, Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Hi
>
> Am 04.11.21 um 17:07 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 the vgacon_text_force() function and related logic to the DRM
>> subsystem. While doing that, rename the function to drm_check_modeset()
>> which better reflects what the function is really used to test for.
>> 
>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>> ---
>> 
>> 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.
>> 
>>   drivers/gpu/drm/Makefile                |  2 ++
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
>>   drivers/gpu/drm/ast/ast_drv.c           |  1 -
>>   drivers/gpu/drm/drm_drv.c               |  9 +++++----
>>   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 ------
>>   18 files changed, 39 insertions(+), 44 deletions(-)
>>   create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>> 
>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>> index 1c41156deb5f..c74810c285af 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
>> +
>
> This now depends on the VGA textmode console. Even if you have no VGA 
> console, you'd want drm_nomodeset.o. Simpledrm might be built-in and can 
> provide graphics. Non-PC systems don't even have a VGA device.

This was discussed in an earlier version, which had this builtin but the
header still had a stub for CONFIG_VGA_CONSOLE=n.

> I think we really want a separate boolean config option that gets 
> selected by CONFIG_DRM.

Perhaps that should be a separate change on top.

BR,
Jani.

>
>
>>   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 7fde40d06181..b4b6993861e6 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>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
>> index 802063279b86..6222082c3082 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_drv.c b/drivers/gpu/drm/drm_drv.c
>> index 3fb567d62881..80b85b8ea776 100644
>> --- a/drivers/gpu/drm/drm_drv.c
>> +++ b/drivers/gpu/drm/drm_drv.c
>> @@ -986,12 +986,13 @@ EXPORT_SYMBOL(drm_dev_set_unique);
>>    */
>>   int drm_drv_enabled(const struct drm_driver *driver)
>>   {
>> -	if (vgacon_text_force()) {
>> +	int ret;
>> +
>> +	ret = drm_check_modeset();
>> +	if (ret)
>>   		DRM_INFO("%s driver is disabled\n", driver->name);
>> -		return -ENODEV;
>> -	}
>>   
>> -	return 0;
>> +	return ret;
>>   }
>>   EXPORT_SYMBOL(drm_drv_enabled);
>>   
>> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
>> new file mode 100644
>> index 000000000000..6683e396d2c5
>> --- /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 drm_nomodeset;
>> +
>> +int drm_check_modeset(void)
>> +{
>> +	return drm_nomodeset ? -ENODEV : 0;
>> +}
>> +EXPORT_SYMBOL(drm_check_modeset);
>> +
>> +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");
>
> I'd update this text to be less sensational.
>
>> +
>> +	return 1;
>> +}
>> +
>> +/* 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 45cb3e540eff..c890c1ca20c4 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>
>> -
>
> These changes should be in patch 1?
>
>>   #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 2a581094ba2b..8e000cac11ba 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 8844d3602d87..bd1456521b7c 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 3ac2ef2bf545..ff070ac76111 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 56d688c04346..f59cc971ec95 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 ee6b1ff9128b..6e9a31f1a0f3 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 4706c5bc3067..659208d5aef9 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 e4377c37cf33..b1e63fd543bb 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 28200dfba2d1..ba9c0c2f8ae6 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 05e9949293d5..115ec9518277 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..18982d3507e4 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 int drm_check_modeset(void);
>> +#else
>> +static inline int drm_check_modeset(void) { return 0; }
>> +#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 Nov. 5, 2021, 9:39 a.m. UTC | #3
Hi

Am 05.11.21 um 10:22 schrieb Jani Nikula:
> On Fri, 05 Nov 2021, Thomas Zimmermann <tzimmermann@suse.de> wrote:
>> Hi
>>
>> Am 04.11.21 um 17:07 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 the vgacon_text_force() function and related logic to the DRM
>>> subsystem. While doing that, rename the function to drm_check_modeset()
>>> which better reflects what the function is really used to test for.
>>>
>>> Suggested-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>>> Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
>>> ---
>>>
>>> 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.
>>>
>>>    drivers/gpu/drm/Makefile                |  2 ++
>>>    drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 -
>>>    drivers/gpu/drm/ast/ast_drv.c           |  1 -
>>>    drivers/gpu/drm/drm_drv.c               |  9 +++++----
>>>    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 ------
>>>    18 files changed, 39 insertions(+), 44 deletions(-)
>>>    create mode 100644 drivers/gpu/drm/drm_nomodeset.c
>>>
>>> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
>>> index 1c41156deb5f..c74810c285af 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-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
>>> +
>>
>> This now depends on the VGA textmode console. Even if you have no VGA
>> console, you'd want drm_nomodeset.o. Simpledrm might be built-in and can
>> provide graphics. Non-PC systems don't even have a VGA device.
> 
> This was discussed in an earlier version, which had this builtin but the
> header still had a stub for CONFIG_VGA_CONSOLE=n.
> 
>> I think we really want a separate boolean config option that gets
>> selected by CONFIG_DRM.
> 
> Perhaps that should be a separate change on top.

Sure, make it a separate patch.

We want to make this work on ARM systems. I even have a request to 
replace offb on Power architecture by simpledrm. So the final config has 
to be system agnostic.

Best regards
Thomas

> 
> BR,
> Jani.
> 
>>
>>
>>>    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 7fde40d06181..b4b6993861e6 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>
>>> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
>>> index 802063279b86..6222082c3082 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_drv.c b/drivers/gpu/drm/drm_drv.c
>>> index 3fb567d62881..80b85b8ea776 100644
>>> --- a/drivers/gpu/drm/drm_drv.c
>>> +++ b/drivers/gpu/drm/drm_drv.c
>>> @@ -986,12 +986,13 @@ EXPORT_SYMBOL(drm_dev_set_unique);
>>>     */
>>>    int drm_drv_enabled(const struct drm_driver *driver)
>>>    {
>>> -	if (vgacon_text_force()) {
>>> +	int ret;
>>> +
>>> +	ret = drm_check_modeset();
>>> +	if (ret)
>>>    		DRM_INFO("%s driver is disabled\n", driver->name);
>>> -		return -ENODEV;
>>> -	}
>>>    
>>> -	return 0;
>>> +	return ret;
>>>    }
>>>    EXPORT_SYMBOL(drm_drv_enabled);
>>>    
>>> diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
>>> new file mode 100644
>>> index 000000000000..6683e396d2c5
>>> --- /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 drm_nomodeset;
>>> +
>>> +int drm_check_modeset(void)
>>> +{
>>> +	return drm_nomodeset ? -ENODEV : 0;
>>> +}
>>> +EXPORT_SYMBOL(drm_check_modeset);
>>> +
>>> +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");
>>
>> I'd update this text to be less sensational.
>>
>>> +
>>> +	return 1;
>>> +}
>>> +
>>> +/* 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 45cb3e540eff..c890c1ca20c4 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>
>>> -
>>
>> These changes should be in patch 1?
>>
>>>    #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 2a581094ba2b..8e000cac11ba 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 8844d3602d87..bd1456521b7c 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 3ac2ef2bf545..ff070ac76111 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 56d688c04346..f59cc971ec95 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 ee6b1ff9128b..6e9a31f1a0f3 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 4706c5bc3067..659208d5aef9 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 e4377c37cf33..b1e63fd543bb 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 28200dfba2d1..ba9c0c2f8ae6 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 05e9949293d5..115ec9518277 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..18982d3507e4 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 int drm_check_modeset(void);
>>> +#else
>>> +static inline int drm_check_modeset(void) { return 0; }
>>> +#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 */
>>>
>
Javier Martinez Canillas Nov. 5, 2021, 9:55 a.m. UTC | #4
On 11/5/21 10:00, Thomas Zimmermann wrote:

[snip]

>> +
>> +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");
> 
> I'd update this text to be less sensational.
>

This is indeed quite sensational. But think we can do it as a follow-up patch.

Since we will have to stick with nomodeset for backward compatibility, I was
planning to add documentation to explain what this parameter is about and what
is the actual effect of setting it.

So I think we can change this as a part of that patch-set.
 
>> +
>> +	return 1;
>> +}
>> +
>> +/* 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 45cb3e540eff..c890c1ca20c4 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>
>> -
>
> These changes should be in patch 1?
>

Yes, I forgot to move these when changed the order of the patches.

Best regards, -- 
Javier Martinez Canillas
Linux Engineering
Red Hat
Javier Martinez Canillas Nov. 5, 2021, 9:58 a.m. UTC | #5
On 11/5/21 10:39, Thomas Zimmermann wrote:

[snip]

>>>>    
>>>> +obj-$(CONFIG_VGA_CONSOLE) += drm_nomodeset.o
>>>> +
>>>
>>> This now depends on the VGA textmode console. Even if you have no VGA
>>> console, you'd want drm_nomodeset.o. Simpledrm might be built-in and can
>>> provide graphics. Non-PC systems don't even have a VGA device.
>>
>> This was discussed in an earlier version, which had this builtin but the
>> header still had a stub for CONFIG_VGA_CONSOLE=n.
>>
>>> I think we really want a separate boolean config option that gets
>>> selected by CONFIG_DRM.
>>
>> Perhaps that should be a separate change on top.
> 
> Sure, make it a separate patch.
>

Agreed. I was planning to do it as a follow-up as well and drop the
#ifdef CONFIG_VGA_CONSOLE guard in the header.
 
> We want to make this work on ARM systems. I even have a request to 
> replace offb on Power architecture by simpledrm. So the final config has 
> to be system agnostic.
>

Same, since we want to drop the fbdev drivers in Fedora, for all arches.
 
> Best regards
> Thomas
> 
Best regards,
diff mbox series

Patch

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 1c41156deb5f..c74810c285af 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-$(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 a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 7fde40d06181..b4b6993861e6 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>
diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 802063279b86..6222082c3082 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_drv.c b/drivers/gpu/drm/drm_drv.c
index 3fb567d62881..80b85b8ea776 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -986,12 +986,13 @@  EXPORT_SYMBOL(drm_dev_set_unique);
  */
 int drm_drv_enabled(const struct drm_driver *driver)
 {
-	if (vgacon_text_force()) {
+	int ret;
+
+	ret = drm_check_modeset();
+	if (ret)
 		DRM_INFO("%s driver is disabled\n", driver->name);
-		return -ENODEV;
-	}
 
-	return 0;
+	return ret;
 }
 EXPORT_SYMBOL(drm_drv_enabled);
 
diff --git a/drivers/gpu/drm/drm_nomodeset.c b/drivers/gpu/drm/drm_nomodeset.c
new file mode 100644
index 000000000000..6683e396d2c5
--- /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 drm_nomodeset;
+
+int drm_check_modeset(void)
+{
+	return drm_nomodeset ? -ENODEV : 0;
+}
+EXPORT_SYMBOL(drm_check_modeset);
+
+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 a/drivers/gpu/drm/i915/i915_module.c b/drivers/gpu/drm/i915/i915_module.c
index 45cb3e540eff..c890c1ca20c4 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 2a581094ba2b..8e000cac11ba 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 8844d3602d87..bd1456521b7c 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 3ac2ef2bf545..ff070ac76111 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 56d688c04346..f59cc971ec95 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 ee6b1ff9128b..6e9a31f1a0f3 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 4706c5bc3067..659208d5aef9 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 e4377c37cf33..b1e63fd543bb 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 28200dfba2d1..ba9c0c2f8ae6 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 05e9949293d5..115ec9518277 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..18982d3507e4 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 int drm_check_modeset(void);
+#else
+static inline int drm_check_modeset(void) { return 0; }
+#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 */