[Bug 218198] Suspend/Resume Regression with attached ATA devices

[bugzilla-daemon@kernel.org]

https://bugzilla.kernel.org/show_bug.cgi?id=218198

--- Comment #7 from Dieter Mummenschanz (dmummenschanz@web.de) ---
 (In reply to Niklas.Cassel from comment #6)

Hello Niklas,

thanks for looking into this.

> It would be nice if you could test with latest v6.7-rcX.

Okay I took the latest 6.7-rc3 from Linux's git and included your patch. Booted
up my Laptop, put it back to sleep and brought it up again. See attached log.

I've disabed my link_power_management_policy override so all the time my system
was stuck at pc2 package state according to powertop.

> For devsleep to get enabled, you need "sadm sds" in the SATA controller

Yep I can see that:
[    0.379531] ahci 0000:00:17.0: flags: 64bit ncq sntf stag pm clo only pio
slum part ems sxs deso sadm sds apst 

> and "Dev-Sleep" in the SATA device print.

I guess this is it?
[    1.031169] ata5.00: Features: Trust Dev-Sleep NCQ-sndrcv

> Additionally, your lpm-policy (lpm-pol) has to be either ATA_LPM_MIN_POWER or
> ATA_LPM_MIN_POWER_WITH_PARTIAL (i.e. lpm-pol has to print either 4 or 5).

I'm afraid this is not the case here:
[    0.399005] ata1: SATA max UDMA/133 abar m2048@0x42233000 port 0x42233100
irq 125 lpm-pol 0

> Note that even if you have LPM_POLICY=3 (ATA_LPM_MED_POWER_WITH_DIPM) in your
> Kconfig, ahci_update_initial_lpm_policy() will possibly override this by
> default

That would explain why I'm seeing "max_performance" in
link_power_management_policy without overriding it.


> The reason why many platforms do this override, is because:
> "One of the requirement for modern x86 system to enter lowest power mode
> (SLP_S0) is SATA IP block to be off. This is true even during when
> platform is suspended to idle and not only in opportunistic (runtime)
> suspend."
> 
> "SATA IP block doesn't get turned off till SATA is in DEVSLP mode. Here
> user has to either use scsi-host sysfs or tools like powertop to set
> the sata-host link_power_management_policy to min_power."
> 
> See:
> https://github.com/torvalds/linux/commit/
> b1a9585cc396cac5a9e5a09b2721f3b8568e62d0

Thanks for the insight.

> I would really not recommend you doing this, because when you force set
> lpm policy via sysfs, ahci_update_initial_lpm_policy() is not called,
> so if your platform requires ATA_LPM_MIN_* to enter lower power states,
> you forcing lpm-policy to ATA_LPM_MED_POWER_WITH_DIPM will ensure that
> you never enter lower power states.

I understand however I do not know of any other way to enable lower package
states on my machine.

> So it appears that your port does not support devsleep...

Anything we can do to preserve the old bwhaviour until 6.6?

> PxDEVSLP.DSP (in AHCI specification) is the bit that determines if devsleep
> is supported for a specific port. This bit is initialized by BIOS.
> 
> So this could be a BIOS bug...
> But you said that it works if you revert Damien's patch...

Yes, I had no issues up to 6.7-rc1. 

> Perhaps you could test this patch:

Yweah, see logs.  

> You mentioned that it works when you revert Damien's patch.
> It could be interesting to see these prints, before/after the
> suspend/resume both with and without the revert.

I've attached the dmesg logs from kernel 6.6.


Thanks again for your help
Dieter

-- 
You may reply to this email to add a comment.

You are receiving this mail because:
You are watching the assignee of the bug.