Re: [PATCH 05/11] fbdev: Convert drivers to aperture helpers

Hi

Am 11.07.22 um 13:01 schrieb Javier Martinez Canillas:

On 7/7/22 17:39, Thomas Zimmermann wrote:

quoted

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

[...]

quoted

  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");

Do you know why this can't just use aperture_remove_conflicting_pci_devices() ?

I simply don't want change too much at once, because there was this problem with the PCI helper on ast.

At some point we can make a push to really fix this throughout the code base. And that would include an update to fb_is_primary_device(), [1] which doesn't fill well into the new interfaces.

Best regards
Thomas

[1] https://elixir.bootlin.com/linux/latest/source/arch/x86/video/fbdev.c#L14

It seems that the driver is open coding part of the logic already in that helper.
For example, figuring out if is a primary by checking the IORESOURCE_ROM_SHADOW
flag in the PCI_ROM_RESOURCE.

But also getting the base and size for PCI BAR 0, since the loop in that helper
would already take care of that (it also starts at BAR 0).

quoted

  }
  
  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);

Same for this.

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