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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



[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@xxxxxxx>

Evan
> -----Original Message-----
> From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Mario
> Limonciello
> Sent: Thursday, July 6, 2023 2:07 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: koba.ko@xxxxxxxxxxxxx; Limonciello, Mario
> <Mario.Limonciello@xxxxxxx>
> 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@xxxxxxx>
> ---
>  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





[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux