diff mbox series

ACPI: scan: Release PM resources blocked by unused objects

Message ID 5507158.DvuYhMxLoT@kreacher (mailing list archive)
State Mainlined, archived
Headers show
Series ACPI: scan: Release PM resources blocked by unused objects | expand

Commit Message

Rafael J. Wysocki Oct. 9, 2021, 2:22 p.m. UTC
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

On some systems the ACPI namespace contains device objects that are
not used in certain configurations of the system.  If they start off
in the D0 power state configuration, they will stay in it until the
system reboots, because of the lack of any mechanism possibly causing
their configuration to change.  If that happens, they may prevent
some power resources from being turned off or generally they may
prevent the platform from getting into the deepest low-power states
thus causing some energy to be wasted.

Address this issue by changing the configuration of unused ACPI
device objects to the D3cold power state one after carrying out
the ACPI-based enumeration of devices.

BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=214091
Link: https://lore.kernel.org/linux-acpi/20211007205126.11769-1-mario.limonciello@amd.com/
Reported-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---

Mario,

I've changed acpi_dev_turn_off_if_unused() to check pnp.type.hardware_id
instead of pnp.type.bus_address, because on some systems there are objects
with both _ADR and _HID (which is against the spec) and the former is bogus.

Please test this one, even though for PCI devices there should be no difference.

Thanks!

---
 drivers/acpi/glue.c     |   25 +++++++++++++++++++++++++
 drivers/acpi/internal.h |    1 +
 drivers/acpi/scan.c     |    6 ++++++
 3 files changed, 32 insertions(+)

Comments

Mario Limonciello Oct. 12, 2021, 9:10 p.m. UTC | #1
On 10/9/2021 09:22, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> On some systems the ACPI namespace contains device objects that are
> not used in certain configurations of the system.  If they start off
> in the D0 power state configuration, they will stay in it until the
> system reboots, because of the lack of any mechanism possibly causing
> their configuration to change.  If that happens, they may prevent
> some power resources from being turned off or generally they may
> prevent the platform from getting into the deepest low-power states
> thus causing some energy to be wasted.
> 
> Address this issue by changing the configuration of unused ACPI
> device objects to the D3cold power state one after carrying out
> the ACPI-based enumeration of devices.
> 
> BugLink: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fbugzilla.kernel.org%2Fshow_bug.cgi%3Fid%3D214091&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C6e87f8a1465c424d721c08d98b303310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637693861879301335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=zCknM%2FyK8unP%2BI0fstH8OoB5UUVfwOKqeERZOVu9lQY%3D&amp;reserved=0
> Link: https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Flinux-acpi%2F20211007205126.11769-1-mario.limonciello%40amd.com%2F&amp;data=04%7C01%7Cmario.limonciello%40amd.com%7C6e87f8a1465c424d721c08d98b303310%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637693861879301335%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000&amp;sdata=4yE1GvAineqku3yjQyfXv6nifmx1uqoEAAbf57eDiFY%3D&amp;reserved=0
> Reported-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

Tested-by: Mario Limonciello <mario.limonciello@amd.com>

> ---
> 
> Mario,
> 
> I've changed acpi_dev_turn_off_if_unused() to check pnp.type.hardware_id
> instead of pnp.type.bus_address, because on some systems there are objects
> with both _ADR and _HID (which is against the spec) and the former is bogus.
> 
> Please test this one, even though for PCI devices there should be no difference.
> 

Yes, Thanks.  I had a try and this new version is also working correctly 
on the system this was found.

> Thanks!
> 
> ---
>   drivers/acpi/glue.c     |   25 +++++++++++++++++++++++++
>   drivers/acpi/internal.h |    1 +
>   drivers/acpi/scan.c     |    6 ++++++
>   3 files changed, 32 insertions(+)
> 
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -2559,6 +2559,12 @@ int __init acpi_scan_init(void)
>   		}
>   	}
>   
> +	/*
> +	 * Make sure that power management resources are not blocked by ACPI
> +	 * device objects with no users.
> +	 */
> +	bus_for_each_dev(&acpi_bus_type, NULL, NULL, acpi_dev_turn_off_if_unused);
> +
>   	acpi_turn_off_unused_power_resources();
>   
>   	acpi_scan_initialized = true;
> Index: linux-pm/drivers/acpi/glue.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/glue.c
> +++ linux-pm/drivers/acpi/glue.c
> @@ -350,3 +350,28 @@ void acpi_device_notify_remove(struct de
>   
>   	acpi_unbind_one(dev);
>   }
> +
> +int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used)
> +{
> +	struct acpi_device *adev = to_acpi_device(dev);
> +
> +	/*
> +	 * Skip device objects with device IDs, because they may be in use even
> +	 * if they are not companions of any physical device objects.
> +	 */
> +	if (adev->pnp.type.hardware_id)
> +		return 0;
> +
> +	mutex_lock(&adev->physical_node_lock);
> +
> +	/*
> +	 * Device objects without device IDs are not in use if they have no
> +	 * corresponding physical device objects.
> +	 */
> +	if (list_empty(&adev->physical_node_list))
> +		acpi_device_set_power(adev, ACPI_STATE_D3_COLD);
> +
> +	mutex_unlock(&adev->physical_node_lock);
> +
> +	return 0;
> +}
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -117,6 +117,7 @@ bool acpi_device_is_battery(struct acpi_
>   bool acpi_device_is_first_physical_node(struct acpi_device *adev,
>   					const struct device *dev);
>   int acpi_bus_register_early_device(int type);
> +int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used);
>   
>   /* --------------------------------------------------------------------------
>                        Device Matching and Notification
> 
> 
>
diff mbox series

Patch

Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -2559,6 +2559,12 @@  int __init acpi_scan_init(void)
 		}
 	}
 
+	/*
+	 * Make sure that power management resources are not blocked by ACPI
+	 * device objects with no users.
+	 */
+	bus_for_each_dev(&acpi_bus_type, NULL, NULL, acpi_dev_turn_off_if_unused);
+
 	acpi_turn_off_unused_power_resources();
 
 	acpi_scan_initialized = true;
Index: linux-pm/drivers/acpi/glue.c
===================================================================
--- linux-pm.orig/drivers/acpi/glue.c
+++ linux-pm/drivers/acpi/glue.c
@@ -350,3 +350,28 @@  void acpi_device_notify_remove(struct de
 
 	acpi_unbind_one(dev);
 }
+
+int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used)
+{
+	struct acpi_device *adev = to_acpi_device(dev);
+
+	/*
+	 * Skip device objects with device IDs, because they may be in use even
+	 * if they are not companions of any physical device objects.
+	 */
+	if (adev->pnp.type.hardware_id)
+		return 0;
+
+	mutex_lock(&adev->physical_node_lock);
+
+	/*
+	 * Device objects without device IDs are not in use if they have no
+	 * corresponding physical device objects.
+	 */
+	if (list_empty(&adev->physical_node_list))
+		acpi_device_set_power(adev, ACPI_STATE_D3_COLD);
+
+	mutex_unlock(&adev->physical_node_lock);
+
+	return 0;
+}
Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -117,6 +117,7 @@  bool acpi_device_is_battery(struct acpi_
 bool acpi_device_is_first_physical_node(struct acpi_device *adev,
 					const struct device *dev);
 int acpi_bus_register_early_device(int type);
+int acpi_dev_turn_off_if_unused(struct device *dev, void *not_used);
 
 /* --------------------------------------------------------------------------
                      Device Matching and Notification