Re: [PATCH v7 00/12] Improvements to S5 power consumption

[Mario Limonciello <superm1@kernel.org>]

On 9/10/25 1:11 PM, Rafael J. Wysocki wrote:
> Hi Mario,
> 
> On Tue, Sep 9, 2025 at 9:16 PM Mario Limonciello (AMD)
> <superm1@kernel.org> wrote:
>>
>> A variety of issues both in function and in power consumption have been
>> raised as a result of devices not being put into a low power state when
>> the system is powered off.
>>
>> There have been some localized changes[1] to PCI core to help these issues,
>> but they have had various downsides.
>>
>> This series instead uses the driver hibernate flows when the system is
>> being powered off or halted.  This lines up the behavior with what other
>> operating systems do as well.  If for some reason that fails or is not
>> supported, run driver shutdown() callbacks.
>>
>> Rafael did mention in earlier versions of the series concerns about
>> regression risk.  He was looking for thoughts from Greg who isn't against
>> it but also isn't sure about how to maintain it. [1]
>>
>> This has been validated by me and several others in AMD
>> on a variety of AMD hardware platforms. It's been validated by some
>> community members on their Intel hardware. To my knowledge it has not
>> been validated on non-x86.
> 
> Still, the patches need more work (see my replies to the relevant patches).

Yes, thanks for the review.
> 
>> On my development laptop I have also contrived failures in the hibernation
>> callbacks to make sure that the fallback to shutdown callback works.
>>
>> In order to assist with potential regressions the series also includes
>> documentation to help with getting a kernel log at shutdown after
>> the disk is unmounted.
>>
>> Cc: AceLan Kao <acelan.kao@canonical.com>
>> Cc: Kai-Heng Feng <kaihengf@nvidia.com>
>> Cc: Mark Pearson <mpearson-lenovo@squebb.ca>
>> Cc: Merthan Karakaş <m3rthn.k@gmail.com>
>> Cc: Eric Naim <dnaim@cachyos.org>
>> Link: https://lore.kernel.org/linux-usb/2025090852-coma-tycoon-9f37@gregkh/ [1]
>> ---
>> v6->v7:
>>   * Add documentation on how to debug a shutdown hang
>>   * Adjust commit messages per feedback from Bjorn
>>
>> Mario Limonciello (AMD) (12):
>>    PM: Introduce new PMSG_POWEROFF event
>>    scsi: Add PM_EVENT_POWEROFF into suspend callbacks
>>    usb: sl811-hcd: Add PM_EVENT_POWEROFF into suspend callbacks
>>    USB: Pass PMSG_POWEROFF event to suspend_common()
>>    PCI/PM: Disable device wakeups when halting or powering off system
>>    PCI/PM: Split out code from pci_pm_suspend_noirq() into helper
>>    PCI/PM: Run bridge power up actions as part of restore phase
>>    PCI/PM: Use pci_power_manageable() in pci_pm_poweroff_noirq()
>>    PCI: Put PCIe bridges with downstream devices into D3 at hibernate
>>    drm/amd: Avoid evicting resources at S5
>>    PM: Use hibernate flows for system power off
>>    Documentation: power: Add document on debugging shutdown hangs
> 
> If I were you, I'd split this series into 3 parts.
> 
> The first part would be the addition of PMSG_POWEROFF just for
> hibernation, which should not be objectionable (the first 4 patches
> above).
> 
> The next one would be changes to allow PCI bridges to go into
> D3hot/cold during the last stage of hibernation (the "power-off"
> transition).  This can be justified by itself even before starting to
> use the same power-off flow for the last stage of hibernation and for
> system power-down.
> 
> The last one would be the hibernation/power-down integration.
> 
> Each of the above can be posted separately and arguably you need to
> get the first part in before the other two and the second part in
> before the third one, preferably not in the same cycle.
> 
> This way, if there are any regressions in the first two parts, there
> will be at least some time to address them before the last part goes
> in.
> 
> Thanks!

Thanks for this proposal.

I do like the idea of splitting it in 3 parts to give time for 
regression control.

It's getting close to the end of this cycle, would you be opposed to a 
re-spun first 4 patches for 6.18?

Thanks,