Hi
Am 08.10.24 um 14:39 schrieb Niklas Schnelle:
In a future patch HAS_IOPORT=n will disable inb()/outb() and friends at
compile time. We thus need to add HAS_IOPORT as dependency for those
drivers using them. In the bochs driver there is optional MMIO support
detected at runtime, warn if this isn't taken when HAS_IOPORT is not
defined.
There is also a direct and hard coded use in cirrus.c which according to
the comment is only necessary during resume. Let's just skip this as
for example s390 which doesn't have I/O port support also doesen't
support suspend/resume.
Co-developed-by: Arnd Bergmann <arnd@xxxxxxxxxx>
Signed-off-by: Arnd Bergmann <arnd@xxxxxxxxxx>
Signed-off-by: Niklas Schnelle <schnelle@xxxxxxxxxxxxx>
I feel like I reviewed this before, but can't find it.
---
drivers/gpu/drm/gma500/Kconfig | 2 +-
drivers/gpu/drm/qxl/Kconfig | 1 +
drivers/gpu/drm/tiny/bochs.c | 17 +++++++++++++++++
drivers/gpu/drm/tiny/cirrus.c | 2 ++
drivers/gpu/drm/xe/Kconfig | 2 +-
5 files changed, 22 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/gma500/Kconfig b/drivers/gpu/drm/gma500/Kconfig
index efb4a2dd2f80885cb59c925d09401002278d7d61..23b7c14de5e29238ece939d5822d8a9ffc4675cc 100644
--- a/drivers/gpu/drm/gma500/Kconfig
+++ b/drivers/gpu/drm/gma500/Kconfig
@@ -1,7 +1,7 @@
# SPDX-License-Identifier: GPL-2.0-only
config DRM_GMA500
tristate "Intel GMA500/600/3600/3650 KMS Framebuffer"
- depends on DRM && PCI && X86 && MMU
+ depends on DRM && PCI && X86 && MMU && HAS_IOPORT
select DRM_KMS_HELPER
select FB_IOMEM_HELPERS if DRM_FBDEV_EMULATION
select I2C
diff --git a/drivers/gpu/drm/qxl/Kconfig b/drivers/gpu/drm/qxl/Kconfig
index ca3f51c2a8fe1a383f8a2479f04b5c0b3fb14e44..d0e0d440c8d96564cb7b8ffd2385c44fc43f873d 100644
--- a/drivers/gpu/drm/qxl/Kconfig
+++ b/drivers/gpu/drm/qxl/Kconfig
@@ -2,6 +2,7 @@
config DRM_QXL
tristate "QXL virtual GPU"
depends on DRM && PCI && MMU
+ depends on HAS_IOPORT
Is there a difference between this style (multiple 'depends on') and the
one used for gma500 (&& && &&)?
select DRM_KMS_HELPER
select DRM_TTM
select DRM_TTM_HELPER
diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c
index 31fc5d839e106ea4d5c8fe42d1bfc3c70291e3fb..0ed78d3d5774778f91de972ac27056938036e722 100644
--- a/drivers/gpu/drm/tiny/bochs.c
+++ b/drivers/gpu/drm/tiny/bochs.c
@@ -2,6 +2,7 @@
#include <linux/module.h>
#include <linux/pci.h>
+#include <linux/bug.h>
Alphabetic sorting please.
#include <drm/drm_aperture.h>
#include <drm/drm_atomic_helper.h>
@@ -105,7 +106,9 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val)
writeb(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outb(val, ioport);
+#endif
Could you provide empty defines for the out() interfaces at the top of
the file?
}
}
@@ -119,7 +122,11 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 ioport)
return readb(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
return inb(ioport);
+#else
+ return 0xff;
+#endif
And the in() interfaces could be defined to 0xff[ff].
I assume that you don't want to provide such empty macros in the
kernel's io.h header?
}
}
@@ -132,8 +139,12 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg)
ret = readw(bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
ret = inw(VBE_DISPI_IOPORT_DATA);
+#else
+ ret = 0xffff;
+#endif
}
return ret;
}
@@ -145,8 +156,10 @@ static void bochs_dispi_write(struct bochs_device *bochs, u16 reg, u16 val)
writew(val, bochs->mmio + offset);
} else {
+#ifdef CONFIG_HAS_IOPORT
outw(reg, VBE_DISPI_IOPORT_INDEX);
outw(val, VBE_DISPI_IOPORT_DATA);
+#endif
}
}
@@ -229,6 +242,10 @@ static int bochs_hw_init(struct drm_device *dev)
return -ENOMEM;
}
} else {
+ if (!IS_ENABLED(CONFIG_HAS_IOPORT)) {
+ DRM_ERROR("I/O ports are not supported\n");
+ return -EIO;
+ }
It would be nicer to use an "} else if(IOPORT) {" here and put the
"return -EIO" into a trailing else branch.
If you want to add an error message, please don't use DRM_ERROR(). In
this case, dev_err(dev->dev, "...\n") seems appropriate.
Best regards
Thomas
ioaddr = VBE_DISPI_IOPORT_INDEX;
iosize = 2;
if (!request_region(ioaddr, iosize, "bochs-drm")) {
diff --git a/drivers/gpu/drm/tiny/cirrus.c b/drivers/gpu/drm/tiny/cirrus.c
index 751326e3d9c374baf72115492aeefff2b73869f0..e31e1df029ab0272c4a1ff0ab3eb026ca679b560 100644
--- a/drivers/gpu/drm/tiny/cirrus.c
+++ b/drivers/gpu/drm/tiny/cirrus.c
@@ -509,8 +509,10 @@ static void cirrus_crtc_helper_atomic_enable(struct drm_crtc *crtc,
cirrus_mode_set(cirrus, &crtc_state->mode);
+#ifdef CONFIG_HAS_IOPORT
/* Unblank (needed on S3 resume, vgabios doesn't do it then) */
outb(VGA_AR_ENABLE_DISPLAY, VGA_ATT_W);
+#endif
drm_dev_exit(idx);
}
diff --git a/drivers/gpu/drm/xe/Kconfig b/drivers/gpu/drm/xe/Kconfig
index 7bbe46a98ff1f449bc2af30686585a00e9e8af93..116f58774135fc3a9f37d6d72d41340f5c812297 100644
--- a/drivers/gpu/drm/xe/Kconfig
+++ b/drivers/gpu/drm/xe/Kconfig
@@ -49,7 +49,7 @@ config DRM_XE
config DRM_XE_DISPLAY
bool "Enable display support"
- depends on DRM_XE && DRM_XE=m
+ depends on DRM_XE && DRM_XE=m && HAS_IOPORT
select FB_IOMEM_HELPERS
select I2C
select I2C_ALGOBIT
--
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Frankenstrasse 146, 90461 Nuernberg, Germany
GF: Ivo Totev, Andrew Myers, Andrew McDonald, Boudien Moerman
HRB 36809 (AG Nuernberg)
