[PATCH v2] ata: ahci: Add Elkhart Lake AHCI controller

[PATCH v2] ata: ahci: Add Elkhart Lake AHCI controller

[Werner Fischer]

Elkhart Lake is the successor of Apollo Lake and Gemini Lake. These
CPUs and their PCHs are used in mobile and embedded environments.

With this patch I suggest that Elkhart Lake SATA controllers [1] should
use the default LPM policy for mobile chipsets.
The disadvantage of missing hot-plug support with this setting should
not be an issue, as those CPUs are used in embedded environments and
not in servers with hot-plug backplanes.

We discovered that the Elkhart Lake SATA controllers have been missing
in ahci.c after a customer reported the throttling of his SATA SSD
after a short period of higher I/O. We determined the high temperature
of the SSD controller in idle mode as the root cause for that.

Depending on the used SSD, we have seen up to 1.8 Watt lower system
idle power usage and up to 30°C lower SSD controller temperatures in
our tests, when we set med_power_with_dipm manually. I have provided a
table showing seven different SATA SSDs from ATP, Intel/Solidigm and
Samsung [2].

Intel lists a total of 3 SATA controller IDs (4B60, 4B62, 4B63) in [1]
for those mobile PCHs.
This commit just adds 0x4b63 as I do not have test systems with 0x4b60
and 0x4b62 SATA controllers.
I have tested this patch with a system which uses 0x4b63 as SATA
controller.

[1] https://sata-io.org/product/8803
[2] https://www.thomas-krenn.com/en/wiki/SATA_Link_Power_Management#Example_LES_v4

Signed-off-by: Werner Fischer <devlists@wefi.net>
Cc: stable@vger.kernel.org
---
V1 -> V2: Added comment mentioning the additional IDs 4b60 and 4b62
          added "ahci:" in summary phrase

--- a/drivers/ata/ahci.c	2023-08-29 09:14:09.537450842 +0200
+++ b/drivers/ata/ahci.c	2023-08-29 11:22:50.779830526 +0200
@@ -421,6 +421,8 @@
 	{ PCI_VDEVICE(INTEL, 0x34d3), board_ahci_low_power }, /* Ice Lake LP AHCI */
 	{ PCI_VDEVICE(INTEL, 0x02d3), board_ahci_low_power }, /* Comet Lake PCH-U AHCI */
 	{ PCI_VDEVICE(INTEL, 0x02d7), board_ahci_low_power }, /* Comet Lake PCH RAID */
+	/* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
+	{ PCI_VDEVICE(INTEL, 0x4b63), board_ahci_low_power }, /* Elkhart Lake AHCI */
 
 	/* JMicron 360/1/3/5/6, match class to avoid IDE function */
 	{ PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,

Re: [PATCH v2] ata: ahci: Add Elkhart Lake AHCI controller

[Niklas Cassel]

On Tue, Aug 29, 2023 at 01:33:58PM +0200, Werner Fischer wrote:
> Elkhart Lake is the successor of Apollo Lake and Gemini Lake. These
> CPUs and their PCHs are used in mobile and embedded environments.
> 
> With this patch I suggest that Elkhart Lake SATA controllers [1] should
> use the default LPM policy for mobile chipsets.
> The disadvantage of missing hot-plug support with this setting should
> not be an issue, as those CPUs are used in embedded environments and
> not in servers with hot-plug backplanes.
> 
> We discovered that the Elkhart Lake SATA controllers have been missing
> in ahci.c after a customer reported the throttling of his SATA SSD
> after a short period of higher I/O. We determined the high temperature
> of the SSD controller in idle mode as the root cause for that.
> 
> Depending on the used SSD, we have seen up to 1.8 Watt lower system
> idle power usage and up to 30°C lower SSD controller temperatures in
> our tests, when we set med_power_with_dipm manually. I have provided a
> table showing seven different SATA SSDs from ATP, Intel/Solidigm and
> Samsung [2].
> 
> Intel lists a total of 3 SATA controller IDs (4B60, 4B62, 4B63) in [1]
> for those mobile PCHs.
> This commit just adds 0x4b63 as I do not have test systems with 0x4b60
> and 0x4b62 SATA controllers.
> I have tested this patch with a system which uses 0x4b63 as SATA
> controller.
> 
> [1] https://sata-io.org/product/8803
> [2] https://www.thomas-krenn.com/en/wiki/SATA_Link_Power_Management#Example_LES_v4
> 
> Signed-off-by: Werner Fischer <devlists@wefi.net>
> Cc: stable@vger.kernel.org
> ---
> V1 -> V2: Added comment mentioning the additional IDs 4b60 and 4b62
>           added "ahci:" in summary phrase
> 
> --- a/drivers/ata/ahci.c	2023-08-29 09:14:09.537450842 +0200
> +++ b/drivers/ata/ahci.c	2023-08-29 11:22:50.779830526 +0200
> @@ -421,6 +421,8 @@
>  	{ PCI_VDEVICE(INTEL, 0x34d3), board_ahci_low_power }, /* Ice Lake LP AHCI */
>  	{ PCI_VDEVICE(INTEL, 0x02d3), board_ahci_low_power }, /* Comet Lake PCH-U AHCI */
>  	{ PCI_VDEVICE(INTEL, 0x02d7), board_ahci_low_power }, /* Comet Lake PCH RAID */
> +	/* Elkhart Lake IDs 0x4b60 & 0x4b62 https://sata-io.org/product/8803 not tested yet */
> +	{ PCI_VDEVICE(INTEL, 0x4b63), board_ahci_low_power }, /* Elkhart Lake AHCI */
>  
>  	/* JMicron 360/1/3/5/6, match class to avoid IDE function */
>  	{ PCI_VENDOR_ID_JMICRON, PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,

Damien,

considering that we have many Intel AHCI controllers using
board_ahci_low_power already, and we haven't heard any complaints
on those other than for Tiger Lake...

I guess we could accept this one as well, and hope that it is just
Intel Tiger Lake that is problematic.. I guess the only real way to
know if Elkhart Lake is problematic as well is to see if we hear
people complain...


(Yes, one problem with all AHCI controllers using board_ahci_low_power,
is that hotplug is not working if you use what many distros are using,
CONFIG_SATA_MOBILE_LPM_POLICY=3 or higher... but that is expected as
AHCI 1.3.1 section 7.3.1.1 clearly states that you need to disable both
partial and slumber states to have reliable hot plug detection.)


Kind regards,
Niklas

Re: [PATCH v2] ata: ahci: Add Elkhart Lake AHCI controller

[Damien Le Moal]

On 8/29/23 20:33, Werner Fischer wrote:
> Elkhart Lake is the successor of Apollo Lake and Gemini Lake. These
> CPUs and their PCHs are used in mobile and embedded environments.
> 
> With this patch I suggest that Elkhart Lake SATA controllers [1] should
> use the default LPM policy for mobile chipsets.
> The disadvantage of missing hot-plug support with this setting should
> not be an issue, as those CPUs are used in embedded environments and
> not in servers with hot-plug backplanes.
> 
> We discovered that the Elkhart Lake SATA controllers have been missing
> in ahci.c after a customer reported the throttling of his SATA SSD
> after a short period of higher I/O. We determined the high temperature
> of the SSD controller in idle mode as the root cause for that.
> 
> Depending on the used SSD, we have seen up to 1.8 Watt lower system
> idle power usage and up to 30°C lower SSD controller temperatures in
> our tests, when we set med_power_with_dipm manually. I have provided a
> table showing seven different SATA SSDs from ATP, Intel/Solidigm and
> Samsung [2].
> 
> Intel lists a total of 3 SATA controller IDs (4B60, 4B62, 4B63) in [1]
> for those mobile PCHs.
> This commit just adds 0x4b63 as I do not have test systems with 0x4b60
> and 0x4b62 SATA controllers.
> I have tested this patch with a system which uses 0x4b63 as SATA
> controller.
> 
> [1] https://sata-io.org/product/8803
> [2] https://www.thomas-krenn.com/en/wiki/SATA_Link_Power_Management#Example_LES_v4
> 
> Signed-off-by: Werner Fischer <devlists@wefi.net>
> Cc: stable@vger.kernel.org

Applied to for-6.6. Thanks !

Note: when sending patches, please also add the maintainer(s) to your email
"to:" list.

-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2] ata: ahci: Add Elkhart Lake AHCI controller

[Werner Fischer]

On Sat, 2023-09-02 at 12:09 +0900, Damien Le Moal wrote:
> Applied to for-6.6. Thanks !
Thank you, too.

> Note: when sending patches, please also add the maintainer(s) to your
> email "to:" list.
Thank you for the hint, I will do it next time.

Best regards,
Werner