diff mbox series

ACPI: scan: Turn off unused power resources during initialization

Message ID 2527835.vZkJICojNU@kreacher (mailing list archive)
State Superseded, archived
Headers show
Series ACPI: scan: Turn off unused power resources during initialization | expand

Commit Message

Rafael J. Wysocki March 17, 2021, 4:49 p.m. UTC 
From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>

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 <david.e.box@linux.intel.com>
Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
---
 drivers/acpi/internal.h |    1 +
 drivers/acpi/scan.c     |    2 ++
 drivers/acpi/sleep.h    |    1 -
 3 files changed, 3 insertions(+), 1 deletion(-)

Comments

kernel test robot March 17, 2021, 7:05 p.m. UTC | #1 
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 <lkp@intel.com>

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/kbuild-all@lists.01.org
Paul Menzel March 17, 2021, 9:26 p.m. UTC | #2 
Dear Rafael,


Am 17.03.21 um 17:49 schrieb Rafael J. Wysocki:
> From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> 
> 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 <david.e.box@linux.intel.com>

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 <rafael.j.wysocki@intel.com>
> ---
>   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)
>   {
kernel test robot March 17, 2021, 9:27 p.m. UTC | #3 
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-a013-20210317 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 8ef111222a3dd12a9175f69c3bff598c46e8bdf7)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # 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
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All errors (new ones prefixed by >>):

>> ld.lld: error: undefined symbol: acpi_turn_off_unused_power_resources
   >>> referenced by scan.c
   >>>               acpi/scan.o:(acpi_scan_init) in archive drivers/built-in.a

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Rafael J. Wysocki March 18, 2021, 11:07 a.m. UTC | #4 
On Wed, Mar 17, 2021 at 10:27 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:
>
> Dear Rafael,
>
>
> Am 17.03.21 um 17:49 schrieb Rafael J. Wysocki:
> > From: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
> >
> > 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 <david.e.box@linux.intel.com>
>
> 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).
diff mbox series

Patch

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)
 {