| Message ID | 20241008-b4-has_ioport-v8-3-793e68aeadda@linux.ibm.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | treewide: Remove I/O port accessors for HAS_IOPORT=n | expand |
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@kernel.org> > Signed-off-by: Arnd Bergmann <arnd@kernel.org> > Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com> 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 >
On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote: > Am 08.10.24 um 14:39 schrieb Niklas Schnelle: d 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 (&& && &&)? No, it's the same. Doing it in one line is mainly useful if you have some '||' as well. >> @@ -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? That no longer works since there are now __compiletime_error() versions of these funcitons. However we can do it more nicely like: diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c index 9b337f948434..034af6e32200 100644 --- a/drivers/gpu/drm/tiny/bochs.c +++ b/drivers/gpu/drm/tiny/bochs.c @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val) if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) return; - if (bochs->mmio) { + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { int offset = ioport - 0x3c0 + 0x400; writeb(val, bochs->mmio + offset); } else { -#ifdef CONFIG_HAS_IOPORT outb(val, ioport); -#endif } } @@ -128,16 +126,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 ioport) if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) return 0xff; - if (bochs->mmio) { + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { int offset = ioport - 0x3c0 + 0x400; return readb(bochs->mmio + offset); } else { -#ifdef CONFIG_HAS_IOPORT return inb(ioport); -#else - return 0xff; -#endif } } @@ -145,32 +139,26 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg) { u16 ret = 0; - if (bochs->mmio) { + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { int offset = 0x500 + (reg << 1); 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; } static void bochs_dispi_write(struct bochs_device *bochs, u16 reg, u16 val) { - if (bochs->mmio) { + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { int offset = 0x500 + (reg << 1); writew(val, bochs->mmio + offset); } else { -#ifdef CONFIG_HAS_IOPORT outw(reg, VBE_DISPI_IOPORT_INDEX); outw(val, VBE_DISPI_IOPORT_DATA); -#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? That was the original idea many years ago, but Linus rejected my pull request for it, so Niklas worked through all drivers individually to add the dependencies instead. Arnd
Hi Am 21.10.24 um 12:08 schrieb Arnd Bergmann: > On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote: >> Am 08.10.24 um 14:39 schrieb Niklas Schnelle: > d 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 (&& && &&)? > No, it's the same. Doing it in one line is mainly useful > if you have some '||' as well. > >>> @@ -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? > That no longer works since there are now __compiletime_error() > versions of these funcitons. However we can do it more nicely like: > > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c > index 9b337f948434..034af6e32200 100644 > --- a/drivers/gpu/drm/tiny/bochs.c > +++ b/drivers/gpu/drm/tiny/bochs.c > @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val) > if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) > return; > > - if (bochs->mmio) { > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { > int offset = ioport - 0x3c0 + 0x400; > > writeb(val, bochs->mmio + offset); > } else { > -#ifdef CONFIG_HAS_IOPORT > outb(val, ioport); > -#endif > } For all functions with such a pattern, could we use: bool bochs_uses_mmio(bochs) { return !IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio } void writeb_func() { if (bochs_uses_mmio()) { writeb() #if CONFIG_HAS_IOPORT } else { outb() #endif } } u8 readb_func() { u8 ret = 0xff if (bochs_uses_mmio()) { ret = readb() #if CONFIG_HAS_IOPORT } else { ret = inb() #endif } return ret; } ? The code in bochs_uses_mmio() could then also print a drm_warn_once if we have neither MMIO nor I/O ports. I'd review a separate series for such a change. > } > > @@ -128,16 +126,12 @@ static u8 bochs_vga_readb(struct bochs_device *bochs, u16 ioport) > if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) > return 0xff; > > - if (bochs->mmio) { > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { > int offset = ioport - 0x3c0 + 0x400; > > return readb(bochs->mmio + offset); > } else { > -#ifdef CONFIG_HAS_IOPORT > return inb(ioport); > -#else > - return 0xff; > -#endif > } > } > > @@ -145,32 +139,26 @@ static u16 bochs_dispi_read(struct bochs_device *bochs, u16 reg) > { > u16 ret = 0; > > - if (bochs->mmio) { > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { > int offset = 0x500 + (reg << 1); > > 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; > } > > static void bochs_dispi_write(struct bochs_device *bochs, u16 reg, u16 val) > { > - if (bochs->mmio) { > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { > int offset = 0x500 + (reg << 1); > > writew(val, bochs->mmio + offset); > } else { > -#ifdef CONFIG_HAS_IOPORT > outw(reg, VBE_DISPI_IOPORT_INDEX); > outw(val, VBE_DISPI_IOPORT_DATA); > -#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? > That was the original idea many years ago, but Linus rejected > my pull request for it, so Niklas worked through all drivers > individually to add the dependencies instead. I see. Best regards Thomas > > Arnd
> > I'd review a separate series for such a change. Could also be a single patch here, of course.
On Mon, 2024-10-21 at 12:58 +0200, Thomas Zimmermann wrote: > Hi > > Am 21.10.24 um 12:08 schrieb Arnd Bergmann: > > On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote: > > > Am 08.10.24 um 14:39 schrieb Niklas Schnelle: > > d 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 (&& && &&)? > > No, it's the same. Doing it in one line is mainly useful > > if you have some '||' as well. > > > > > > @@ -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? > > That no longer works since there are now __compiletime_error() > > versions of these funcitons. However we can do it more nicely like: > > > > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c > > index 9b337f948434..034af6e32200 100644 > > --- a/drivers/gpu/drm/tiny/bochs.c > > +++ b/drivers/gpu/drm/tiny/bochs.c > > @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val) > > if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) > > return; > > > > - if (bochs->mmio) { > > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { > > int offset = ioport - 0x3c0 + 0x400; > > > > writeb(val, bochs->mmio + offset); > > } else { > > -#ifdef CONFIG_HAS_IOPORT > > outb(val, ioport); > > -#endif > > } > > For all functions with such a pattern, could we use: > > bool bochs_uses_mmio(bochs) > { > return !IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio > } > > void writeb_func() > { > if (bochs_uses_mmio()) { > writeb() > #if CONFIG_HAS_IOPORT > } else { > outb() > #endif > } > } > I think if the helper were __always_inline we could still take advantage of the dead code elimination and combine this with Arnd's approach. Though I feel like it is a bit odd to try to do the MMIO approach despite bochs->mmio being false on !HAS_IOPORT systems. Is that what you wanted to correct by keeping the #ifdef CONFIG_HAS_IOPORT around the else? And yes the warning makes sense to me too. Thanks, Niklas
On Mon, Oct 21, 2024, at 10:58, Thomas Zimmermann wrote: > Am 21.10.24 um 12:08 schrieb Arnd Bergmann: >> On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote: >> --- a/drivers/gpu/drm/tiny/bochs.c >> +++ b/drivers/gpu/drm/tiny/bochs.c >> @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val) >> if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) >> return; >> >> - if (bochs->mmio) { >> + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { I meant IS_ENABLED() of course. > For all functions with such a pattern, could we use: > > bool bochs_uses_mmio(bochs) > { > return !IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio > } > > void writeb_func() > { > if (bochs_uses_mmio()) { > writeb() > #if CONFIG_HAS_IOPORT > } else { > outb() > #endif > } Yes, that helper function look fine, but it should then be either __always_inline or a macro. With that, the #ifdef is not needed since gcc only warns if there is a path that leads to outb() actually getting called. Arnd
On Mon, Oct 21, 2024 at 01:18:20PM +0200, Niklas Schnelle wrote: > On Mon, 2024-10-21 at 12:58 +0200, Thomas Zimmermann wrote: > > Hi > > > > Am 21.10.24 um 12:08 schrieb Arnd Bergmann: > > > On Mon, Oct 21, 2024, at 07:52, Thomas Zimmermann wrote: > > > > Am 08.10.24 um 14:39 schrieb Niklas Schnelle: > > > d 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 (&& && &&)? > > > No, it's the same. Doing it in one line is mainly useful > > > if you have some '||' as well. > > > > > > > > @@ -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? > > > That no longer works since there are now __compiletime_error() > > > versions of these funcitons. However we can do it more nicely like: > > > > > > diff --git a/drivers/gpu/drm/tiny/bochs.c b/drivers/gpu/drm/tiny/bochs.c > > > index 9b337f948434..034af6e32200 100644 > > > --- a/drivers/gpu/drm/tiny/bochs.c > > > +++ b/drivers/gpu/drm/tiny/bochs.c > > > @@ -112,14 +112,12 @@ static void bochs_vga_writeb(struct bochs_device *bochs, u16 ioport, u8 val) > > > if (WARN_ON(ioport < 0x3c0 || ioport > 0x3df)) > > > return; > > > > > > - if (bochs->mmio) { > > > + if (!IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio) { > > > int offset = ioport - 0x3c0 + 0x400; > > > > > > writeb(val, bochs->mmio + offset); > > > } else { > > > -#ifdef CONFIG_HAS_IOPORT > > > outb(val, ioport); > > > -#endif > > > } > > > > For all functions with such a pattern, could we use: > > > > bool bochs_uses_mmio(bochs) > > { > > return !IS_DEFINED(CONFIG_HAS_IOPORT) || bochs->mmio > > } > > > > void writeb_func() > > { > > if (bochs_uses_mmio()) { > > writeb() > > #if CONFIG_HAS_IOPORT > > } else { > > outb() > > #endif > > } > > } > > > > I think if the helper were __always_inline we could still take > advantage of the dead code elimination and combine this with Arnd's > approach. Though I feel like it is a bit odd to try to do the MMIO > approach despite bochs->mmio being false on !HAS_IOPORT systems. > Is that what you wanted to correct by keeping the #ifdef > CONFIG_HAS_IOPORT around the else? And yes the warning makes sense to > me too. Working on this now, I think we don't need a warning in the bochs_uses_mmio() helper because we should never get here with !IS_ENABLED(CONFIG_HAS_IOPORT) at runtime thanks to the check added in bochs_hw_init(). This also takes care of my original worry that we might try writeb()/readb() with an invalid bochs->mmio value. I'll sent a v9 with the helper added and #ifdefs's removed. Thanks, Niklas
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 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> #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 } } @@ -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 } } @@ -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; + } 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