[RFC 1/2] drm/amd: Extend Intel ASPM quirk to all dGPUs

[Limonciello, Mario]

[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.Quan at amd.com>
> Sent: Wednesday, July 5, 2023 20:04
> To: Limonciello, Mario <Mario.Limonciello at amd.com>; amd-
> gfx at lists.freedesktop.org
> Cc: koba.ko at canonical.com; Limonciello, Mario
> <Mario.Limonciello at 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.quan at amd.com>
>
> Evan
> > -----Original Message-----
> > From: amd-gfx <amd-gfx-bounces at lists.freedesktop.org> On Behalf Of
> Mario
> > Limonciello
> > Sent: Thursday, July 6, 2023 2:07 AM
> > To: amd-gfx at lists.freedesktop.org
> > Cc: koba.ko at canonical.com; Limonciello, Mario
> > <Mario.Limonciello at 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.limonciello at 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
>