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

[Thomas Zimmermann <tzimmermann@suse.de>]

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

Hi

Am 11.07.22 um 13:01 schrieb 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@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() ?

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

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