Re: [PATCH 8/9] drm/amdgpu: Remove driver side BAR release before resize

["Christian König" <christian.koenig@amd.com>]

On 11/11/25 12:08, Ilpo Järvinen wrote:
> On Tue, 11 Nov 2025, Christian König wrote:
> 
>> Sorry for the late reply I'm really busy at the moment.
>>
>> On 10/28/25 18:35, Ilpo Järvinen wrote:
>>> PCI core handles releasing device's resources and their rollback in
>>> case of failure of a BAR resizing operation. Releasing resource prior
>>> to calling pci_resize_resource() prevents PCI core from restoring the
>>> BARs as they were.
>>
>> I've intentionally didn't do it this way because at least on AMD HW we 
>> could only release the VRAM and doorbell BAR (both 64bit), but not the 
>> register BAR (32bit only).
>>
>> This patch set looks like the right thing in general, but which BARs are 
>> now released by pci_resize_resource()?
>>
>> If we avoid releasing the 32bit BAR as well then that should work, 
>> otherwise we will probably cause problems.
> 
> After these changes, pci_resize_resource() releases BARs that share the 
> bridge window with the BAR to be resized. So the answer depends on the 
> upstream bridge.
> 
> However, amdgpu_device_resize_fb_bar() also checks that root bus has a
> resource with a 64-bit address. That won't tell what the nearest bridge 
> has though. Maybe that check should be converted to check the resources of 
> the nearest bus instead? It would make it impossible to have the 
> 32-bit resource share the bridge window with the 64-bit resources so the 
> resize would be safe.

Mhm, I don't think that will work.


I've added the check for the root bus to avoid a couple of issues during resize, but checking the nearest bridge would block a whole bunch of use cases and isn't even 100% save.

See one use case of this is that all the BARs of the device start in the same 32bit bridge window (or a mixture of 64bit and 32bit window).

What we have is that BAR 0 and 2 are 64bit BARs which can (after some preparation) move around freely. But IIRC BAR 4 are the legacy I/O ports and BAR 5 is the 32bit MMIO registers (don't nail me on that, could be just the other way around).

Especially that 32bit MMIO BAR *can't* move! Not only because it is 32bit, but also because the amdgpu driver as well as the HW itself through the VGA emulation, as well as the EFI/VESA/VBIOS code might reference its absolute address.


Could we give pci_resize_resource() a mask of BARs which are save to release? Or maybe a flag to indicate that it can only free up 64bit BARs?

Regards,
Christian.

> 
> Thanks a lot for checking this out!
>