Re: [RESEND PATCH 3/5] drm: Rename vgacon_text_force() function to drm_modeset_disabled()

From: Thomas Zimmermann
Date: Wed Nov 03 2021 - 08:57:52 EST


Hi

Am 03.11.21 um 13:28 schrieb Javier Martinez Canillas:
This function is used by some DRM drivers to determine if the "nomodeset"
kernel command line parameter was set and prevent these drivers to probe.

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

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

Suggested-by: Suggested-by: Daniel Vetter <daniel.vetter@xxxxxxxx>
Signed-off-by: Javier Martinez Canillas <javierm@xxxxxxxxxx>
---

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

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2680a2aaa877..f7bd2616cf23 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2513,7 +2513,7 @@ static int __init amdgpu_init(void)
{
int r;
- if (vgacon_text_force()) {
+ if (drm_modeset_disabled()) {
DRM_ERROR("amdgpu kernel modesetting disabled.\n");

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


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

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

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


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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature