[AMD Official Use Only - General]
Thanks. I'm going to leave this series as the backup option, have another idea
that I'll have Koba try first.
> -----Original Message-----
> From: Quan, Evan <evan.q...@amd.com>
> Sent: Wednesday, July 5, 2023 20:04
> To: Limonciello, Mario <mario.limoncie...@amd.com>; amd-
> g...@lists.freedesktop.org
> Cc: koba...@canonical.com; Limonciello, Mario
> <mario.limoncie...@amd.com>
> Subject: RE: [RFC 1/2] drm/amd: Extend Intel ASPM quirk to all dGPUs
>
> [AMD Official Use Only - General]
>
> One small nitpick:
> It seems there is missing a default clause for the switch statement.
> Will that hit the compile warning about "a switch statement must have a
> default clause"?
> With that checked, the series is reviewed-by: Evan Quan
> <evan.q...@amd.com>
>
> Evan
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-boun...@lists.freedesktop.org> On Behalf Of
> Mario
> > Limonciello
> > Sent: Thursday, July 6, 2023 2:07 AM
> > To: amd-gfx@lists.freedesktop.org
> > Cc: koba...@canonical.com; Limonciello, Mario
> > <mario.limoncie...@amd.com>
> > Subject: [RFC 1/2] drm/amd: Extend Intel ASPM quirk to all dGPUs
> >
> > More failures are reported across additional products and so it seems
> > unless we have a handle on the fundmental ASPM incompatibilities with
> > Intel host and AMD dGPU, we should not allow them on problematic hosts.
> >
> > Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
> > Signed-off-by: Mario Limonciello <mario.limoncie...@amd.com>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 -
> > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 39 +++++++++++++++-
> --
> > ----
> > drivers/gpu/drm/amd/amdgpu/nv.c | 2 +-
> > drivers/gpu/drm/amd/amdgpu/vi.c | 2 +-
> > 4 files changed, 29 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > index dc4dc1446a19..294a549e7499 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> > @@ -1314,7 +1314,6 @@ void amdgpu_device_pci_config_reset(struct
> > amdgpu_device *adev);
> > int amdgpu_device_pci_reset(struct amdgpu_device *adev);
> > bool amdgpu_device_need_post(struct amdgpu_device *adev);
> > bool amdgpu_device_should_use_aspm(struct amdgpu_device *adev);
> > -bool amdgpu_device_aspm_support_quirk(void);
> >
> > void amdgpu_cs_report_moved_bytes(struct amdgpu_device *adev, u64
> > num_bytes,
> > u64 num_vis_bytes);
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > index 7f069e1731fe..ef22a0a6065e 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> > @@ -1458,6 +1458,30 @@ bool amdgpu_device_need_post(struct
> > amdgpu_device *adev)
> > return true;
> > }
> >
> > +static bool amdgpu_device_aspm_support_quirk(void)
> > +{
> > +#if IS_ENABLED(CONFIG_X86)
> > + struct cpuinfo_x86 *c = &cpu_data(0);
> > +
> > + if (c->x86 != 6)
> > + return true;
> > +
> > + switch (c->x86_model) {
> > + /* Problems reported for Alder Lake
> > + * Volcanic Islands:
> > + * https://gitlab.freedesktop.org/drm/amd/-/issues/1885
> > + * e02fe3bc7aba2 ("drm/amdgpu: vi: disable ASPM on Intel Alder
> > Lake based systems")
> > + * Navi 1x cards:
> > + * https://gitlab.freedesktop.org/drm/amd/-/issues/2458
> > + * c08c079692da0 ("drm/amdgpu/nv: Apply ASPM quirk on Intel
> > ADL + AMD Navi")
> > + */
> > + case INTEL_FAM6_ALDERLAKE:
> > + return false;
> > + }
> > +#endif
> > + return true;
> > +}
> > +
> > /**
> > * amdgpu_device_should_use_aspm - check if the device should program
> > ASPM
> > *
> > @@ -1480,18 +1504,9 @@ bool amdgpu_device_should_use_aspm(struct
> > amdgpu_device *adev)
> > default:
> > return false;
> > }
> > - return pcie_aspm_enabled(adev->pdev);
> > -}
> > -
> > -bool amdgpu_device_aspm_support_quirk(void)
> > -{
> > -#if IS_ENABLED(CONFIG_X86)
> > - struct cpuinfo_x86 *c = &cpu_data(0);
> > -
> > - return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
> > -#else
> > - return true;
> > -#endif
> > + if (!pcie_aspm_enabled(adev->pdev))
> > + return false;
> > + return amdgpu_device_aspm_support_quirk();
> > }
> >
> > /* if we get transitioned to only one device, take VGA back */
> > diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c
> > b/drivers/gpu/drm/amd/amdgpu/nv.c
> > index 51523b27a186..4f8e696d9ba2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/nv.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
> > @@ -527,7 +527,7 @@ static int nv_set_vce_clocks(struct amdgpu_device
> > *adev, u32 evclk, u32 ecclk)
> >
> > static void nv_program_aspm(struct amdgpu_device *adev)
> > {
> > - if (!amdgpu_device_should_use_aspm(adev)
> > || !amdgpu_device_aspm_support_quirk())
> > + if (!amdgpu_device_should_use_aspm(adev))
> > return;
> >
> > if (!(adev->flags & AMD_IS_APU) &&
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
> > b/drivers/gpu/drm/amd/amdgpu/vi.c
> > index 6a8494f98d3e..c413cac369e4 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vi.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
> > @@ -1124,7 +1124,7 @@ static void vi_program_aspm(struct
> > amdgpu_device *adev)
> > bool bL1SS = false;
> > bool bClkReqSupport = true;
> >
> > - if (!amdgpu_device_should_use_aspm(adev)
> > || !amdgpu_device_aspm_support_quirk())
> > + if (!amdgpu_device_should_use_aspm(adev))
> > return;
> >
> > if (adev->flags & AMD_IS_APU ||
> > --
> > 2.34.1
>
