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

["Ilpo Järvinen" <ilpo.jarvinen@linux.intel.com>]

[-- Attachment #1: Type: text/plain, Size: 3273 bytes --]

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.

Thanks a lot for checking this out!

-- 
 i.

> Regards,
> Christian.
> 
> > 
> > Remove driver-side release of BARs from the amdgpu driver.
> > 
> > Also remove the driver initiated assignment as pci_resize_resource()
> > should try to assign as much as possible. If the driver side call
> > manages to get more required resources assigned in some scenario, such
> > a problem should be fixed inside pci_resize_resource() instead.
> > 
> > Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
> > ---
> >  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 8 +-------
> >  1 file changed, 1 insertion(+), 7 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 7a899fb4de29..65474d365229 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1729,12 +1729,8 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
> >  	pci_write_config_word(adev->pdev, PCI_COMMAND,
> >  			      cmd & ~PCI_COMMAND_MEMORY);
> >  
> > -	/* Free the VRAM and doorbell BAR, we most likely need to move both. */
> > +	/* Tear down doorbell as resizing will release BARs */
> >  	amdgpu_doorbell_fini(adev);
> > -	if (adev->asic_type >= CHIP_BONAIRE)
> > -		pci_release_resource(adev->pdev, 2);
> > -
> > -	pci_release_resource(adev->pdev, 0);
> >  
> >  	r = pci_resize_resource(adev->pdev, 0, rbar_size);
> >  	if (r == -ENOSPC)
> > @@ -1743,8 +1739,6 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
> >  	else if (r && r != -ENOTSUPP)
> >  		dev_err(adev->dev, "Problem resizing BAR0 (%d).", r);
> >  
> > -	pci_assign_unassigned_bus_resources(adev->pdev->bus);
> > -
> >  	/* When the doorbell or fb BAR isn't available we have no chance of
> >  	 * using the device.
> >  	 */
>