<html><head>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
  </head>
  <body>
    <p>Hi Nils,<br>
    </p>
    <div class="moz-cite-prefix">On 4/10/2022 8:54 AM, Nils Wallménius
      wrote:<br>
    </div>
    <blockquote type="cite" cite="mid:CA+nq7DsCU97Ana34GcyQZ_GzR4eiQbzkzPLUBRQTAtNYa-OiLA@mail.gmail.com">
      
      <div dir="auto">
        <div>Hi Richard, see inline comment.<br>
          <br>
          <div class="gmail_quote">
            <div dir="ltr" class="gmail_attr">Den fre 8 apr. 2022
              21:05Richard Gong <<a href="mailto:richard.gong@amd.com" moz-do-not-send="true" class="moz-txt-link-freetext">richard.gong@amd.com</a>>
              skrev:<br>
            </div>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">Active
              State Power Management (ASPM) feature is enabled since
              kernel 5.14.<br>
              There are some AMD GFX cards (such as WX3200 and RX640)
              that cannot be<br>
              used with Intel AlderLake based systems to enable ASPM.
              Using these GFX<br>
              cards as video/display output, Intel Alder Lake based
              systems will hang<br>
              during suspend/resume.<br>
              <br>
              Add extra check to disable ASPM on Intel AlderLake based
              systems.<br>
              <br>
              Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")<br>
              Link: <a href="https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1885&data=04%7C01%7Crichard.gong%40amd.com%7Cd46b7fb14c6d44a277d408da1af99f63%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637851956929855129%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&sdata=B3SXfB65dqx3EBQzUZh6xCmtpO9ynPj3PvwrWe9Q%2BpU%3D&reserved=0" originalsrc="https://gitlab.freedesktop.org/drm/amd/-/issues/1885" shash="QJ86gCfnpDRZOYDu6DNKyhICvZCTAiKthyZJL7UwoaEmZWBseLQiGXPOKcbAes5grQMqNdYcl0xvG4KtbFZYpfMpiwjpPMWdd8ZKhwzKyne9wejyvRaDfhuo7NK6KnKdzlOtP8yxfR4sMoMZaOxTy/GCBFSKgVT6cmg5w2urqSE=" rel="noreferrer noreferrer" target="_blank" moz-do-not-send="true">https://gitlab.freedesktop.org/drm/amd/-/issues/1885</a><br>
              Signed-off-by: Richard Gong <<a href="mailto:richard.gong@amd.com" target="_blank" rel="noreferrer" moz-do-not-send="true" class="moz-txt-link-freetext">richard.gong@amd.com</a>><br>
              ---<br>
              v2: correct commit description<br>
                  move the check from chip family to problematic
              platform<br>
              ---<br>
               drivers/gpu/drm/amd/amdgpu/vi.c | 17 ++++++++++++++++-<br>
               1 file changed, 16 insertions(+), 1 deletion(-)<br>
              <br>
              diff --git a/drivers/gpu/drm/amd/amdgpu/vi.c
              b/drivers/gpu/drm/amd/amdgpu/vi.c<br>
              index 039b90cdc3bc..8b4eaf54b23e 100644<br>
              --- a/drivers/gpu/drm/amd/amdgpu/vi.c<br>
              +++ b/drivers/gpu/drm/amd/amdgpu/vi.c<br>
              @@ -81,6 +81,10 @@<br>
               #include "mxgpu_vi.h"<br>
               #include "amdgpu_dm.h"<br>
              <br>
              +#if IS_ENABLED(CONFIG_X86_64)<br>
              +#include <asm/intel-family.h><br>
              +#endif<br>
              +<br>
               #define ixPCIE_LC_L1_PM_SUBSTATE       0x100100C6<br>
               #define
              PCIE_LC_L1_PM_SUBSTATE__LC_L1_SUBSTATES_OVERRIDE_EN_MASK 
                   0x00000001L<br>
               #define
              PCIE_LC_L1_PM_SUBSTATE__LC_PCI_PM_L1_2_OVERRIDE_MASK 
               0x00000002L<br>
              @@ -1134,13 +1138,24 @@ static void vi_enable_aspm(struct
              amdgpu_device *adev)<br>
                              WREG32_PCIE(ixPCIE_LC_CNTL, data);<br>
               }<br>
            </blockquote>
          </div>
        </div>
        <div dir="auto"><br>
        </div>
        <div dir="auto">There's a typo in the new function name
          apsm/aspm. Btw might be worth a comment why this check is
          done?</div>
        <div dir="auto"><br>
        </div>
      </div>
    </blockquote>
    <p>Thanks for review, <br>
    </p>
    <p>s/intel_core_apsm_chk/intel_core_aspm_chk in the next view.</p>
    <p>Regards,</p>
    <p>Richard<br>
    </p>
    <blockquote type="cite" cite="mid:CA+nq7DsCU97Ana34GcyQZ_GzR4eiQbzkzPLUBRQTAtNYa-OiLA@mail.gmail.com">
      <div dir="auto">
        <div dir="auto">Regards</div>
        <div dir="auto">Nils</div>
        <div dir="auto"><br>
        </div>
        <div dir="auto">
          <div class="gmail_quote">
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex"><br>
            </blockquote>
            <blockquote class="gmail_quote" style="margin:0 0 0
              .8ex;border-left:1px #ccc solid;padding-left:1ex">
              +static bool intel_core_apsm_chk(void)<br>
              +{<br>
              +#if IS_ENABLED(CONFIG_X86_64)<br>
              +       struct cpuinfo_x86 *c = &cpu_data(0);<br>
              +<br>
              +       return (c->x86 == 6 && c->x86_model
              == INTEL_FAM6_ALDERLAKE);<br>
              +#else<br>
              +       return false;<br>
              +#endif<br>
              +}<br>
              +<br>
               static void vi_program_aspm(struct amdgpu_device *adev)<br>
               {<br>
                      u32 data, data1, orig;<br>
                      bool bL1SS = false;<br>
                      bool bClkReqSupport = true;<br>
              <br>
              -       if (!amdgpu_device_should_use_aspm(adev))<br>
              +       if (!amdgpu_device_should_use_aspm(adev) ||
              intel_core_apsm_chk())<br>
                              return;<br>
              <br>
                      if (adev->flags & AMD_IS_APU ||<br>
              -- <br>
              2.25.1<br>
              <br>
            </blockquote>
          </div>
        </div>
      </div>
    </blockquote>
  </body>
</html>