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: <2603fe3e-2b8b-0161-2812-6c1f21eda02b@amd.com>
Date:   Fri, 8 Apr 2022 11:15:39 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.2.0
Subject: Re: [PATCH] drm/amdgpu: disable ASPM for legacy products that don't
 support ASPM
Content-Language: en-US
To:     Alex Deucher <alexdeucher@gmail.com>,
        "Limonciello, Mario" <Mario.Limonciello@amd.com>
Cc:     "Deucher, Alexander" <Alexander.Deucher@amd.com>,
        "Koenig, Christian" <Christian.Koenig@amd.com>,
        "Pan, Xinhui" <Xinhui.Pan@amd.com>,
        "airlied@linux.ie" <airlied@linux.ie>,
        "daniel@ffwll.ch" <daniel@ffwll.ch>,
        "dri-devel@lists.freedesktop.org" <dri-devel@lists.freedesktop.org>,
        "amd-gfx@lists.freedesktop.org" <amd-gfx@lists.freedesktop.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
References: <20220408154447.3519453-1-richard.gong@amd.com>
 <BL1PR12MB51576654D3EEB10F5DF862A7E2E99@BL1PR12MB5157.namprd12.prod.outlook.com>
 <CADnq5_PmxGxrJG5uZkkFXQ1YbJbDZTvAqb2oYqdCE=NtqBojqw@mail.gmail.com>
From:   "Gong, Richard" <richard.gong@amd.com>
In-Reply-To: <CADnq5_PmxGxrJG5uZkkFXQ1YbJbDZTvAqb2oYqdCE=NtqBojqw@mail.gmail.com>
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?RlZoMktiWitxMVpLa0JhSFNxWjRZWE9qeG9LYUk1eEQ3NW0rRXNkb2FoODcx?=
 =?utf-8?B?MkRucUphdkg5cURVM0EyV0NIRjZaK1VMbmQrZDM1WTlWaDY4ZHNySXo2TXZt?=
 =?utf-8?B?STN3NndNMW1QWU55RkgyTFN5aStqYlh6OS9IMThXWWF1eTNGSVp5K1c0VjFk?=
 =?utf-8?B?UWMxTHJoVFJlSUVTNHhqK1V2OVdYSVFycEF1VVZENWlpQkpZYm95WExEdHYv?=
 =?utf-8?B?dFZBaENnUm9FMHJHWGZGZUFYejZjcGlUa2g1V3RBRzFsNjlOTFcxR1YrMVVl?=
 =?utf-8?B?Ylk2QmZsQ1FMS01wajEzSXB5c1cxVytPWktPZFVGa2xHbHFFQVd0dzZIVUcx?=
 =?utf-8?B?WXdoa2tuSWxjeHpRbWhyV2JvMmVsWnZ6d1BQaXVDUjIydTBEYm16L2FXM25K?=
 =?utf-8?B?S3hnY0FPUUNiMkJxQUluZGttb2hyNVRIRUY5N28vMzlIUTlXbGsxZnFQQU5M?=
 =?utf-8?B?Mk9GTC9pSk9NWncraHk1UkVwSHcvR2h0Qk1BVFJiVklhSjdRZ0lXQWhNem5o?=
 =?utf-8?B?RUJ5QVM1RkZvd1RhaXdsTG5iSENMWjlzYkZyOWMyamlrZSs2aVRsdUUvTnZR?=
 =?utf-8?B?c2UxYldMVkNWWWg5RTB1STBrQk1nV2FVSTREbVpWVG5ucjlLTFNubXk1dHht?=
 =?utf-8?B?a2htdWRRQk81N3FBRExQd0dKSFU2am95ejhvMWdVQVNtUXF2UVFreUd5MHho?=
 =?utf-8?B?LzNmUkZ0cHlvd3l1eEpLbEF6VExuell5T3pkRTFGYjRPWXJPZDEwYTROd0FB?=
 =?utf-8?B?MUR4RWhpaFBDWjUwdjhxQjhvWE83Z1BsNFg2Ly9UOG1yZjBsTTh6SXdHT2V6?=
 =?utf-8?B?Zmh0SDdGeXM3UjE1TFNLVlFINS9ZV1FWdmh4NmFPUFRoMmltRlFneC9hQmJ0?=
 =?utf-8?B?TXh2K2NpRFIwcU0rbHdYSVNPUjdWY3BXZnRyZk1PQUZzWkJ0bzgwRGYrYlgz?=
 =?utf-8?B?Qmk2aXBiaXVXY29lRDF3VVVjTGk5NjBpSTNzYVJYRGlld3BMNjIzbVl5bHZO?=
 =?utf-8?B?ejkzRnhKYWxJdmRyZkpsYWxYSHVJK1d5cjVSTCt1Y1Njc0VNRFZuOVFaZWFG?=
 =?utf-8?B?OURuUnR0eEJUdzhLRktMdENOa2c5YTNYaWtPV2xPdWEvN2kvUE52ZXJCcE9l?=
 =?utf-8?B?d1FleThDeGVpOTZydE13ZGhsd3RhR3B3MEpPcFQ3WU9HVDduUGFBcWRzUExy?=
 =?utf-8?B?dVJ5Q2h0b3J4ajFXeDVpVHQ5YXhOd3NNTHFMVEtIcG5ya3gwVTdJUUw3RVZJ?=
 =?utf-8?B?emFDSHcyOXdPQ3RiSkFSTEZJRFdPRDFsUm4zZ2x5ZFYvcE04dm9wVjBKRWtV?=
 =?utf-8?B?Wm1nWTBOanRKQ210VjdHcDNEQUJvZENVUjc1YzI1dG8veEc0elFQUUhXUTVi?=
 =?utf-8?B?KzdqMVRMbHdLZkFLbVV4ZlM1b1RMYW03dENhZnJkblQyMkUyM1Jhb2Y2STNR?=
 =?utf-8?B?c0NnRGU5aG1WZkdpL0pYcFJ4KzVUeHprRUlVVUE1MjRxMDF6YnVhSFNDdDh3?=
 =?utf-8?B?UUlldUs2M0lnZjZTRTJjTUpESWtmajdINFI2VEFsQ3ViTktOTHVPcWlxQXlK?=
 =?utf-8?B?WVBieC9HVUh2UTdDa0xNaGJDWXdXMjFRL2JmY1ZqZ3gxQVR4bGt6cDhNMDFI?=
 =?utf-8?B?T0xldG9STHVSWi9rNlRBc2RpZ2xnWHUrTThuTUJURHdCcUYwVm9yTlpwVFlm?=
 =?utf-8?B?bzNHSHpleVJSeHNNTml2SHZ5QTUwNGlCNGZ6RFJ2dE5lUUFzS3Z2ZDBXRXNI?=
 =?utf-8?B?Y3k4QjUvc2JiaWVWZ0s1aXRHWG00LzEvbzlNOWRlcUQzdW9tQnEwVVhOSVVB?=
 =?utf-8?B?bGRMdFdhaG1HbnE2RXVGNFVxOFRya0tXTVhTdHJmQzc3QWNrcHp2aEk0S1NF?=
 =?utf-8?B?eEdQdmI5TzJOcjB3bDZDRVI3SVZPY0FqRFRzc0ZxT3QwSEZTalZGR2RNbGt6?=
 =?utf-8?B?MXlaMUx2NmtmOSt3SFdhczJKUWR1N05QRjdmcmhQTFpiN25nbVNzZjJERjgr?=
 =?utf-8?B?eHlZNGtZd2V0a0hSMWN1OUtxZk04OW1jYk1rNmJOUG4yN1cwSzZ3ck9pbUJV?=
 =?utf-8?B?aldLNU51QmMyMU5vWmRvWGcyQUprU283UmJrWEhacE9jY1I1dWZzWVU1WUJa?=
 =?utf-8?B?R2dqZllJQ0R6VDFVaGp2Zk5FOUF0SFBEZXpLODBZUG03eWdWcmJ1UzUrVjdl?=
 =?utf-8?B?QklyK1VEV01rMVZ1dFdjWG9WL3V5NnlhSnVaZ3pkdmRiajJVMEFiKzBVYjNE?=
 =?utf-8?B?MWwyK0kreHpNdFNMcVdFUllodWJleWxDclJLejNWSm9lemsvTVFCY2NzTlVI?=
 =?utf-8?B?TXFyY2ZsNFpKaTliU2MydmtZQld6Yno3aWs2SUVyQkV5S3IzaUVkQT09?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a5fab33b-00e9-47f7-bc55-08da197b0825
X-MS-Exchange-CrossTenant-AuthSource: SA0PR12MB4526.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Apr 2022 16:15:43.6427
 (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: qGgOKDU0oW24I/+urudUfCc2RM6sEm624bd1McQjc2EnWwE3ioYpRGzMjinaNGcYEjelO23WjC4hjVKOPiVejw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB5023
X-Spam-Status: No, score=-4.9 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,
        URIBL_BLOCKED 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 Alex,

On 4/8/2022 10:54 AM, Alex Deucher wrote:
> On Fri, Apr 8, 2022 at 11:47 AM Limonciello, Mario
> <Mario.Limonciello@amd.com> wrote:
>> [Public]
>>
>>
>>
>>> -----Original Message-----
>>> From: Gong, Richard <Richard.Gong@amd.com>
>>> Sent: Friday, April 8, 2022 10:45
>>> To: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian
>>> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>;
>>> airlied@linux.ie; daniel@ffwll.ch
>>> Cc: amd-gfx@lists.freedesktop.org; dri-devel@lists.freedesktop.org; linux-
>>> kernel@vger.kernel.org; Limonciello, Mario <Mario.Limonciello@amd.com>;
>>> Gong, Richard <Richard.Gong@amd.com>
>>> Subject: [PATCH] drm/amdgpu: disable ASPM for legacy products that don't
>>> support ASPM
>>>
>>> Active State Power Management (ASPM) feature is enabled since kernel
>>> 5.14.
>>> However there are some legacy products (WX3200 and RX640 are examples)
>>> that
>>> do not support ASPM. Use them as video/display output and system would
>>> hang
>>> during suspend/resume.
>>>
>>> Add extra check to disable ASPM for old products that don't have
>>> ASPM support.
> The patch description is incorrect.  ASPM works just fine on these
> GPUs.  It's more of an issue with whether the underlying platform
> supports ASPM or not.  Rather than disabling a chip family, I would
> prefer to add a check for problematic platforms and disable ASPM on
> those platforms.

I thought that initially.

But I found out that suspend/resume works just fine on the "problematic" 
platform (Dell Precision 3660, Intel ADL based) + AMD W6400 GFX card. 
With WX3200 or RX640, suspend/resume works only when ASPM was disabled. 
Both WX3200 and RX640 are from CHIP_POLARIS12 family.

This is why I take chip family approach.

Regards,

Richard

> Alex
>
>>> Signed-off-by: Richard Gong <richard.gong@amd.com>
>>> 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%7C96f8f686f75f43abb5ed08da19780fab%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637850300760921285%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=xVKC0Q16ho5Y2GDuN%2Fnx68wm6NzOIyR5xJbiXPgqPpQ%3D&amp;reserved=0
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 4 ++++
>>>   1 file changed, 4 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> index bb1c025d9001..8987107f41ee 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
>>> @@ -2012,6 +2012,10 @@ static int amdgpu_pci_probe(struct pci_dev
>>> *pdev,
>>>        if (amdgpu_aspm == -1 && !pcie_aspm_enabled(pdev))
>>>                amdgpu_aspm = 0;
>>>
>>> +     /* disable ASPM for the legacy products that don't support ASPM */
>>> +     if ((flags & AMD_ASIC_MASK) == CHIP_POLARIS12)
>>> +             amdgpu_aspm = 0;
>>> +
>> I think it's problematic to disable it for the entire driver.  There might be multiple
>> AMDGPUs in the system, and others may support ASPM.
>>
>> Can it be done just as part of probe for Polaris?
>>
>>>        if (amdgpu_virtual_display ||
>>>            amdgpu_device_asic_has_dc_support(flags & AMD_ASIC_MASK))
>>>                supports_atomic = true;
>>> --
>>> 2.25.1