Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit f4ac6476 authored by Hans de Goede's avatar Hans de Goede Committed by Tejun Heo
Browse files

libata: Add new med_power_with_dipm link_power_management_policy setting 

As described by Matthew Garret quite a while back:
https://mjg59.dreamwidth.org/34868.html

Intel CPUs starting with the Haswell generation need SATA links to power
down for the "package" part of the CPU to reach low power-states like
PC7 / P8 which bring a significant power-saving with them.

The default max_performance lpm policy does not allow for these high
PC states, both the medium_power and min_power policies do allow this.

The min_power policy saves significantly more power, but there are some
reports of some disks / SSDs not liking min_power leading to system
crashes and in some cases even data corruption has been reported.

Matthew has found a document documenting the default settings of
Intel's IRST Windows driver with which most laptops ship:
https://www-ssl.intel.com/content/dam/doc/reference-guide/sata-devices-implementation-recommendations.pdf



Matthew wrote a patch changing med_power to match those defaults, but
that never got anywhere as some people where reporting issues with the
patch-set that patch was a part of.

This commit is another attempt to make the default IRST driver settings
available under Linux, but instead of changing medium_power and
potentially introducing regressions, this commit adds a new
med_power_with_dipm setting which is identical to the existing
medium_power accept that it enables dipm on top, which makes it match
the Windows IRST driver settings, which should hopefully be safe to
use on most devices.

The med_power_with_dipm setting is close to min_power, except that:
a) It does not use host-initiated slumber mode (ASP not set),
   but it does allow device-initiated slumber
b) It does not enable DevSlp mode

On my T440s test laptop I get the following power savings when idle:
medium_power		0.9W
med_power_with_dipm	1.2W
min_power		1.2W

Suggested-by: default avatarMatthew Garrett <mjg59@srcf.ucam.org>
Cc: Matthew Garrett <mjg59@srcf.ucam.org>
Signed-off-by: default avatarHans de Goede <hdegoede@redhat.com>
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
parent 23e4c67a

drivers/ata/libata-core.c

+1 −0
Original line number Original line Diff line number Diff line
@@ -3964,6 +3964,7 @@ int sata_link_scr_lpm(struct ata_link *link, enum ata_lpm_policy policy, 
		scontrol &= ~(0x1 << 8);

		scontrol &= ~(0x1 << 8);

		scontrol |= (0x6 << 8);

		scontrol |= (0x6 << 8);

		break;

		break;

	case ATA_LPM_MED_POWER_WITH_DIPM:

	case ATA_LPM_MIN_POWER:

	case ATA_LPM_MIN_POWER:

		if (ata_link_nr_enabled(link) > 0)

		if (ata_link_nr_enabled(link) > 0)

			/* no restrictions on LPM transitions */

			/* no restrictions on LPM transitions */

drivers/ata/libata-eh.c

+5 −5
Original line number Original line Diff line number Diff line
@@ -3454,9 +3454,9 @@ static int ata_eh_maybe_retry_flush(struct ata_device *dev) 
 *	@r_failed_dev: out parameter for failed device

 *	@r_failed_dev: out parameter for failed device

 *

 *

 *	Enable SATA Interface power management.  This will enable

 *	Enable SATA Interface power management.  This will enable

 *	Device Interface Power Management (DIPM) for min_power

 *	Device Interface Power Management (DIPM) for min_power and

 * 	policy, and then call driver specific callbacks for

 *	medium_power_with_dipm policies, and then call driver specific

 *	enabling Host Initiated Power management.

 *	callbacks for enabling Host Initiated Power management.

 *

 *

 *	LOCKING:

 *	LOCKING:

 *	EH context.

 *	EH context.
@@ -3502,7 +3502,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, 
			hints &= ~ATA_LPM_HIPM;

			hints &= ~ATA_LPM_HIPM;





		/* disable DIPM before changing link config */

		/* disable DIPM before changing link config */

		if (policy != ATA_LPM_MIN_POWER && dipm) {

		if (policy < ATA_LPM_MED_POWER_WITH_DIPM && dipm) {

			err_mask = ata_dev_set_feature(dev,

			err_mask = ata_dev_set_feature(dev,

					SETFEATURES_SATA_DISABLE, SATA_DIPM);

					SETFEATURES_SATA_DISABLE, SATA_DIPM);

			if (err_mask && err_mask != AC_ERR_DEV) {

			if (err_mask && err_mask != AC_ERR_DEV) {
@@ -3545,7 +3545,7 @@ static int ata_eh_set_lpm(struct ata_link *link, enum ata_lpm_policy policy, 




	/* host config updated, enable DIPM if transitioning to MIN_POWER */

	/* host config updated, enable DIPM if transitioning to MIN_POWER */

	ata_for_each_dev(dev, link, ENABLED) {

	ata_for_each_dev(dev, link, ENABLED) {

		if (policy == ATA_LPM_MIN_POWER && !no_dipm &&

		if (policy >= ATA_LPM_MED_POWER_WITH_DIPM && !no_dipm &&

		    ata_id_has_dipm(dev->id)) {

		    ata_id_has_dipm(dev->id)) {

			err_mask = ata_dev_set_feature(dev,

			err_mask = ata_dev_set_feature(dev,

					SETFEATURES_SATA_ENABLE, SATA_DIPM);

					SETFEATURES_SATA_ENABLE, SATA_DIPM);

drivers/ata/libata-scsi.c

+5 −4
Original line number Original line Diff line number Diff line
@@ -109,6 +109,7 @@ static const char *ata_lpm_policy_names[] = { 
	[ATA_LPM_UNKNOWN]		= "max_performance",

	[ATA_LPM_UNKNOWN]		= "max_performance",

	[ATA_LPM_MAX_POWER]		= "max_performance",

	[ATA_LPM_MAX_POWER]		= "max_performance",

	[ATA_LPM_MED_POWER]		= "medium_power",

	[ATA_LPM_MED_POWER]		= "medium_power",

	[ATA_LPM_MED_POWER_WITH_DIPM]	= "med_power_with_dipm",

	[ATA_LPM_MIN_POWER]		= "min_power",

	[ATA_LPM_MIN_POWER]		= "min_power",

};

};

include/linux/libata.h

+1 −0
Original line number Original line Diff line number Diff line
@@ -522,6 +522,7 @@ enum ata_lpm_policy { 
	ATA_LPM_UNKNOWN,

	ATA_LPM_UNKNOWN,

	ATA_LPM_MAX_POWER,

	ATA_LPM_MAX_POWER,

	ATA_LPM_MED_POWER,

	ATA_LPM_MED_POWER,

	ATA_LPM_MED_POWER_WITH_DIPM, /* Med power + DIPM as win IRST does */

	ATA_LPM_MIN_POWER,

	ATA_LPM_MIN_POWER,

};

};