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

[Javier Martinez Canillas]

On 7/7/22 17:39, Thomas Zimmermann wrote:
> 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 at suse.de>
> ---

[...]

>  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() ?

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

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

-- 
Best regards,

Javier Martinez Canillas
Linux Engineering
Red Hat