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: <5b1a8e39-1e59-211b-47eb-20ef21cff84c@amd.com>
Date:   Wed, 17 May 2023 21:00:51 -0500
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.1
Subject: Re: [PATCH] PCI: Only put >= 2015 root ports into D3 on Intel
Content-Language: en-US
From:   "Limonciello, Mario" <mario.limonciello@amd.com>
To:     Mika Westerberg <mika.westerberg@linux.intel.com>
Cc:     Bjorn Helgaas <bhelgaas@google.com>, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        S-k Shyam-sundar <Shyam-sundar.S-k@amd.com>,
        Natikar Basavaraj <Basavaraj.Natikar@amd.com>,
        Deucher Alexander <Alexander.Deucher@amd.com>,
        "Rafael J. Wysocki" <rafael.j.wysocki@intel.com>,
        Iain Lane <iain@orangesquash.org.uk>
References: <20230515231515.1440-1-mario.limonciello@amd.com>
 <20230516055918.GS66750@black.fi.intel.com>
 <f0417d28-0c1a-f46e-9cbd-f20f2ac1e9f9@amd.com>
 <20230517071527.GU66750@black.fi.intel.com>
 <8e7e23dc-f01b-6f78-f383-7706795e386e@amd.com>
 <20230517125811.GG45886@black.fi.intel.com>
 <a71a662d-27a6-5c74-a44a-9538ed503a05@amd.com>
In-Reply-To: <a71a662d-27a6-5c74-a44a-9538ed503a05@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?VzdkVGVIeWtLeDA0R0JkMmJzZ1dqc05ndEZib0RhOGozRWFyOTJlM3kzZCt4?=
 =?utf-8?B?ajdhYnRFWXFGS0hyYmdEanBwOUVYRlNJcUZFaG9XQ3hDbjMydERmWlBVSlBX?=
 =?utf-8?B?MlFBUUE0VE04eDR3YUsySmxPNnA5d29SZGRxa2dBR2N5MGlKMlFSVlBuVEhL?=
 =?utf-8?B?Nnp3dDNMZTc0WnV6emp2RkxhTTVNcTNPU1d2bGlvY1pubmNDdXh4ZWVUa1Av?=
 =?utf-8?B?S3lvQkVKV3UvaHRrQnFnM2JLN3RTRS96YURha1ZDSXRuRXRmTUQvRHdEbk5k?=
 =?utf-8?B?T1h5SXFWUkN0RG1yNm40NlorNFExeWZSb2tLS3hpUVZTcjN2UkU1RVFKemtp?=
 =?utf-8?B?UDlFRTJmWm9sUisraUtpQ1BKbSt6TmhnaERialM4a1JjWE1oV1ZMUVVBTEdp?=
 =?utf-8?B?YnlXQkNxdFNEWlh6YTFXaUM1T0xUU291c0dYSzdRMld6M3VvNGxTZmI3RCtp?=
 =?utf-8?B?bXJ1UXdnSExMMThnVmc4elNnazFtTzBqd3FnOUtaeFpTUFhuOE9vQWtkZi8r?=
 =?utf-8?B?Z09RWFljL2pQMHlCMVB6dGp3K3k4SkV2MG5UZ21rNld0dVVSdWlvNzhmMEd1?=
 =?utf-8?B?L1czeUxsSWlBQzdlYWV4b2xhQnk1M3JDQ0Y1WUhLVm9XRFV0Tm9hbE8vNUpT?=
 =?utf-8?B?RDU0SjlQTDNqdTA1WHhOTWltMldod0dPWC9NYkpMT21kNHp5VU5oTXRCY2Jp?=
 =?utf-8?B?aytMQjNyWG1rcUVTZUZsSGVTbDJpUjJqb09mY2dFV01NaHl4TmJCaWlGT09U?=
 =?utf-8?B?VldjdElNTDNBc1ZGR2tRQ3VTdnd4aFhkcE5LVDFtaGxobXRrQ212ZnNlR2ta?=
 =?utf-8?B?NWZrVU9oZnQ1SFpLSnRDcGlQSzNXS0VHb1NHQ0RyL0hHMENBS01XTStlcDRU?=
 =?utf-8?B?K3ZXVGVIRWYyU2lrUStaZmZHdm5xS1FGeWVYbjl0d0p3Y3B4OFh6eFNMN0c2?=
 =?utf-8?B?L0xwLzdKN2VBOVVramRGRXZla0dBV0NpdTNlN1RBY2ZyMjdpaGU2bHcyNG9S?=
 =?utf-8?B?RGx0eGhzamszZHB3aVlZS2IwQWVlY2pYeGlwekNXb0xXdnVEMk1laVBqeHly?=
 =?utf-8?B?dDJtei9mZ0xnNGFLcHNJTmtvTXZsUWxPeUovOHY3dzl2Tm03REhQdHF5anU5?=
 =?utf-8?B?TWx0cTBqc0h6VlNMZFFWUmtaS2xRemtGNDR5TG5Xa0F3SE5QVERNSjNEaTBY?=
 =?utf-8?B?SU14aXdMYVZlMFNacS93Y05hODVFQVRSN2p3RnpXNTFyeXV2Z09FUmFpa2pQ?=
 =?utf-8?B?ZEVtTzlFNUFDT0o5cjdlVi8wN213dnlYK0ttNmFuRHEvcG5nWHVPcTgyWGpm?=
 =?utf-8?B?ZGpaSWhCbG9pZ1hxaXhWeVF6d3dKbHp3OEh5UExLTzRZd0hOYXhCemwvQWlX?=
 =?utf-8?B?bUZMTGgrT2MzZHBzUnlnWnNDcG9aV1hXS1ZVQ0g5VE5ZdzZOdTJsUDVEc0Zx?=
 =?utf-8?B?ZEszcHVzVGhvMGJlRHFYSTVzRFRBSktENUlFdnVkTUFhL3hJQjM5U3haZVMv?=
 =?utf-8?B?aC9EeGFJR042aEdvWEd0VWlPZWU2UTBDc2VUZnA4M0JJQTROU0FsQ1AwZnVh?=
 =?utf-8?B?ME55d0kzdSs5Vy96T1c1Z1pHZnkwQ0VGU3pEMlZsUHYxamhhK3l6ZUJGT1hl?=
 =?utf-8?B?cFlGSnBCOTdNMWIzT1E2ckcrMjhvRVpEaytFcStlVWZOR3NHR0VUYkpWTWpw?=
 =?utf-8?B?RE9qWVJOZ21QWkdJQ1kwUDlJcFExMDFuaHVOdWVZSmp0K3lBWVhOKzNDbFVp?=
 =?utf-8?B?YXFsVk1XcjF3WTV5aS81VkhaQ2lDUzUvZGNENDN5cGYrdzhxbU5YZzlPd3oz?=
 =?utf-8?B?a0x1UGJEYytnVHlaSlk3S1NYQXhMNTFIQzhucjEvaFozVlhFaUV1aHlRYktk?=
 =?utf-8?B?QW14dndRSU1wd3FJdUgxR3MzUG9TUXVteWltWVJ1UXJTK0x3ZUtxUHEzZFha?=
 =?utf-8?B?dFpDVG9WSldxWUJUYTJqSitaZGFULzFGMWhyQmdTY08wMTFINlAwUjFwNmZ4?=
 =?utf-8?B?ZE1laGlSemR6UnVMUW12c2lyYThuYUJ1czFyYmJvUFkrNFZUSHNSMkROWVNQ?=
 =?utf-8?B?ekx2Z0hKRHcxQWlIOWJOSk10VUF6UVRtMjVuRXIzUlpBZkN5VXdBdTVhdXIy?=
 =?utf-8?Q?LhdA7WspjFX/7dh4YM6lR1ro+?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dc4a49af-9d21-450d-7c5e-08db5743b6ea
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 18 May 2023 02:00:54.7890
 (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: JEbfp2XOYExwWILNVZcPUlp/rqPz/EB5Nin5ucNAzOnRj1B3Klp3vRafLhTaemTEokTwSOewzYG+Bk9dsNebOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR12MB8223
X-Spam-Status: No, score=-2.5 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_NONE,RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_NONE,
        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


On 5/17/2023 8:48 PM, Limonciello, Mario wrote:
>
> On 5/17/2023 7:58 AM, Mika Westerberg wrote:
>> On Wed, May 17, 2023 at 07:33:17AM -0500, Limonciello, Mario wrote:
>>>>> AFAICT the actual issue is entirely a wakeup platform firmware 
>>>>> sequencing
>>>>> issue
>>>>> while in a hardware sleep state and not PMEs.
>>>>>
>>>>> It's only exposed by putting the root ports into D3 over s2idle.
>>>> But there are two ways to enter s2idle (well or the S0ix whatever 
>>>> is the
>>>> AMD term for that). Either through system sleep or simply waiting that
>>>> all the needed devices runtime suspend. There should be no difference
>>>> from device perspective AFAICT.
>> I should correct that the wakes may be configured differently, though.
>>
>>> On AMD all devices in runtime suspend and SoC entering system
>>> suspend aren't the same state.
>> Okay.
>>
>>>>> As an experiment on an unpatched kernel if I avoid letting amd-pmc 
>>>>> bind then
>>>>> the
>>>>> hardware will never enter a hardware sleep state over Linux s2idle 
>>>>> and this
>>>>> issue
>>>>> doesn't occur.
>>>>>
>>>>> That shows that PMEs *do* work from D3cold.
>>>>>
>>>>> With all of this I have to wonder if the Windows behavior of what 
>>>>> to do with
>>>>> the root
>>>>> ports is tied to the uPEP requirements specified in the firmware.
>>>>>
>>>>> Linux doesn't do any enforcement or adjustments from what uPEP 
>>>>> indicates.
>>>>>
>>>>> The uPEP constraints for the root port in question in an affected 
>>>>> AMD system
>>>>> has:
>>>>>
>>>>>                       Package (0x04)
>>>>>                       {
>>>>>                           Zero,
>>>>>                           "\\_SB.PCI0.GP19",
>>>>>                           Zero,
>>>>>                           Zero
>>>>>                       },
>>>>>
>>>>> AMD's parsing is through 'lpi_device_get_constraints_amd' so that 
>>>>> structure
>>>>> shows
>>>>> as not enabled and doesn't specify any D-state requirements.
>>>> AFAIK this object does not exist in ChromeOS so Linux cannot use it
>>>> there.
>>> OK that means that if we came up with a solution that utilized
>>> uPEP that it would have to remain optional.
>> Right.
>>
>>>>> What do they specify for Intel on a matching root port?
>>>> I think the corresponding entry in ADL-P system for TBT PCIe root 
>>>> port 0
>>>> looks like this:
>>>>
>>>>     Package (0x03)
>>>>     {
>>>>         "\\_SB.PC00.TRP0",
>>>>         Zero,
>>>>         Package (0x02)
>>>>         {
>>>>         Zero,
>>>>         Package (0x02)
>>>>         {
>>>>             0xFF,
>>>>             0x03
>>>>         }
>>>>         }
>>>>     },
>>>>
>>>> I'm not entirely sure what does it tell? ;-)
>>> It's parsed using `lpi_device_get_constraints`.
>>>
>>> So should I follow it right this means for ACPI device
>>> \\_SB.PC00.TRP0 the constraint is disabled.
>>>
>>> It's described as
>>> Version 0, UID 0xFF has a minimum D-state of 3.
>> I see, so it needs to be in D3 for this "constraint" to be valid.
>>
>>> That means my idea to try to only change D-states at
>>> suspend for enabled constraints won't help.
>> :(
>
> At least on an Alder Lake P system can you check
> whether your root ports actually respond
> affirmatively to acpi_pci_bridge_d3() or they need
> to fall back to that logic?
>
> In my case the problematic ones don't have _PRW or
> _S0W, which might explain why Windows doesn't try
> to use D3 (hot or cold) for them either.
>
> If the root ports on your current systems do respond
> through acpi_pci_bridge_d3() a possible solution here
> might be an allow list for systems from 2015-2018 rather
> than everything > 2015.
>
Actually this is exactly it.  The function is missing a call

for platform_pci_power_manageable().

I'll send out a v2.