Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Message-ID: <766d621c-695d-4ae7-87cf-690cb8d066df@amd.com>
Date:   Wed, 13 Dec 2023 12:42:04 -0600
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 2/2] PCI/portdrv: Place PCIe port hierarchy into D3cold at
 shutdown
Content-Language: en-US
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     Bjorn Helgaas <bhelgaas@google.com>,
        "Rafael J . Wysocki" <rjw@rjwysocki.net>,
        linux-pci@vger.kernel.org, linux-acpi@vger.kernel.org,
        linux-kernel@vger.kernel.org, mpearson-lenovo@squebb.ca
References: <20231213182656.6165-1-mario.limonciello@amd.com>
 <20231213182656.6165-3-mario.limonciello@amd.com>
 <CAJZ5v0gDjwEpx-WNSY0=qchoSGxizsD3XM7Bgq=i0xufBm=Cag@mail.gmail.com>
From:   Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <CAJZ5v0gDjwEpx-WNSY0=qchoSGxizsD3XM7Bgq=i0xufBm=Cag@mail.gmail.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?U0ZTWXZYaURHS2gyV09ZNGh4RTd6U3BZUWhuZXhiRWF3dC81UVlkeUxZUzdN?=
 =?utf-8?B?SWNhZ01vWTdyQzRXTzVXVTBNZE44Z2k4SU9ZTkt3cU80LzV6azBFem11bk91?=
 =?utf-8?B?NnIxQnZLeVpvcVdHRG4rS2hvRDBrY1lVd01mYWdaUTIvYUtKZjFyaktibUlJ?=
 =?utf-8?B?anIxMzRDcllSTm9qY3NBVHJPb0dYQTdQK2UrMk5hK1d4SFYrTnFOSjh6SWhJ?=
 =?utf-8?B?VDE1bkR4RFJ0ZEdSdlVjeFh2dFZRa1gyNUIwQ29jT012alFITzRoSEI5eU1H?=
 =?utf-8?B?VE9zMTg4VitEQ0RjLzB0YnFDRzA1MDJPMUNZTTdiL3FzbVNxZVhHZCtCSENY?=
 =?utf-8?B?Y1VmejlGMEMyMW54cm1hNjJSMnlLSWdEd3lKOVE0eE9Ya0treDcvTXZFTlFo?=
 =?utf-8?B?a0lzMXBMb0FMaHM5cHZlUUxPOHNQNGZqaUQ5b3pHYy9rOGNpdDBHbnRsalhD?=
 =?utf-8?B?MmxROTFMT1lZd2dJYUZLeWVDSVhOZk1rYUJQZkx1UUo5TWJJMUhmVlh6NmFM?=
 =?utf-8?B?T1k2TmI1Z1dCbmJSRTFZK3ZRd2RGMkZYRFFoNFAxdFU4NU5iRzVJZTNiRGZU?=
 =?utf-8?B?YjR0Q1lJcHVYOURyN0FIN3JoV3dOeFp1NTR2QWt6SEJSVzJ3UHd4aUxwTXRQ?=
 =?utf-8?B?Q1pmMDRVcUV6M21UWWpGZU52TUUzbDlJVFVlTnBZQ0N4S2FiUVBiWUFiaG5N?=
 =?utf-8?B?Zmp1VkNWTGhlRnpZVlVkbEFRZDdTM08vbnhKbTRrV2xycmc1SWZJSFJLQkE5?=
 =?utf-8?B?ZzFzbkhONHorV0haNTVmUnVDR1pkb3VzaFJJSGFXT2FrQnpmTjFnTzRhTGpU?=
 =?utf-8?B?VUcvZFJXS3h4QkdGdVlGMXdLM2JTRC9EZTF2b2V1YThpbkF2dWY4STlNWE1H?=
 =?utf-8?B?WEN1Wi9ncVJ4bnMveXlzRExEZHAzbjhpa3Z1dFpGT0RIUTM0Tk9FWlRJK0Nx?=
 =?utf-8?B?S0JpUmhRMjVTSnBpYkEyejBnd2hpdTJtUk4vblJyUGY3Rko1dHlLbGxITzJP?=
 =?utf-8?B?WmgxSzZtakR3dWc1ZzJZRU9teCtocGk2MjFwT0MvTlhtdzZFYlBmR2NlUGpN?=
 =?utf-8?B?Qmk0UkFhS0JtN3JaQkhIYW5wRTNrT3NYckFRUGF1bGhlMFY1bUFJZE5IaVho?=
 =?utf-8?B?SkdtdlRsMjk3RjhRM09ha0Y3L2JUOU9pS0dTdTVkK2k0b2pwMDd2MTRrVjYw?=
 =?utf-8?B?WGhBOVpUTzU0NjVzblNDdE52amQxZVoxRTVwVDI2TnlZNEtteWp1YkRRMjJ6?=
 =?utf-8?B?YTdjbEF2MjltL1BheWI5QzBUS0pNQUFYaUh5dmdRR1hPd2QrZjJFVUMzVlEr?=
 =?utf-8?B?MGpvcVBUT3Fld1JOWWdCbVVUaU9aUTJ3ajI3ZzBJbjF5dTNiRkRjN29lVlZr?=
 =?utf-8?B?Wk9nT2Y2V1hyLzNiWksvcWtmcloySFRYV0pBOHh2NzlWOTZURHZLUUEzeFRI?=
 =?utf-8?B?MlRsNmpYTHlRbmpQSEJubXhQeklaQzRzVFZWUldhL0NYWks1Q0RNc0tiU0Jl?=
 =?utf-8?B?c2NPSm5OUWEzV0xoWCtYcVg4MXdxZGVkT1JBY3lRZVZVSGFtZFJrU0J5Z2sy?=
 =?utf-8?B?Nkg2NGtHZlJZU2RvVm5qdHVURzF0c05zT0h4WnRFT2RJWHZ5WGd6YjVYZFpQ?=
 =?utf-8?B?UkVLajZCV1RSYnBwanZMZVBxeEZFMCttMEcwOG1ZbDhHNDhOTndMTUZJUU5H?=
 =?utf-8?B?anplK2ZzUGRObnVPZEcva1cvbk9hMkJyenY1c0pialhtbmxTZERMeTJ0b05N?=
 =?utf-8?B?NUtKV2VPTzhXUHFOZ3VGa1RhZGc2eEErYll3WTh3QjNOcGN6Z0dzOXByVFE3?=
 =?utf-8?B?NXkvZkJnUEJ2TmhSTC80UGtxV3NDcTJsODNOSHJMYjZJUTRCNEVRMk1tWC9O?=
 =?utf-8?B?MUZqY0RsQ2s2djdzTERJb0lrVlFaSHlKQStsY2hpdDJFbjFnanpPWVpPTTRr?=
 =?utf-8?B?M0wzTlhjQS9xTFg5WWZIbUxHS0E3TndrTU9udU1jdG1qNFlZVGlEQis2dm9S?=
 =?utf-8?B?aXl3dlZTM01VcXYzbVVubjZrYmFzRjFHbUJFY2s5Si9oT0FBcHRTQjBOVDdk?=
 =?utf-8?B?LysvU0kwNWVtWXBjT2lTeUlrWkFCYjMvOHpiOTFOMyt1L3pMemVnS3p5M3p6?=
 =?utf-8?Q?JwUtCsrSm7HFTDmWtKfd6nZas?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: bcbf6834-e579-40c1-efb9-08dbfc0b3530
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 18:42:07.1295
 (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: yFAobbkg+GLi7Z9soW7vHAbmDAPkaPxcKEpWdaYjFJYrQhHSmUy7pYzdOe+5xk0fQx9MghwkgaslKaokUIfmDw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR12MB4356
X-Spam-Status: No, score=-1.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=no 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
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Wed, 13 Dec 2023 10:42:23 -0800 (PST)

On 12/13/2023 12:38, Rafael J. Wysocki wrote:
> On Wed, Dec 13, 2023 at 7:27 PM Mario Limonciello
> <mario.limonciello@amd.com> wrote:
>>
>> When a system is being powered off it's important that PCIe ports
>> have been put into D3cold as there is no other software to turn
>> off the devices at S5.
>>
>> If PCIe ports are left in D0 then any GPIOs toggled by the ACPI
>> power resources may be left enabled and devices may consume excess
>> power.
> 
> Isn't that a platform firmware issue?
> 
> It is the responsibility of the platform firmware to properly put the
> platform into S5, including power removal from devices that are not
> armed for power-on.

The specific issues that triggered this series were tied to the PCIe 
ports for dGPUs.  There is a GPIO that is toggled by _ON or _OFF.

Windows calls _OFF as part of S5..

> 
>> Cc: mpearson-lenovo@squebb.ca
>> Signed-off-by: Mario Limonciello <mario.limonciello@amd.com>
>> ---
>>   drivers/pci/pcie/portdrv.c | 11 ++++++++---
>>   1 file changed, 8 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/pci/pcie/portdrv.c b/drivers/pci/pcie/portdrv.c
>> index 14a4b89a3b83..08238680c481 100644
>> --- a/drivers/pci/pcie/portdrv.c
>> +++ b/drivers/pci/pcie/portdrv.c
>> @@ -734,9 +734,14 @@ static void pcie_portdrv_remove(struct pci_dev *dev)
>>   static void pcie_portdrv_shutdown(struct pci_dev *dev)
>>   {
>>          if (pci_bridge_d3_possible(dev)) {
>> -               pm_runtime_forbid(&dev->dev);
>> -               pm_runtime_get_noresume(&dev->dev);
>> -               pm_runtime_dont_use_autosuspend(&dev->dev);
>> +               /* whole hierarchy goes into a low power state for S5 */
>> +               if (system_state == SYSTEM_POWER_OFF) {
>> +                       pci_set_power_state(dev, PCI_D3cold);
>> +               } else {
>> +                       pm_runtime_forbid(&dev->dev);
>> +                       pm_runtime_get_noresume(&dev->dev);
>> +                       pm_runtime_dont_use_autosuspend(&dev->dev);
>> +               }
>>          }
> 
> Wouldn't it be better to remove power from the port after running the
> code below?
> 

Yes; I think you're right.  I'll do some more testing with this.

>>          pcie_port_device_remove(dev);
>> --