Re: [PATCH v2] PCI/PM: Restore BARs after powering up during enumeration

[Mario Limonciello <mario.limonciello@amd.com>]

On 10/13/26 11:57, Rafael J. Wysocki (Intel) wrote:
> On Tue, Oct 13, 2026 at 5:52 PM Rafael J. Wysocki (Intel)
> <rafael@kernel.org> wrote:
>>
>> On Tue, Oct 13, 2026 at 5:41 PM Mario Limonciello
>> <mario.limonciello@amd.com> wrote:
>>>
>>>
>>>
>>> On 10/13/26 10:23, Rafael J. Wysocki (Intel) wrote:
>>>> On Tue, Oct 13, 2026 at 8:10 AM Keng-Yu Lin <keng-yu.lin@hpe.com> wrote:
>>>>>
>>>>> Hi Rafael, Mario,
>>>>>
>>>>> The initial read is not needed to fix the reported D3hot-to-D0
>>>>> failure. I added it to avoid extra BAR writes on devices already
>>>>> in D0.
>>>>>
>>>>> pci_power_up() does update current_state, but need_restore is
>>>>> calculated before that update. The check includes:
>>>>>
>>>>>           dev->current_state >= PCI_D3hot
>>>>>
>>>>> For an initially unknown device with PMCSR reporting D0 and
>>>>> No_Soft_Reset clear, this still requests a BAR restore. Reading
>>>>> the state first avoids that request, although the final
>>>>> current_state is D0 either way.
>>>>
>>>> Fair enough, but if the device is in D0 already, it need not be
>>>> powered up, so if you make a point to update the power state upfront,
>>>> you may as well skip the power up if the outcome is D0.
>>>
>>> It actually does need to have all the ACPI calls that happen as part of
>>> the D0 transition though like _REG.
>>
>> Good point.
>>
>> This basically means that the upfront power state check belongs to the
>> logic to determine whether or not to restore the BARs which should be
>> part of pci_power_up().
>>
>> So now the question is, if this upfront power state check is done, is
>> it still useful to read PMCSR after calling
>> platform_pci_set_power_state() in pci_power_up()?
> 
> Or what about using the following sequence:
> 
> pci_update_current_state(dev, PCI_UNKNOWN);
> if (dev->current_state == PCI_D0) {
>          /* No need to do a full power-up, just sync-up the platform state. */
>          platform_pci_set_power_state(dev, PCI_D0);
> } else {
>          /* Power up the device and restore BARs if necessary. */
>          pci_set_full_power_state(dev, false);
> }
> pci_update_current_state(dev, PCI_D0);

That looks great to me.