linux-fbdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers
@ 2022-07-18  7:23 Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 02/11] fbdev/vga16fb: Create EGA/VGA devices in sysfb code Thomas Zimmermann
                   ` (9 more replies)
  0 siblings, 10 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Fbdev firmware drivers acquire ownership of framebuffer I/O ranges and
hand them over to native drivers during the boot process. Re-implement
this mechanism with aperture helpers and remove the respective fbdev
code.

This change allows to perform hand-over from DRM firmware drivers. In a
later patchset, device ownership can be moved from DRM and fbdev entirely
into aperture helpers.

Patches 1 and 4 are cleanups.

Patches 2 and 3 integrate EGA/VGA support into sysfb, although it's not
clear if the x86 architecture code actually still supports VGA graphics
mode.

Patches 5 to 10 replace fbdev's ownership management with aperture
helpers. This includes removal of conflicting framebuffer drivers,
removal of conflicting VGA drivers and registration of fbdev firmware
devices. Notably, many PCI-based fbdev drivers failed to remove firmware
devices until now; and therefore probably haven't worked correctly for
some time.

Patch 11 removes the implementation of fbdev ownership management.

The patchset has been tested by handing over device ownership between
firmware and native drivers of DRM and fbdev in various combinations.

v2:
	* remove unused options handling from vga16fb (Javier)
	* more elaborate commit messages (Javier)
	* remove unused internal functions in fbmem.c

Thomas Zimmermann (11):
  fbdev: Remove trailing whitespaces
  fbdev/vga16fb: Create EGA/VGA devices in sysfb code
  fbdev/vga16fb: Auto-generate module init/exit code
  fbdev/core: Remove remove_conflicting_pci_framebuffers()
  fbdev: Convert drivers to aperture helpers
  fbdev: Remove conflicting devices on PCI bus
  video/aperture: Disable and unregister sysfb devices via aperture
    helpers
  video: Provide constants for VGA I/O range
  video/aperture: Remove conflicting VGA devices, if any
  fbdev: Acquire framebuffer apertures for firmware devices
  fbdev: Remove conflict-handling code

 drivers/firmware/sysfb.c                     |   4 +
 drivers/staging/sm750fb/sm750.c              |  15 +-
 drivers/video/aperture.c                     |  69 ++--
 drivers/video/fbdev/arkfb.c                  |   5 +
 drivers/video/fbdev/asiliantfb.c             |   5 +
 drivers/video/fbdev/aty/aty128fb.c           |  57 ++--
 drivers/video/fbdev/aty/atyfb_base.c         |   7 +-
 drivers/video/fbdev/aty/radeon_base.c        |  83 +++--
 drivers/video/fbdev/carminefb.c              |   5 +
 drivers/video/fbdev/chipsfb.c                |  13 +-
 drivers/video/fbdev/cirrusfb.c               |   5 +
 drivers/video/fbdev/core/fbmem.c             | 213 ++-----------
 drivers/video/fbdev/cyber2000fb.c            |   5 +
 drivers/video/fbdev/geode/gx1fb_core.c       |   5 +
 drivers/video/fbdev/geode/gxfb_core.c        |   5 +
 drivers/video/fbdev/geode/lxfb_core.c        |   5 +
 drivers/video/fbdev/gxt4500.c                |   5 +
 drivers/video/fbdev/hyperv_fb.c              |   6 +-
 drivers/video/fbdev/i740fb.c                 |   5 +
 drivers/video/fbdev/i810/i810_main.c         | 315 ++++++++++---------
 drivers/video/fbdev/imsttfb.c                |  36 ++-
 drivers/video/fbdev/intelfb/intelfbdrv.c     |   5 +
 drivers/video/fbdev/kyro/fbdev.c             |   5 +
 drivers/video/fbdev/matrox/matroxfb_base.c   |   5 +
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c   |   5 +
 drivers/video/fbdev/neofb.c                  |  41 +--
 drivers/video/fbdev/nvidia/nvidia.c          |   7 +-
 drivers/video/fbdev/pm2fb.c                  |   5 +
 drivers/video/fbdev/pm3fb.c                  |   5 +
 drivers/video/fbdev/pvr2fb.c                 |   5 +
 drivers/video/fbdev/riva/fbdev.c             |  67 ++--
 drivers/video/fbdev/s3fb.c                   |   5 +
 drivers/video/fbdev/savage/savagefb_driver.c |   5 +
 drivers/video/fbdev/sis/sis_main.c           |   5 +
 drivers/video/fbdev/skeletonfb.c             | 210 +++++++------
 drivers/video/fbdev/sm712fb.c                |   5 +
 drivers/video/fbdev/sstfb.c                  |  43 +--
 drivers/video/fbdev/sunxvr2500.c             |   5 +
 drivers/video/fbdev/sunxvr500.c              |   5 +
 drivers/video/fbdev/tdfxfb.c                 |   5 +
 drivers/video/fbdev/tgafb.c                  |  17 +-
 drivers/video/fbdev/tridentfb.c              |   5 +
 drivers/video/fbdev/vermilion/vermilion.c    |   7 +-
 drivers/video/fbdev/vga16fb.c                | 191 +++++------
 drivers/video/fbdev/via/via-core.c           |   5 +
 drivers/video/fbdev/vt8623fb.c               |   5 +
 include/linux/fb.h                           |   4 -
 include/video/vga.h                          |  20 +-
 48 files changed, 776 insertions(+), 784 deletions(-)


base-commit: ebea934e2651857c9b56cc80bf99460ee18a3592
-- 
2.36.1


^ permalink raw reply	[flat|nested] 20+ messages in thread

* [PATCH v2 02/11] fbdev/vga16fb: Create EGA/VGA devices in sysfb code
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 03/11] fbdev/vga16fb: Auto-generate module init/exit code Thomas Zimmermann
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Move the device-creation from vga16fb to sysfb code. The driver's
videomode checks are independent from device creation, so move them
into vga16fb's probe function. This will allow to create the module
init/exit code automatically.

The vga16fb driver requires a screen_info for type VIDEO_TYPE_VGAC
or VIDEO_TYPE_EGAC. Such code is nowhere present in the kernel, except
for some MIPS systems. It's not clear if the vga16fb driver actually
works in practice.

v2:
	* keep driver name to "vga16fb" (Javier)
	* give rational for moving mode checks (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/firmware/sysfb.c      |  4 +++
 drivers/video/fbdev/vga16fb.c | 57 +++++++++++++++++------------------
 2 files changed, 31 insertions(+), 30 deletions(-)

diff --git a/drivers/firmware/sysfb.c b/drivers/firmware/sysfb.c
index 1f276f108cc9..3fd3563d962b 100644
--- a/drivers/firmware/sysfb.c
+++ b/drivers/firmware/sysfb.c
@@ -94,6 +94,10 @@ static __init int sysfb_init(void)
 		name = "efi-framebuffer";
 	else if (si->orig_video_isVGA == VIDEO_TYPE_VLFB)
 		name = "vesa-framebuffer";
+	else if (si->orig_video_isVGA == VIDEO_TYPE_VGAC)
+		name = "vga-framebuffer";
+	else if (si->orig_video_isVGA == VIDEO_TYPE_EGAC)
+		name = "ega-framebuffer";
 	else
 		name = "platform-framebuffer";
 
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index faf76972114d..9e614c1db03d 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -185,19 +185,19 @@ static inline void setindex(int index)
 }
 
 /* Check if the video mode is supported by the driver */
-static inline int check_mode_supported(void)
+static inline int check_mode_supported(const struct screen_info *si)
 {
 	/* non-x86 architectures treat orig_video_isVGA as a boolean flag */
 #if defined(CONFIG_X86)
 	/* only EGA and VGA in 16 color graphic mode are supported */
-	if (screen_info.orig_video_isVGA != VIDEO_TYPE_EGAC &&
-	    screen_info.orig_video_isVGA != VIDEO_TYPE_VGAC)
+	if (si->orig_video_isVGA != VIDEO_TYPE_EGAC &&
+	    si->orig_video_isVGA != VIDEO_TYPE_VGAC)
 		return -ENODEV;
 
-	if (screen_info.orig_video_mode != 0x0D &&	/* 320x200/4 (EGA) */
-	    screen_info.orig_video_mode != 0x0E &&	/* 640x200/4 (EGA) */
-	    screen_info.orig_video_mode != 0x10 &&	/* 640x350/4 (EGA) */
-	    screen_info.orig_video_mode != 0x12)	/* 640x480/4 (VGA) */
+	if (si->orig_video_mode != 0x0D &&	/* 320x200/4 (EGA) */
+	    si->orig_video_mode != 0x0E &&	/* 640x200/4 (EGA) */
+	    si->orig_video_mode != 0x10 &&	/* 640x350/4 (EGA) */
+	    si->orig_video_mode != 0x12)	/* 640x480/4 (VGA) */
 		return -ENODEV;
 #endif
 	return 0;
@@ -1321,11 +1321,20 @@ static int __init vga16fb_setup(char *options)
 
 static int vga16fb_probe(struct platform_device *dev)
 {
+	struct screen_info *si;
 	struct fb_info *info;
 	struct vga16fb_par *par;
 	int i;
 	int ret = 0;
 
+	si = dev_get_platdata(&dev->dev);
+	if (!si)
+		return -ENODEV;
+
+	ret = check_mode_supported(si);
+	if (ret)
+		return ret;
+
 	printk(KERN_DEBUG "vga16fb: initializing\n");
 	info = framebuffer_alloc(sizeof(struct vga16fb_par), &dev->dev);
 
@@ -1352,10 +1361,10 @@ static int vga16fb_probe(struct platform_device *dev)
 	par = info->par;
 
 #if defined(CONFIG_X86)
-	par->isVGA = screen_info.orig_video_isVGA == VIDEO_TYPE_VGAC;
+	par->isVGA = si->orig_video_isVGA == VIDEO_TYPE_VGAC;
 #else
 	/* non-x86 architectures treat orig_video_isVGA as a boolean flag */
-	par->isVGA = screen_info.orig_video_isVGA;
+	par->isVGA = si->orig_video_isVGA;
 #endif
 	par->palette_blanked = 0;
 	par->vesa_blanked = 0;
@@ -1425,16 +1434,21 @@ static int vga16fb_remove(struct platform_device *dev)
 	return 0;
 }
 
+static const struct platform_device_id vga16fb_driver_id_table[] = {
+	{"ega-framebuffer", 0},
+	{"vga-framebuffer", 0},
+	{ }
+};
+
 static struct platform_driver vga16fb_driver = {
 	.probe = vga16fb_probe,
 	.remove = vga16fb_remove,
 	.driver = {
 		.name = "vga16fb",
 	},
+	.id_table = vga16fb_driver_id_table,
 };
 
-static struct platform_device *vga16fb_device;
-
 static int __init vga16fb_init(void)
 {
 	int ret;
@@ -1447,32 +1461,15 @@ static int __init vga16fb_init(void)
 	vga16fb_setup(option);
 #endif
 
-	ret = check_mode_supported();
+	ret = platform_driver_register(&vga16fb_driver);
 	if (ret)
 		return ret;
 
-	ret = platform_driver_register(&vga16fb_driver);
-
-	if (!ret) {
-		vga16fb_device = platform_device_alloc("vga16fb", 0);
-
-		if (vga16fb_device)
-			ret = platform_device_add(vga16fb_device);
-		else
-			ret = -ENOMEM;
-
-		if (ret) {
-			platform_device_put(vga16fb_device);
-			platform_driver_unregister(&vga16fb_driver);
-		}
-	}
-
-	return ret;
+	return 0;
 }
 
 static void __exit vga16fb_exit(void)
 {
-	platform_device_unregister(vga16fb_device);
 	platform_driver_unregister(&vga16fb_driver);
 }
 
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 03/11] fbdev/vga16fb: Auto-generate module init/exit code
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 02/11] fbdev/vga16fb: Create EGA/VGA devices in sysfb code Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 04/11] fbdev/core: Remove remove_conflicting_pci_framebuffers() Thomas Zimmermann
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Move vgag16fb's option parsing into the driver's probe function and
generate the rest of the module's init/exit functions from macros.
Keep the options code, although there are no options defined.

v2:
	* no options are supported, remove the code (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/vga16fb.c | 41 +----------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index 9e614c1db03d..3312b6a4e00d 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -1304,21 +1304,6 @@ static const struct fb_ops vga16fb_ops = {
 	.fb_imageblit	= vga16fb_imageblit,
 };
 
-#ifndef MODULE
-static int __init vga16fb_setup(char *options)
-{
-	char *this_opt;
-
-	if (!options || !*options)
-		return 0;
-
-	while ((this_opt = strsep(&options, ",")) != NULL) {
-		if (!*this_opt) continue;
-	}
-	return 0;
-}
-#endif
-
 static int vga16fb_probe(struct platform_device *dev)
 {
 	struct screen_info *si;
@@ -1449,31 +1434,7 @@ static struct platform_driver vga16fb_driver = {
 	.id_table = vga16fb_driver_id_table,
 };
 
-static int __init vga16fb_init(void)
-{
-	int ret;
-#ifndef MODULE
-	char *option = NULL;
-
-	if (fb_get_options("vga16fb", &option))
-		return -ENODEV;
-
-	vga16fb_setup(option);
-#endif
-
-	ret = platform_driver_register(&vga16fb_driver);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static void __exit vga16fb_exit(void)
-{
-	platform_driver_unregister(&vga16fb_driver);
-}
+module_platform_driver(vga16fb_driver);
 
 MODULE_DESCRIPTION("Legacy VGA framebuffer device driver");
 MODULE_LICENSE("GPL");
-module_init(vga16fb_init);
-module_exit(vga16fb_exit);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 04/11] fbdev/core: Remove remove_conflicting_pci_framebuffers()
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 02/11] fbdev/vga16fb: Create EGA/VGA devices in sysfb code Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 03/11] fbdev/vga16fb: Auto-generate module init/exit code Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 05/11] fbdev: Convert drivers to aperture helpers Thomas Zimmermann
                   ` (6 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Remove remove_conflicting_pci_framebuffers() and implement similar
functionality in aperture_remove_conflicting_pci_device(), which was
the only caller. Removes an otherwise unused interface and streamlines
the aperture helper. No functional changes.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/aperture.c         | 30 ++++++++++++--------
 drivers/video/fbdev/core/fbmem.c | 48 --------------------------------
 include/linux/fb.h               |  2 --
 3 files changed, 18 insertions(+), 62 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 538f2d40acda..f42a0d8bc211 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -321,30 +321,36 @@ EXPORT_SYMBOL(aperture_remove_conflicting_devices);
  */
 int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name)
 {
+	bool primary = false;
 	resource_size_t base, size;
 	int bar, ret;
 
-	/*
-	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
-	 * otherwise the vga fbdev driver falls over.
-	 */
-#if IS_REACHABLE(CONFIG_FB)
-	ret = remove_conflicting_pci_framebuffers(pdev, name);
-	if (ret)
-		return ret;
+#ifdef CONFIG_X86
+	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
 #endif
-	ret = vga_remove_vgacon(pdev);
-	if (ret)
-		return ret;
 
 	for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
 		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
 			continue;
+
 		base = pci_resource_start(pdev, bar);
 		size = pci_resource_len(pdev, bar);
-		aperture_detach_devices(base, size);
+		ret = aperture_remove_conflicting_devices(base, size, primary, name);
+		if (ret)
+			break;
 	}
 
+	if (ret)
+		return ret;
+
+	/*
+	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
+	 * otherwise the vga fbdev driver falls over.
+	 */
+	ret = vga_remove_vgacon(pdev);
+	if (ret)
+		return ret;
+
 	return 0;
 
 }
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index c4a18322dee9..877de85309f3 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1787,54 +1787,6 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
 }
 EXPORT_SYMBOL(remove_conflicting_framebuffers);
 
-/**
- * remove_conflicting_pci_framebuffers - remove firmware-configured framebuffers for PCI devices
- * @pdev: PCI device
- * @name: requesting driver name
- *
- * This function removes framebuffer devices (eg. initialized by firmware)
- * using memory range configured for any of @pdev's memory bars.
- *
- * The function assumes that PCI device with shadowed ROM drives a primary
- * display and so kicks out vga16fb.
- */
-int remove_conflicting_pci_framebuffers(struct pci_dev *pdev, const char *name)
-{
-	struct apertures_struct *ap;
-	bool primary = false;
-	int err, idx, bar;
-
-	for (idx = 0, bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
-		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
-			continue;
-		idx++;
-	}
-
-	ap = alloc_apertures(idx);
-	if (!ap)
-		return -ENOMEM;
-
-	for (idx = 0, bar = 0; bar < PCI_STD_NUM_BARS; bar++) {
-		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
-			continue;
-		ap->ranges[idx].base = pci_resource_start(pdev, bar);
-		ap->ranges[idx].size = pci_resource_len(pdev, bar);
-		pci_dbg(pdev, "%s: bar %d: 0x%lx -> 0x%lx\n", __func__, bar,
-			(unsigned long)pci_resource_start(pdev, bar),
-			(unsigned long)pci_resource_end(pdev, bar));
-		idx++;
-	}
-
-#ifdef CONFIG_X86
-	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
-					IORESOURCE_ROM_SHADOW;
-#endif
-	err = remove_conflicting_framebuffers(ap, name, primary);
-	kfree(ap);
-	return err;
-}
-EXPORT_SYMBOL(remove_conflicting_pci_framebuffers);
-
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
diff --git a/include/linux/fb.h b/include/linux/fb.h
index 07fcd0e56682..b91c77016560 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -615,8 +615,6 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern void unregister_framebuffer(struct fb_info *fb_info);
-extern int remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
-					       const char *name);
 extern int remove_conflicting_framebuffers(struct apertures_struct *a,
 					   const char *name, bool primary);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 05/11] fbdev: Convert drivers to aperture helpers
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (2 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 04/11] fbdev/core: Remove remove_conflicting_pci_framebuffers() Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  9:57   ` kernel test robot
  2022-07-18  7:23 ` [PATCH v2 06/11] fbdev: Remove conflicting devices on PCI bus Thomas Zimmermann
                   ` (5 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Convert fbdev drivers from fbdev's remove_conflicting_framebuffers() to
the framework-independent aperture_remove_conflicting_devices(). Calling
this function will also remove conflicting DRM drivers.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/staging/sm750fb/sm750.c       | 15 +++++----------
 drivers/video/fbdev/aty/radeon_base.c | 17 ++++-------------
 drivers/video/fbdev/hyperv_fb.c       |  6 ++++--
 3 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index dbd1159a2ef0..ce04c38f6afd 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -1,4 +1,5 @@
 // SPDX-License-Identifier: GPL-2.0
+#include <linux/aperture.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/errno.h>
@@ -987,22 +988,16 @@ static int sm750fb_framebuffer_alloc(struct sm750_dev *sm750_dev, int fbidx)
 
 static int lynxfb_kick_out_firmware_fb(struct pci_dev *pdev)
 {
-	struct apertures_struct *ap;
+	resource_size_t base = pci_resource_start(pdev, 0);
+	resource_size_t size = pci_resource_len(pdev, 0);
 	bool primary = false;
 
-	ap = alloc_apertures(1);
-	if (!ap)
-		return -ENOMEM;
-
-	ap->ranges[0].base = pci_resource_start(pdev, 0);
-	ap->ranges[0].size = pci_resource_len(pdev, 0);
 #ifdef CONFIG_X86
 	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
 					IORESOURCE_ROM_SHADOW;
 #endif
-	remove_conflicting_framebuffers(ap, "sm750_fb1", primary);
-	kfree(ap);
-	return 0;
+
+	return aperture_remove_conflicting_devices(base, size, primary, "sm750_fb1");
 }
 
 static int lynxfb_pci_probe(struct pci_dev *pdev,
diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index b311c07fe66d..e5e362b8c9da 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -54,6 +54,7 @@
 
 #include "radeonfb.h"
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
@@ -2239,20 +2240,10 @@ static const struct bin_attribute edid2_attr = {
 
 static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
 {
-	struct apertures_struct *ap;
+	resource_size_t base = pci_resource_start(pdev, 0);
+	resource_size_t size = pci_resource_len(pdev, 0);
 
-	ap = alloc_apertures(1);
-	if (!ap)
-		return -ENOMEM;
-
-	ap->ranges[0].base = pci_resource_start(pdev, 0);
-	ap->ranges[0].size = pci_resource_len(pdev, 0);
-
-	remove_conflicting_framebuffers(ap, KBUILD_MODNAME, false);
-
-	kfree(ap);
-
-	return 0;
+	return aperture_remove_conflicting_devices(base, size, KBUILD_MODNAME, false);
 }
 
 static int radeonfb_pci_register(struct pci_dev *pdev,
diff --git a/drivers/video/fbdev/hyperv_fb.c b/drivers/video/fbdev/hyperv_fb.c
index 886c564787f1..a944a6620527 100644
--- a/drivers/video/fbdev/hyperv_fb.c
+++ b/drivers/video/fbdev/hyperv_fb.c
@@ -45,6 +45,7 @@
 
 #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/vmalloc.h>
@@ -1074,8 +1075,9 @@ static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
 	info->screen_size = dio_fb_size;
 
 getmem_done:
-	remove_conflicting_framebuffers(info->apertures,
-					KBUILD_MODNAME, false);
+	aperture_remove_conflicting_devices(info->apertures->ranges[0].base,
+					    info->apertures->ranges[0].size,
+					    KBUILD_MODNAME, false);
 
 	if (gen2vm) {
 		/* framebuffer is reallocated, clear screen_info to avoid misuse from kexec */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 06/11] fbdev: Remove conflicting devices on PCI bus
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (3 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 05/11] fbdev: Convert drivers to aperture helpers Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-21 14:17   ` Geert Uytterhoeven
  2022-07-18  7:23 ` [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers Thomas Zimmermann
                   ` (4 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Remove firmware devices on the PCI bus, by calling
aperture_remove_conflicting_pci_devices() in the probe function of
each related fbdev driver. iSo far, most of these drivers did not
remove conflicting VESA or EFI devices, or outride failed for
resource conflicts (i.e., matroxfb.) This must have been broken
for quite some time.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/arkfb.c                  | 5 +++++
 drivers/video/fbdev/asiliantfb.c             | 5 +++++
 drivers/video/fbdev/aty/aty128fb.c           | 5 +++++
 drivers/video/fbdev/aty/atyfb_base.c         | 7 ++++++-
 drivers/video/fbdev/carminefb.c              | 5 +++++
 drivers/video/fbdev/chipsfb.c                | 7 ++++++-
 drivers/video/fbdev/cirrusfb.c               | 5 +++++
 drivers/video/fbdev/cyber2000fb.c            | 5 +++++
 drivers/video/fbdev/geode/gx1fb_core.c       | 5 +++++
 drivers/video/fbdev/geode/gxfb_core.c        | 5 +++++
 drivers/video/fbdev/geode/lxfb_core.c        | 5 +++++
 drivers/video/fbdev/gxt4500.c                | 5 +++++
 drivers/video/fbdev/i740fb.c                 | 5 +++++
 drivers/video/fbdev/i810/i810_main.c         | 5 +++++
 drivers/video/fbdev/imsttfb.c                | 8 +++++++-
 drivers/video/fbdev/intelfb/intelfbdrv.c     | 5 +++++
 drivers/video/fbdev/kyro/fbdev.c             | 5 +++++
 drivers/video/fbdev/matrox/matroxfb_base.c   | 5 +++++
 drivers/video/fbdev/mb862xx/mb862xxfbdrv.c   | 5 +++++
 drivers/video/fbdev/neofb.c                  | 5 +++++
 drivers/video/fbdev/nvidia/nvidia.c          | 7 ++++++-
 drivers/video/fbdev/pm2fb.c                  | 5 +++++
 drivers/video/fbdev/pm3fb.c                  | 5 +++++
 drivers/video/fbdev/pvr2fb.c                 | 5 +++++
 drivers/video/fbdev/riva/fbdev.c             | 5 +++++
 drivers/video/fbdev/s3fb.c                   | 5 +++++
 drivers/video/fbdev/savage/savagefb_driver.c | 5 +++++
 drivers/video/fbdev/sis/sis_main.c           | 5 +++++
 drivers/video/fbdev/skeletonfb.c             | 8 ++++++++
 drivers/video/fbdev/sm712fb.c                | 5 +++++
 drivers/video/fbdev/sstfb.c                  | 5 +++++
 drivers/video/fbdev/sunxvr2500.c             | 5 +++++
 drivers/video/fbdev/sunxvr500.c              | 5 +++++
 drivers/video/fbdev/tdfxfb.c                 | 5 +++++
 drivers/video/fbdev/tgafb.c                  | 7 +++++++
 drivers/video/fbdev/tridentfb.c              | 5 +++++
 drivers/video/fbdev/vermilion/vermilion.c    | 7 ++++++-
 drivers/video/fbdev/via/via-core.c           | 5 +++++
 drivers/video/fbdev/vt8623fb.c               | 5 +++++
 39 files changed, 206 insertions(+), 5 deletions(-)

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index eb3e47c58c5f..453daa072f53 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -11,6 +11,7 @@
  *  Code is based on s3fb
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -949,6 +950,10 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	int rc;
 	u8 regval;
 
+	rc = aperture_remove_conflicting_pci_devices(dev, "arkfb");
+	if (rc < 0)
+		return rc;
+
 	/* Ignore secondary VGA device because there is no VGA arbitration */
 	if (! svga_primary_device(dev)) {
 		dev_info(&(dev->dev), "ignoring secondary device\n");
diff --git a/drivers/video/fbdev/asiliantfb.c b/drivers/video/fbdev/asiliantfb.c
index f8ef62542f7f..3818437a8f69 100644
--- a/drivers/video/fbdev/asiliantfb.c
+++ b/drivers/video/fbdev/asiliantfb.c
@@ -29,6 +29,7 @@
  *  more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -545,6 +546,10 @@ static int asiliantfb_pci_init(struct pci_dev *dp,
 	struct fb_info *p;
 	int err;
 
+	err = aperture_remove_conflicting_pci_devices(dp, "asiliantfb");
+	if (err)
+		return err;
+
 	if ((dp->resource[0].flags & IORESOURCE_MEM) == 0)
 		return -ENODEV;
 	addr = pci_resource_start(dp, 0);
diff --git a/drivers/video/fbdev/aty/aty128fb.c b/drivers/video/fbdev/aty/aty128fb.c
index 5cdbbba2a013..57e398fe7a81 100644
--- a/drivers/video/fbdev/aty/aty128fb.c
+++ b/drivers/video/fbdev/aty/aty128fb.c
@@ -47,6 +47,7 @@
  */
 
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
@@ -2055,6 +2056,10 @@ static int aty128_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	void __iomem *bios = NULL;
 #endif
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "aty128fb");
+	if (err)
+		return err;
+
 	/* Enable device in PCI config */
 	if ((err = pci_enable_device(pdev))) {
 		printk(KERN_ERR "aty128fb: Cannot enable PCI device: %d\n",
diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
index a3e6faed7745..4804b6e9f3f4 100644
--- a/drivers/video/fbdev/aty/atyfb_base.c
+++ b/drivers/video/fbdev/aty/atyfb_base.c
@@ -48,6 +48,7 @@
 
 ******************************************************************************/
 
+#include <linux/aperture.h>
 #include <linux/compat.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
@@ -3533,7 +3534,11 @@ static int atyfb_pci_probe(struct pci_dev *pdev,
 	struct fb_info *info;
 	struct resource *rp;
 	struct atyfb_par *par;
-	int rc = -ENOMEM;
+	int rc;
+
+	rc = aperture_remove_conflicting_pci_devices(pdev, "atyfb");
+	if (rc)
+		return rc;
 
 	/* Enable device in PCI config */
 	if (pci_enable_device(pdev)) {
diff --git a/drivers/video/fbdev/carminefb.c b/drivers/video/fbdev/carminefb.c
index 3a1c2e0739a1..4651b48a87f9 100644
--- a/drivers/video/fbdev/carminefb.c
+++ b/drivers/video/fbdev/carminefb.c
@@ -7,6 +7,7 @@
  * - FB1 is display 1 with unique memory area
  * - both display use 32 bit colors
  */
+#include <linux/aperture.h>
 #include <linux/delay.h>
 #include <linux/errno.h>
 #include <linux/fb.h>
@@ -614,6 +615,10 @@ static int carminefb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 	struct fb_info *info;
 	int ret;
 
+	ret = aperture_remove_conflicting_pci_devices(dev, "carminefb");
+	if (ret)
+		return ret;
+
 	ret = pci_enable_device(dev);
 	if (ret)
 		return ret;
diff --git a/drivers/video/fbdev/chipsfb.c b/drivers/video/fbdev/chipsfb.c
index 618fb6dbbedb..5ad64714d39e 100644
--- a/drivers/video/fbdev/chipsfb.c
+++ b/drivers/video/fbdev/chipsfb.c
@@ -14,6 +14,7 @@
  *  more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -351,7 +352,11 @@ static int chipsfb_pci_init(struct pci_dev *dp, const struct pci_device_id *ent)
 	struct fb_info *p;
 	unsigned long addr;
 	unsigned short cmd;
-	int rc = -ENODEV;
+	int rc;
+
+	rc = aperture_remove_conflicting_pci_devices(dp, "chipsfb");
+	if (rc)
+		return rc;
 
 	if (pci_enable_device(dp) < 0) {
 		dev_err(&dp->dev, "Cannot enable PCI device\n");
diff --git a/drivers/video/fbdev/cirrusfb.c b/drivers/video/fbdev/cirrusfb.c
index 51e072c03e1c..4ff6f624f912 100644
--- a/drivers/video/fbdev/cirrusfb.c
+++ b/drivers/video/fbdev/cirrusfb.c
@@ -34,6 +34,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -2085,6 +2086,10 @@ static int cirrusfb_pci_register(struct pci_dev *pdev,
 	unsigned long board_addr, board_size;
 	int ret;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "cirrusfb");
+	if (ret)
+		return ret;
+
 	ret = pci_enable_device(pdev);
 	if (ret < 0) {
 		printk(KERN_ERR "cirrusfb: Cannot enable PCI device\n");
diff --git a/drivers/video/fbdev/cyber2000fb.c b/drivers/video/fbdev/cyber2000fb.c
index d45355b9a58c..be7bcf95c96a 100644
--- a/drivers/video/fbdev/cyber2000fb.c
+++ b/drivers/video/fbdev/cyber2000fb.c
@@ -33,6 +33,7 @@
  * (which, incidentally, is about the same saving as a 2.5in hard disk
  * entering standby mode.)
  */
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1720,6 +1721,10 @@ static int cyberpro_pci_probe(struct pci_dev *dev,
 
 	sprintf(name, "CyberPro%4X", id->device);
 
+	err = aperture_remove_conflicting_pci_devices(dev, name);
+	if (err)
+		return err;
+
 	err = pci_enable_device(dev);
 	if (err)
 		return err;
diff --git a/drivers/video/fbdev/geode/gx1fb_core.c b/drivers/video/fbdev/geode/gx1fb_core.c
index 5d34d89fb665..4cac7e3bb1a0 100644
--- a/drivers/video/fbdev/geode/gx1fb_core.c
+++ b/drivers/video/fbdev/geode/gx1fb_core.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2005 Arcom Control Systems Ltd.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -320,6 +321,10 @@ static int gx1fb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct fb_info *info;
 	int ret;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "gx1fb");
+	if (ret)
+		return ret;
+
 	info = gx1fb_init_fbinfo(&pdev->dev);
 	if (!info)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/geode/gxfb_core.c b/drivers/video/fbdev/geode/gxfb_core.c
index 44089b331f91..2527bd80ec5f 100644
--- a/drivers/video/fbdev/geode/gxfb_core.c
+++ b/drivers/video/fbdev/geode/gxfb_core.c
@@ -15,6 +15,7 @@
  *
  * 16 MiB of framebuffer memory is assumed to be available.
  */
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -364,6 +365,10 @@ static int gxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct fb_videomode *modedb_ptr;
 	unsigned int modedb_size;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "gxfb");
+	if (ret)
+		return ret;
+
 	info = gxfb_init_fbinfo(&pdev->dev);
 	if (!info)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/geode/lxfb_core.c b/drivers/video/fbdev/geode/lxfb_core.c
index 66c81262d18f..9d26592dbfce 100644
--- a/drivers/video/fbdev/geode/lxfb_core.c
+++ b/drivers/video/fbdev/geode/lxfb_core.c
@@ -6,6 +6,7 @@
  * Built from gxfb (which is Copyright (C) 2006 Arcom Control Systems Ltd.)
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -484,6 +485,10 @@ static int lxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct fb_videomode *modedb_ptr;
 	unsigned int modedb_size;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "lxfb");
+	if (ret)
+		return ret;
+
 	info = lxfb_init_fbinfo(&pdev->dev);
 
 	if (info == NULL)
diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index e5475ae1e158..f0d36a4fdaef 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -6,6 +6,7 @@
  * Copyright (C) 2006 Paul Mackerras, IBM Corp. <paulus@samba.org>
  */
 
+#include <linux/aperture.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
 #include <linux/fb.h>
@@ -621,6 +622,10 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct fb_var_screeninfo var;
 	enum gxt_cards cardtype;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "gxt4500fb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(pdev);
 	if (err) {
 		dev_err(&pdev->dev, "gxt4500: cannot enable PCI device: %d\n",
diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
index 09dd85553d4f..23329de28e77 100644
--- a/drivers/video/fbdev/i740fb.c
+++ b/drivers/video/fbdev/i740fb.c
@@ -12,6 +12,7 @@
  *  i740fb by Patrick LERDA, v0.9
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1008,6 +1009,10 @@ static int i740fb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 	bool found = false;
 	u8 *edid;
 
+	ret = aperture_remove_conflicting_pci_devices(dev, "i740fb");
+	if (ret)
+		return ret;
+
 	info = framebuffer_alloc(sizeof(struct i740fb_par), &(dev->dev));
 	if (!info)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/i810/i810_main.c b/drivers/video/fbdev/i810/i810_main.c
index 41a86efca516..ff09f8c20bfc 100644
--- a/drivers/video/fbdev/i810/i810_main.c
+++ b/drivers/video/fbdev/i810/i810_main.c
@@ -28,6 +28,7 @@
  *  more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -2016,6 +2017,10 @@ static int i810fb_init_pci(struct pci_dev *dev,
 	struct fb_videomode mode;
 	int err = -1, vfreq, hfreq, pixclock;
 
+	err = aperture_remove_conflicting_pci_devices(dev, "i810fb");
+	if (err)
+		return err;
+
 	info = framebuffer_alloc(sizeof(struct i810fb_par), &dev->dev);
 	if (!info)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/imsttfb.c b/drivers/video/fbdev/imsttfb.c
index f489386855c0..d7edb9c5d3a3 100644
--- a/drivers/video/fbdev/imsttfb.c
+++ b/drivers/video/fbdev/imsttfb.c
@@ -16,6 +16,7 @@
  *  more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1469,7 +1470,12 @@ static int imsttfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	struct imstt_par *par;
 	struct fb_info *info;
 	struct device_node *dp;
-	int ret = -ENOMEM;
+	int ret;
+
+	ret = aperture_remove_conflicting_pci_devices(pdev, "imsttfb");
+	if (ret)
+		return ret;
+	ret = -ENOMEM;
 
 	dp = pci_device_to_OF_node(pdev);
 	if(dp)
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index 5647fca8c49a..d4a2891a9a7a 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -107,6 +107,7 @@
  *              Add support for 945GME. (Phil Endecott <spam_from_intelfb@chezphil.org>)
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -483,6 +484,10 @@ static int intelfb_pci_register(struct pci_dev *pdev,
 
 	DBG_MSG("intelfb_pci_register\n");
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "intelfb");
+	if (err)
+		return err;
+
 	num_registered++;
 	if (num_registered != 1) {
 		ERR_MSG("Attempted to register %d devices "
diff --git a/drivers/video/fbdev/kyro/fbdev.c b/drivers/video/fbdev/kyro/fbdev.c
index d57772f96ad2..b4b93054c520 100644
--- a/drivers/video/fbdev/kyro/fbdev.c
+++ b/drivers/video/fbdev/kyro/fbdev.c
@@ -9,6 +9,7 @@
  * for more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/types.h>
 #include <linux/kernel.h>
@@ -676,6 +677,10 @@ static int kyrofb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	unsigned long size;
 	int err;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "kyrofb");
+	if (err)
+		return err;
+
 	if ((err = pci_enable_device(pdev))) {
 		printk(KERN_WARNING "kyrofb: Can't enable pdev: %d\n", err);
 		return err;
diff --git a/drivers/video/fbdev/matrox/matroxfb_base.c b/drivers/video/fbdev/matrox/matroxfb_base.c
index 236521b19daf..3e26346c05a2 100644
--- a/drivers/video/fbdev/matrox/matroxfb_base.c
+++ b/drivers/video/fbdev/matrox/matroxfb_base.c
@@ -100,6 +100,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/version.h>
 
 #include "matroxfb_base.h"
@@ -2044,6 +2045,10 @@ static int matroxfb_probe(struct pci_dev* pdev, const struct pci_device_id* dumm
 	u_int32_t cmd;
 	DBG(__func__)
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "matroxfb");
+	if (err)
+		return err;
+
 	svid = pdev->subsystem_vendor;
 	sid = pdev->subsystem_device;
 	for (b = dev_list; b->vendor; b++) {
diff --git a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
index a7508f5be343..96800c9c9cd9 100644
--- a/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
+++ b/drivers/video/fbdev/mb862xx/mb862xxfbdrv.c
@@ -10,6 +10,7 @@
 
 #undef DEBUG
 
+#include <linux/aperture.h>
 #include <linux/fb.h>
 #include <linux/delay.h>
 #include <linux/uaccess.h>
@@ -999,6 +1000,10 @@ static int mb862xx_pci_probe(struct pci_dev *pdev,
 	struct device *dev = &pdev->dev;
 	int ret;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "mb862xxfb");
+	if (ret)
+		return ret;
+
 	ret = pci_enable_device(pdev);
 	if (ret < 0) {
 		dev_err(dev, "Cannot enable PCI device\n");
diff --git a/drivers/video/fbdev/neofb.c b/drivers/video/fbdev/neofb.c
index 8ad2a79623af..93a2d2d1abe8 100644
--- a/drivers/video/fbdev/neofb.c
+++ b/drivers/video/fbdev/neofb.c
@@ -54,6 +54,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -2029,6 +2030,10 @@ static int neofb_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	DBG("neofb_probe");
 
+	err = aperture_remove_conflicting_pci_devices(dev, "neofb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(dev);
 	if (err)
 		return err;
diff --git a/drivers/video/fbdev/nvidia/nvidia.c b/drivers/video/fbdev/nvidia/nvidia.c
index a372a183c1f0..329e2e8133c6 100644
--- a/drivers/video/fbdev/nvidia/nvidia.c
+++ b/drivers/video/fbdev/nvidia/nvidia.c
@@ -9,6 +9,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1276,11 +1277,15 @@ static int nvidiafb_probe(struct pci_dev *pd, const struct pci_device_id *ent)
 	struct nvidia_par *par;
 	struct fb_info *info;
 	unsigned short cmd;
-
+	int ret;
 
 	NVTRACE_ENTER();
 	assert(pd != NULL);
 
+	ret = aperture_remove_conflicting_pci_devices(pd, "nvidiafb");
+	if (ret)
+		return ret;
+
 	info = framebuffer_alloc(sizeof(struct nvidia_par), &pd->dev);
 
 	if (!info)
diff --git a/drivers/video/fbdev/pm2fb.c b/drivers/video/fbdev/pm2fb.c
index d3be2c64f1c0..bc80d8498aeb 100644
--- a/drivers/video/fbdev/pm2fb.c
+++ b/drivers/video/fbdev/pm2fb.c
@@ -27,6 +27,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
@@ -1516,6 +1517,10 @@ static int pm2fb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	int err;
 	int retval = -ENXIO;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "pm2fb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(pdev);
 	if (err) {
 		printk(KERN_WARNING "pm2fb: Can't enable pdev: %d\n", err);
diff --git a/drivers/video/fbdev/pm3fb.c b/drivers/video/fbdev/pm3fb.c
index a8faf46adeb1..ba69846d444f 100644
--- a/drivers/video/fbdev/pm3fb.c
+++ b/drivers/video/fbdev/pm3fb.c
@@ -22,6 +22,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1315,6 +1316,10 @@ static int pm3fb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 	int err;
 	int retval = -ENXIO;
 
+	err = aperture_remove_conflicting_pci_devices(dev, "pm3fb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(dev);
 	if (err) {
 		printk(KERN_WARNING "pm3fb: Can't enable PCI dev: %d\n", err);
diff --git a/drivers/video/fbdev/pvr2fb.c b/drivers/video/fbdev/pvr2fb.c
index f4add36cb5f4..b73ad14efa20 100644
--- a/drivers/video/fbdev/pvr2fb.c
+++ b/drivers/video/fbdev/pvr2fb.c
@@ -45,6 +45,7 @@
 
 #undef DEBUG
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -942,6 +943,10 @@ static int pvr2fb_pci_probe(struct pci_dev *pdev,
 {
 	int ret;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "pvrfb");
+	if (ret)
+		return ret;
+
 	ret = pci_enable_device(pdev);
 	if (ret) {
 		printk(KERN_ERR "pvr2fb: PCI enable failed\n");
diff --git a/drivers/video/fbdev/riva/fbdev.c b/drivers/video/fbdev/riva/fbdev.c
index 5bafc44c591b..0ea74e28f915 100644
--- a/drivers/video/fbdev/riva/fbdev.c
+++ b/drivers/video/fbdev/riva/fbdev.c
@@ -29,6 +29,7 @@
  *	doublescan modes are broken
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1898,6 +1899,10 @@ static int rivafb_probe(struct pci_dev *pd, const struct pci_device_id *ent)
 	NVTRACE_ENTER();
 	assert(pd != NULL);
 
+	ret = aperture_remove_conflicting_pci_devices(pd, "rivafb");
+	if (ret)
+		return ret;
+
 	info = framebuffer_alloc(sizeof(struct riva_par), &pd->dev);
 	if (!info) {
 		ret = -ENOMEM;
diff --git a/drivers/video/fbdev/s3fb.c b/drivers/video/fbdev/s3fb.c
index b93c8eb02336..f66c4de0e188 100644
--- a/drivers/video/fbdev/s3fb.c
+++ b/drivers/video/fbdev/s3fb.c
@@ -11,6 +11,7 @@
  * which is based on the code of neofb.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1129,6 +1130,10 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return -ENODEV;
 	}
 
+	rc = aperture_remove_conflicting_pci_devices(dev, "s3fb");
+	if (rc)
+		return rc;
+
 	/* Allocate and fill driver data structure */
 	info = framebuffer_alloc(sizeof(struct s3fb_info), &(dev->dev));
 	if (!info)
diff --git a/drivers/video/fbdev/savage/savagefb_driver.c b/drivers/video/fbdev/savage/savagefb_driver.c
index 8114c921ceb8..b7818b652698 100644
--- a/drivers/video/fbdev/savage/savagefb_driver.c
+++ b/drivers/video/fbdev/savage/savagefb_driver.c
@@ -41,6 +41,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -2176,6 +2177,10 @@ static int savagefb_probe(struct pci_dev *dev, const struct pci_device_id *id)
 
 	DBG("savagefb_probe");
 
+	err = aperture_remove_conflicting_pci_devices(dev, "savagefb");
+	if (err)
+		return err;
+
 	info = framebuffer_alloc(sizeof(struct savagefb_par), &dev->dev);
 	if (!info)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/sis/sis_main.c b/drivers/video/fbdev/sis/sis_main.c
index f28fd69d5eb7..7114c5c17c91 100644
--- a/drivers/video/fbdev/sis/sis_main.c
+++ b/drivers/video/fbdev/sis/sis_main.c
@@ -19,6 +19,7 @@
  * which is (c) 1998 Gerd Knorr <kraxel@goldbach.in-berlin.de>
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/moduleparam.h>
 #include <linux/kernel.h>
@@ -5849,6 +5850,10 @@ static int sisfb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	if(sisfb_off)
 		return -ENXIO;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "sisfb");
+	if (ret)
+		return ret;
+
 	sis_fb_info = framebuffer_alloc(sizeof(*ivideo), &pdev->dev);
 	if(!sis_fb_info)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/skeletonfb.c b/drivers/video/fbdev/skeletonfb.c
index 304320ce6c6f..125df366e23a 100644
--- a/drivers/video/fbdev/skeletonfb.c
+++ b/drivers/video/fbdev/skeletonfb.c
@@ -42,6 +42,7 @@
  *  more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -668,6 +669,13 @@ static int xxxfb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
     struct device *device = &dev->dev; /* or &pdev->dev */
     int cmap_len, retval;
 
+    /*
+     * Remove firmware-based drivers that create resource conflicts.
+     */
+    retval = aperture_remove_conflicting_pci_devices(pdev, "xxxfb");
+    if (retval)
+	    return retval;
+
     /*
      * Dynamically allocate info and par
      */
diff --git a/drivers/video/fbdev/sm712fb.c b/drivers/video/fbdev/sm712fb.c
index 092a1caa1208..3baf33635e65 100644
--- a/drivers/video/fbdev/sm712fb.c
+++ b/drivers/video/fbdev/sm712fb.c
@@ -18,6 +18,7 @@
  * Framebuffer driver for Silicon Motion SM710, SM712, SM721 and SM722 chips
  */
 
+#include <linux/aperture.h>
 #include <linux/io.h>
 #include <linux/fb.h>
 #include <linux/pci.h>
@@ -1502,6 +1503,10 @@ static int smtcfb_pci_probe(struct pci_dev *pdev,
 
 	dev_info(&pdev->dev, "Silicon Motion display driver.\n");
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "smtcfb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(pdev);	/* enable SMTC chip */
 	if (err)
 		return err;
diff --git a/drivers/video/fbdev/sstfb.c b/drivers/video/fbdev/sstfb.c
index 535ef4693cd4..73ca2782ebfc 100644
--- a/drivers/video/fbdev/sstfb.c
+++ b/drivers/video/fbdev/sstfb.c
@@ -80,6 +80,7 @@
  * Includes
  */
 
+#include <linux/aperture.h>
 #include <linux/string.h>
 #include <linux/kernel.h>
 #include <linux/module.h>
@@ -1326,6 +1327,10 @@ static int sstfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct sst_spec *spec;
 	int err;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "sstfb");
+	if (err)
+		return err;
+
 	/* Enable device in PCI config. */
 	if ((err=pci_enable_device(pdev))) {
 		printk(KERN_ERR "cannot enable device\n");
diff --git a/drivers/video/fbdev/sunxvr2500.c b/drivers/video/fbdev/sunxvr2500.c
index 1d3bacd9d5ac..81d59613ea1f 100644
--- a/drivers/video/fbdev/sunxvr2500.c
+++ b/drivers/video/fbdev/sunxvr2500.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2007 David S. Miller (davem@davemloft.net)
  */
 
+#include <linux/aperture.h>
 #include <linux/kernel.h>
 #include <linux/fb.h>
 #include <linux/pci.h>
@@ -123,6 +124,10 @@ static int s3d_pci_register(struct pci_dev *pdev,
 	struct s3d_info *sp;
 	int err;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "s3dfb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(pdev);
 	if (err < 0) {
 		printk(KERN_ERR "s3d: Cannot enable PCI device %s\n",
diff --git a/drivers/video/fbdev/sunxvr500.c b/drivers/video/fbdev/sunxvr500.c
index 9daf17b11106..3a51b2a1480c 100644
--- a/drivers/video/fbdev/sunxvr500.c
+++ b/drivers/video/fbdev/sunxvr500.c
@@ -5,6 +5,7 @@
  * Copyright (C) 2007 David S. Miller (davem@davemloft.net)
  */
 
+#include <linux/aperture.h>
 #include <linux/kernel.h>
 #include <linux/fb.h>
 #include <linux/pci.h>
@@ -249,6 +250,10 @@ static int e3d_pci_register(struct pci_dev *pdev,
 	unsigned int line_length;
 	int err;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "e3dfb");
+	if (err)
+		return err;
+
 	of_node = pci_device_to_OF_node(pdev);
 	if (!of_node) {
 		printk(KERN_ERR "e3d: Cannot find OF node of %s\n",
diff --git a/drivers/video/fbdev/tdfxfb.c b/drivers/video/fbdev/tdfxfb.c
index 67e37a62b07c..059e0174e139 100644
--- a/drivers/video/fbdev/tdfxfb.c
+++ b/drivers/video/fbdev/tdfxfb.c
@@ -64,6 +64,7 @@
  *
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -1376,6 +1377,10 @@ static int tdfxfb_probe(struct pci_dev *pdev, const struct pci_device_id *id)
 	struct fb_monspecs *specs;
 	bool found;
 
+	err = aperture_remove_conflicting_pci_devices(pdev, "tdfxfb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(pdev);
 	if (err) {
 		printk(KERN_ERR "tdfxfb: Can't enable pdev: %d\n", err);
diff --git a/drivers/video/fbdev/tgafb.c b/drivers/video/fbdev/tgafb.c
index 9266c76783cc..4600138e3bef 100644
--- a/drivers/video/fbdev/tgafb.c
+++ b/drivers/video/fbdev/tgafb.c
@@ -12,6 +12,7 @@
  *  more details.
  */
 
+#include <linux/aperture.h>
 #include <linux/bitrev.h>
 #include <linux/compiler.h>
 #include <linux/delay.h>
@@ -106,6 +107,12 @@ static struct pci_driver tgafb_pci_driver = {
 static int tgafb_pci_register(struct pci_dev *pdev,
 			      const struct pci_device_id *ent)
 {
+	int ret;
+
+	ret = aperture_remove_conflicting_pci_devices(pdev, "tgafb");
+	if (ret)
+		return ret;
+
 	return tgafb_register(&pdev->dev);
 }
 
diff --git a/drivers/video/fbdev/tridentfb.c b/drivers/video/fbdev/tridentfb.c
index 319131bd72cf..6813df793c49 100644
--- a/drivers/video/fbdev/tridentfb.c
+++ b/drivers/video/fbdev/tridentfb.c
@@ -16,6 +16,7 @@
  *	timing value tweaking so it looks good on every monitor in every mode
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/fb.h>
 #include <linux/init.h>
@@ -1470,6 +1471,10 @@ static int trident_pci_probe(struct pci_dev *dev,
 	int chip_id;
 	bool found = false;
 
+	err = aperture_remove_conflicting_pci_devices(dev, "tridentfb");
+	if (err)
+		return err;
+
 	err = pci_enable_device(dev);
 	if (err)
 		return err;
diff --git a/drivers/video/fbdev/vermilion/vermilion.c b/drivers/video/fbdev/vermilion/vermilion.c
index ff61605b8764..82b36dbb5b1a 100644
--- a/drivers/video/fbdev/vermilion/vermilion.c
+++ b/drivers/video/fbdev/vermilion/vermilion.c
@@ -14,6 +14,7 @@
  *   Alan Hourihane <alanh-at-tungstengraphics-dot-com>
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -442,7 +443,11 @@ static int vml_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 	struct vml_info *vinfo;
 	struct fb_info *info;
 	struct vml_par *par;
-	int err = 0;
+	int err;
+
+	err = aperture_remove_conflicting_pci_devices(dev, "vmlfb");
+	if (err)
+		return err;
 
 	par = kzalloc(sizeof(*par), GFP_KERNEL);
 	if (par == NULL)
diff --git a/drivers/video/fbdev/via/via-core.c b/drivers/video/fbdev/via/via-core.c
index 89d75079b730..2ee8fcae08df 100644
--- a/drivers/video/fbdev/via/via-core.c
+++ b/drivers/video/fbdev/via/via-core.c
@@ -8,6 +8,7 @@
 /*
  * Core code for the Via multifunction framebuffer device.
  */
+#include <linux/aperture.h>
 #include <linux/via-core.h>
 #include <linux/via_i2c.h>
 #include <linux/via-gpio.h>
@@ -617,6 +618,10 @@ static int via_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	int ret;
 
+	ret = aperture_remove_conflicting_pci_devices(pdev, "viafb");
+	if (ret)
+		return ret;
+
 	ret = pci_enable_device(pdev);
 	if (ret)
 		return ret;
diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c
index a92a8c670cf0..62318cef5f8c 100644
--- a/drivers/video/fbdev/vt8623fb.c
+++ b/drivers/video/fbdev/vt8623fb.c
@@ -12,6 +12,7 @@
  * (http://davesdomain.org.uk/viafb/)
  */
 
+#include <linux/aperture.h>
 #include <linux/module.h>
 #include <linux/kernel.h>
 #include <linux/errno.h>
@@ -670,6 +671,10 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)
 		return -ENODEV;
 	}
 
+	rc = aperture_remove_conflicting_pci_devices(dev, "vt8623fb");
+	if (rc)
+		return rc;
+
 	/* Allocate and fill driver data structure */
 	info = framebuffer_alloc(sizeof(struct vt8623fb_info), &(dev->dev));
 	if (!info)
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (4 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 06/11] fbdev: Remove conflicting devices on PCI bus Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2023-03-20  1:47   ` Samuel Čavoj
  2022-07-18  7:23 ` [PATCH v2 08/11] video: Provide constants for VGA I/O range Thomas Zimmermann
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann,
	Zack Rusin, Daniel Vetter, Alex Deucher, Zhen Lei,
	Changcheng Deng, Maarten Lankhorst, Maxime Ripard

Call sysfb_disable() before removing conflicting devices in aperture
helpers. Fixes sysfb state if fbdev has been disabled.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
Fixes: fb84efa28a48 ("drm/aperture: Run fbdev removal before internal helpers")
Cc: Zack Rusin <zackr@vmware.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: Sam Ravnborg <sam@ravnborg.org>
Cc: Helge Deller <deller@gmx.de>
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: Zhen Lei <thunder.leizhen@huawei.com>
Cc: Changcheng Deng <deng.changcheng@zte.com.cn>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: dri-devel@lists.freedesktop.org
---
 drivers/video/aperture.c         | 14 ++++++++++++++
 drivers/video/fbdev/core/fbmem.c | 12 ------------
 2 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index f42a0d8bc211..101e13c2cf41 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -8,6 +8,7 @@
 #include <linux/pci.h>
 #include <linux/platform_device.h>
 #include <linux/slab.h>
+#include <linux/sysfb.h>
 #include <linux/types.h>
 #include <linux/vgaarb.h>
 
@@ -286,7 +287,20 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
 #if IS_REACHABLE(CONFIG_FB)
 	struct apertures_struct *a;
 	int ret;
+#endif
+
+	/*
+	 * If a driver asked to unregister a platform device registered by
+	 * sysfb, then can be assumed that this is a driver for a display
+	 * that is set up by the system firmware and has a generic driver.
+	 *
+	 * Drivers for devices that don't have a generic driver will never
+	 * ask for this, so let's assume that a real driver for the display
+	 * was already probed and prevent sysfb to register devices later.
+	 */
+	sysfb_disable();
 
+#if IS_REACHABLE(CONFIG_FB)
 	a = alloc_apertures(1);
 	if (!a)
 		return -ENOMEM;
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 877de85309f3..3ee3ea018245 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -19,7 +19,6 @@
 #include <linux/kernel.h>
 #include <linux/major.h>
 #include <linux/slab.h>
-#include <linux/sysfb.h>
 #include <linux/mm.h>
 #include <linux/mman.h>
 #include <linux/vt.h>
@@ -1765,17 +1764,6 @@ int remove_conflicting_framebuffers(struct apertures_struct *a,
 		do_free = true;
 	}
 
-	/*
-	 * If a driver asked to unregister a platform device registered by
-	 * sysfb, then can be assumed that this is a driver for a display
-	 * that is set up by the system firmware and has a generic driver.
-	 *
-	 * Drivers for devices that don't have a generic driver will never
-	 * ask for this, so let's assume that a real driver for the display
-	 * was already probed and prevent sysfb to register devices later.
-	 */
-	sysfb_disable();
-
 	mutex_lock(&registration_lock);
 	do_remove_conflicting_framebuffers(a, name, primary);
 	mutex_unlock(&registration_lock);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 08/11] video: Provide constants for VGA I/O range
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (5 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 09/11] video/aperture: Remove conflicting VGA devices, if any Thomas Zimmermann
                   ` (2 subsequent siblings)
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Provide VGA_FB_ constants for the VGA framebuffer I/O range and convert
fbdev code. In the case of vga16fb, this is a rename of the existing
constants VGA_FB_PHYS and VGA_FB_PHYS_LEN.

v2:
	* clarify relationship with old constants in vga16fb (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/core/fbmem.c |  4 ++--
 drivers/video/fbdev/vga16fb.c    | 15 ++++++---------
 include/video/vga.h              |  2 ++
 3 files changed, 10 insertions(+), 11 deletions(-)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 3ee3ea018245..2237049327db 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -39,6 +39,7 @@
 
 #include <asm/fb.h>
 
+#include <video/vga.h>
 
     /*
      *  Frame buffer device initialization and setup routines
@@ -1549,7 +1550,6 @@ static bool fb_do_apertures_overlap(struct apertures_struct *gena,
 
 static void do_unregister_framebuffer(struct fb_info *fb_info);
 
-#define VGA_FB_PHYS 0xA0000
 static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
 					       const char *name, bool primary)
 {
@@ -1568,7 +1568,7 @@ static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
 		device = registered_fb[i]->device;
 		if (fb_do_apertures_overlap(gen_aper, a) ||
 			(primary && gen_aper && gen_aper->count &&
-			 gen_aper->ranges[0].base == VGA_FB_PHYS)) {
+			 gen_aper->ranges[0].base == VGA_FB_PHYS_BASE)) {
 
 			printk(KERN_INFO "fb%d: switching to %s from %s\n",
 			       i, name, registered_fb[i]->fix.id);
diff --git a/drivers/video/fbdev/vga16fb.c b/drivers/video/fbdev/vga16fb.c
index 3312b6a4e00d..35cf51ae3292 100644
--- a/drivers/video/fbdev/vga16fb.c
+++ b/drivers/video/fbdev/vga16fb.c
@@ -25,9 +25,6 @@
 #include <asm/io.h>
 #include <video/vga.h>
 
-#define VGA_FB_PHYS 0xA0000
-#define VGA_FB_PHYS_LEN 65536
-
 #define MODE_SKIP4	1
 #define MODE_8BPP	2
 #define MODE_CFB	4
@@ -87,8 +84,8 @@ static struct fb_var_screeninfo vga16fb_defined = {
 /* name should not depend on EGA/VGA */
 static const struct fb_fix_screeninfo vga16fb_fix = {
 	.id		= "VGA16 VGA",
-	.smem_start	= VGA_FB_PHYS,
-	.smem_len	= VGA_FB_PHYS_LEN,
+	.smem_start	= VGA_FB_PHYS_BASE,
+	.smem_len	= VGA_FB_PHYS_SIZE,
 	.type		= FB_TYPE_VGA_PLANES,
 	.type_aux	= FB_AUX_VGA_PLANES_VGA4,
 	.visual		= FB_VISUAL_PSEUDOCOLOR,
@@ -1333,8 +1330,8 @@ static int vga16fb_probe(struct platform_device *dev)
 		goto err_ioremap;
 	}
 
-	/* XXX share VGA_FB_PHYS and I/O region with vgacon and others */
-	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS, 0);
+	/* XXX share VGA_FB_PHYS_BASE and I/O region with vgacon and others */
+	info->screen_base = (void __iomem *)VGA_MAP_MEM(VGA_FB_PHYS_BASE, 0);
 
 	if (!info->screen_base) {
 		printk(KERN_ERR "vga16fb: unable to map device\n");
@@ -1385,8 +1382,8 @@ static int vga16fb_probe(struct platform_device *dev)
 
 	vga16fb_update_fix(info);
 
-	info->apertures->ranges[0].base = VGA_FB_PHYS;
-	info->apertures->ranges[0].size = VGA_FB_PHYS_LEN;
+	info->apertures->ranges[0].base = VGA_FB_PHYS_BASE;
+	info->apertures->ranges[0].size = VGA_FB_PHYS_SIZE;
 
 	if (register_framebuffer(info) < 0) {
 		printk(KERN_ERR "vga16fb: unable to register framebuffer\n");
diff --git a/include/video/vga.h b/include/video/vga.h
index ef8e9fa9b9bd..947c0abd04ef 100644
--- a/include/video/vga.h
+++ b/include/video/vga.h
@@ -22,6 +22,8 @@
 #include <asm/vga.h>
 #include <asm/byteorder.h>
 
+#define VGA_FB_PHYS_BASE	0xA0000 /* VGA framebuffer I/O base */
+#define VGA_FB_PHYS_SIZE	65536	/* VGA framebuffer I/O size */
 
 /* Some of the code below is taken from SVGAlib.  The original,
    unmodified copyright notice for that code is below. */
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 09/11] video/aperture: Remove conflicting VGA devices, if any
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (6 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 08/11] video: Provide constants for VGA I/O range Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 10/11] fbdev: Acquire framebuffer apertures for firmware devices Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 11/11] fbdev: Remove conflict-handling code Thomas Zimmermann
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

On the primary graphics adapter, a driver might conflict with a VGA
driver that controls the VGA framebuffer I/O range. Remove the VGA
driver from the aperture helpers. Until now, this case has been
hendled by fbdev, but it should work even with fbdev disabled.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/aperture.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 101e13c2cf41..abc691284a77 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -12,6 +12,8 @@
 #include <linux/types.h>
 #include <linux/vgaarb.h>
 
+#include <video/vga.h>
+
 /**
  * DOC: overview
  *
@@ -300,6 +302,16 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
 	 */
 	sysfb_disable();
 
+	aperture_detach_devices(base, size);
+
+	/*
+	 * If this is the primary adapter, there could be a VGA device
+	 * that consumes the VGA framebuffer I/O range. Remove this device
+	 * as well.
+	 */
+	if (primary)
+		aperture_detach_devices(VGA_FB_PHYS_BASE, VGA_FB_PHYS_SIZE);
+
 #if IS_REACHABLE(CONFIG_FB)
 	a = alloc_apertures(1);
 	if (!a)
@@ -315,8 +327,6 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
 		return ret;
 #endif
 
-	aperture_detach_devices(base, size);
-
 	return 0;
 }
 EXPORT_SYMBOL(aperture_remove_conflicting_devices);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 10/11] fbdev: Acquire framebuffer apertures for firmware devices
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (7 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 09/11] video/aperture: Remove conflicting VGA devices, if any Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  2022-07-18  7:23 ` [PATCH v2 11/11] fbdev: Remove conflict-handling code Thomas Zimmermann
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

When registering a generic framebuffer, automatically acquire ownership
of the framebuffer's I/O range. The device will now be handled by the
aperture helpers. Fbdev-based conflict handling is no longer required.

v2:
	* use fb_ prefix instead of fbm_ (Javier)

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/fbdev/core/fbmem.c | 33 ++++++++++++++++++++++++++++++++
 1 file changed, 33 insertions(+)

diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 2237049327db..928a8afcc5a1 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -13,6 +13,7 @@
 
 #include <linux/module.h>
 
+#include <linux/aperture.h>
 #include <linux/compat.h>
 #include <linux/types.h>
 #include <linux/errno.h>
@@ -1739,6 +1740,32 @@ static void do_unregister_framebuffer(struct fb_info *fb_info)
 	put_fb_info(fb_info);
 }
 
+static int fb_aperture_acquire_for_platform_device(struct fb_info *fb_info)
+{
+	struct apertures_struct *ap = fb_info->apertures;
+	struct device *dev = fb_info->device;
+	struct platform_device *pdev;
+	unsigned int i;
+	int ret;
+
+	if (!ap)
+		return 0;
+
+	if (!dev_is_platform(dev))
+		return 0;
+
+	pdev = to_platform_device(dev);
+
+	for (ret = 0, i = 0; i < ap->count; ++i) {
+		ret = devm_aperture_acquire_for_platform_device(pdev, ap->ranges[i].base,
+								ap->ranges[i].size);
+		if (ret)
+			break;
+	}
+
+	return ret;
+}
+
 /**
  * remove_conflicting_framebuffers - remove firmware-configured framebuffers
  * @a: memory range, users of which are to be removed
@@ -1789,6 +1816,12 @@ register_framebuffer(struct fb_info *fb_info)
 {
 	int ret;
 
+	if (fb_info->flags & FBINFO_MISC_FIRMWARE) {
+		ret = fb_aperture_acquire_for_platform_device(fb_info);
+		if (ret)
+			return ret;
+	}
+
 	mutex_lock(&registration_lock);
 	ret = do_register_framebuffer(fb_info);
 	mutex_unlock(&registration_lock);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [PATCH v2 11/11] fbdev: Remove conflict-handling code
  2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
                   ` (8 preceding siblings ...)
  2022-07-18  7:23 ` [PATCH v2 10/11] fbdev: Acquire framebuffer apertures for firmware devices Thomas Zimmermann
@ 2022-07-18  7:23 ` Thomas Zimmermann
  9 siblings, 0 replies; 20+ messages in thread
From: Thomas Zimmermann @ 2022-07-18  7:23 UTC (permalink / raw)
  To: javierm, deller, daniel, sam, maxime
  Cc: linux-fbdev, linux-staging, dri-devel, Thomas Zimmermann

Remove the call to do_remove_conflicting_framebuffers() from the
framebuffer registration. Aperture helpers take care of removing
conflicting devices. With all ownership information stored in the
aperture datastrcutures, remove remove_conflicting_framebuffers()
entirely.

This change also rectifies DRM generic-framebuffer registration, which
tried to unregister conflicting framebuffers, even though it's entirely
build on top of DRM.

v2:
	* remove internal aperture-overlap helpers, which are
	  now unused

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>
---
 drivers/video/aperture.c         |  21 -----
 drivers/video/fbdev/core/fbmem.c | 136 -------------------------------
 include/linux/fb.h               |   2 -
 3 files changed, 159 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index abc691284a77..9e6bcc03a1a4 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -2,7 +2,6 @@
 
 #include <linux/aperture.h>
 #include <linux/device.h>
-#include <linux/fb.h> /* for old fbdev helpers */
 #include <linux/list.h>
 #include <linux/mutex.h>
 #include <linux/pci.h>
@@ -286,11 +285,6 @@ static void aperture_detach_devices(resource_size_t base, resource_size_t size)
 int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
 					bool primary, const char *name)
 {
-#if IS_REACHABLE(CONFIG_FB)
-	struct apertures_struct *a;
-	int ret;
-#endif
-
 	/*
 	 * If a driver asked to unregister a platform device registered by
 	 * sysfb, then can be assumed that this is a driver for a display
@@ -312,21 +306,6 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
 	if (primary)
 		aperture_detach_devices(VGA_FB_PHYS_BASE, VGA_FB_PHYS_SIZE);
 
-#if IS_REACHABLE(CONFIG_FB)
-	a = alloc_apertures(1);
-	if (!a)
-		return -ENOMEM;
-
-	a->ranges[0].base = base;
-	a->ranges[0].size = size;
-
-	ret = remove_conflicting_framebuffers(a, name, primary);
-	kfree(a);
-
-	if (ret)
-		return ret;
-#endif
-
 	return 0;
 }
 EXPORT_SYMBOL(aperture_remove_conflicting_devices);
diff --git a/drivers/video/fbdev/core/fbmem.c b/drivers/video/fbdev/core/fbmem.c
index 928a8afcc5a1..c80305d34dae 100644
--- a/drivers/video/fbdev/core/fbmem.c
+++ b/drivers/video/fbdev/core/fbmem.c
@@ -1514,102 +1514,6 @@ static int fb_check_foreignness(struct fb_info *fi)
 	return 0;
 }
 
-static bool apertures_overlap(struct aperture *gen, struct aperture *hw)
-{
-	/* is the generic aperture base the same as the HW one */
-	if (gen->base == hw->base)
-		return true;
-	/* is the generic aperture base inside the hw base->hw base+size */
-	if (gen->base > hw->base && gen->base < hw->base + hw->size)
-		return true;
-	return false;
-}
-
-static bool fb_do_apertures_overlap(struct apertures_struct *gena,
-				    struct apertures_struct *hwa)
-{
-	int i, j;
-	if (!hwa || !gena)
-		return false;
-
-	for (i = 0; i < hwa->count; ++i) {
-		struct aperture *h = &hwa->ranges[i];
-		for (j = 0; j < gena->count; ++j) {
-			struct aperture *g = &gena->ranges[j];
-			printk(KERN_DEBUG "checking generic (%llx %llx) vs hw (%llx %llx)\n",
-				(unsigned long long)g->base,
-				(unsigned long long)g->size,
-				(unsigned long long)h->base,
-				(unsigned long long)h->size);
-			if (apertures_overlap(g, h))
-				return true;
-		}
-	}
-
-	return false;
-}
-
-static void do_unregister_framebuffer(struct fb_info *fb_info);
-
-static void do_remove_conflicting_framebuffers(struct apertures_struct *a,
-					       const char *name, bool primary)
-{
-	int i;
-
-restart_removal:
-	/* check all firmware fbs and kick off if the base addr overlaps */
-	for_each_registered_fb(i) {
-		struct apertures_struct *gen_aper;
-		struct device *device;
-
-		if (!(registered_fb[i]->flags & FBINFO_MISC_FIRMWARE))
-			continue;
-
-		gen_aper = registered_fb[i]->apertures;
-		device = registered_fb[i]->device;
-		if (fb_do_apertures_overlap(gen_aper, a) ||
-			(primary && gen_aper && gen_aper->count &&
-			 gen_aper->ranges[0].base == VGA_FB_PHYS_BASE)) {
-
-			printk(KERN_INFO "fb%d: switching to %s from %s\n",
-			       i, name, registered_fb[i]->fix.id);
-
-			/*
-			 * If we kick-out a firmware driver, we also want to remove
-			 * the underlying platform device, such as simple-framebuffer,
-			 * VESA, EFI, etc. A native driver will then be able to
-			 * allocate the memory range.
-			 *
-			 * If it's not a platform device, at least print a warning. A
-			 * fix would add code to remove the device from the system. For
-			 * framebuffers without any Linux device, print a warning as
-			 * well.
-			 */
-			if (!device) {
-				pr_warn("fb%d: no device set\n", i);
-				do_unregister_framebuffer(registered_fb[i]);
-			} else if (dev_is_platform(device)) {
-				/*
-				 * Drop the lock because if the device is unregistered, its
-				 * driver will call to unregister_framebuffer(), that takes
-				 * this lock.
-				 */
-				mutex_unlock(&registration_lock);
-				platform_device_unregister(to_platform_device(device));
-				mutex_lock(&registration_lock);
-			} else {
-				pr_warn("fb%d: cannot remove device\n", i);
-				do_unregister_framebuffer(registered_fb[i]);
-			}
-			/*
-			 * Restart the removal loop now that the device has been
-			 * unregistered and its associated framebuffer gone.
-			 */
-			goto restart_removal;
-		}
-	}
-}
-
 static int do_register_framebuffer(struct fb_info *fb_info)
 {
 	int i;
@@ -1618,10 +1522,6 @@ static int do_register_framebuffer(struct fb_info *fb_info)
 	if (fb_check_foreignness(fb_info))
 		return -ENOSYS;
 
-	do_remove_conflicting_framebuffers(fb_info->apertures,
-					   fb_info->fix.id,
-					   fb_is_primary_device(fb_info));
-
 	if (num_registered_fb == FB_MAX)
 		return -ENXIO;
 
@@ -1766,42 +1666,6 @@ static int fb_aperture_acquire_for_platform_device(struct fb_info *fb_info)
 	return ret;
 }
 
-/**
- * remove_conflicting_framebuffers - remove firmware-configured framebuffers
- * @a: memory range, users of which are to be removed
- * @name: requesting driver name
- * @primary: also kick vga16fb if present
- *
- * This function removes framebuffer devices (initialized by firmware/bootloader)
- * which use memory range described by @a. If @a is NULL all such devices are
- * removed.
- */
-int remove_conflicting_framebuffers(struct apertures_struct *a,
-				    const char *name, bool primary)
-{
-	bool do_free = false;
-
-	if (!a) {
-		a = alloc_apertures(1);
-		if (!a)
-			return -ENOMEM;
-
-		a->ranges[0].base = 0;
-		a->ranges[0].size = ~0;
-		do_free = true;
-	}
-
-	mutex_lock(&registration_lock);
-	do_remove_conflicting_framebuffers(a, name, primary);
-	mutex_unlock(&registration_lock);
-
-	if (do_free)
-		kfree(a);
-
-	return 0;
-}
-EXPORT_SYMBOL(remove_conflicting_framebuffers);
-
 /**
  *	register_framebuffer - registers a frame buffer device
  *	@fb_info: frame buffer info structure
diff --git a/include/linux/fb.h b/include/linux/fb.h
index b91c77016560..453c3b2b6b8e 100644
--- a/include/linux/fb.h
+++ b/include/linux/fb.h
@@ -615,8 +615,6 @@ extern ssize_t fb_sys_write(struct fb_info *info, const char __user *buf,
 /* drivers/video/fbmem.c */
 extern int register_framebuffer(struct fb_info *fb_info);
 extern void unregister_framebuffer(struct fb_info *fb_info);
-extern int remove_conflicting_framebuffers(struct apertures_struct *a,
-					   const char *name, bool primary);
 extern int fb_prepare_logo(struct fb_info *fb_info, int rotate);
 extern int fb_show_logo(struct fb_info *fb_info, int rotate);
 extern char* fb_get_buffer_offset(struct fb_info *info, struct fb_pixmap *buf, u32 size);
-- 
2.36.1


^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 05/11] fbdev: Convert drivers to aperture helpers
  2022-07-18  7:23 ` [PATCH v2 05/11] fbdev: Convert drivers to aperture helpers Thomas Zimmermann
@ 2022-07-18  9:57   ` kernel test robot
  0 siblings, 0 replies; 20+ messages in thread
From: kernel test robot @ 2022-07-18  9:57 UTC (permalink / raw)
  To: Thomas Zimmermann, javierm, deller, daniel, sam, maxime
  Cc: llvm, kbuild-all, linux-fbdev, linux-staging, dri-devel,
	Thomas Zimmermann

Hi Thomas,

I love your patch! Perhaps something to improve:

[auto build test WARNING on ebea934e2651857c9b56cc80bf99460ee18a3592]

url:    https://github.com/intel-lab-lkp/linux/commits/Thomas-Zimmermann/fbdev-Maintain-device-ownership-with-aperture-helpers/20220718-152559
base:   ebea934e2651857c9b56cc80bf99460ee18a3592
config: x86_64-randconfig-a003 (https://download.01.org/0day-ci/archive/20220718/202207181701.mB5xBuhX-lkp@intel.com/config)
compiler: clang version 15.0.0 (https://github.com/llvm/llvm-project d74b88c69dc2644bd0dc5d64e2d7413a0d4040e5)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/64ab1ffe4e3accdef429db81ec645a1cbad540df
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Thomas-Zimmermann/fbdev-Maintain-device-ownership-with-aperture-helpers/20220718-152559
        git checkout 64ab1ffe4e3accdef429db81ec645a1cbad540df
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash drivers/video/fbdev/

If you fix the issue, kindly add following tag where applicable
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/video/fbdev/hyperv_fb.c:1080:26: warning: expression which evaluates to zero treated as a null pointer constant of type 'const char *' [-Wnon-literal-null-conversion]
                                               KBUILD_MODNAME, false);
                                                               ^~~~~
   1 warning generated.


vim +1080 drivers/video/fbdev/hyperv_fb.c

3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09   987  
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29   988  
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29   989  /* Get framebuffer memory from Hyper-V video pci space */
3546448338e76a drivers/video/fbdev/hyperv_fb.c Jake Oshins       2015-08-05   990  static int hvfb_getmem(struct hv_device *hdev, struct fb_info *info)
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29   991  {
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26   992  	struct hvfb_par *par = info->par;
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26   993  	struct pci_dev *pdev  = NULL;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29   994  	void __iomem *fb_virt;
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26   995  	int gen2vm = efi_enabled(EFI_BOOT);
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09   996  	phys_addr_t paddr;
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26   997  	int ret;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29   998  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09   999  	info->apertures = alloc_apertures(1);
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1000  	if (!info->apertures)
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1001  		return -ENOMEM;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1002  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1003  	if (!gen2vm) {
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1004  		pdev = pci_get_device(PCI_VENDOR_ID_MICROSOFT,
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1005  			PCI_DEVICE_ID_HYPERV_VIDEO, NULL);
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1006  		if (!pdev) {
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1007  			pr_err("Unable to find PCI Hyper-V video\n");
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1008  			return -ENODEV;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1009  		}
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1010  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1011  		info->apertures->ranges[0].base = pci_resource_start(pdev, 0);
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1012  		info->apertures->ranges[0].size = pci_resource_len(pdev, 0);
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1013  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1014  		/*
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1015  		 * For Gen 1 VM, we can directly use the contiguous memory
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1016  		 * from VM. If we succeed, deferred IO happens directly
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1017  		 * on this allocated framebuffer memory, avoiding extra
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1018  		 * memory copy.
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1019  		 */
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1020  		paddr = hvfb_get_phymem(hdev, screen_fb_size);
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1021  		if (paddr != (phys_addr_t) -1) {
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1022  			par->mmio_pp = paddr;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1023  			par->mmio_vp = par->dio_vp = __va(paddr);
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1024  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1025  			info->fix.smem_start = paddr;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1026  			info->fix.smem_len = screen_fb_size;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1027  			info->screen_base = par->mmio_vp;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1028  			info->screen_size = screen_fb_size;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1029  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1030  			par->need_docopy = false;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1031  			goto getmem_done;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1032  		}
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1033  		pr_info("Unable to allocate enough contiguous physical memory on Gen 1 VM. Using MMIO instead.\n");
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1034  	} else {
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1035  		info->apertures->ranges[0].base = screen_info.lfb_base;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1036  		info->apertures->ranges[0].size = screen_info.lfb_size;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1037  	}
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1038  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1039  	/*
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1040  	 * Cannot use the contiguous physical memory.
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1041  	 * Allocate mmio space for framebuffer.
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1042  	 */
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1043  	dio_fb_size =
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1044  		screen_width * screen_height * screen_depth / 8;
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1045  
c4b4d7047f16a8 drivers/video/fbdev/hyperv_fb.c Saurabh Sengar    2022-04-27  1046  	ret = vmbus_allocate_mmio(&par->mem, hdev, 0, -1,
3546448338e76a drivers/video/fbdev/hyperv_fb.c Jake Oshins       2015-08-05  1047  				  screen_fb_size, 0x100000, true);
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26  1048  	if (ret != 0) {
3546448338e76a drivers/video/fbdev/hyperv_fb.c Jake Oshins       2015-08-05  1049  		pr_err("Unable to allocate framebuffer memory\n");
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1050  		goto err1;
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26  1051  	}
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1052  
5f1251a48c17b5 drivers/video/fbdev/hyperv_fb.c Dexuan Cui        2020-11-17  1053  	/*
5f1251a48c17b5 drivers/video/fbdev/hyperv_fb.c Dexuan Cui        2020-11-17  1054  	 * Map the VRAM cacheable for performance. This is also required for
5f1251a48c17b5 drivers/video/fbdev/hyperv_fb.c Dexuan Cui        2020-11-17  1055  	 * VM Connect to display properly for ARM64 Linux VM, as the host also
5f1251a48c17b5 drivers/video/fbdev/hyperv_fb.c Dexuan Cui        2020-11-17  1056  	 * maps the VRAM cacheable.
5f1251a48c17b5 drivers/video/fbdev/hyperv_fb.c Dexuan Cui        2020-11-17  1057  	 */
5f1251a48c17b5 drivers/video/fbdev/hyperv_fb.c Dexuan Cui        2020-11-17  1058  	fb_virt = ioremap_cache(par->mem->start, screen_fb_size);
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1059  	if (!fb_virt)
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1060  		goto err2;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1061  
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1062  	/* Allocate memory for deferred IO */
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1063  	par->dio_vp = vzalloc(round_up(dio_fb_size, PAGE_SIZE));
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1064  	if (par->dio_vp == NULL)
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1065  		goto err3;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1066  
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1067  	/* Physical address of FB device */
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1068  	par->mmio_pp = par->mem->start;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1069  	/* Virtual address of FB device */
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1070  	par->mmio_vp = (unsigned char *) fb_virt;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1071  
3546448338e76a drivers/video/fbdev/hyperv_fb.c Jake Oshins       2015-08-05  1072  	info->fix.smem_start = par->mem->start;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1073  	info->fix.smem_len = dio_fb_size;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1074  	info->screen_base = par->dio_vp;
d21987d709e807 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-09-18  1075  	info->screen_size = dio_fb_size;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1076  
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09  1077  getmem_done:
64ab1ffe4e3acc drivers/video/fbdev/hyperv_fb.c Thomas Zimmermann 2022-07-18  1078  	aperture_remove_conflicting_devices(info->apertures->ranges[0].base,
64ab1ffe4e3acc drivers/video/fbdev/hyperv_fb.c Thomas Zimmermann 2022-07-18  1079  					    info->apertures->ranges[0].size,
3a6fb6c4255c38 drivers/video/fbdev/hyperv_fb.c Wei Hu            2019-12-09 @1080  					    KBUILD_MODNAME, false);
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1081  
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1082  	if (gen2vm) {
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1083  		/* framebuffer is reallocated, clear screen_info to avoid misuse from kexec */
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1084  		screen_info.lfb_size = 0;
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1085  		screen_info.lfb_base = 0;
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1086  		screen_info.orig_video_isVGA = 0;
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1087  	} else {
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1088  		pci_dev_put(pdev);
3cb73bc3fa2a3c drivers/video/fbdev/hyperv_fb.c Kairui Song       2020-10-14  1089  	}
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26  1090  
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1091  	return 0;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1092  
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1093  err3:
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1094  	iounmap(fb_virt);
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1095  err2:
696ca5e82c057a drivers/video/fbdev/hyperv_fb.c Jake Oshins       2016-04-05  1096  	vmbus_free_mmio(par->mem->start, screen_fb_size);
3546448338e76a drivers/video/fbdev/hyperv_fb.c Jake Oshins       2015-08-05  1097  	par->mem = NULL;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1098  err1:
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26  1099  	if (!gen2vm)
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1100  		pci_dev_put(pdev);
9069fd54960304 drivers/video/hyperv_fb.c       Gerd Hoffmann     2014-02-26  1101  
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1102  	return -ENOMEM;
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1103  }
68a2d20b79b105 drivers/video/hyperv_fb.c       Haiyang Zhang     2013-04-29  1104  

-- 
0-DAY CI Kernel Test Service
https://01.org/lkp

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 06/11] fbdev: Remove conflicting devices on PCI bus
  2022-07-18  7:23 ` [PATCH v2 06/11] fbdev: Remove conflicting devices on PCI bus Thomas Zimmermann
@ 2022-07-21 14:17   ` Geert Uytterhoeven
  0 siblings, 0 replies; 20+ messages in thread
From: Geert Uytterhoeven @ 2022-07-21 14:17 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: Javier Martinez Canillas, Helge Deller, Daniel Vetter,
	Sam Ravnborg, Maxime Ripard, Linux Fbdev development list,
	linux-staging, DRI Development

Hi Thomas,

On Mon, Jul 18, 2022 at 9:24 AM Thomas Zimmermann <tzimmermann@suse.de> wrote:
> Remove firmware devices on the PCI bus, by calling
> aperture_remove_conflicting_pci_devices() in the probe function of
> each related fbdev driver. iSo far, most of these drivers did not
> remove conflicting VESA or EFI devices, or outride failed for
> resource conflicts (i.e., matroxfb.) This must have been broken
> for quite some time.
>
> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
> Reviewed-by: Javier Martinez Canillas <javierm@redhat.com>

Thanks for your patch!

> --- a/drivers/video/fbdev/tgafb.c
> +++ b/drivers/video/fbdev/tgafb.c
> @@ -12,6 +12,7 @@
>   *  more details.
>   */
>
> +#include <linux/aperture.h>
>  #include <linux/bitrev.h>
>  #include <linux/compiler.h>
>  #include <linux/delay.h>
> @@ -106,6 +107,12 @@ static struct pci_driver tgafb_pci_driver = {
>  static int tgafb_pci_register(struct pci_dev *pdev,
>                               const struct pci_device_id *ent)
>  {
> +       int ret;
> +
> +       ret = aperture_remove_conflicting_pci_devices(pdev, "tgafb");
> +       if (ret)
> +               return ret;
> +
>         return tgafb_register(&pdev->dev);
>  }

I am wondering which driver could possibly conflict with TGA?

Probably there are a few other drivers in the same category (tdfxfb?).

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2022-07-18  7:23 ` [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers Thomas Zimmermann
@ 2023-03-20  1:47   ` Samuel Čavoj
  2023-03-20  9:46     ` Thomas Zimmermann
  0 siblings, 1 reply; 20+ messages in thread
From: Samuel Čavoj @ 2023-03-20  1:47 UTC (permalink / raw)
  To: Thomas Zimmermann
  Cc: javierm, deller, daniel, sam, maxime, linux-fbdev, linux-staging,
	dri-devel, Thomas Zimmermann, Zack Rusin, Daniel Vetter,
	Alex Deucher, Zhen Lei, Changcheng Deng, Maarten Lankhorst,
	Maxime Ripard

Hi,

> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> index f42a0d8bc211..101e13c2cf41 100644
> --- a/drivers/video/aperture.c
> +++ b/drivers/video/aperture.c
> @@ -8,6 +8,7 @@
>  #include <linux/pci.h>
>  #include <linux/platform_device.h>
>  #include <linux/slab.h>
> +#include <linux/sysfb.h>
>  #include <linux/types.h>
>  #include <linux/vgaarb.h>
> 
> @@ -286,7 +287,20 @@ int 
> aperture_remove_conflicting_devices(resource_size_t base, 
> resource_size_t si
>  #if IS_REACHABLE(CONFIG_FB)
>  	struct apertures_struct *a;
>  	int ret;
> +#endif
> +
> +	/*
> +	 * If a driver asked to unregister a platform device registered by
> +	 * sysfb, then can be assumed that this is a driver for a display
> +	 * that is set up by the system firmware and has a generic driver.
> +	 *
> +	 * Drivers for devices that don't have a generic driver will never
> +	 * ask for this, so let's assume that a real driver for the display
> +	 * was already probed and prevent sysfb to register devices later.
> +	 */
> +	sysfb_disable();

This call to sysfb_disable() has been causing trouble with regard to
VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices to
get rid of any console drivers (d173780620792c) using the device in
question, but now even unrelated drivers are getting killed. Example
situation:

Machine has two GPUs and uses efifb for the console. Efifb registers
with the aperture system the efi framebuffer region, which is covered
by a BAR resource of GPU 1. VFIO grabs GPU 2 and calls
aperture_remove_conflicting_pci_devices(GPU 2). GPU 2 has no overlap
with the efifb on GPU1 but the efifb is killed regardless due to
the unconditional call to sysfb_disable(). The console switches
to dummy and locks up from the user perspective.
This seems unnecessary, as the device is unrelated.

I do not quite understand the comment justifying the call.

Some discussions with workarounds:
https://old.reddit.com/r/VFIO/comments/11qei4t/framebuffer_doesnt_work_anymore_after_passthrough/
https://bbs.archlinux.org/viewtopic.php?id=280512


Thanks,
Samuel

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2023-03-20  1:47   ` Samuel Čavoj
@ 2023-03-20  9:46     ` Thomas Zimmermann
  2023-03-20 10:13       ` Javier Martinez Canillas
  0 siblings, 1 reply; 20+ messages in thread
From: Thomas Zimmermann @ 2023-03-20  9:46 UTC (permalink / raw)
  To: Samuel Čavoj
  Cc: javierm, deller, daniel, sam, maxime, linux-fbdev, linux-staging,
	dri-devel, Zack Rusin, Daniel Vetter, Alex Deucher, Zhen Lei,
	Changcheng Deng, Maarten Lankhorst, Maxime Ripard


[-- Attachment #1.1: Type: text/plain, Size: 2613 bytes --]

Hi

Am 20.03.23 um 02:47 schrieb Samuel Čavoj:
> Hi,
> 
>> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
>> index f42a0d8bc211..101e13c2cf41 100644
>> --- a/drivers/video/aperture.c
>> +++ b/drivers/video/aperture.c
>> @@ -8,6 +8,7 @@
>>  #include <linux/pci.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/slab.h>
>> +#include <linux/sysfb.h>
>>  #include <linux/types.h>
>>  #include <linux/vgaarb.h>
>>
>> @@ -286,7 +287,20 @@ int 
>> aperture_remove_conflicting_devices(resource_size_t base, 
>> resource_size_t si
>>  #if IS_REACHABLE(CONFIG_FB)
>>      struct apertures_struct *a;
>>      int ret;
>> +#endif
>> +
>> +    /*
>> +     * If a driver asked to unregister a platform device registered by
>> +     * sysfb, then can be assumed that this is a driver for a display
>> +     * that is set up by the system firmware and has a generic driver.
>> +     *
>> +     * Drivers for devices that don't have a generic driver will never
>> +     * ask for this, so let's assume that a real driver for the display
>> +     * was already probed and prevent sysfb to register devices later.
>> +     */
>> +    sysfb_disable();
> 
> This call to sysfb_disable() has been causing trouble with regard to
> VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices to
> get rid of any console drivers (d173780620792c) using the device in
> question, but now even unrelated drivers are getting killed. Example
> situation:

Which drivers do you use?

Best regards
Thomas

> 
> Machine has two GPUs and uses efifb for the console. Efifb registers
> with the aperture system the efi framebuffer region, which is covered
> by a BAR resource of GPU 1. VFIO grabs GPU 2 and calls
> aperture_remove_conflicting_pci_devices(GPU 2). GPU 2 has no overlap
> with the efifb on GPU1 but the efifb is killed regardless due to
> the unconditional call to sysfb_disable(). The console switches
> to dummy and locks up from the user perspective.
> This seems unnecessary, as the device is unrelated.
> 
> I do not quite understand the comment justifying the call.
> 
> Some discussions with workarounds:
> https://old.reddit.com/r/VFIO/comments/11qei4t/framebuffer_doesnt_work_anymore_after_passthrough/
> https://bbs.archlinux.org/viewtopic.php?id=280512
> 
> 
> Thanks,
> Samuel

-- 
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 #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2023-03-20  9:46     ` Thomas Zimmermann
@ 2023-03-20 10:13       ` Javier Martinez Canillas
  2023-03-20 11:08         ` Samuel Čavoj
  0 siblings, 1 reply; 20+ messages in thread
From: Javier Martinez Canillas @ 2023-03-20 10:13 UTC (permalink / raw)
  To: Thomas Zimmermann, Samuel Čavoj
  Cc: deller, daniel, sam, maxime, linux-fbdev, linux-staging,
	dri-devel, Zack Rusin, Daniel Vetter, Alex Deucher, Zhen Lei,
	Changcheng Deng, Maarten Lankhorst, Maxime Ripard

Thomas Zimmermann <tzimmermann@suse.de> writes:

[...]

>>> +    /*
>>> +     * If a driver asked to unregister a platform device registered by
>>> +     * sysfb, then can be assumed that this is a driver for a display
>>> +     * that is set up by the system firmware and has a generic driver.
>>> +     *
>>> +     * Drivers for devices that don't have a generic driver will never
>>> +     * ask for this, so let's assume that a real driver for the display
>>> +     * was already probed and prevent sysfb to register devices later.
>>> +     */
>>> +    sysfb_disable();
>> 
>> This call to sysfb_disable() has been causing trouble with regard to
>> VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices to
>> get rid of any console drivers (d173780620792c) using the device in
>> question, but now even unrelated drivers are getting killed. Example
>> situation:
>
> Which drivers do you use?
>

Also, what kernel version?

[...]

>> 
>> Machine has two GPUs and uses efifb for the console. Efifb registers
>> with the aperture system the efi framebuffer region, which is covered
>> by a BAR resource of GPU 1. VFIO grabs GPU 2 and calls
>> aperture_remove_conflicting_pci_devices(GPU 2). GPU 2 has no overlap
>> with the efifb on GPU1 but the efifb is killed regardless due to
>> the unconditional call to sysfb_disable(). The console switches
>> to dummy and locks up from the user perspective.
>> This seems unnecessary, as the device is unrelated.
>> 

That's a bug indeed but I thought that was already fixed...

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2023-03-20 10:13       ` Javier Martinez Canillas
@ 2023-03-20 11:08         ` Samuel Čavoj
  2023-03-20 12:12           ` Javier Martinez Canillas
  0 siblings, 1 reply; 20+ messages in thread
From: Samuel Čavoj @ 2023-03-20 11:08 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Thomas Zimmermann, deller, daniel, sam, maxime, linux-fbdev,
	linux-staging, dri-devel, Zack Rusin, Daniel Vetter,
	Alex Deucher, Zhen Lei, Changcheng Deng, Maarten Lankhorst,
	Maxime Ripard

On 2023-03-20 11:13, Javier Martinez Canillas wrote:
> Thomas Zimmermann <tzimmermann@suse.de> writes:
> 
> [...]
> 
>>>> +    /*
>>>> +     * If a driver asked to unregister a platform device registered 
>>>> by
>>>> +     * sysfb, then can be assumed that this is a driver for a 
>>>> display
>>>> +     * that is set up by the system firmware and has a generic 
>>>> driver.
>>>> +     *
>>>> +     * Drivers for devices that don't have a generic driver will 
>>>> never
>>>> +     * ask for this, so let's assume that a real driver for the 
>>>> display
>>>> +     * was already probed and prevent sysfb to register devices 
>>>> later.
>>>> +     */
>>>> +    sysfb_disable();
>>> 
>>> This call to sysfb_disable() has been causing trouble with regard to
>>> VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices 
>>> to
>>> get rid of any console drivers (d173780620792c) using the device in
>>> question, but now even unrelated drivers are getting killed. Example
>>> situation:
>> 
>> Which drivers do you use?

This happens with either no drivers loaded or the proprietary nvidia
driver. Nouveau is fine as it doesn't rely on efifb but brings its own.

>> 
> 
> Also, what kernel version?

I tried with 6.2.6, can build mainline and test there as well.

Thanks for help!

> 
> [...]
> 
>>> 
>>> Machine has two GPUs and uses efifb for the console. Efifb registers
>>> with the aperture system the efi framebuffer region, which is covered
>>> by a BAR resource of GPU 1. VFIO grabs GPU 2 and calls
>>> aperture_remove_conflicting_pci_devices(GPU 2). GPU 2 has no overlap
>>> with the efifb on GPU1 but the efifb is killed regardless due to
>>> the unconditional call to sysfb_disable(). The console switches
>>> to dummy and locks up from the user perspective.
>>> This seems unnecessary, as the device is unrelated.
>>> 
> 
> That's a bug indeed but I thought that was already fixed...

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2023-03-20 11:08         ` Samuel Čavoj
@ 2023-03-20 12:12           ` Javier Martinez Canillas
  2023-03-28 15:19             ` Samuel Čavoj
  0 siblings, 1 reply; 20+ messages in thread
From: Javier Martinez Canillas @ 2023-03-20 12:12 UTC (permalink / raw)
  To: Samuel Čavoj
  Cc: Thomas Zimmermann, deller, daniel, sam, maxime, linux-fbdev,
	linux-staging, dri-devel, Zack Rusin, Daniel Vetter,
	Alex Deucher, Zhen Lei, Changcheng Deng, Maarten Lankhorst,
	Maxime Ripard

Samuel Čavoj <samuel@cavoj.net> writes:

[...]

>>>> This call to sysfb_disable() has been causing trouble with regard to
>>>> VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices 
>>>> to
>>>> get rid of any console drivers (d173780620792c) using the device in
>>>> question, but now even unrelated drivers are getting killed. Example
>>>> situation:
>>> 
>>> Which drivers do you use?
>
> This happens with either no drivers loaded or the proprietary nvidia
> driver. Nouveau is fine as it doesn't rely on efifb but brings its own.
>

Which is what all DRM drivers should do. If they want to make sure that a
fbdev will be present after the DRM driver probes, then should register an
emulated fbdev.

There was an attempt to workaround that in [0], in particular patch [1]
but that effort was not continued since the only DRM driver that would be
affected is the Nvidia proprietary driver that relies on efifb/simpledrm
to have a VT.

[0]: https://patchwork.kernel.org/project/dri-devel/list/?series=711019&archive=both
[1]: https://patchwork.kernel.org/project/dri-devel/patch/20230111154112.90575-11-daniel.vetter@ffwll.ch/

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2023-03-20 12:12           ` Javier Martinez Canillas
@ 2023-03-28 15:19             ` Samuel Čavoj
  2023-04-04 11:36               ` Javier Martinez Canillas
  0 siblings, 1 reply; 20+ messages in thread
From: Samuel Čavoj @ 2023-03-28 15:19 UTC (permalink / raw)
  To: Javier Martinez Canillas
  Cc: Thomas Zimmermann, deller, daniel, sam, maxime, linux-fbdev,
	linux-staging, dri-devel, Zack Rusin, Daniel Vetter,
	Alex Deucher, Zhen Lei, Changcheng Deng, Maarten Lankhorst,
	Maxime Ripard

On 2023-03-20 13:12, Javier Martinez Canillas wrote:
> Samuel Čavoj <samuel@cavoj.net> writes:
> 
> [...]
> 
>>>>> This call to sysfb_disable() has been causing trouble with regard 
>>>>> to
>>>>> VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices
>>>>> to
>>>>> get rid of any console drivers (d173780620792c) using the device in
>>>>> question, but now even unrelated drivers are getting killed. 
>>>>> Example
>>>>> situation:
>>>> 
>>>> Which drivers do you use?
>> 
>> This happens with either no drivers loaded or the proprietary nvidia
>> driver. Nouveau is fine as it doesn't rely on efifb but brings its 
>> own.
>> 
> 
> Which is what all DRM drivers should do. If they want to make sure that 
> a
> fbdev will be present after the DRM driver probes, then should register 
> an
> emulated fbdev.

I don't see how this is specific to Nvidia or DRM drivers.

The efifb is killed if vfio-pci (or another driver which uses the
aperture system to remove conflicting drivers) is bound to ANY pci
device, regardless of whether it's nvidia's fault for not implementing
a framebuffer. Fair enough, I agree that they should, but
I for one expect my efifb to not die at a random time
when a random unrelated driver does a random thing with another
unrelated GPU.

Or is the efifb considered a stop-gap solution the only purpose of
which is early boot--before another GPU driver is loaded?

> 
> There was an attempt to workaround that in [0], in particular patch [1]
> but that effort was not continued since the only DRM driver that would 
> be
> affected is the Nvidia proprietary driver that relies on 
> efifb/simpledrm
> to have a VT.
> 
> [0]: 
> https://patchwork.kernel.org/project/dri-devel/list/?series=711019&archive=both
> [1]: 
> https://patchwork.kernel.org/project/dri-devel/patch/20230111154112.90575-11-daniel.vetter@ffwll.ch/

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers
  2023-03-28 15:19             ` Samuel Čavoj
@ 2023-04-04 11:36               ` Javier Martinez Canillas
  0 siblings, 0 replies; 20+ messages in thread
From: Javier Martinez Canillas @ 2023-04-04 11:36 UTC (permalink / raw)
  To: Samuel Čavoj
  Cc: Thomas Zimmermann, deller, daniel, sam, maxime, linux-fbdev,
	linux-staging, dri-devel, Zack Rusin, Daniel Vetter,
	Alex Deucher, Zhen Lei, Changcheng Deng, Maarten Lankhorst,
	Maxime Ripard

Samuel Čavoj <samuel@cavoj.net> writes:

Hello Samuel,

> On 2023-03-20 13:12, Javier Martinez Canillas wrote:
>> Samuel Čavoj <samuel@cavoj.net> writes:
>> 
>> [...]
>> 
>>>>>> This call to sysfb_disable() has been causing trouble with regard 
>>>>>> to
>>>>>> VFIO. VFIO has been calling aperture_remove_conflicting_pci_devices
>>>>>> to
>>>>>> get rid of any console drivers (d173780620792c) using the device in
>>>>>> question, but now even unrelated drivers are getting killed. 
>>>>>> Example
>>>>>> situation:
>>>>> 
>>>>> Which drivers do you use?
>>> 
>>> This happens with either no drivers loaded or the proprietary nvidia
>>> driver. Nouveau is fine as it doesn't rely on efifb but brings its 
>>> own.
>>> 
>> 
>> Which is what all DRM drivers should do. If they want to make sure that 
>> a
>> fbdev will be present after the DRM driver probes, then should register 
>> an
>> emulated fbdev.
>
> I don't see how this is specific to Nvidia or DRM drivers.
>

Not specific to Nvidia per se but as mentioned it only affected Nvidia due
that driver relying on a different graphics driver to get a VT console.

> The efifb is killed if vfio-pci (or another driver which uses the
> aperture system to remove conflicting drivers) is bound to ANY pci
> device, regardless of whether it's nvidia's fault for not implementing
> a framebuffer. Fair enough, I agree that they should, but
> I for one expect my efifb to not die at a random time
> when a random unrelated driver does a random thing with another
> unrelated GPU.
>

There was a patch series to address that:

https://patchwork.kernel.org/project/dri-devel/list/?series=711019&archive=both

In particular, this patch:

https://patchwork.kernel.org/project/dri-devel/patch/20230111154112.90575-11-daniel.vetter@ffwll.ch/

> Or is the efifb considered a stop-gap solution the only purpose of
> which is early boot--before another GPU driver is loaded?
>

All the firmware-provided graphics drivers are really a best effort IMO,
that is something only to be used to get early video output and any in the
case of "nomodeset" (i.e: some distros have a "Safe graphics mode" boot
entry that prevents DRM drivers to be loaded but used for troubleshooting.

But as soon as a real DRM driver is probed (either in the host or a guest
when the device is passed-through), I believe that is very likely that it
won't work anymore. In other words, is not a robust way to get output and
is just a best effort.

-- 
Best regards,

Javier Martinez Canillas
Core Platforms
Red Hat


^ permalink raw reply	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2023-04-04 11:38 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-18  7:23 [PATCH v2 00/11] fbdev: Maintain device ownership with aperture helpers Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 02/11] fbdev/vga16fb: Create EGA/VGA devices in sysfb code Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 03/11] fbdev/vga16fb: Auto-generate module init/exit code Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 04/11] fbdev/core: Remove remove_conflicting_pci_framebuffers() Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 05/11] fbdev: Convert drivers to aperture helpers Thomas Zimmermann
2022-07-18  9:57   ` kernel test robot
2022-07-18  7:23 ` [PATCH v2 06/11] fbdev: Remove conflicting devices on PCI bus Thomas Zimmermann
2022-07-21 14:17   ` Geert Uytterhoeven
2022-07-18  7:23 ` [PATCH v2 07/11] video/aperture: Disable and unregister sysfb devices via aperture helpers Thomas Zimmermann
2023-03-20  1:47   ` Samuel Čavoj
2023-03-20  9:46     ` Thomas Zimmermann
2023-03-20 10:13       ` Javier Martinez Canillas
2023-03-20 11:08         ` Samuel Čavoj
2023-03-20 12:12           ` Javier Martinez Canillas
2023-03-28 15:19             ` Samuel Čavoj
2023-04-04 11:36               ` Javier Martinez Canillas
2022-07-18  7:23 ` [PATCH v2 08/11] video: Provide constants for VGA I/O range Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 09/11] video/aperture: Remove conflicting VGA devices, if any Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 10/11] fbdev: Acquire framebuffer apertures for firmware devices Thomas Zimmermann
2022-07-18  7:23 ` [PATCH v2 11/11] fbdev: Remove conflict-handling code Thomas Zimmermann

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).