On 7/25/2025 4:03 PM, Damien Le Moal wrote:
On 7/25/25 15:43, Borah, Chaitanya Kumar wrote:
Hello Damien,
Hope you are doing well. I am Chaitanya from the linux graphics team in
Intel.
This mail is regarding a regression we are seeing in our CI runs[1] on
linux-next repository.
Since the version next-20250708 [2], we are seeing the following regression
`````````````````````````````````````````````````````````````````````````````````
(kms_pm_rpm:5821) igt_pm-CRITICAL: Test assertion failure function
__igt_pm_enable_sata_link_power_management, file ../lib/igt_pm.c:392:
(kms_pm_rpm:5821) igt_pm-CRITICAL: Failed assertion: write(fd,
"min_power\n", strlen("min_power\n")) == strlen("min_power\n")
(kms_pm_rpm:5821) igt_pm-CRITICAL: Last errno: 95, Operation not supported
(kms_pm_rpm:5821) igt_pm-CRITICAL: error: -1 != 10
Test kms_pm_rpm failed.
`````````````````````````````````````````````````````````````````````````````````
Details log can be found in [3].
After bisecting the tree, the following patch [4] seems to be the first
"bad" commit
`````````````````````````````````````````````````````````````````````````````````````````````````````````
commit 4edf1505b76d30e1e1e283d431e4f84ad01ddcef
Author: Damien Le Moal dlem...@kernel.org
Date: Tue Jul 1 21:53:18 2025 +0900
ata: ahci: Disallow LPM policy control for external ports
`````````````````````````````````````````````````````````````````````````````````````````````````````````
For some context in our kms_pm_rpm tests, we enable min_power policy for
SATA so that we can reach deep runtime power states and restore the
original policy after finishing. [5][6]
IIUC, the above change is based on spec and not something which can be
reverted. So as I see it, we have to drop this code path for external
ports. However I am not sure if we can achieve deep power states without
enforcing it through the sysfs entry.
I am not entirely sure what you mean with the last sentence above, but for
external ports, LPM cannot be used if you want to keep the port hotplug
capability alive and working. Without keeping such port at max power state, we
cannot detect hotplug events (which is super annoying when you have e.g. a
server with front loading drive bays allowing swapping drives without shutting
the machine down).
Atleast for the basic-rte subtest, the test passes if we comment out the
functions controlling the SATA ports. We will need more testing to
determine if this approach work. Any thoughts on it?
Niklas and I actually suspected that we would be getting "complaints" about this
change. Well... We did :)
The problem really is that external ports have never been properly handled by
libata so SATA hot-plugging never really worked reliably. Patches queued up for
6.17 before this patch prevent the kernel from changing the power state of
external port. And this patch was introduced after seeing systemd.udevd setting
external ports power state to min_power or lower states, thus breaking again the
hotplug capability.
The error you are seeing is thus entirely correct and expected.
The question is though: do we want the user to "ignore" hotplug capability and
instead priviledge low power states. I guess we should have such capability.
Atleast a case can be made for debugging and testing use-cases.
Also, are there other ways to detect a port is external other than
receiving EOPNOTSUPP on the sysfs write?
There is not. But it would be easy to add a sysfs port attribute, e.g.
/sys/class/ata_port/ata1/external which says "0" for regular ports and "1" for
external ports. We could also make this attribute writable in the case of
external port so that doing:
echo 0 > /sys/class/ata_port/ata1/external
forces the kernel to ignore the external nature of the port and allow user
control of the port/device LPM state.
Would that work for your case ?
Something like this should solve our problem.
Regards
Chaitanya
