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: <0fa90fa4-af41-52c1-0e6f-3ce6a84fa461@amd.com>
Date:   Wed, 2 Aug 2023 09:35:35 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH v7 2/2] PCI: Don't put non-power manageable PCIe root
 ports into D3
Content-Language: en-US
To:     Mika Westerberg <mika.westerberg@linux.intel.com>
Cc:     "Rafael J. Wysocki" <rafael@kernel.org>,
        Bjorn Helgaas <helgaas@kernel.org>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org, Len Brown <lenb@kernel.org>,
        linux-acpi@vger.kernel.org, Iain Lane <iain@orangesquash.org.uk>,
        Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
References: <b82a50eb-8182-84ca-5b24-dbe8870fa871@amd.com>
 <CAJZ5v0i6PviqW7u3i8hmvSCvR_VHqP-mWRy3Da8Ev_1vi9qBQA@mail.gmail.com>
 <a309e3fe-b1f9-e269-cb97-8af87c8d483b@amd.com>
 <CAJZ5v0jvxrDMR6YHFpYZ4yYpp82-3TtrH==SMRFtUMJsv7=i=g@mail.gmail.com>
 <37b005d5-68fb-f8dd-67e2-c953d677fca2@amd.com>
 <8298c01c-abec-914b-0542-459f38c635fe@amd.com>
 <CAJZ5v0i3g0JujMwikB8niRZ93hXJZqWtjrCjbaDmkMLUbMmwMA@mail.gmail.com>
 <d1b2cf1b-de5f-6c2e-c8dc-fdf60cd0882d@amd.com>
 <20230802052601.GM14638@black.fi.intel.com>
 <e82ec662-22d9-b331-0880-886bd28624eb@amd.com>
 <20230802143142.GS14638@black.fi.intel.com>
From:   Mario Limonciello <mario.limonciello@amd.com>
In-Reply-To: <20230802143142.GS14638@black.fi.intel.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?N1laQkpPVnFMU21FdUN6TjZqc1Nad2FGQWRqSE1FMDFUdDE2TWI4QzFVT2Iz?=
 =?utf-8?B?Q3Y1WFZBSGgzdzkvZXJLK3Iwb1lXdDNNSnM0YlgvYXpwMVFHL293WFNyaWpO?=
 =?utf-8?B?R1c0ckw3SmtmWDFSa1FhVG9ndG5icndQUVpzWWtvdlNyclBUdkl6SEtzUlR5?=
 =?utf-8?B?OUNnTUoybFBRRjNlbXd3MFNqVFRick8wZm5TR3VVTWhDTjlrdmVRYjJUVGlS?=
 =?utf-8?B?T0dxb3BhNDJtaUJZZE1PNU5SaitDdFVpOGFoT3U4OHBRYkpzVmtiVFVlSmZI?=
 =?utf-8?B?K3ZpTHA0Z1dlOVRzUmVnQ0NSMG9GcmdnRUs1VllQeVdYMkVLYzNQRkoyTVVr?=
 =?utf-8?B?V1draU9OTDRzV05wNUJOZDFLSG5wK2MrZzh4aVFvekhGa3BJYkZ2dGEyekJD?=
 =?utf-8?B?VUx5UWx2M0N3Z3E0RDRUQzhIY2hOSGx3SHBzUGR4NkJRMW00bERLVnFoNkFl?=
 =?utf-8?B?QVlGTFdGZG5zRzVzTklHemt3WmJIK21mTWRUdk12OTk2RWlHa0gwc0ZCckdi?=
 =?utf-8?B?d1l5SHNobnArOTBPSGhzNmZsNWtBZzhkN2Z1QlV2SlpvSUJjYWVYYUpha0ds?=
 =?utf-8?B?dzJIcnRmSkMrajAwQnJrUVZtU2xld21hK0FtSWVXN1doMytrVk0za0xYbVJH?=
 =?utf-8?B?ckpKZmRjM2RIUFl1RjAwQVZSRmJ2eWtxN1ZFcVlWbktlYnJiMTIxeG5nbW44?=
 =?utf-8?B?Q0s1cmhmVFBFQWUvbTZKNm9MYTkybzBsMEpzVUtsZ2lmL1Z3QXlua05CRy84?=
 =?utf-8?B?L2REeUtPRmpsN29NOHhRQjVpaHRaS0xWUWxCVjJIazlKL3NneVhUbmdBRXI4?=
 =?utf-8?B?em5ObHNzR1BuRHNzbTVCeitpQ2tjWHpnb05GdjVWUGV5WlNDZ1VuQzJ0b0lV?=
 =?utf-8?B?eDM2ckI3ajBUQS9LTldHQnlmQmtJQklPcStNcFUwWG1IRGFaQldaaHRyTU44?=
 =?utf-8?B?ZlNYd0RWQlZHMmtHbnBPOU1XYjJIdmcva250SDFvWUJ0MTdCNTNEV2p1c2pU?=
 =?utf-8?B?emFncUZIYmtsckZjeFRBTy95aXZrWDNqSDlvL2F3dmR5WTc2WVdTdkRIRU9k?=
 =?utf-8?B?aFFwb3pmN2M1MWRpUngzM1crV29tdVhOSFYzTHo5eTF0VHY0eWd4Zk5BUlRz?=
 =?utf-8?B?LzBjaGFiQjJ2cHcwZnNJdUp6UjZwUmJoM0V1T0s5aUJoSTcwQWkrVHdQaVJn?=
 =?utf-8?B?a3hoM2Q3Zld3QTFLbTdDYTlydXpQSjByQllwQjhaV05TWVpYbjBkb24yYWg4?=
 =?utf-8?B?RU9vNE8rR3N5MThDMDZuenAxczVxV2xlS3U0dzl5VndlZ201Vml0VFhvblpC?=
 =?utf-8?B?SHE4QVFZV09WNFFRcURoZWV3ZERhS0hsQXhlenpUZ1VoUlJmNVJyeTVuYlJ2?=
 =?utf-8?B?eTFrYXgxRHUvUDZEeHVxMjE2VkhBV1ZmQnV1cGp3dVFaYm85K0VaRzYyRE90?=
 =?utf-8?B?SHZJM20vV2luYW82U1ZWOWR3QVVSUlFLalZ6cDRmaFRnZ0QxbEhUT2ZEWVZT?=
 =?utf-8?B?eUpXZEM2OHU5b3VQWnBIWWpjbGJOdnd3cDR4SVhYNDZNMDdoTUhxTkRxUlZu?=
 =?utf-8?B?blY3MlhmTlZVWjdRK2FvU0N4OHRsdjRTVXBJNGlVb1d1d0huckJDQnd3UG5p?=
 =?utf-8?B?TFkyb2JadkFrWEl1alJhVk84QitxcjhqNjB0WVJmV0RUVWRvWUJMcFFiZTFN?=
 =?utf-8?B?ZWN2NlA3em5Wbng4TWt3akdOWStHUk9rMC9jSjdKbUFoSVBpRm45Y20rL0tB?=
 =?utf-8?B?T3E1b3EzL2lCUUJyWVV1QTFNanZMNElvZUVpYnZkMVFEcnN6cE5CTUZvalV6?=
 =?utf-8?B?OGRYZXRQWFNseEczK2xITFBLNWZvaERCOExXdnkrZW5mKzR2eFNiWnVYdHlj?=
 =?utf-8?B?Ly9sRCtEU3FVZ0VtT0lLSkNWSTEyRjR1YlRwVU03c3M0aHFPZzFRUXkyOHBK?=
 =?utf-8?B?VGFvT280NjVSU3czY1RITE1YZkUxOHE5Y0hDOHFUU3pKYnloMFRYWDVvSDBN?=
 =?utf-8?B?OWljdVRvNWthbE1yVDE1cnBYRHNiRHdmSVRxWDJSUEI4SUZqU1ZNdmtUVkxy?=
 =?utf-8?B?R0hUSVFOcVNwRDdISGRaMytabXJUMHlraHN3dHZFWDEvYlZxbmNKc25HWXhC?=
 =?utf-8?Q?vEaLa4D+BRpeFsUqNX0+hbAM4?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9be3c4a4-98e1-450e-1367-08db9365bd28
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Aug 2023 14:35:37.9094
 (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: 4cumBxTe4Jzh3YP2Fnry7uqjin7d5yrfbliU+YLhXnnty5TitkKorEoF9EfMyFoTUhnqcoMd7kU6+z4WjIiNbw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR12MB8614
X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FORGED_SPF_HELO,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        T_SCC_BODY_TEXT_LINE,URIBL_BLOCKED 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


On 8/2/23 09:31, Mika Westerberg wrote:
> Hi,
> 
> On Wed, Aug 02, 2023 at 09:10:38AM -0500, Mario Limonciello wrote:
>>
>>
>> On 8/2/23 00:26, Mika Westerberg wrote:
>>> Hi Mario,
>>>
>>> On Tue, Aug 01, 2023 at 10:17:11PM -0500, Mario Limonciello wrote:
>>>>> Consequently, platform_pci_bridge_d3() will return false and the only
>>>>> thing that may allow the port to go into D0 is the dmi_get_bios_year()
>>>>> check at the end of pci_bridge_d3_possible().
>>>>>
>>>>> However, that was added, because there are Intel platforms on which
>>>>> Root Ports need to be programmed into D3hot on suspend (which allows
>>>>> the whole platform to reduce power significantly) and there are no
>>>>> ACPI device power management objects associated with them (Mika should
>>>>> know the gory details related to this).  It looks like under Windows
>>>>> the additional power reduction would not be possible on those systems,
>>>>> but that would be a problem, wouldn't it?
>>>>>
>>>>
>>>> I've been thinking on this today, and I at least have a hypothesis about
>>>> this behavior.  Perhaps Windows is actually utilizing enabled PEP
>>>> constraints to enforce what state device should be put into over Modern
>>>> Standby cycles in the absence of ACPI objects.
>>>>
>>>> In the case of one of my problematic system the PEP constraints for the root
>>>> port are:
>>>>
>>>> Package (0x04)
>>>> {
>>>> 	0x00,
>>>> 	"\\_SB.PCI0.GP17",
>>>> 	0x00,
>>>> 	0x00
>>>> },
>>>>
>>>> That first 0x00 means the constraint isn't actually enabled for the root
>>>> port.
>>>>
>>>> Mika,
>>>>
>>>> Could you get an acpidump from one of these problematic Intel systems so we
>>>> can check the PEP constraints to see if this theory works? Or maybe you have
>>>> some other ideas why this is different?
>>>
>>> The patch adding this was merged in 2016 and unfortunately I don't have
>>> any of the ACPI dumps from them available anymore (and do not recall the
>>> details either). I think these were Apollo Lake-P based systems with the
>>> initial runtime D3cold and S0ix support at the time.
>>
>>
>> I scoured the web looking for acpidumps a bit an Apollo Lake system and came
>> across this random bug report:
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=1591307
>>
>> "Intel(R) Celeron(R) CPU N3450 @ 1.10GHz (family: 0x6, model: 0x5c,
>> stepping: 0x9)"
>>
>> I looked at the acpidump, and I notice:
>>
>> Low Power S0 Idle (V5) : 0
>>
>> That means that Windows wouldn't actually be putting it into Modern Standby
>> at suspend but would rather use S3.
> 
> Same goes for Linux AFAICT. The ones needed this actually used S0ix so
> the bit should definitely be set.

OK.

> 
>> Considering that result, could we perhaps adjust the check to:
>>
>> if ((c->x86_vendor == X86_VENDOR_INTEL) && !(acpi_gbl_FADT.flags &
>> ACPI_FADT_LOW_POWER_S0))
>>
>> Or could we quirk the PCI root ports from Apollo Lake to opt into D3?
> 
> It is not just Apollo Lake, but all "modern" systems as well (sorry if
> this was unclear). Apollo Lake just was the first one that needed this.
> We also have the Low Power S0 Idle bit set in recent systems too.

Ah got it; I misunderstood it as Apollo Lake was the only one that 
needed it.

So modern systems that set the bit in the FADT, do they also lack _S0W 
and _S0D on the root ports?

Does my PEP constraints theory hold steam at all?