Re: [PATCHv2] drm/amdgpu: disable ASPM on Intel AlderLake based systems

From: Gong, Richard
Date: Mon Apr 11 2022 - 07:12:18 EST


Hi Lijo,

On 4/10/2022 11:15 PM, Lazar, Lijo wrote:


On 4/9/2022 12:35 AM, Richard Gong wrote:
Active State Power Management (ASPM) feature is enabled since kernel 5.14.
There are some AMD GFX cards (such as WX3200 and RX640) that cannot be
used with Intel AlderLake based systems to enable ASPM. Using these GFX
cards as video/display output, Intel Alder Lake based systems will hang
during suspend/resume.

Add extra check to disable ASPM on Intel AlderLake based systems.

Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
Link: https://gitlab.freedesktop.org/drm/amd/-/issues/1885
Signed-off-by: Richard Gong <richard.gong@xxxxxxx>
---
v2: correct commit description
     move the check from chip family to problematic platform
---
  drivers/gpu/drm/amd/amdgpu/vi.c | 17 ++++++++++++++++-
  1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c b/drivers/gpu/drm/amd/amdgpu/vi.c
index 039b90cdc3bc..8b4eaf54b23e 100644
--- a/drivers/gpu/drm/amd/amdgpu/vi.c
+++ b/drivers/gpu/drm/amd/amdgpu/vi.c
@@ -81,6 +81,10 @@
  #include "mxgpu_vi.h"
  #include "amdgpu_dm.h"
  +#if IS_ENABLED(CONFIG_X86_64)
+#include <asm/intel-family.h>
+#endif
+
  #define ixPCIE_LC_L1_PM_SUBSTATE    0x100100C6
  #define PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK 0x00000001L
  #define PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK 0x00000002L
@@ -1134,13 +1138,24 @@ static void vi_enable_aspm(struct amdgpu_device *adev)
          WREG32_PCIE(ixPCIE_LC_CNTL, data);
  }
  +static bool intel_core_apsm_chk(void)

If this is only for Dell systems, use DMI_SYS_VENDOR/DMI_PRODUCT_NAME to identify the platform information from SMBIOS.
Not sure, the report issue was originally for Dell system but may occur with others. This is why I just check CPU info.

Better to rename to aspm_support_quirk_check() or similar, and return false on is_alderlake() or is_dell_xyz();

Ok, will rename to aspm_support_quick_check in the next versin.

Thanks,
Lijo

+{
+#if IS_ENABLED(CONFIG_X86_64)
+    struct cpuinfo_x86 *c = &cpu_data(0);
+
+    return (c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
+#else
+    return false;
+#endif
+}
+
  static void vi_program_aspm(struct amdgpu_device *adev)
  {
      u32 data, data1, orig;
      bool bL1SS = false;
      bool bClkReqSupport = true;
  -    if (!amdgpu_device_should_use_aspm(adev))
+    if (!amdgpu_device_should_use_aspm(adev) || intel_core_apsm_chk())
          return;
        if (adev->flags & AMD_IS_APU ||