[PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Daniel Vetter]

It's just open coded and matches.

Note that Thomas said that his version apparently failed for some
reason, but hey maybe we should try again.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/gpu/drm/ast/ast_drv.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
index 420fc75c240e..3ac24a780f50 100644
--- a/drivers/gpu/drm/ast/ast_drv.c
+++ b/drivers/gpu/drm/ast/ast_drv.c
@@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] = {
 
 MODULE_DEVICE_TABLE(pci, ast_pciidlist);
 
-static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
-{
-	bool primary = false;
-	resource_size_t base, size;
-
-	base = pci_resource_start(pdev, 0);
-	size = pci_resource_len(pdev, 0);
-#ifdef CONFIG_X86
-	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
-#endif
-
-	return drm_aperture_remove_conflicting_framebuffers(base, size, primary, &ast_driver);
-}
-
 static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 {
 	struct ast_private *ast;
 	struct drm_device *dev;
 	int ret;
 
-	ret = ast_remove_conflicting_framebuffers(pdev);
+	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &ast_driver);
 	if (ret)
 		return ret;
 
-- 
2.39.0

[PATCH 03/11] drm/aperture: Remove primary argument

[Daniel Vetter]

Only really pci devices have a business setting this - it's for
figuring out whether the legacy vga stuff should be nuked too. And
with the preceeding two patches those are all using the pci version of
this.

Which means for all other callers primary == false and we can remove
it now.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Maxime Ripard <mripard@kernel.org>
Cc: Deepak Rawat <drawat.floss@gmail.com>
Cc: Neil Armstrong <neil.armstrong@linaro.org>
Cc: Kevin Hilman <khilman@baylibre.com>
Cc: Jerome Brunet <jbrunet@baylibre.com>
Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
Cc: Thierry Reding <thierry.reding@gmail.com>
Cc: Jonathan Hunter <jonathanh@nvidia.com>
Cc: Emma Anholt <emma@anholt.net>
Cc: Helge Deller <deller@gmx.de>
Cc: David Airlie <airlied@gmail.com>
Cc: Daniel Vetter <daniel@ffwll.ch>
Cc: linux-hyperv@vger.kernel.org
Cc: linux-amlogic@lists.infradead.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-tegra@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
---
 drivers/gpu/drm/arm/hdlcd_drv.c             |  2 +-
 drivers/gpu/drm/armada/armada_drv.c         |  2 +-
 drivers/gpu/drm/drm_aperture.c              | 11 +++--------
 drivers/gpu/drm/hyperv/hyperv_drm_drv.c     |  1 -
 drivers/gpu/drm/meson/meson_drv.c           |  2 +-
 drivers/gpu/drm/msm/msm_fbdev.c             |  2 +-
 drivers/gpu/drm/rockchip/rockchip_drm_drv.c |  2 +-
 drivers/gpu/drm/stm/drv.c                   |  2 +-
 drivers/gpu/drm/sun4i/sun4i_drv.c           |  2 +-
 drivers/gpu/drm/tegra/drm.c                 |  2 +-
 drivers/gpu/drm/vc4/vc4_drv.c               |  2 +-
 include/drm/drm_aperture.h                  |  7 +++----
 12 files changed, 15 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index 7043d1c9ed8f..98267e355918 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -297,7 +297,7 @@ static int hdlcd_drm_bind(struct device *dev)
 	 */
 	if (hdlcd_read(hdlcd, HDLCD_REG_COMMAND)) {
 		hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
-		drm_aperture_remove_framebuffers(false, &hdlcd_driver);
+		drm_aperture_remove_framebuffers(&hdlcd_driver);
 	}
 
 	drm_mode_config_reset(drm);
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 0643887800b4..c99ec7078301 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -95,7 +95,7 @@ static int armada_drm_bind(struct device *dev)
 	}
 
 	/* Remove early framebuffers */
-	ret = drm_aperture_remove_framebuffers(false, &armada_drm_driver);
+	ret = drm_aperture_remove_framebuffers(&armada_drm_driver);
 	if (ret) {
 		dev_err(dev, "[" DRM_NAME ":%s] can't kick out simple-fb: %d\n",
 			__func__, ret);
diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 3b8fdeeafd53..697cffbfd603 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -32,17 +32,13 @@
  *
  *	static int remove_conflicting_framebuffers(struct pci_dev *pdev)
  *	{
- *		bool primary = false;
  *		resource_size_t base, size;
  *		int ret;
  *
  *		base = pci_resource_start(pdev, 0);
  *		size = pci_resource_len(pdev, 0);
- *	#ifdef CONFIG_X86
- *		primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
- *	#endif
  *
- *		return drm_aperture_remove_conflicting_framebuffers(base, size, primary,
+ *		return drm_aperture_remove_conflicting_framebuffers(base, size,
  *		                                                    &example_driver);
  *	}
  *
@@ -161,7 +157,6 @@ EXPORT_SYMBOL(devm_aperture_acquire_from_firmware);
  * drm_aperture_remove_conflicting_framebuffers - remove existing framebuffers in the given range
  * @base: the aperture's base address in physical memory
  * @size: aperture size in bytes
- * @primary: also kick vga16fb if present
  * @req_driver: requesting DRM driver
  *
  * This function removes graphics device drivers which use the memory range described by
@@ -171,9 +166,9 @@ EXPORT_SYMBOL(devm_aperture_acquire_from_firmware);
  * 0 on success, or a negative errno code otherwise
  */
 int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
-						 bool primary, const struct drm_driver *req_driver)
+						 const struct drm_driver *req_driver)
 {
-	return aperture_remove_conflicting_devices(base, size, primary, req_driver->name);
+	return aperture_remove_conflicting_devices(base, size, false, req_driver->name);
 }
 EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
 
diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
index 427c20ba3404..7e81d58c083f 100644
--- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
+++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
@@ -74,7 +74,6 @@ static int hyperv_setup_vram(struct hyperv_drm_device *hv,
 
 	drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
 						     screen_info.lfb_size,
-						     false,
 						     &hyperv_driver);
 
 	hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024;
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 79bfe3938d3c..c8d39809d897 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -285,7 +285,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 	 * Remove early framebuffers (ie. simplefb). The framebuffer can be
 	 * located anywhere in RAM
 	 */
-	ret = drm_aperture_remove_framebuffers(false, &meson_driver);
+	ret = drm_aperture_remove_framebuffers(&meson_driver);
 	if (ret)
 		goto free_drm;
 
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 31e1e30cb52a..84dfbccb6912 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -155,7 +155,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 	}
 
 	/* the fw fb could be anywhere in memory */
-	ret = drm_aperture_remove_framebuffers(false, dev->driver);
+	ret = drm_aperture_remove_framebuffers(dev->driver);
 	if (ret)
 		goto fini;
 
diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
index 6e0788d14c10..d97f2edc646b 100644
--- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
+++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
@@ -140,7 +140,7 @@ static int rockchip_drm_bind(struct device *dev)
 	int ret;
 
 	/* Remove existing drivers that may own the framebuffer memory. */
-	ret = drm_aperture_remove_framebuffers(false, &rockchip_drm_driver);
+	ret = drm_aperture_remove_framebuffers(&rockchip_drm_driver);
 	if (ret) {
 		DRM_DEV_ERROR(dev,
 			      "Failed to remove existing framebuffers - %d.\n",
diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
index 50410bd99dfe..354349c6e085 100644
--- a/drivers/gpu/drm/stm/drv.c
+++ b/drivers/gpu/drm/stm/drv.c
@@ -185,7 +185,7 @@ static int stm_drm_platform_probe(struct platform_device *pdev)
 
 	DRM_DEBUG("%s\n", __func__);
 
-	ret = drm_aperture_remove_framebuffers(false, &drv_driver);
+	ret = drm_aperture_remove_framebuffers(&drv_driver);
 	if (ret)
 		return ret;
 
diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
index cc94efbbf2d4..6367b89cbab1 100644
--- a/drivers/gpu/drm/sun4i/sun4i_drv.c
+++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
@@ -98,7 +98,7 @@ static int sun4i_drv_bind(struct device *dev)
 		goto cleanup_mode_config;
 
 	/* Remove early framebuffers (ie. simplefb) */
-	ret = drm_aperture_remove_framebuffers(false, &sun4i_drv_driver);
+	ret = drm_aperture_remove_framebuffers(&sun4i_drv_driver);
 	if (ret)
 		goto cleanup_mode_config;
 
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 7bd2e65c2a16..d2ff527cf6d7 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -1252,7 +1252,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
 
 	drm_mode_config_reset(drm);
 
-	err = drm_aperture_remove_framebuffers(false, &tegra_drm_driver);
+	err = drm_aperture_remove_framebuffers(&tegra_drm_driver);
 	if (err < 0)
 		goto hub;
 
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index 0ccaee57fe9a..0a9e922636b1 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -350,7 +350,7 @@ static int vc4_drm_bind(struct device *dev)
 			return -EPROBE_DEFER;
 	}
 
-	ret = drm_aperture_remove_framebuffers(false, driver);
+	ret = drm_aperture_remove_framebuffers(driver);
 	if (ret)
 		return ret;
 
diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
index 7096703c3949..cbe33b49fd5d 100644
--- a/include/drm/drm_aperture.h
+++ b/include/drm/drm_aperture.h
@@ -13,14 +13,13 @@ int devm_aperture_acquire_from_firmware(struct drm_device *dev, resource_size_t
 					resource_size_t size);
 
 int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
-						 bool primary, const struct drm_driver *req_driver);
+						 const struct drm_driver *req_driver);
 
 int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
 						     const struct drm_driver *req_driver);
 
 /**
  * drm_aperture_remove_framebuffers - remove all existing framebuffers
- * @primary: also kick vga16fb if present
  * @req_driver: requesting DRM driver
  *
  * This function removes all graphics device drivers. Use this function on systems
@@ -30,9 +29,9 @@ int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
  * 0 on success, or a negative errno code otherwise
  */
 static inline int
-drm_aperture_remove_framebuffers(bool primary, const struct drm_driver *req_driver)
+drm_aperture_remove_framebuffers(const struct drm_driver *req_driver)
 {
-	return drm_aperture_remove_conflicting_framebuffers(0, (resource_size_t)-1, primary,
+	return drm_aperture_remove_conflicting_framebuffers(0, (resource_size_t)-1,
 							    req_driver);
 }
 
-- 
2.39.0

[PATCH 04/11] video/aperture: use generic code to figure out the vga default device

[Daniel Vetter]

Since vgaarb has been promoted to be a core piece of the pci subsystem
we don't have to open code random guesses anymore, we actually know
this in a platform agnostic way, and there's no need for an x86
specific hack. See also 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to
drivers/pci")

This should not result in any functional change, and the non-x86
multi-gpu pci systems are probably rare enough to not matter (I don't
know of any tbh). But it's a nice cleanup, so let's do it.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: linux-pci@vger.kernel.org
---
 drivers/video/aperture.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 41e77de1ea82..3d8c925c7365 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -324,13 +324,11 @@ EXPORT_SYMBOL(aperture_remove_conflicting_devices);
  */
 int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name)
 {
-	bool primary = false;
+	bool primary;
 	resource_size_t base, size;
 	int bar, ret;
 
-#ifdef CONFIG_X86
-	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
-#endif
+	primary = pdev == vga_default_device();
 
 	for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
 		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
-- 
2.39.0

[PATCH 05/11] video/aperture: Only kick vgacon when the pdev is decoding vga

[Daniel Vetter]

Otherwise it's bit silly, and we might throw out the driver for the
screen the user is actually looking at. I haven't found a bug report
for this case yet, but we did get bug reports for the analog case
where we're throwing out the efifb driver.

References: https://bugzilla.kernel.org/show_bug.cgi?id=216303
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/video/aperture.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 3d8c925c7365..6f351a58f6c6 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -341,6 +341,9 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
 			return ret;
 	}
 
+	if (!primary)
+		return 0;
+
 	/*
 	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over.
-- 
2.39.0

[PATCH 06/11] staging/lynxfb: Use pci aperture helper

[Daniel Vetter]

It exists! Note that since this is an exact copy, there shouldn't be
any functional difference here.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
Cc: Teddy Wang <teddy.wang@siliconmotion.com>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/staging/sm750fb/sm750.c | 16 +---------------
 1 file changed, 1 insertion(+), 15 deletions(-)

diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
index effc7fcc3703..22ace3168723 100644
--- a/drivers/staging/sm750fb/sm750.c
+++ b/drivers/staging/sm750fb/sm750.c
@@ -989,20 +989,6 @@ static int sm750fb_framebuffer_alloc(struct sm750_dev *sm750_dev, int fbidx)
 	return err;
 }
 
-static int lynxfb_kick_out_firmware_fb(struct pci_dev *pdev)
-{
-	resource_size_t base = pci_resource_start(pdev, 0);
-	resource_size_t size = pci_resource_len(pdev, 0);
-	bool primary = false;
-
-#ifdef CONFIG_X86
-	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
-					IORESOURCE_ROM_SHADOW;
-#endif
-
-	return aperture_remove_conflicting_devices(base, size, primary, "sm750_fb1");
-}
-
 static int lynxfb_pci_probe(struct pci_dev *pdev,
 			    const struct pci_device_id *ent)
 {
@@ -1011,7 +997,7 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
 	int fbidx;
 	int err;
 
-	err = lynxfb_kick_out_firmware_fb(pdev);
+	err = aperture_remove_conflicting_pci_devices(pdev, "sm750_fb1");
 	if (err)
 		return err;
 
-- 
2.39.0

[PATCH 07/11] fbdev/radeon: use pci aperture helpers

[Daniel Vetter]

It's not exactly the same since the open coded version doesn't set
primary correctly. But that's a bugfix, so shouldn't hurt really.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/video/fbdev/aty/radeon_base.c | 10 +---------
 1 file changed, 1 insertion(+), 9 deletions(-)

diff --git a/drivers/video/fbdev/aty/radeon_base.c b/drivers/video/fbdev/aty/radeon_base.c
index 657064227de8..972c4bbedfa3 100644
--- a/drivers/video/fbdev/aty/radeon_base.c
+++ b/drivers/video/fbdev/aty/radeon_base.c
@@ -2238,14 +2238,6 @@ static const struct bin_attribute edid2_attr = {
 	.read	= radeon_show_edid2,
 };
 
-static int radeon_kick_out_firmware_fb(struct pci_dev *pdev)
-{
-	resource_size_t base = pci_resource_start(pdev, 0);
-	resource_size_t size = pci_resource_len(pdev, 0);
-
-	return aperture_remove_conflicting_devices(base, size, false, KBUILD_MODNAME);
-}
-
 static int radeonfb_pci_register(struct pci_dev *pdev,
 				 const struct pci_device_id *ent)
 {
@@ -2296,7 +2288,7 @@ static int radeonfb_pci_register(struct pci_dev *pdev,
 	rinfo->fb_base_phys = pci_resource_start (pdev, 0);
 	rinfo->mmio_base_phys = pci_resource_start (pdev, 2);
 
-	ret = radeon_kick_out_firmware_fb(pdev);
+	ret = aperture_remove_conflicting_pci_devices(pdev, KBUILD_MODNAME);
 	if (ret)
 		goto err_release_fb;
 
-- 
2.39.0

[PATCH 09/11] video/aperture: Move vga handling to pci function

[Daniel Vetter]

A few reasons for this:

- It's really the only one where this matters. I tried looking around,
  and I didn't find any non-pci vga-compatible controllers for x86
  (since that's the only platform where we had this until a few
  patches ago), where a driver participating in the aperture claim
  dance would interfere.

- I also don't expect that any future bus anytime soon will
  not just look like pci towards the OS, that's been the case for like
  25+ years by now for practically everything (even non non-x86).

- Also it's a bit funny if we have one part of the vga removal in the
  pci function, and the other in the generic one.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/video/aperture.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 6f351a58f6c6..03f8a5e95238 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -298,14 +298,6 @@ int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t si
 
 	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);
-
 	return 0;
 }
 EXPORT_SYMBOL(aperture_remove_conflicting_devices);
@@ -344,6 +336,13 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
 	if (!primary)
 		return 0;
 
+	/*
+	 * 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.
+	 */
+	aperture_detach_devices(VGA_FB_PHYS_BASE, VGA_FB_PHYS_SIZE);
+
 	/*
 	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
 	 * otherwise the vga fbdev driver falls over.
-- 
2.39.0

[PATCH 10/11] video/aperture: Drop primary argument

[Daniel Vetter]

With the preceeding patches it's become defunct. Also I'm about to add
a different boolean argument, so it's better to keep the confusion
down to the absolute minimum.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Cc: Thomas Zimmermann <tzimmermann@suse.de>
Cc: Javier Martinez Canillas <javierm@redhat.com>
Cc: Helge Deller <deller@gmx.de>
Cc: linux-fbdev@vger.kernel.org
---
 drivers/gpu/drm/drm_aperture.c | 2 +-
 drivers/video/aperture.c       | 7 +++----
 include/linux/aperture.h       | 9 ++++-----
 3 files changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
index 697cffbfd603..5729f3bb4398 100644
--- a/drivers/gpu/drm/drm_aperture.c
+++ b/drivers/gpu/drm/drm_aperture.c
@@ -168,7 +168,7 @@ EXPORT_SYMBOL(devm_aperture_acquire_from_firmware);
 int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
 						 const struct drm_driver *req_driver)
 {
-	return aperture_remove_conflicting_devices(base, size, false, req_driver->name);
+	return aperture_remove_conflicting_devices(base, size, req_driver->name);
 }
 EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
 
diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
index 03f8a5e95238..ba565515480d 100644
--- a/drivers/video/aperture.c
+++ b/drivers/video/aperture.c
@@ -43,7 +43,7 @@
  *		base = mem->start;
  *		size = resource_size(mem);
  *
- *		ret = aperture_remove_conflicting_devices(base, size, false, "example");
+ *		ret = aperture_remove_conflicting_devices(base, size, "example");
  *		if (ret)
  *			return ret;
  *
@@ -274,7 +274,6 @@ static void aperture_detach_devices(resource_size_t base, resource_size_t size)
  * aperture_remove_conflicting_devices - remove devices in the given range
  * @base: the aperture's base address in physical memory
  * @size: aperture size in bytes
- * @primary: also kick vga16fb if present; only relevant for VGA devices
  * @name: a descriptive name of the requesting driver
  *
  * This function removes devices that own apertures within @base and @size.
@@ -283,7 +282,7 @@ static void aperture_detach_devices(resource_size_t base, resource_size_t size)
  * 0 on success, or a negative errno code otherwise
  */
 int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
-					bool primary, const char *name)
+					const char *name)
 {
 	/*
 	 * If a driver asked to unregister a platform device registered by
@@ -328,7 +327,7 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
 
 		base = pci_resource_start(pdev, bar);
 		size = pci_resource_len(pdev, bar);
-		ret = aperture_remove_conflicting_devices(base, size, primary, name);
+		ret = aperture_remove_conflicting_devices(base, size, name);
 		if (ret)
 			return ret;
 	}
diff --git a/include/linux/aperture.h b/include/linux/aperture.h
index 442f15a57cad..7248727753be 100644
--- a/include/linux/aperture.h
+++ b/include/linux/aperture.h
@@ -14,7 +14,7 @@ int devm_aperture_acquire_for_platform_device(struct platform_device *pdev,
 					      resource_size_t size);
 
 int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
-					bool primary, const char *name);
+					const char *name);
 
 int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name);
 #else
@@ -26,7 +26,7 @@ static inline int devm_aperture_acquire_for_platform_device(struct platform_devi
 }
 
 static inline int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
-						      bool primary, const char *name)
+						      const char *name)
 {
 	return 0;
 }
@@ -39,7 +39,6 @@ static inline int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
 
 /**
  * aperture_remove_all_conflicting_devices - remove all existing framebuffers
- * @primary: also kick vga16fb if present; only relevant for VGA devices
  * @name: a descriptive name of the requesting driver
  *
  * This function removes all graphics device drivers. Use this function on systems
@@ -48,9 +47,9 @@ static inline int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
  * Returns:
  * 0 on success, or a negative errno code otherwise
  */
-static inline int aperture_remove_all_conflicting_devices(bool primary, const char *name)
+static inline int aperture_remove_all_conflicting_devices(const char *name)
 {
-	return aperture_remove_conflicting_devices(0, (resource_size_t)-1, primary, name);
+	return aperture_remove_conflicting_devices(0, (resource_size_t)-1, name);
 }
 
 #endif
-- 
2.39.0

Re: [PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Thomas Zimmermann]

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

Hi

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> It's just open coded and matches.
> 
> Note that Thomas said that his version apparently failed for some
> reason, but hey maybe we should try again.

I'll give this patch a test tomorrow.

Best regards
Thomas

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> ---
>   drivers/gpu/drm/ast/ast_drv.c | 16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 420fc75c240e..3ac24a780f50 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] = {
>   
>   MODULE_DEVICE_TABLE(pci, ast_pciidlist);
>   
> -static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
> -{
> -	bool primary = false;
> -	resource_size_t base, size;
> -
> -	base = pci_resource_start(pdev, 0);
> -	size = pci_resource_len(pdev, 0);
> -#ifdef CONFIG_X86
> -	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> -#endif
> -
> -	return drm_aperture_remove_conflicting_framebuffers(base, size, primary, &ast_driver);
> -}
> -
>   static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   {
>   	struct ast_private *ast;
>   	struct drm_device *dev;
>   	int ret;
>   
> -	ret = ast_remove_conflicting_framebuffers(pdev);
> +	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &ast_driver);
>   	if (ret)
>   		return ret;
>   

-- 
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 --]

Re: [PATCH 03/11] drm/aperture: Remove primary argument

[Thomas Zimmermann]

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

Hi

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> Only really pci devices have a business setting this - it's for
> figuring out whether the legacy vga stuff should be nuked too. And
> with the preceeding two patches those are all using the pci version of
> this.
> 
> Which means for all other callers primary == false and we can remove
> it now.

AFAICS this patch needs to be merged with patch 4 to build.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Deepak Rawat <drawat.floss@gmail.com>
> Cc: Neil Armstrong <neil.armstrong@linaro.org>
> Cc: Kevin Hilman <khilman@baylibre.com>
> Cc: Jerome Brunet <jbrunet@baylibre.com>
> Cc: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> Cc: Thierry Reding <thierry.reding@gmail.com>
> Cc: Jonathan Hunter <jonathanh@nvidia.com>
> Cc: Emma Anholt <emma@anholt.net>
> Cc: Helge Deller <deller@gmx.de>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Daniel Vetter <daniel@ffwll.ch>
> Cc: linux-hyperv@vger.kernel.org
> Cc: linux-amlogic@lists.infradead.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: linux-tegra@vger.kernel.org
> Cc: linux-fbdev@vger.kernel.org
> ---
>   drivers/gpu/drm/arm/hdlcd_drv.c             |  2 +-
>   drivers/gpu/drm/armada/armada_drv.c         |  2 +-
>   drivers/gpu/drm/drm_aperture.c              | 11 +++--------
>   drivers/gpu/drm/hyperv/hyperv_drm_drv.c     |  1 -
>   drivers/gpu/drm/meson/meson_drv.c           |  2 +-
>   drivers/gpu/drm/msm/msm_fbdev.c             |  2 +-
>   drivers/gpu/drm/rockchip/rockchip_drm_drv.c |  2 +-
>   drivers/gpu/drm/stm/drv.c                   |  2 +-
>   drivers/gpu/drm/sun4i/sun4i_drv.c           |  2 +-
>   drivers/gpu/drm/tegra/drm.c                 |  2 +-
>   drivers/gpu/drm/vc4/vc4_drv.c               |  2 +-
>   include/drm/drm_aperture.h                  |  7 +++----
>   12 files changed, 15 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index 7043d1c9ed8f..98267e355918 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -297,7 +297,7 @@ static int hdlcd_drm_bind(struct device *dev)
>   	 */
>   	if (hdlcd_read(hdlcd, HDLCD_REG_COMMAND)) {
>   		hdlcd_write(hdlcd, HDLCD_REG_COMMAND, 0);
> -		drm_aperture_remove_framebuffers(false, &hdlcd_driver);
> +		drm_aperture_remove_framebuffers(&hdlcd_driver);
>   	}
>   
>   	drm_mode_config_reset(drm);
> diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
> index 0643887800b4..c99ec7078301 100644
> --- a/drivers/gpu/drm/armada/armada_drv.c
> +++ b/drivers/gpu/drm/armada/armada_drv.c
> @@ -95,7 +95,7 @@ static int armada_drm_bind(struct device *dev)
>   	}
>   
>   	/* Remove early framebuffers */
> -	ret = drm_aperture_remove_framebuffers(false, &armada_drm_driver);
> +	ret = drm_aperture_remove_framebuffers(&armada_drm_driver);
>   	if (ret) {
>   		dev_err(dev, "[" DRM_NAME ":%s] can't kick out simple-fb: %d\n",
>   			__func__, ret);
> diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
> index 3b8fdeeafd53..697cffbfd603 100644
> --- a/drivers/gpu/drm/drm_aperture.c
> +++ b/drivers/gpu/drm/drm_aperture.c
> @@ -32,17 +32,13 @@
>    *
>    *	static int remove_conflicting_framebuffers(struct pci_dev *pdev)
>    *	{
> - *		bool primary = false;
>    *		resource_size_t base, size;
>    *		int ret;
>    *
>    *		base = pci_resource_start(pdev, 0);
>    *		size = pci_resource_len(pdev, 0);
> - *	#ifdef CONFIG_X86
> - *		primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> - *	#endif
>    *
> - *		return drm_aperture_remove_conflicting_framebuffers(base, size, primary,
> + *		return drm_aperture_remove_conflicting_framebuffers(base, size,
>    *		                                                    &example_driver);
>    *	}
>    *
> @@ -161,7 +157,6 @@ EXPORT_SYMBOL(devm_aperture_acquire_from_firmware);
>    * drm_aperture_remove_conflicting_framebuffers - remove existing framebuffers in the given range
>    * @base: the aperture's base address in physical memory
>    * @size: aperture size in bytes
> - * @primary: also kick vga16fb if present
>    * @req_driver: requesting DRM driver
>    *
>    * This function removes graphics device drivers which use the memory range described by
> @@ -171,9 +166,9 @@ EXPORT_SYMBOL(devm_aperture_acquire_from_firmware);
>    * 0 on success, or a negative errno code otherwise
>    */
>   int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
> -						 bool primary, const struct drm_driver *req_driver)
> +						 const struct drm_driver *req_driver)
>   {
> -	return aperture_remove_conflicting_devices(base, size, primary, req_driver->name);
> +	return aperture_remove_conflicting_devices(base, size, false, req_driver->name);
>   }
>   EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
>   
> diff --git a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> index 427c20ba3404..7e81d58c083f 100644
> --- a/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> +++ b/drivers/gpu/drm/hyperv/hyperv_drm_drv.c
> @@ -74,7 +74,6 @@ static int hyperv_setup_vram(struct hyperv_drm_device *hv,
>   
>   	drm_aperture_remove_conflicting_framebuffers(screen_info.lfb_base,
>   						     screen_info.lfb_size,
> -						     false,
>   						     &hyperv_driver);
>   
>   	hv->fb_size = (unsigned long)hv->mmio_megabytes * 1024 * 1024;
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 79bfe3938d3c..c8d39809d897 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -285,7 +285,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>   	 * Remove early framebuffers (ie. simplefb). The framebuffer can be
>   	 * located anywhere in RAM
>   	 */
> -	ret = drm_aperture_remove_framebuffers(false, &meson_driver);
> +	ret = drm_aperture_remove_framebuffers(&meson_driver);
>   	if (ret)
>   		goto free_drm;
>   
> diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
> index 31e1e30cb52a..84dfbccb6912 100644
> --- a/drivers/gpu/drm/msm/msm_fbdev.c
> +++ b/drivers/gpu/drm/msm/msm_fbdev.c
> @@ -155,7 +155,7 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
>   	}
>   
>   	/* the fw fb could be anywhere in memory */
> -	ret = drm_aperture_remove_framebuffers(false, dev->driver);
> +	ret = drm_aperture_remove_framebuffers(dev->driver);
>   	if (ret)
>   		goto fini;
>   
> diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> index 6e0788d14c10..d97f2edc646b 100644
> --- a/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> +++ b/drivers/gpu/drm/rockchip/rockchip_drm_drv.c
> @@ -140,7 +140,7 @@ static int rockchip_drm_bind(struct device *dev)
>   	int ret;
>   
>   	/* Remove existing drivers that may own the framebuffer memory. */
> -	ret = drm_aperture_remove_framebuffers(false, &rockchip_drm_driver);
> +	ret = drm_aperture_remove_framebuffers(&rockchip_drm_driver);
>   	if (ret) {
>   		DRM_DEV_ERROR(dev,
>   			      "Failed to remove existing framebuffers - %d.\n",
> diff --git a/drivers/gpu/drm/stm/drv.c b/drivers/gpu/drm/stm/drv.c
> index 50410bd99dfe..354349c6e085 100644
> --- a/drivers/gpu/drm/stm/drv.c
> +++ b/drivers/gpu/drm/stm/drv.c
> @@ -185,7 +185,7 @@ static int stm_drm_platform_probe(struct platform_device *pdev)
>   
>   	DRM_DEBUG("%s\n", __func__);
>   
> -	ret = drm_aperture_remove_framebuffers(false, &drv_driver);
> +	ret = drm_aperture_remove_framebuffers(&drv_driver);
>   	if (ret)
>   		return ret;
>   
> diff --git a/drivers/gpu/drm/sun4i/sun4i_drv.c b/drivers/gpu/drm/sun4i/sun4i_drv.c
> index cc94efbbf2d4..6367b89cbab1 100644
> --- a/drivers/gpu/drm/sun4i/sun4i_drv.c
> +++ b/drivers/gpu/drm/sun4i/sun4i_drv.c
> @@ -98,7 +98,7 @@ static int sun4i_drv_bind(struct device *dev)
>   		goto cleanup_mode_config;
>   
>   	/* Remove early framebuffers (ie. simplefb) */
> -	ret = drm_aperture_remove_framebuffers(false, &sun4i_drv_driver);
> +	ret = drm_aperture_remove_framebuffers(&sun4i_drv_driver);
>   	if (ret)
>   		goto cleanup_mode_config;
>   
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 7bd2e65c2a16..d2ff527cf6d7 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -1252,7 +1252,7 @@ static int host1x_drm_probe(struct host1x_device *dev)
>   
>   	drm_mode_config_reset(drm);
>   
> -	err = drm_aperture_remove_framebuffers(false, &tegra_drm_driver);
> +	err = drm_aperture_remove_framebuffers(&tegra_drm_driver);
>   	if (err < 0)
>   		goto hub;
>   
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 0ccaee57fe9a..0a9e922636b1 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -350,7 +350,7 @@ static int vc4_drm_bind(struct device *dev)
>   			return -EPROBE_DEFER;
>   	}
>   
> -	ret = drm_aperture_remove_framebuffers(false, driver);
> +	ret = drm_aperture_remove_framebuffers(driver);
>   	if (ret)
>   		return ret;
>   
> diff --git a/include/drm/drm_aperture.h b/include/drm/drm_aperture.h
> index 7096703c3949..cbe33b49fd5d 100644
> --- a/include/drm/drm_aperture.h
> +++ b/include/drm/drm_aperture.h
> @@ -13,14 +13,13 @@ int devm_aperture_acquire_from_firmware(struct drm_device *dev, resource_size_t
>   					resource_size_t size);
>   
>   int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
> -						 bool primary, const struct drm_driver *req_driver);
> +						 const struct drm_driver *req_driver);
>   
>   int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
>   						     const struct drm_driver *req_driver);
>   
>   /**
>    * drm_aperture_remove_framebuffers - remove all existing framebuffers
> - * @primary: also kick vga16fb if present
>    * @req_driver: requesting DRM driver
>    *
>    * This function removes all graphics device drivers. Use this function on systems
> @@ -30,9 +29,9 @@ int drm_aperture_remove_conflicting_pci_framebuffers(struct pci_dev *pdev,
>    * 0 on success, or a negative errno code otherwise
>    */
>   static inline int
> -drm_aperture_remove_framebuffers(bool primary, const struct drm_driver *req_driver)
> +drm_aperture_remove_framebuffers(const struct drm_driver *req_driver)
>   {
> -	return drm_aperture_remove_conflicting_framebuffers(0, (resource_size_t)-1, primary,
> +	return drm_aperture_remove_conflicting_framebuffers(0, (resource_size_t)-1,
>   							    req_driver);
>   }
>   

-- 
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 --]

Re: [PATCH 04/11] video/aperture: use generic code to figure out the vga default device

[Thomas Zimmermann]

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

Hi

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> Since vgaarb has been promoted to be a core piece of the pci subsystem
> we don't have to open code random guesses anymore, we actually know
> this in a platform agnostic way, and there's no need for an x86
> specific hack. See also 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to
> drivers/pci")
> 
> This should not result in any functional change, and the non-x86
> multi-gpu pci systems are probably rare enough to not matter (I don't
> know of any tbh). But it's a nice cleanup, so let's do it.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: linux-pci@vger.kernel.org
> ---
>   drivers/video/aperture.c | 6 ++----
>   1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> index 41e77de1ea82..3d8c925c7365 100644
> --- a/drivers/video/aperture.c
> +++ b/drivers/video/aperture.c
> @@ -324,13 +324,11 @@ EXPORT_SYMBOL(aperture_remove_conflicting_devices);
>    */
>   int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name)
>   {
> -	bool primary = false;
> +	bool primary;
>   	resource_size_t base, size;
>   	int bar, ret;
>   
> -#ifdef CONFIG_X86
> -	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> -#endif
> +	primary = pdev == vga_default_device();

vga_default_device() is part of vgaarb and can return NULL. [1] That new 
test is likely to be incorrect on many systems.

I suggest to implement a helper like fb_is_primary_device() on x86: it 
uses the default VGA if set, or falls back to the original test. [2]

It's noteworthy that on most architectures, fb_is_primary_device() 
returns 0. But at least on Sparc [3] and some Parisc [4] machines, it 
does not.

I've long wanted to rework this helper anyway. So this is a good 
opportunity.

Best regards
Thomas

[1] 
https://elixir.bootlin.com/linux/latest/source/include/linux/vgaarb.h#L69
[2] 
https://elixir.bootlin.com/linux/latest/source/arch/x86/video/fbdev.c#L14
[3] 
https://elixir.bootlin.com/linux/latest/source/arch/sparc/include/asm/fb.h#L18
[4] 
https://elixir.bootlin.com/linux/latest/source/drivers/video/console/sticore.c#L1153


>   
>   	for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
>   		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))

-- 
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 --]

Re: [PATCH 05/11] video/aperture: Only kick vgacon when the pdev is decoding vga

[Thomas Zimmermann]

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

Hi

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> Otherwise it's bit silly, and we might throw out the driver for the
> screen the user is actually looking at. I haven't found a bug report
> for this case yet, but we did get bug reports for the analog case
> where we're throwing out the efifb driver.
> 
> References: https://bugzilla.kernel.org/show_bug.cgi?id=216303
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> ---
>   drivers/video/aperture.c | 3 +++
>   1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> index 3d8c925c7365..6f351a58f6c6 100644
> --- a/drivers/video/aperture.c
> +++ b/drivers/video/aperture.c
> @@ -341,6 +341,9 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
>   			return ret;
>   	}
>   
> +	if (!primary)
> +		return 0;
> +

The original code from fbdev didn't do this, so this code didn't either.

It appears more to be a special case than an early-out branch. So can we 
write it as

if (primary) {
   // kick_vgacon
}

?

Best regards
Thomas

>   	/*
>   	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
>   	 * otherwise the vga fbdev driver falls over.

-- 
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 --]

Re: [PATCH 06/11] staging/lynxfb: Use pci aperture helper

[Thomas Zimmermann]

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

Hi

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> It exists! Note that since this is an exact copy, there shouldn't be
> any functional difference here.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Teddy Wang <teddy.wang@siliconmotion.com>
> Cc: linux-fbdev@vger.kernel.org
> ---
>   drivers/staging/sm750fb/sm750.c | 16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/staging/sm750fb/sm750.c b/drivers/staging/sm750fb/sm750.c
> index effc7fcc3703..22ace3168723 100644
> --- a/drivers/staging/sm750fb/sm750.c
> +++ b/drivers/staging/sm750fb/sm750.c
> @@ -989,20 +989,6 @@ static int sm750fb_framebuffer_alloc(struct sm750_dev *sm750_dev, int fbidx)
>   	return err;
>   }
>   
> -static int lynxfb_kick_out_firmware_fb(struct pci_dev *pdev)
> -{
> -	resource_size_t base = pci_resource_start(pdev, 0);
> -	resource_size_t size = pci_resource_len(pdev, 0);
> -	bool primary = false;
> -
> -#ifdef CONFIG_X86
> -	primary = pdev->resource[PCI_ROM_RESOURCE].flags &
> -					IORESOURCE_ROM_SHADOW;
> -#endif
> -
> -	return aperture_remove_conflicting_devices(base, size, primary, "sm750_fb1");

This still had the primary argument, it needs to be handled in an early 
patch in some way.

> -}
> -
>   static int lynxfb_pci_probe(struct pci_dev *pdev,
>   			    const struct pci_device_id *ent)
>   {
> @@ -1011,7 +997,7 @@ static int lynxfb_pci_probe(struct pci_dev *pdev,
>   	int fbidx;
>   	int err;
>   
> -	err = lynxfb_kick_out_firmware_fb(pdev);
> +	err = aperture_remove_conflicting_pci_devices(pdev, "sm750_fb1");
>   	if (err)
>   		return err;
>   

-- 
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 --]

Re: [PATCH 10/11] video/aperture: Drop primary argument

[Thomas Zimmermann]

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

Hi

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> With the preceeding patches it's become defunct. Also I'm about to add
> a different boolean argument, so it's better to keep the confusion
> down to the absolute minimum.

OK, maybe my earlier comments about the use of 'primary' in some 
function calls were not correct. Ignore them then.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> ---
>   drivers/gpu/drm/drm_aperture.c | 2 +-
>   drivers/video/aperture.c       | 7 +++----
>   include/linux/aperture.h       | 9 ++++-----
>   3 files changed, 8 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_aperture.c b/drivers/gpu/drm/drm_aperture.c
> index 697cffbfd603..5729f3bb4398 100644
> --- a/drivers/gpu/drm/drm_aperture.c
> +++ b/drivers/gpu/drm/drm_aperture.c
> @@ -168,7 +168,7 @@ EXPORT_SYMBOL(devm_aperture_acquire_from_firmware);
>   int drm_aperture_remove_conflicting_framebuffers(resource_size_t base, resource_size_t size,
>   						 const struct drm_driver *req_driver)
>   {
> -	return aperture_remove_conflicting_devices(base, size, false, req_driver->name);
> +	return aperture_remove_conflicting_devices(base, size, req_driver->name);
>   }
>   EXPORT_SYMBOL(drm_aperture_remove_conflicting_framebuffers);
>   
> diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> index 03f8a5e95238..ba565515480d 100644
> --- a/drivers/video/aperture.c
> +++ b/drivers/video/aperture.c
> @@ -43,7 +43,7 @@
>    *		base = mem->start;
>    *		size = resource_size(mem);
>    *
> - *		ret = aperture_remove_conflicting_devices(base, size, false, "example");
> + *		ret = aperture_remove_conflicting_devices(base, size, "example");
>    *		if (ret)
>    *			return ret;
>    *
> @@ -274,7 +274,6 @@ static void aperture_detach_devices(resource_size_t base, resource_size_t size)
>    * aperture_remove_conflicting_devices - remove devices in the given range
>    * @base: the aperture's base address in physical memory
>    * @size: aperture size in bytes
> - * @primary: also kick vga16fb if present; only relevant for VGA devices
>    * @name: a descriptive name of the requesting driver
>    *
>    * This function removes devices that own apertures within @base and @size.
> @@ -283,7 +282,7 @@ static void aperture_detach_devices(resource_size_t base, resource_size_t size)
>    * 0 on success, or a negative errno code otherwise
>    */
>   int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
> -					bool primary, const char *name)
> +					const char *name)
>   {
>   	/*
>   	 * If a driver asked to unregister a platform device registered by
> @@ -328,7 +327,7 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
>   
>   		base = pci_resource_start(pdev, bar);
>   		size = pci_resource_len(pdev, bar);
> -		ret = aperture_remove_conflicting_devices(base, size, primary, name);
> +		ret = aperture_remove_conflicting_devices(base, size, name);
>   		if (ret)
>   			return ret;
>   	}
> diff --git a/include/linux/aperture.h b/include/linux/aperture.h
> index 442f15a57cad..7248727753be 100644
> --- a/include/linux/aperture.h
> +++ b/include/linux/aperture.h
> @@ -14,7 +14,7 @@ int devm_aperture_acquire_for_platform_device(struct platform_device *pdev,
>   					      resource_size_t size);
>   
>   int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
> -					bool primary, const char *name);
> +					const char *name);
>   
>   int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name);
>   #else
> @@ -26,7 +26,7 @@ static inline int devm_aperture_acquire_for_platform_device(struct platform_devi
>   }
>   
>   static inline int aperture_remove_conflicting_devices(resource_size_t base, resource_size_t size,
> -						      bool primary, const char *name)
> +						      const char *name)
>   {
>   	return 0;
>   }
> @@ -39,7 +39,6 @@ static inline int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
>   
>   /**
>    * aperture_remove_all_conflicting_devices - remove all existing framebuffers
> - * @primary: also kick vga16fb if present; only relevant for VGA devices
>    * @name: a descriptive name of the requesting driver
>    *
>    * This function removes all graphics device drivers. Use this function on systems
> @@ -48,9 +47,9 @@ static inline int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev,
>    * Returns:
>    * 0 on success, or a negative errno code otherwise
>    */
> -static inline int aperture_remove_all_conflicting_devices(bool primary, const char *name)
> +static inline int aperture_remove_all_conflicting_devices(const char *name)
>   {
> -	return aperture_remove_conflicting_devices(0, (resource_size_t)-1, primary, name);
> +	return aperture_remove_conflicting_devices(0, (resource_size_t)-1, name);
>   }
>   
>   #endif

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

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]

Re: [PATCH 04/11] video/aperture: use generic code to figure out the vga default device

[Daniel Vetter]

On Wed, Jan 11, 2023 at 04:59:30PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> > Since vgaarb has been promoted to be a core piece of the pci subsystem
> > we don't have to open code random guesses anymore, we actually know
> > this in a platform agnostic way, and there's no need for an x86
> > specific hack. See also 1d38fe6ee6a8 ("PCI/VGA: Move vgaarb to
> > drivers/pci")
> > 
> > This should not result in any functional change, and the non-x86
> > multi-gpu pci systems are probably rare enough to not matter (I don't
> > know of any tbh). But it's a nice cleanup, so let's do it.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Javier Martinez Canillas <javierm@redhat.com>
> > Cc: Helge Deller <deller@gmx.de>
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: Bjorn Helgaas <bhelgaas@google.com>
> > Cc: linux-pci@vger.kernel.org
> > ---
> >   drivers/video/aperture.c | 6 ++----
> >   1 file changed, 2 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> > index 41e77de1ea82..3d8c925c7365 100644
> > --- a/drivers/video/aperture.c
> > +++ b/drivers/video/aperture.c
> > @@ -324,13 +324,11 @@ EXPORT_SYMBOL(aperture_remove_conflicting_devices);
> >    */
> >   int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *name)
> >   {
> > -	bool primary = false;
> > +	bool primary;
> >   	resource_size_t base, size;
> >   	int bar, ret;
> > -#ifdef CONFIG_X86
> > -	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> > -#endif
> > +	primary = pdev == vga_default_device();
> 
> vga_default_device() is part of vgaarb and can return NULL. [1] That new
> test is likely to be incorrect on many systems.
> 
> I suggest to implement a helper like fb_is_primary_device() on x86: it uses
> the default VGA if set, or falls back to the original test. [2]
> 
> It's noteworthy that on most architectures, fb_is_primary_device() returns
> 0. But at least on Sparc [3] and some Parisc [4] machines, it does not.

Afaik these neither do legacy vga nor sysfb, so we should be fine. I'll
augment the commit message.

> I've long wanted to rework this helper anyway. So this is a good
> opportunity.

Hm yeah that sounds like a good thing to copy. I'm honestly not sure
whether it's needed, but at least it shouldn't hurt. I thought at least
that the boot default device should be set on all pci architectures.

I'll also reorder this with the previous patch to avoid the compile fail.
-Daniel

> 
> Best regards
> Thomas
> 
> [1]
> https://elixir.bootlin.com/linux/latest/source/include/linux/vgaarb.h#L69
> [2]
> https://elixir.bootlin.com/linux/latest/source/arch/x86/video/fbdev.c#L14
> [3] https://elixir.bootlin.com/linux/latest/source/arch/sparc/include/asm/fb.h#L18
> [4] https://elixir.bootlin.com/linux/latest/source/drivers/video/console/sticore.c#L1153
> 
> 
> >   	for (bar = 0; bar < PCI_STD_NUM_BARS; ++bar) {
> >   		if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM))
> 
> -- 
> 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




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 05/11] video/aperture: Only kick vgacon when the pdev is decoding vga

[Daniel Vetter]

On Wed, Jan 11, 2023 at 05:03:02PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> > Otherwise it's bit silly, and we might throw out the driver for the
> > screen the user is actually looking at. I haven't found a bug report
> > for this case yet, but we did get bug reports for the analog case
> > where we're throwing out the efifb driver.
> > 
> > References: https://bugzilla.kernel.org/show_bug.cgi?id=216303
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Javier Martinez Canillas <javierm@redhat.com>
> > Cc: Helge Deller <deller@gmx.de>
> > Cc: linux-fbdev@vger.kernel.org
> > ---
> >   drivers/video/aperture.c | 3 +++
> >   1 file changed, 3 insertions(+)
> > 
> > diff --git a/drivers/video/aperture.c b/drivers/video/aperture.c
> > index 3d8c925c7365..6f351a58f6c6 100644
> > --- a/drivers/video/aperture.c
> > +++ b/drivers/video/aperture.c
> > @@ -341,6 +341,9 @@ int aperture_remove_conflicting_pci_devices(struct pci_dev *pdev, const char *na
> >   			return ret;
> >   	}
> > +	if (!primary)
> > +		return 0;
> > +
> 
> The original code from fbdev didn't do this, so this code didn't either.
> 
> It appears more to be a special case than an early-out branch. So can we
> write it as

Yeah I think this was a mistake going way back to when I added this to
i915 originally. It is a real change, but also I guess the people who have
machines without efifb or vesafb are ... really not many :-) Iirc you had
some very funny kernels going way back when vgacon was considered the only
safe choice to even hit this stuff.

> if (primary) {
>   // kick_vgacon
> }

Yeah, but next patch adds the vga aperture, and then I think it makes a
bit more sense.
-Daniel

> 
> ?
> 
> Best regards
> Thomas
> 
> >   	/*
> >   	 * WARNING: Apparently we must kick fbdev drivers before vgacon,
> >   	 * otherwise the vga fbdev driver falls over.
> 
> -- 
> 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




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Daniel Vetter]

On Wed, Jan 11, 2023 at 04:48:39PM +0100, Thomas Zimmermann wrote:
> Hi
> 
> Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> > It's just open coded and matches.
> > 
> > Note that Thomas said that his version apparently failed for some
> > reason, but hey maybe we should try again.
> 
> I'll give this patch a test tomorrow.

Thanks a lot!
-Daniel

> 
> Best regards
> Thomas
> 
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Thomas Zimmermann <tzimmermann@suse.de>
> > Cc: Javier Martinez Canillas <javierm@redhat.com>
> > Cc: Helge Deller <deller@gmx.de>
> > Cc: linux-fbdev@vger.kernel.org
> > ---
> >   drivers/gpu/drm/ast/ast_drv.c | 16 +---------------
> >   1 file changed, 1 insertion(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> > index 420fc75c240e..3ac24a780f50 100644
> > --- a/drivers/gpu/drm/ast/ast_drv.c
> > +++ b/drivers/gpu/drm/ast/ast_drv.c
> > @@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] = {
> >   MODULE_DEVICE_TABLE(pci, ast_pciidlist);
> > -static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
> > -{
> > -	bool primary = false;
> > -	resource_size_t base, size;
> > -
> > -	base = pci_resource_start(pdev, 0);
> > -	size = pci_resource_len(pdev, 0);
> > -#ifdef CONFIG_X86
> > -	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> > -#endif
> > -
> > -	return drm_aperture_remove_conflicting_framebuffers(base, size, primary, &ast_driver);
> > -}
> > -
> >   static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
> >   {
> >   	struct ast_private *ast;
> >   	struct drm_device *dev;
> >   	int ret;
> > -	ret = ast_remove_conflicting_framebuffers(pdev);
> > +	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &ast_driver);
> >   	if (ret)
> >   		return ret;
> 
> -- 
> 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




-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

Re: [PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Thomas Zimmermann]

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



Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> It's just open coded and matches.
> 
> Note that Thomas said that his version apparently failed for some
> reason, but hey maybe we should try again.

It apparently worked this time. Tested on an AST2100 chip.

> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org

Tested-by: Thomas Zimmmermann <tzimmermann@suse.de>
Reviewed-by: Thomas Zimmermann <tzimmermann@suse.de>

> ---
>   drivers/gpu/drm/ast/ast_drv.c | 16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 420fc75c240e..3ac24a780f50 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] = {
>   
>   MODULE_DEVICE_TABLE(pci, ast_pciidlist);
>   
> -static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
> -{
> -	bool primary = false;
> -	resource_size_t base, size;
> -
> -	base = pci_resource_start(pdev, 0);
> -	size = pci_resource_len(pdev, 0);
> -#ifdef CONFIG_X86
> -	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> -#endif
> -
> -	return drm_aperture_remove_conflicting_framebuffers(base, size, primary, &ast_driver);
> -}
> -
>   static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   {
>   	struct ast_private *ast;
>   	struct drm_device *dev;
>   	int ret;
>   
> -	ret = ast_remove_conflicting_framebuffers(pdev);
> +	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &ast_driver);
>   	if (ret)
>   		return ret;
>   

-- 
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 --]

Re: [PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Thomas Zimmermann]

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

Hi,

FYI I have merged patches 1, 6 and 7 of this patchset. They look fine 
and are worthwhile fixes on their own.

Best regards
Thomas

Am 11.01.23 um 16:41 schrieb Daniel Vetter:
> It's just open coded and matches.
> 
> Note that Thomas said that his version apparently failed for some
> reason, but hey maybe we should try again.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Thomas Zimmermann <tzimmermann@suse.de>
> Cc: Javier Martinez Canillas <javierm@redhat.com>
> Cc: Helge Deller <deller@gmx.de>
> Cc: linux-fbdev@vger.kernel.org
> ---
>   drivers/gpu/drm/ast/ast_drv.c | 16 +---------------
>   1 file changed, 1 insertion(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/ast/ast_drv.c b/drivers/gpu/drm/ast/ast_drv.c
> index 420fc75c240e..3ac24a780f50 100644
> --- a/drivers/gpu/drm/ast/ast_drv.c
> +++ b/drivers/gpu/drm/ast/ast_drv.c
> @@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] = {
>   
>   MODULE_DEVICE_TABLE(pci, ast_pciidlist);
>   
> -static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
> -{
> -	bool primary = false;
> -	resource_size_t base, size;
> -
> -	base = pci_resource_start(pdev, 0);
> -	size = pci_resource_len(pdev, 0);
> -#ifdef CONFIG_X86
> -	primary = pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_ROM_SHADOW;
> -#endif
> -
> -	return drm_aperture_remove_conflicting_framebuffers(base, size, primary, &ast_driver);
> -}
> -
>   static int ast_pci_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
>   {
>   	struct ast_private *ast;
>   	struct drm_device *dev;
>   	int ret;
>   
> -	ret = ast_remove_conflicting_framebuffers(pdev);
> +	ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, &ast_driver);
>   	if (ret)
>   		return ret;
>   

-- 
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 --]

Re: [PATCH 01/11] drm/ast: Use drm_aperture_remove_conflicting_pci_framebuffers

[Jammy Huang]

Hi Thomas,

The Intel(x86) CPUs have a separate address space for "IO", but the ARM 
architecture only has "memory", so all IO devices are accessed as if 
they were memory. Which means ARM does not support isolated IO. Here is 
a related discussion on ARM's forum.

https://community.arm.com/support-forums/f/architectures-and-processors-forum/52046/how-to-read-write-an-i-o-port-in-aarch64

Thus, we adapt MMIO only after this patch.


On 2023/4/4 下午 10:45, Thomas Zimmermann wrote:
> Hi,
>
> FYI I have merged patches 1, 6 and 7 of this patchset. They look fine 
> and are worthwhile fixes on their own.
>
> Best regards
> Thomas
>
> Am 11.01.23 um 16:41 schrieb Daniel Vetter:
>> It's just open coded and matches.
>>
>> Note that Thomas said that his version apparently failed for some
>> reason, but hey maybe we should try again.
>>
>> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
>> Cc: Dave Airlie <airlied@redhat.com>
>> Cc: Thomas Zimmermann <tzimmermann@suse.de>
>> Cc: Javier Martinez Canillas <javierm@redhat.com>
>> Cc: Helge Deller <deller@gmx.de>
>> Cc: linux-fbdev@vger.kernel.org
>> ---
>>   drivers/gpu/drm/ast/ast_drv.c | 16 +---------------
>>   1 file changed, 1 insertion(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/ast/ast_drv.c 
>> b/drivers/gpu/drm/ast/ast_drv.c
>> index 420fc75c240e..3ac24a780f50 100644
>> --- a/drivers/gpu/drm/ast/ast_drv.c
>> +++ b/drivers/gpu/drm/ast/ast_drv.c
>> @@ -90,27 +90,13 @@ static const struct pci_device_id ast_pciidlist[] 
>> = {
>>     MODULE_DEVICE_TABLE(pci, ast_pciidlist);
>>   -static int ast_remove_conflicting_framebuffers(struct pci_dev *pdev)
>> -{
>> -    bool primary = false;
>> -    resource_size_t base, size;
>> -
>> -    base = pci_resource_start(pdev, 0);
>> -    size = pci_resource_len(pdev, 0);
>> -#ifdef CONFIG_X86
>> -    primary = pdev->resource[PCI_ROM_RESOURCE].flags & 
>> IORESOURCE_ROM_SHADOW;
>> -#endif
>> -
>> -    return drm_aperture_remove_conflicting_framebuffers(base, size, 
>> primary, &ast_driver);
>> -}
>> -
>>   static int ast_pci_probe(struct pci_dev *pdev, const struct 
>> pci_device_id *ent)
>>   {
>>       struct ast_private *ast;
>>       struct drm_device *dev;
>>       int ret;
>>   -    ret = ast_remove_conflicting_framebuffers(pdev);
>> +    ret = drm_aperture_remove_conflicting_pci_framebuffers(pdev, 
>> &ast_driver);
>>       if (ret)
>>           return ret;
>
-- 
Best Regards
Jammy