LinuxLists.cc - [PATCH] ACPI: scan: Turn off unused power resources during initialization

2021-03-17 16:53:44

Subject: [PATCH] ACPI: scan: Turn off unused power resources during initialization

From: Rafael J. Wysocki <[email protected]>

It is reported that on certain platforms unused ACPI power resources
that have not been explicitly turned off prevent the platform from
reaching the lowest power state in suspend-to-idle which leads to
excessive power draw.

For this reason, turn all of the unused ACPI power resources off
at the end of the initial namespace scan for devices in analogy with
resume from suspend-to-RAM.

Reported-by: David Box <[email protected]>
Signed-off-by: Rafael J. Wysocki <[email protected]>
---
drivers/acpi/internal.h | 1 +
drivers/acpi/scan.c | 2 ++
drivers/acpi/sleep.h | 1 -
3 files changed, 3 insertions(+), 1 deletion(-)

Index: linux-pm/drivers/acpi/internal.h
===================================================================
--- linux-pm.orig/drivers/acpi/internal.h
+++ linux-pm/drivers/acpi/internal.h
@@ -139,6 +139,7 @@ int acpi_device_sleep_wake(struct acpi_d
int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
int acpi_power_on_resources(struct acpi_device *device, int state);
int acpi_power_transition(struct acpi_device *device, int state);
+void acpi_turn_off_unused_power_resources(void);

/* --------------------------------------------------------------------------
Device Power Management
Index: linux-pm/drivers/acpi/scan.c
===================================================================
--- linux-pm.orig/drivers/acpi/scan.c
+++ linux-pm/drivers/acpi/scan.c
@@ -2360,6 +2360,8 @@ int __init acpi_scan_init(void)
}
}

+ acpi_turn_off_unused_power_resources();
+
acpi_scan_initialized = true;

out:
Index: linux-pm/drivers/acpi/sleep.h
===================================================================
--- linux-pm.orig/drivers/acpi/sleep.h
+++ linux-pm/drivers/acpi/sleep.h
@@ -8,7 +8,6 @@ extern struct list_head acpi_wakeup_devi
extern struct mutex acpi_device_lock;

extern void acpi_resume_power_resources(void);
-extern void acpi_turn_off_unused_power_resources(void);

static inline acpi_status acpi_set_waking_vector(u32 wakeup_address)
{

2021-03-17 21:43:27

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH] ACPI: scan: Turn off unused power resources during initialization

Hi "Rafael,

I love your patch! Yet something to improve:

[auto build test ERROR on pm/linux-next]
[also build test ERROR on linux/master linus/master v5.12-rc3 next-20210317]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url: https://github.com/0day-ci/linux/commits/Rafael-J-Wysocki/ACPI-scan-Turn-off-unused-power-resources-during-initialization/20210318-005323
base: https://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm.git linux-next
config: x86_64-randconfig-c022-20210317 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-22) 9.3.0
reproduce (this is a W=1 build):
# https://github.com/0day-ci/linux/commit/2462e52396b65921f0c83dfe19d5fcd9927d3498
git remote add linux-review https://github.com/0day-ci/linux
git fetch --no-tags linux-review Rafael-J-Wysocki/ACPI-scan-Turn-off-unused-power-resources-during-initialization/20210318-005323
git checkout 2462e52396b65921f0c83dfe19d5fcd9927d3498
# save the attached .config to linux build tree
make W=1 ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <[email protected]>

All errors (new ones prefixed by >>):

ld: drivers/acpi/scan.o: in function `acpi_scan_init':
>> drivers/acpi/scan.c:2363: undefined reference to `acpi_turn_off_unused_power_resources'

vim +2363 drivers/acpi/scan.c

2290
2291 int __init acpi_scan_init(void)
2292 {
2293 int result;
2294 acpi_status status;
2295 struct acpi_table_stao *stao_ptr;
2296
2297 acpi_pci_root_init();
2298 acpi_pci_link_init();
2299 acpi_processor_init();
2300 acpi_platform_init();
2301 acpi_lpss_init();
2302 acpi_apd_init();
2303 acpi_cmos_rtc_init();
2304 acpi_container_init();
2305 acpi_memory_hotplug_init();
2306 acpi_watchdog_init();
2307 acpi_pnp_init();
2308 acpi_int340x_thermal_init();
2309 acpi_amba_init();
2310 acpi_init_lpit();
2311
2312 acpi_scan_add_handler(&generic_device_handler);
2313
2314 /*
2315 * If there is STAO table, check whether it needs to ignore the UART
2316 * device in SPCR table.
2317 */
2318 status = acpi_get_table(ACPI_SIG_STAO, 0,
2319 (struct acpi_table_header **)&stao_ptr);
2320 if (ACPI_SUCCESS(status)) {
2321 if (stao_ptr->header.length > sizeof(struct acpi_table_stao))
2322 pr_info(PREFIX "STAO Name List not yet supported.\n");
2323
2324 if (stao_ptr->ignore_uart)
2325 acpi_get_spcr_uart_addr();
2326
2327 acpi_put_table((struct acpi_table_header *)stao_ptr);
2328 }
2329
2330 acpi_gpe_apply_masked_gpes();
2331 acpi_update_all_gpes();
2332
2333 /*
2334 * Although we call __add_memory() that is documented to require the
2335 * device_hotplug_lock, it is not necessary here because this is an
2336 * early code when userspace or any other code path cannot trigger
2337 * hotplug/hotunplug operations.
2338 */
2339 mutex_lock(&acpi_scan_lock);
2340 /*
2341 * Enumerate devices in the ACPI namespace.
2342 */
2343 result = acpi_bus_scan(ACPI_ROOT_OBJECT);
2344 if (result)
2345 goto out;
2346
2347 result = acpi_bus_get_device(ACPI_ROOT_OBJECT, &acpi_root);
2348 if (result)
2349 goto out;
2350
2351 /* Fixed feature devices do not exist on HW-reduced platform */
2352 if (!acpi_gbl_reduced_hardware) {
2353 result = acpi_bus_scan_fixed();
2354 if (result) {
2355 acpi_detach_data(acpi_root->handle,
2356 acpi_scan_drop_device);
2357 acpi_device_del(acpi_root);
2358 put_device(&acpi_root->dev);
2359 goto out;
2360 }
2361 }
2362
> 2363 acpi_turn_off_unused_power_resources();
2364
2365 acpi_scan_initialized = true;
2366
2367 out:
2368 mutex_unlock(&acpi_scan_lock);
2369 return result;
2370 }
2371

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/[email protected]

Attachments:

(No filename) (4.13 kB)
.config.gz (27.85 kB)
Download all attachments

2021-03-17 21:47:52

by Paul Menzel

[permalink] [raw]

Subject: Re: [PATCH] ACPI: scan: Turn off unused power resources during initialization

Dear Rafael,

Am 17.03.21 um 17:49 schrieb Rafael J. Wysocki:
> From: Rafael J. Wysocki <[email protected]>
>
> It is reported that on certain platforms unused ACPI power resources
> that have not been explicitly turned off prevent the platform from
> reaching the lowest power state in suspend-to-idle which leads to
> excessive power draw.
>
> For this reason, turn all of the unused ACPI power resources off
> at the end of the initial namespace scan for devices in analogy with
> resume from suspend-to-RAM.
>
> Reported-by: David Box <[email protected]>

Thank you for the patch. Could you please add more details to the commit
message, saying what device this was on, and if there were some
error/warning messages pointing to the problem?

Kind regards,

Paul

> Signed-off-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/internal.h | 1 +
> drivers/acpi/scan.c | 2 ++
> drivers/acpi/sleep.h | 1 -
> 3 files changed, 3 insertions(+), 1 deletion(-)
>
> Index: linux-pm/drivers/acpi/internal.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/internal.h
> +++ linux-pm/drivers/acpi/internal.h
> @@ -139,6 +139,7 @@ int acpi_device_sleep_wake(struct acpi_d
> int acpi_power_get_inferred_state(struct acpi_device *device, int *state);
> int acpi_power_on_resources(struct acpi_device *device, int state);
> int acpi_power_transition(struct acpi_device *device, int state);
> +void acpi_turn_off_unused_power_resources(void);
>
> /* --------------------------------------------------------------------------
> Device Power Management
> Index: linux-pm/drivers/acpi/scan.c
> ===================================================================
> --- linux-pm.orig/drivers/acpi/scan.c
> +++ linux-pm/drivers/acpi/scan.c
> @@ -2360,6 +2360,8 @@ int __init acpi_scan_init(void)
> }
> }
>
> + acpi_turn_off_unused_power_resources();
> +
> acpi_scan_initialized = true;
>
> out:
> Index: linux-pm/drivers/acpi/sleep.h
> ===================================================================
> --- linux-pm.orig/drivers/acpi/sleep.h
> +++ linux-pm/drivers/acpi/sleep.h
> @@ -8,7 +8,6 @@ extern struct list_head acpi_wakeup_devi
> extern struct mutex acpi_device_lock;
>
> extern void acpi_resume_power_resources(void);
> -extern void acpi_turn_off_unused_power_resources(void);
>
> static inline acpi_status acpi_set_waking_vector(u32 wakeup_address)
> {

2021-03-17 21:50:39

by kernel test robot

[permalink] [raw]

Subject: Re: [PATCH] ACPI: scan: Turn off unused power resources during initialization

Attachments:

(No filename) (1.99 kB)
.config.gz (38.88 kB)
Download all attachments

2021-03-18 11:11:47

by Rafael J. Wysocki

[permalink] [raw]

Subject: Re: [PATCH] ACPI: scan: Turn off unused power resources during initialization

On Wed, Mar 17, 2021 at 10:27 PM Paul Menzel <[email protected]> wrote:
>
> Dear Rafael,
>
>
> Am 17.03.21 um 17:49 schrieb Rafael J. Wysocki:
> > From: Rafael J. Wysocki <[email protected]>
> >
> > It is reported that on certain platforms unused ACPI power resources
> > that have not been explicitly turned off prevent the platform from
> > reaching the lowest power state in suspend-to-idle which leads to
> > excessive power draw.
> >
> > For this reason, turn all of the unused ACPI power resources off
> > at the end of the initial namespace scan for devices in analogy with
> > resume from suspend-to-RAM.
> >
> > Reported-by: David Box <[email protected]>
>
> Thank you for the patch. Could you please add more details to the commit
> message, saying what device this was on, and if there were some
> error/warning messages pointing to the problem?

The actual report is not public, so I cannot quote it.

In essence, there is a power resource in the affected system that
would be dedicated to a specific device, but that device is not
present in that variant of the platform.

However, the behavior introduced by this patch is generally mandated
by the spec and evidently depended on by firmware developers (see the
second paragraph in
https://uefi.org/specs/ACPI/6.4/07_Power_and_Performance_Mgmt/device-power-management-objects.html).