Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <39435575-266e-59ef-a0fe-62870ba85129@amd.com>
Date:   Tue, 19 Apr 2022 16:08:39 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [PATCHv4] drm/amdgpu: disable ASPM on Intel Alder Lake based
 systems
Content-Language: en-US
To:     Nathan Chancellor <nathan@kernel.org>
Cc:     alexander.deucher@amd.com, christian.koenig@amd.com,
        xinhui.pan@amd.com, airlied@linux.ie, daniel@ffwll.ch,
        amd-gfx@lists.freedesktop.org, dri-devel@lists.freedesktop.org,
        linux-kernel@vger.kernel.org, mario.limonciello@amd.com,
        kernel test robot <lkp@intel.com>
References: <20220412215000.897344-1-richard.gong@amd.com>
 <Ylbu7OGHVaqnznQb@thelio-3990X>
From:   "Gong, Richard" <richard.gong@amd.com>
In-Reply-To: <Ylbu7OGHVaqnznQb@thelio-3990X>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?bk1mOHdNWUMzc0FJNTA5WlNuaVhHdGg0OVJXQW9lZ1V4UEVEQVZiL1pTcFFQ?=
 =?utf-8?B?WFBvaTB5RzFKbUVjcmZJTnRWNVpEQnpaZ1YvNFdlRERRZzJpN1lvZStTbTY2?=
 =?utf-8?B?Q2VuWlJqa294YVJYV1NGWmhNT3g5R2oyS1VybzNBRHdsSUNVa2phNWFZZWhF?=
 =?utf-8?B?S3loZDhlck5zcnc3M2tIU1FRQ2pPMzN1dHJ3YzQ5c0IvMlh6WWprOExZbHhG?=
 =?utf-8?B?dE90MTJSM1A4MFhMQkJHcy8yUHh4Mml4ZWZTcWlac1hoUXZRNVR4cU9JUk1E?=
 =?utf-8?B?cElrSnY3WVh4MWlSMjNlUmNsbUsyRTFKRHFtdXFTV2RSUEYvL1ZnT1o4dWRt?=
 =?utf-8?B?bUV2S21DU05UZnlIUGw2WGpJMGY0VXhrZFBYQW15Uld1bVZrbDQ4TmVIejZO?=
 =?utf-8?B?a0JzdFcxWXYzREhUOFFuWVRnc212UG11ekR4RXBCcjVWNTdXOVRhQ3NsOE1I?=
 =?utf-8?B?dVdWOFpaZXVEc3lEWlN1OWxvL3BReVlsWXBpa29CN1RLVG9WVkxFRnZsTW4x?=
 =?utf-8?B?aFRIcmk0V0NlcC9PRDlFejRRcG95U2FmWE5udGJaOVhmYTZjRkJoZXg3VmJF?=
 =?utf-8?B?MjROdmNvM3pTN2ZSeVJRUjVhVHlYTnhvZXJOelBDdlpRUGxzOHN3SGNKazRQ?=
 =?utf-8?B?emgzNEdzV3cwWGpKRkdsU3RjKzdVcUFaSlNIbk9pNUpPd1g2TmtjTGRsRWNE?=
 =?utf-8?B?enZKamE3T0V0VGVCZitUbm04bGZ6T21JSUZuMEFLUE9tZFEwelBQVnM5bXdB?=
 =?utf-8?B?SUxZQWtCNTBUejRkWWo4QlloUW1PM29vZ3R0VzVYVEhWWlZuUWttOHl2UE1N?=
 =?utf-8?B?by9EaVY1LzlBNnNZdm5Zamh2Q3U0d1NDUVRFSGdXakYrTWczanhqWVl1NFZk?=
 =?utf-8?B?czlUV0VGeVFjc1JOZFB4eGJTTWNHM0Znb210cVljKzlJUUdyT01tb1AwN0xs?=
 =?utf-8?B?cnRYUURvSU0zL3lKMDltUC9aRGo3ZXBRRWtKSjNNR2NGRS9qeHUyZ29GSUJQ?=
 =?utf-8?B?RlROekNEMW85ZnBpUnN3NEVVYzBYNTV5NG9WdWdCZjI2cWEzNmVFMGtuY2lh?=
 =?utf-8?B?Rm5jQnJZL081RjRiTy9HQ2ZrN1ZTeFdmbjZCUmdKNG9VZUtqWGNtQzQzaXFB?=
 =?utf-8?B?TnBJN2VDVnYxSDRoRy9vc1VrcWludnkzSUlBbXlqSFZVNkRVVEt0QmM0eVJI?=
 =?utf-8?B?ellPdzc3ZC9xNnZldExNUDdiNVJSUHd2aDlGTFh3SDZmYVE1M0RobFY1cGRT?=
 =?utf-8?B?aXF0eXRmWlZldXB3UWVML1VuV2FubkNjZzNLcU5EZHBLaVdCYTFxTS9ER1BC?=
 =?utf-8?B?NVc1SlZ4SEo0TFlWYlpOaFMveXNZSHVNOUJMbFBrUk1uVTZRNGVWcUd4QWJr?=
 =?utf-8?B?T1dna0pRWStSOHllYWFJbzNjRHRiMHZtZTRNelE5MWJSa3NyNlRsWVpyUjFj?=
 =?utf-8?B?WmF0T1BuL0I4MmpTemRDVHU1QUxYVm9qdC9sOERPdkt6L0gvSFBhMkV2NWov?=
 =?utf-8?B?NzNCT3ZGdUtFbXpLM25lZ1RoSkt0Q0laOWUzSkF1U3hqVjVYQisybi83WDZL?=
 =?utf-8?B?dlJJR1VnNEtNSUJuNHEybkVYVlRObFYvY25kSUdvTVBuOW4xTzh2R0h4VnpX?=
 =?utf-8?B?SU1jSHp3dkdBMEF4a3FmWUhwVzRsdC9TRDZQeHhNOFZSUmlERVBSNWx6SXZZ?=
 =?utf-8?B?TVBpVXNjUWNleS81TXlNYnBYUllreEtLQUtHenhOZTBFcTdadmovL3pBQVds?=
 =?utf-8?B?WFc5MSswWGlmTjlLcWFEYzMyWHpuOEpPeWFLM3EwQWxnNVY3MEdOSjBPMUdS?=
 =?utf-8?B?ZFFIVDBGYU8wcW1ZeWllNFN5eVZ5bWJUU3Q4ZHpUc0t5enBoNVhQRXkvTFBq?=
 =?utf-8?B?aXlZYmlyc2U2aXZQOHlTVzlqNVlYMkgvMUlZMzNKOHJBNXFiSFAxdVRhZERX?=
 =?utf-8?B?dzF0b0poSjc4QTVzRzdEQUVqcjdCUmdjL2pIcVdqMDhaeUx2OWhKNVBNSTlC?=
 =?utf-8?B?ZHdRSG1EcytJd2I4L2ZUQ0dXenVyU0Z0eDZIL3F2WG9JUGJSdDlvc3dkbkFI?=
 =?utf-8?B?c0hTOGN1b0srQmRqd2Q1UXJCVDlDaVhDeEQ4S2NwWi91MXdpR0ZLbHhGaEtj?=
 =?utf-8?B?Q3Y5Rmo2Vm1SSjVGYkhxUnN3VlhiMjNTaWcrbmo3cHNNT2UrbXlvc2VjVFJh?=
 =?utf-8?B?QXlWNVdYTUpuK0xlT3FOTDgwNzlRME5OclFOV21DclBJbjJRMVNkS2c0MHJK?=
 =?utf-8?B?ZHI3V2Z6NkhXKzZYQ0ZNSGpZVTc5QnUzRFZTeU5ldFVnVDBDMWc2QTRTVjJp?=
 =?utf-8?B?SGI0b3lXUkVqS3ZRcml0WlFkc2ViSlhHQ2xNNVFBRmRqVU5ZMWhuZz09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1acdd24d-ed04-4662-47a4-08da2248c8fc
X-MS-Exchange-CrossTenant-AuthSource: SA0PR12MB4526.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 19 Apr 2022 21:08:43.2575
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 2prgIdBiF1e24MuNKoIt+PdxS2aCDRn22J+0qFvdwwHBCZN2FNx6ks7BwM0mmqhFngwR8z16S3Es9qernMnF3A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH2PR12MB5545
X-Spam-Status: No, score=-5.4 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Nathan,

On 4/13/2022 10:40 AM, Nathan Chancellor wrote:
> Hi Richard,
>
> On Tue, Apr 12, 2022 at 04:50:00PM -0500, 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 won't work
>> with ASPM-enabled Intel Alder Lake based systems. Using these GFX cards as
>> video/display output, Intel Alder Lake based systems will hang during
>> suspend/resume.
>>
>> The issue was initially reported on one system (Dell Precision 3660 with
>> BIOS version 0.14.81), but was later confirmed to affect at least 4 Alder
>> Lake based systems.
>>
>> Add extra check to disable ASPM on Intel Alder Lake based systems.
>>
>> Fixes: 0064b0ce85bb ("drm/amd/pm: enable ASPM by default")
>> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitlab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1885&amp;data=04%7C01%7Crichard.gong%40amd.com%7C35699b2c088747daedf508da1d63f1f3%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637854612351767549%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=lzgZ3bV0PLFFl9uo3wt6N1dOoZpU2DqpddAk%2BTX8rEI%3D&amp;reserved=0
>> Reported-by: kernel test robot <lkp@intel.com>
>> Signed-off-by: Richard Gong <richard.gong@amd.com>
>> ---
>> v4: s/CONFIG_X86_64/CONFIG_X86
>>      enhanced check logic
>> v3: s/intel_core_asom_chk/aspm_support_quirk_check
>>      correct build error with W=1 option
>> 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..b33e0a9bee65 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)
>> +#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 aspm_support_quirk_check(void)
>> +{
>> +	if (IS_ENABLED(CONFIG_X86)) {
>> +		struct cpuinfo_x86 *c = &cpu_data(0);
>> +
>> +		return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
>> +	}
> I have not seen this reported by a bot, sorry if it is a duplicate. This
> breaks non-x86 builds (arm64 allmodconfig for example):
>
> drivers/gpu/drm/amd/amdgpu/vi.c:1144:28: error: implicit declaration of function 'cpu_data' is invalid in C99 [-Werror,-Wimplicit-function-declaration]
>                  struct cpuinfo_x86 *c = &cpu_data(0);
>                                           ^
> drivers/gpu/drm/amd/amdgpu/vi.c:1144:27: error: cannot take the address of an rvalue of type 'int'
>                  struct cpuinfo_x86 *c = &cpu_data(0);
>                                          ^~~~~~~~~~~~
> drivers/gpu/drm/amd/amdgpu/vi.c:1146:13: error: incomplete definition of type 'struct cpuinfo_x86'
>                  return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
>                           ~^
> drivers/gpu/drm/amd/amdgpu/vi.c:1144:10: note: forward declaration of 'struct cpuinfo_x86'
>                  struct cpuinfo_x86 *c = &cpu_data(0);
>                         ^
> drivers/gpu/drm/amd/amdgpu/vi.c:1146:28: error: incomplete definition of type 'struct cpuinfo_x86'
>                  return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
>                                          ~^
> drivers/gpu/drm/amd/amdgpu/vi.c:1144:10: note: forward declaration of 'struct cpuinfo_x86'
>                  struct cpuinfo_x86 *c = &cpu_data(0);
>                         ^
> drivers/gpu/drm/amd/amdgpu/vi.c:1146:43: error: use of undeclared identifier 'INTEL_FAM6_ALDERLAKE'
>                  return !(c->x86 == 6 && c->x86_model == INTEL_FAM6_ALDERLAKE);
>                                                          ^
> 5 errors generated.
>
> 'struct cpuinfo_x86' is only defined for CONFIG_X86 so this section
> needs to guarded with the preprocessor, which is how it was done in v2.
> Please go back to that.

Thanks, I will do that.

Regards,

Richard

>
> Cheers,
> Nathan