[Public]
Should we just check the amdgpu runpm param value?
Thanks,
Lijo
From: Alex Deucher <alexdeucher@xxxxxxxxx>
Sent: Wednesday, February 19, 2025 9:45:28 PM
To: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: disable BAR resize on Dell G5 SE
Sent: Wednesday, February 19, 2025 9:45:28 PM
To: Lazar, Lijo <Lijo.Lazar@xxxxxxx>
Cc: Deucher, Alexander <Alexander.Deucher@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx <amd-gfx@xxxxxxxxxxxxxxxxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: disable BAR resize on Dell G5 SE
On Wed, Feb 19, 2025 at 10:42 AM Lazar, Lijo <lijo.lazar@xxxxxxx> wrote:
>
>
>
> On 2/19/2025 8:02 PM, Alex Deucher wrote:
> > On Tue, Feb 18, 2025 at 11:05 AM Lazar, Lijo <lijo.lazar@xxxxxxx> wrote:
> >>
> >>
> >>
> >> On 2/18/2025 8:38 PM, Alex Deucher wrote:
> >>> There was a quirk added to add a workaround for a Sapphire
> >>> RX 5600 XT Pulse that didn't allow BAR resizing. However,
> >>> the quirk casused a regression on Dell laptops using those
> >>> chips, rather than narrowing the scope of the resizing
> >>> quirk, add a quirk to prevent amdgpu from resizing the BAR
> >>> on those Dell platforms.
> >>>
> >>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/1707
> >>> Fixes: 907830b0fc9e ("PCI: Add a REBAR size quirk for Sapphire RX 5600 XT Pulse")
> >>> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> >>
> >> As per the issue thread, issue happens when dGPU resumes from runpm. If
> >> so, can't we disable runpm as the workaround (IMO, the current patch
> >> also is a workaround)?
> >
> > This seems like a better workaround than disabling runtime pm.
> > Leaving the dGPU powered up all the time seems worse than limiting the
> > BAR size on this platform.
> >
>
> Probably, it's better to note down that as well in the description.
> Also, check if the device has runpm enabled. If a user prefers
> performance, say runpm disabled through driver option, then no need to
> apply this.
I can update the commit message, but the runtime pm method gets set
much later in the init sequence so we don't know what it is or if it
is enabled when we resize the BAR.
Alex
>
> Thanks,
> Lijo
>
> > Alex
> >
> >>
> >> Thanks,
> >> Lijo
> >>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> index 512e642477a7e..56fd874a22de8 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> @@ -1662,6 +1662,12 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
> >>> if (amdgpu_sriov_vf(adev))
> >>> return 0;
> >>>
> >>> + /* skip resizing on Dell G5 SE platforms */
> >>> + if (adev->pdev->vendor == PCI_VENDOR_ID_ATI &&
> >>> + adev->pdev->device == 0x731f &&
> >>> + adev->pdev->subsystem_vendor == PCI_VENDOR_ID_DELL)
> >>> + return 0;
> >>> +
> >>> /* PCI_EXT_CAP_ID_VNDR extended capability is located at 0x100 */
> >>> if (!pci_find_ext_capability(adev->pdev, PCI_EXT_CAP_ID_VNDR))
> >>> DRM_WARN("System can't access extended configuration space, please check!!\n");
> >>
>
>
>
>
> On 2/19/2025 8:02 PM, Alex Deucher wrote:
> > On Tue, Feb 18, 2025 at 11:05 AM Lazar, Lijo <lijo.lazar@xxxxxxx> wrote:
> >>
> >>
> >>
> >> On 2/18/2025 8:38 PM, Alex Deucher wrote:
> >>> There was a quirk added to add a workaround for a Sapphire
> >>> RX 5600 XT Pulse that didn't allow BAR resizing. However,
> >>> the quirk casused a regression on Dell laptops using those
> >>> chips, rather than narrowing the scope of the resizing
> >>> quirk, add a quirk to prevent amdgpu from resizing the BAR
> >>> on those Dell platforms.
> >>>
> >>> Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/1707
> >>> Fixes: 907830b0fc9e ("PCI: Add a REBAR size quirk for Sapphire RX 5600 XT Pulse")
> >>> Signed-off-by: Alex Deucher <alexander.deucher@xxxxxxx>
> >>
> >> As per the issue thread, issue happens when dGPU resumes from runpm. If
> >> so, can't we disable runpm as the workaround (IMO, the current patch
> >> also is a workaround)?
> >
> > This seems like a better workaround than disabling runtime pm.
> > Leaving the dGPU powered up all the time seems worse than limiting the
> > BAR size on this platform.
> >
>
> Probably, it's better to note down that as well in the description.
> Also, check if the device has runpm enabled. If a user prefers
> performance, say runpm disabled through driver option, then no need to
> apply this.
I can update the commit message, but the runtime pm method gets set
much later in the init sequence so we don't know what it is or if it
is enabled when we resize the BAR.
Alex
>
> Thanks,
> Lijo
>
> > Alex
> >
> >>
> >> Thanks,
> >> Lijo
> >>
> >>> ---
> >>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++++
> >>> 1 file changed, 6 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> index 512e642477a7e..56fd874a22de8 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> >>> @@ -1662,6 +1662,12 @@ int amdgpu_device_resize_fb_bar(struct amdgpu_device *adev)
> >>> if (amdgpu_sriov_vf(adev))
> >>> return 0;
> >>>
> >>> + /* skip resizing on Dell G5 SE platforms */
> >>> + if (adev->pdev->vendor == PCI_VENDOR_ID_ATI &&
> >>> + adev->pdev->device == 0x731f &&
> >>> + adev->pdev->subsystem_vendor == PCI_VENDOR_ID_DELL)
> >>> + return 0;
> >>> +
> >>> /* PCI_EXT_CAP_ID_VNDR extended capability is located at 0x100 */
> >>> if (!pci_find_ext_capability(adev->pdev, PCI_EXT_CAP_ID_VNDR))
> >>> DRM_WARN("System can't access extended configuration space, please check!!\n");
> >>
>
