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: <8298c01c-abec-914b-0542-459f38c635fe@amd.com>
Date:   Mon, 31 Jul 2023 22:25:05 -0500
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.12.0
Subject: Re: [PATCH v7 2/2] PCI: Don't put non-power manageable PCIe root
 ports into D3
Content-Language: en-US
From:   Mario Limonciello <mario.limonciello@amd.com>
To:     "Rafael J. Wysocki" <rafael@kernel.org>
Cc:     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>,
        Mika Westerberg <mika.westerberg@linux.intel.com>,
        Andy Shevchenko <andriy.shevchenko@linux.intel.com>
References: <20230711221427.GA250962@bhelgaas>
 <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>
In-Reply-To: <37b005d5-68fb-f8dd-67e2-c953d677fca2@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?UmlzV0VvdUZBNmdVVnlyYml2bDlhR01ldHFXRHArUm02czgwdE9xUmo3Tjlz?=
 =?utf-8?B?amFHYnV0ZldOS0JmVjNPQjlzWmdPc21mWGhLQnU0Vi91K1I5VVhxVGtYbWpx?=
 =?utf-8?B?L2V4U3dCTXlFUm1jczQ1bGdINWdyeHNEU0M5VVZZR2p2dXdiZi9OOWNOam9v?=
 =?utf-8?B?U1d0RzNKZ1ZDRlZWWGM5cmRBVXZRODU3N1ZpTE5OTXdwaUVpVWNLNVJjOUg3?=
 =?utf-8?B?QUtMQXg1MnRwdk9iUlNJMEJMSmVPOW9iZUhqWGJHT2RIb2dzTnBkeE1hTE1L?=
 =?utf-8?B?Q2UvY3d4RFJnamtDWElTRVVBOTRnalpvZHRQTHVPY3pWWXgxWDdPWFk5SDE2?=
 =?utf-8?B?ai9XWHd6UVp3K25nZjdUQlBOenVNTlJrLzdzbXVSNVAzd09sUXNrVjBudi85?=
 =?utf-8?B?R01jYU1NSllkMzNqVWZMNzJJaUswaThsZElpalpTSlhFMEVSbUR2UHZqbDNV?=
 =?utf-8?B?aWNKYWtrS3QzK092eGNnK2M3YVlPYzJSNVFLb3ZScFo5d01odGFpSnVMWnc3?=
 =?utf-8?B?aWU4OTdNWDB0REo0TkdJdDhyN3hnbTErV25oZTIxUTVkS1JLN1RTZDJ3NnEw?=
 =?utf-8?B?Q1V1MEo2NVJkSGN4eWZlOCtkWHRmMkpITFN3Ny8yWW40SUZTZWNBcFFlY2Ix?=
 =?utf-8?B?ZitteHIvVFFuZnhyYTY5SFQ2WVRNbitOeFBJakQzTkFXMXh5S3VGWU1zaDlq?=
 =?utf-8?B?REp4aG1pd0tPblZqaUxwNS9UeVNwU2RXYzAvc0dEMDcwYjZOOTZaREtoblgw?=
 =?utf-8?B?aFRhZnkrUCtLUFJ1aERUdFE1M1FpNHJ0ZFU1c2x6TU9rNE9hMUdrSkRzbGRH?=
 =?utf-8?B?TlpVRk1MblpnM1BPTjhacWNzdjBtZ01YZFczMVBiYllyc2VhZFNWUkNabk1P?=
 =?utf-8?B?TUZ1N2VGdkdxWXJYaHlNNDBPa3pPZlBtaS9QWU5TbU0vVG05ZGJHQURNL0dx?=
 =?utf-8?B?R054MWZtSXVvc2g0cnBxMXlPa1Z4SWxzMTM3aDZ6M3VhUVpPbk5acjkySzhE?=
 =?utf-8?B?NW8rZGRyU2JIWFgrREFCM055MklMWWdhdXJHWjh0enRMU2VtemtTWUUrelpD?=
 =?utf-8?B?dWk0Z2dtTzlUKy9JSHd0d2FVUnFMaGp6djBHWUFkZXI2cnJxTVgzTlhNWEsr?=
 =?utf-8?B?Yk9lWEJ5UFBScFl6bGFkSkpkMHY1d0FHMDN1K3lpb1J2SjROajBocTFwYWwv?=
 =?utf-8?B?KzNwemhKSTZRZG5EVE1FSUhBQUVJOXhPQm5WUEtLblhiRmN6cXhyeWorbGdm?=
 =?utf-8?B?SFI0MWJHbjlSOUNnc2pKUm9xWUI3YTZVaXRYNlVpbjZYNmZtTjVPNHh4V1VT?=
 =?utf-8?B?L0p2M1daT3FkNloxaFljRkVzWVJDUW9EQ3FJdmpXMUdvang1MTAzQ2cxZ0x2?=
 =?utf-8?B?R2JjU0hpaXNGTWFrUDlOemdKRkVncFF6TU1DZnBaR1FpemUrS3I4Y1Q0VHhu?=
 =?utf-8?B?Qm5ZbUZTTWRGbVB3amE2L2lsS05Hc2toUDFLdnlERHlVQWFDa0YrTS9yQ3NK?=
 =?utf-8?B?MGIxdk9lR1QvWVBmdGpZRXpYKzU5OGpPRks0V0JZbkZxQ0N5U29LeU5iWjVi?=
 =?utf-8?B?MlRNUkxMM1dITXVlclFhb2NqSWFpeW0yZEZ4dmh2Z0JrajByUnVMakNueDhE?=
 =?utf-8?B?LzZGVEh4cGlMWDFHYUhhZGJidlltQUNoNkxlVVp1UHhUQVlRQVJFZUZOaVZk?=
 =?utf-8?B?SXRrZ2tNelNlNzAvRnAyY2htRVFiZkdkWHBadVJ4SWttNzljQk5BV0dVZ05Z?=
 =?utf-8?B?enhwTS81ZFJWNnFYRWxDeVlwVVdFNlM0YXFjSjFmdFdTQ0diM25BN1FaSEcx?=
 =?utf-8?B?aVZ2WWNLakE2cWYyWGZ1dy9ONmVRY2tVd0dpZ0FtdWN4ME5rblhhT1JQR2FZ?=
 =?utf-8?B?VFJqUjY3L2RjT1dtVlBQRFY1bjBIY3FBeTZ4L3JJT1Njc3NCUDRLdThhakNo?=
 =?utf-8?B?aGw4YWNsbnBXZXdnS0Ircm5OSjNZVUlGWkJpM2haR29jNDB0TGtiQVZ2VzQz?=
 =?utf-8?B?WjZYZGNQemdmUGtBRTU4QkpxSnptNnBBUTN4R3lrdXJnL3B6V3IrR3U4bnN3?=
 =?utf-8?B?WkpIUHBZbU5waDJvbStlTU1YRVBDN282cnBrYWZKcFR1V1pJZUtNS1pOWHdJ?=
 =?utf-8?B?TGxBdng3ZC9DNitzcWVqNTM3SXlFRU4xNFo5ck5EMlV2aE8ycmZFVE1BRmxW?=
 =?utf-8?B?aXc9PQ==?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: dc8f9d14-8647-4ed1-4d4e-08db923ee7c6
X-MS-Exchange-CrossTenant-AuthSource: MN0PR12MB6101.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 01 Aug 2023 03:25:07.8780
 (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: 83myY+2BkOzms4Ztkp1rJXart4p67GQr/0+NRAjDSS2nWczCVABrH7p8Uex0EQJLJg4U4ldjikBjVYvbL48hFA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW4PR12MB5642
X-Spam-Status: No, score=-0.0 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 7/14/23 19:46, Limonciello, Mario wrote:
> 
> On 7/14/2023 2:17 PM, Rafael J. Wysocki wrote:
>>>> Generally speaking, pci_bridge_d3_possible() is there to prevent
>>>> bridges (and PCIe ports in particular) from being put into D3hot/cold
>>>> if there are reasons to believe that it may not work.
>>>> acpi_pci_bridge_d3() is part of that.
>>>>
>>>> Even if it returns 'true', the _SxD/_SxW check should still be applied
>>>> via pci_target_state() to determine whether or not the firmware allows
>>>> this particular bridge to go into D3hot/cold.  So arguably, the _SxW
>>>> check in acpi_pci_bridge_d3() should not be necessary and if it makes
>>>> any functional difference, there is a bug somewhere else.
>>> But only if it was power manageable would the _SxD/_SxW check be
>>> applied.  This issue is around the branch of pci_target_state() where
>>> it's not power manageable and so it uses PME or it falls back to D3hot.
>> Well, this looks like a spec interpretation difference.
>>
>> We thought that _SxD/_SxW would only be relevant for devices with ACPI
>> PM support, but the firmware people seem to think that those objects
>> are also relevant for PCI devices that don't have ACPI PM support
>> (because those devices are still power-manageable via PMCSR).  If
>> Windows agrees with that viewpoint, we'll need to adjust, but not
>> through adding _SxW checks in random places.
> I think that depends upon how you want to handle the lack of _S0W.
> 
> On these problematic devices there is no _S0W under the PCIe
> root port.  As I said; Windows puts them into D0 in this case though.
> 
> So acpi_dev_power_state_for_wake should return ACPI_STATE_UNKNOWN.
> 
> Can you suggest where you think adding a acpi_dev_power_state_for_wake() 
> does make sense?
> 
> Two areas that I think would work would be in: pci_pm_suspend_noirq() 
> (to avoid calling pci_prepare_to_sleep)
> 
> or
> 
> directly in pci_prepare_to_sleep() to check that value in lieu of 
> pci_target_state().
> 

Rafael,

Did you have any more thoughts on this?