Messages in this thread | | | Date | Fri, 5 Nov 2021 10:39:09 +0100 | Subject | Re: [PATCH v2 2/2] drm: Move nomodeset kernel parameter to the DRM subsystem | From | Thomas Zimmermann <> |
| |
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 */ >>> >
-- 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 [unhandled content-type:application/pgp-signature] | |