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: <3bef108e-2e62-d7d3-af44-f178779702e5@amd.com>
Date:   Wed, 29 Mar 2023 08:18:02 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.8.0
Subject: Re: [PATCH 2/2] drm/amdgpu: Remove ASPM workaround on VI and NV
Content-Language: en-US
To:     "Gong, Richard" <richard.gong@amd.com>,
        Kai-Heng Feng <kai.heng.feng@canonical.com>,
        alexander.deucher@amd.com, christian.koenig@amd.com,
        Xinhui.Pan@amd.com
Cc:     David Airlie <airlied@gmail.com>, Daniel Vetter <daniel@ffwll.ch>,
        Hawking Zhang <Hawking.Zhang@amd.com>,
        Likun Gao <Likun.Gao@amd.com>,
        Felix Kuehling <Felix.Kuehling@amd.com>,
        Victor Zhao <Victor.Zhao@amd.com>,
        Jack Xiao <Jack.Xiao@amd.com>,
        Somalapuram Amaranath <Amaranath.Somalapuram@amd.com>,
        Maarten Lankhorst <maarten.lankhorst@linux.intel.com>,
        Lijo Lazar <lijo.lazar@amd.com>,
        YiPeng Chai <YiPeng.Chai@amd.com>,
        Andrey Grodzovsky <andrey.grodzovsky@amd.com>,
        Bokun Zhang <Bokun.Zhang@amd.com>,
        Guchun Chen <guchun.chen@amd.com>,
        Veerabadhran Gopalakrishnan <veerabadhran.gopalakrishnan@amd.com>,
        Evan Quan <evan.quan@amd.com>, amd-gfx@lists.freedesktop.org,
        dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org
References: <20230329095933.1203559-1-kai.heng.feng@canonical.com>
 <20230329095933.1203559-2-kai.heng.feng@canonical.com>
 <9c8d928d-f322-e9aa-c811-28682ca70f04@amd.com>
From:   Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <9c8d928d-f322-e9aa-c811-28682ca70f04@amd.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?WVZCcjdySE55YkQwN3BRYkVQZkpIckx3VGsyU2Z1MWJTMWpzL3c3ald4blBl?=
 =?utf-8?B?MHVISUhQRkovUGJIYzNwcFNrVzRPQUM5ZlZPU2J3emh5aEVCQWFXQkpBQmNV?=
 =?utf-8?B?azRDYUg3TXlpeDNlMU1sdUtMK3Ric2NRcGt4QWMzS1laalJGMzJxVW5reUlR?=
 =?utf-8?B?OXdZZEthOFprREc3d2NlOEJzb0ZvV1NwODYxeWhyRWx5UnJIUFJpU2pId0R2?=
 =?utf-8?B?UDdsTk9zNXFITDUyN1htY3ZmUld1RHRXc1lCMHRyTmp1b0VMODloZzZsSVpm?=
 =?utf-8?B?M0RpT2R6NG9GSFFUNzBvY0dkNzBqcWZtaEhzYVVoTHFTTm84RGJ5VGdWTHU1?=
 =?utf-8?B?WmIrZ0xMUnVncXZsT1lnc0wyZTZrUU5mc1dNRVYrZXFhMmpVVkFZZi9hZEZr?=
 =?utf-8?B?Ly9RV2hUeW9mZG1XT2tjak51Yk1rem0zbWI3YjREcDNSdmpQQlJPOFdSa3U5?=
 =?utf-8?B?WHpWbEFVSUUxNnJ6SDF1R1o3bGRZa1NLaFplb1BuZm1JckV3alNySnBiME1P?=
 =?utf-8?B?M3Q4N29oMHZnNXVlQmMzSkcxNnVMbFpFKzNCOEwwN1NHTVRGU3MrKzlnY0Zq?=
 =?utf-8?B?SlllU3hWbHVpVDBsUzNqbXBEcXdMNUZ1dzJhenBhR3Y4R1hzK0pxZWg1MVFP?=
 =?utf-8?B?T3ZHTUl6N1RMVk5ydjNMS3A2bnhQVzFMTHR3YUcyT0VHd2UvalJQbEhLVXBy?=
 =?utf-8?B?K1lzZG9lWHNGSHJaRkZ3VWV5ZklzUTF6RGlCYTZITWNobGV1Y1FWTVZ4R0lT?=
 =?utf-8?B?cE8yc2JKOGNPbGpYU255MXBFbE1pV2lZN0tjcnJIRDhOc2dCQlQxNWV6THpI?=
 =?utf-8?B?Q2ZRdkxOMElkSCtCUUxwYnRJcDN4azB3UWNrcDBQTlhwYlA1MitSditqcHlB?=
 =?utf-8?B?Q2txdE9rTmxpV0xGaGRUTnE3ZU5CeVNkaHZNQVZOSDNEQXA2elJyaGVuZElU?=
 =?utf-8?B?UmZRY1F3Z0xWUjg5ZkRpajdjakFaOGc5RjVvV1UwbFBaSERCbUhkd2Vzdmh5?=
 =?utf-8?B?L09rR3RNZGFNeWIyY3BsaytTZnhmdURydjhnSHV0NjhjRUdEVTQxdUlKODFH?=
 =?utf-8?B?NmlYUjRsQnNnbXdlNVFNdnhLamd4bHFuZkNESUl2Nk1IQTFoVHRiQVZKRHMr?=
 =?utf-8?B?TSs0SlA2blUyT2xnNlpNWWtGNHBQNERFTzRsWU5IYjF5UkVHblc3ZnRpT0dw?=
 =?utf-8?B?bVNhd3d1OVdrTFVLc0pXdGwzVHk1RkhXeXhTTDYyS2xuK3hYYTB1TmVnWjBB?=
 =?utf-8?B?cnVOcTliUWVJVWNqZUVpRENFTnJtWWg1am9UVEJqaldEZGVDVU11c3h5emJI?=
 =?utf-8?B?MDMySDdKYzZiMzFtNyswVEp0SGdyOVhtMkl5NkFUZmZURHU3TngvbUhwZllj?=
 =?utf-8?B?Rk5UTTNwbEJtN2Z5cDc4RlZmK213eTB3OC9sWFg5SlZIR2hmTWFCYmxsaVds?=
 =?utf-8?B?T0FTQ2tYc2Zya2lGa05LMUU5d0c0S3VDd25GTzUxQnBXc2pGUzMwZ2RURlgr?=
 =?utf-8?B?UEF5YndWOWd0OTdEbGE1VXA1Yng4RXZXSlJrU2JTRmV4Qms0RkM0eEx6YXBJ?=
 =?utf-8?B?RFkrYnNwOWJCbEYrV2tWNEVkS2ZLUk9iN2ZadDhmSXBmamplWWF4UWdIY2Vo?=
 =?utf-8?B?d1psTXdaTUp0YlV1YVgrbTJYYVcwVER0SnZpTFpjUjM5RWhVQzJNVWxlRWtr?=
 =?utf-8?B?MHRTUDczVkZ1bHRnWmp2QlNNU0FWT1VjWjdpU09Wa1l5ZUtvTzRweGFwcGls?=
 =?utf-8?B?TldSVURaUml0dmM2ZmkxMy9zSUVMSW5jakRncmFMN1h0TW5sbnlrR2JYV1BX?=
 =?utf-8?B?UnhNVTM3K3ZaKzdzclJ3cDZMaW5YNUZNbyszcDJ5RlNTUVVYVlY3a3FVZUg5?=
 =?utf-8?B?czludWtiTzZXWTZmSzNkTVBPcFpZMUE1Zk44WG1KM0tTK1A0SHRXamZtdVdu?=
 =?utf-8?B?WHUxTGEzREdqZWdKUzdCTWNlQUdyM3hqSS9nR2hObXozaC80cmZkdFpZdW0z?=
 =?utf-8?B?c0FaaG5VaThXRHpWalU1YW94UVBwN25CWGtkVmpZTFMrTGZaMW83Nng0cjFi?=
 =?utf-8?B?WW1yN1NMMGdEUTVWZ1Y4YmtncjRORThOYXVkTUtIZ1UxTXZrSTNHdUVsdlg2?=
 =?utf-8?B?NGoxdStzNzYzMFFmSlE1aGlZMmpLaVZ3VGlnMTFYRHFtajlCaGYrdkZqS0pr?=
 =?utf-8?Q?vGpTkI6yM2oY2knMaJWQQx+Z14O6Ce69G+IFH2qF0yIF?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 54dfec31-d0d7-496d-4991-08db30580885
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Mar 2023 13:18:06.4736
 (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: Fpf7vdbooyOLLfRWnyrEC/MsQKgryvc539+LR6p8YotdYAdB5oFNQj3vfBSw3A+kgxsDerrznopW5WAg4JCqDw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ1PR12MB6340
X-Spam-Status: No, score=-0.2 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
        DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE
        autolearn=unavailable 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


On 3/29/23 08:08, Gong, Richard wrote:
>
> On 3/29/2023 4:59 AM, Kai-Heng Feng wrote:
>> Since the original issue is resolved by a new fix, the ASPM workaround
>> can be dropped.
> What is the new fix? Can you elaborate more or add the new fix commit 
> here?

It's his first patch in the series, but yes I agree it should be 
mentioned in this

message explicitly what about that means this can be dropped.

>>
>> Signed-off-by: Kai-Heng Feng <kai.heng.feng@canonical.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 15 ---------------
>>   drivers/gpu/drm/amd/amdgpu/nv.c            |  2 +-
>>   drivers/gpu/drm/amd/amdgpu/vi.c            |  2 +-
>>   4 files changed, 2 insertions(+), 18 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h 
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> index 8cf2cc50b3de..a19a6489b117 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
>> @@ -1248,7 +1248,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 d56b7a2bafa6..0cacace2d6c2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -81,10 +81,6 @@
>>     #include <drm/drm_drv.h>
>>   -#if IS_ENABLED(CONFIG_X86)
>> -#include <asm/intel-family.h>
>> -#endif
>> -
>>   MODULE_FIRMWARE("amdgpu/vega10_gpu_info.bin");
>>   MODULE_FIRMWARE("amdgpu/vega12_gpu_info.bin");
>>   MODULE_FIRMWARE("amdgpu/raven_gpu_info.bin");
>> @@ -1377,17 +1373,6 @@ bool amdgpu_device_should_use_aspm(struct 
>> amdgpu_device *adev)
>>       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 we get transitioned to only one device, take VGA back */
>>   /**
>>    * amdgpu_device_vga_set_decode - enable/disable vga decode
>> diff --git a/drivers/gpu/drm/amd/amdgpu/nv.c 
>> b/drivers/gpu/drm/amd/amdgpu/nv.c
>> index 47420b403871..15f3c6745ea9 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/nv.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/nv.c
>> @@ -522,7 +522,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 531f173ade2d..81dcb1148a60 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vi.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vi.c
>> @@ -1122,7 +1122,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 ||