[PATCH] ACPI / PM: Infer parent power state from child if unknown

[PATCH] ACPI / PM: Infer parent power state from child if unknown

[Rafael J. Wysocki]

It turns out that there are ACPI BIOSes defining device objects with
_PSx and without either _PSC or _PRx.  For devices corresponding to
those ACPI objetcs __acpi_bus_get_power() returns ACPI_STATE_UNKNOWN
and their initial power states are regarded as unknown as a result.
If such a device is a parent of another power-manageable device, the
child cannot be put into a low-power state through ACPI, because
__acpi_bus_set_power() refuses to change power states of devices
whose parents' power states are unknown.

To work around this problem, observe that the ACPI power state of
a device cannot be higher-power (lower-number) than the power state
of its parent.  Thus, if the device's _PSC method or the
configuration of its power resources indicates that the device is
in D0, the device's parent has to be in D0 as well.  Consequently,
if the parent's power state is unknown when we've just learned that
its child's power state is D0, we can safely set the parent's
power.state field to ACPI_STATE_D0.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Reviewed-by: Aaron Lu <aaron.lu@intel.com>
---

Hi Len,

I suppose we can put this one into -stable.

Thanks,
Rafael

---
 drivers/acpi/bus.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

Index: linux/drivers/acpi/bus.c
===================================================================
--- linux.orig/drivers/acpi/bus.c
+++ linux/drivers/acpi/bus.c
@@ -228,7 +228,16 @@ static int __acpi_bus_get_power(struct a
 		result = psc;
 	}
 	/* The test below covers ACPI_STATE_UNKNOWN too. */
-	if (result <= ACPI_STATE_D2) {
+	if (result == ACPI_STATE_D0) {
+		/*
+		 * If we were unsure about the device parent's power state up to
+		 * this point, the fact that the device is in D0 implies that
+		 * the parent has to be in D0 too.
+		 */
+		if (device->parent
+		    && device->parent->power.state == ACPI_STATE_UNKNOWN)
+			device->parent->power.state = ACPI_STATE_D0;
+	} else if (result <= ACPI_STATE_D2) {
 	  ; /* Do nothing. */
 	} else if (device->power.flags.power_resources) {
 		int error = acpi_power_get_inferred_state(device, &result);

Re: [PATCH] ACPI / PM: Infer parent power state from child if unknown

[Aaron Lu]

On Mon, Sep 10, 2012 at 09:50:22PM +0200, Rafael J. Wysocki wrote:
>  drivers/acpi/bus.c |   11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> Index: linux/drivers/acpi/bus.c
> ===================================================================
> --- linux.orig/drivers/acpi/bus.c
> +++ linux/drivers/acpi/bus.c
> @@ -228,7 +228,16 @@ static int __acpi_bus_get_power(struct a
>  		result = psc;
>  	}
>  	/* The test below covers ACPI_STATE_UNKNOWN too. */
> -	if (result <= ACPI_STATE_D2) {
> +	if (result == ACPI_STATE_D0) {

Oops, I just realized that the check here is too early if device does
not have _PSC but _PRx, its parent will miss the chance to get power
state updated :-)

Sorry for not spotting this earlier.

-Aaron

> +		/*
> +		 * If we were unsure about the device parent's power state up to
> +		 * this point, the fact that the device is in D0 implies that
> +		 * the parent has to be in D0 too.
> +		 */
> +		if (device->parent
> +		    && device->parent->power.state == ACPI_STATE_UNKNOWN)
> +			device->parent->power.state = ACPI_STATE_D0;
> +	} else if (result <= ACPI_STATE_D2) {
>  	  ; /* Do nothing. */
>  	} else if (device->power.flags.power_resources) {
>  		int error = acpi_power_get_inferred_state(device, &result);

Re: [PATCH] ACPI / PM: Infer parent power state from child if unknown

[Rafael J. Wysocki]

On Tuesday, September 11, 2012, Aaron Lu wrote:
> On Mon, Sep 10, 2012 at 09:50:22PM +0200, Rafael J. Wysocki wrote:
> >  drivers/acpi/bus.c |   11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > Index: linux/drivers/acpi/bus.c
> > ===================================================================
> > --- linux.orig/drivers/acpi/bus.c
> > +++ linux/drivers/acpi/bus.c
> > @@ -228,7 +228,16 @@ static int __acpi_bus_get_power(struct a
> >  		result = psc;
> >  	}
> >  	/* The test below covers ACPI_STATE_UNKNOWN too. */
> > -	if (result <= ACPI_STATE_D2) {
> > +	if (result == ACPI_STATE_D0) {
> 
> Oops, I just realized that the check here is too early if device does
> not have _PSC but _PRx, its parent will miss the chance to get power
> state updated :-)

Ah, right.  I'll post an update in a minute.

> Sorry for not spotting this earlier.

Well, I should have figured that myself too. ;-)

Thanks,
Rafael

[PATCH] ACPI / PM: Infer parent power state from child if unknown, v2

[Rafael J. Wysocki]

It turns out that there are ACPI BIOSes defining device objects with
_PSx and without either _PSC or _PRx.  For devices corresponding to
those ACPI objetcs __acpi_bus_get_power() returns ACPI_STATE_UNKNOWN
and their initial power states are regarded as unknown as a result.
If such a device is a parent of another power-manageable device, the
child cannot be put into a low-power state through ACPI, because
__acpi_bus_set_power() refuses to change power states of devices
whose parents' power states are unknown.

To work around this problem, observe that the ACPI power state of
a device cannot be higher-power (lower-number) than the power state
of its parent.  Thus, if the device's _PSC method or the
configuration of its power resources indicates that the device is
in D0, the device's parent has to be in D0 as well.  Consequently,
if the parent's power state is unknown when we've just learned that
its child's power state is D0, we can safely set the parent's
power.state field to ACPI_STATE_D0.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
---
 drivers/acpi/bus.c |   10 ++++++++++
 1 file changed, 10 insertions(+)

Index: linux/drivers/acpi/bus.c
===================================================================
--- linux.orig/drivers/acpi/bus.c
+++ linux/drivers/acpi/bus.c
@@ -237,6 +237,16 @@ static int __acpi_bus_get_power(struct a
 	} else if (result == ACPI_STATE_D3_HOT) {
 		result = ACPI_STATE_D3;
 	}
+
+	/*
+	 * If we were unsure about the device parent's power state up to this
+	 * point, the fact that the device is in D0 implies that the parent has
+	 * to be in D0 too.
+	 */
+	if (device->parent && device->parent->power.state == ACPI_STATE_UNKNOWN
+	    && result == ACPI_STATE_D0)
+		device->parent->power.state = ACPI_STATE_D0;
+
 	*state = result;
 
  out:

Re: [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2

[Aaron Lu]

On Tue, Sep 11, 2012 at 10:38:06PM +0200, Rafael J. Wysocki wrote:
> It turns out that there are ACPI BIOSes defining device objects with
> _PSx and without either _PSC or _PRx.  For devices corresponding to
> those ACPI objetcs __acpi_bus_get_power() returns ACPI_STATE_UNKNOWN
> and their initial power states are regarded as unknown as a result.
> If such a device is a parent of another power-manageable device, the
> child cannot be put into a low-power state through ACPI, because
> __acpi_bus_set_power() refuses to change power states of devices
> whose parents' power states are unknown.
> 
> To work around this problem, observe that the ACPI power state of
> a device cannot be higher-power (lower-number) than the power state
> of its parent.  Thus, if the device's _PSC method or the
> configuration of its power resources indicates that the device is
> in D0, the device's parent has to be in D0 as well.  Consequently,
> if the parent's power state is unknown when we've just learned that
> its child's power state is D0, we can safely set the parent's
> power.state field to ACPI_STATE_D0.
> 
> Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>

Tested-by: Aaron Lu <aaron.lu@intel.com>

-Aaron

> ---
>  drivers/acpi/bus.c |   10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> Index: linux/drivers/acpi/bus.c
> ===================================================================
> --- linux.orig/drivers/acpi/bus.c
> +++ linux/drivers/acpi/bus.c
> @@ -237,6 +237,16 @@ static int __acpi_bus_get_power(struct a
>  	} else if (result == ACPI_STATE_D3_HOT) {
>  		result = ACPI_STATE_D3;
>  	}
> +
> +	/*
> +	 * If we were unsure about the device parent's power state up to this
> +	 * point, the fact that the device is in D0 implies that the parent has
> +	 * to be in D0 too.
> +	 */
> +	if (device->parent && device->parent->power.state == ACPI_STATE_UNKNOWN
> +	    && result == ACPI_STATE_D0)
> +		device->parent->power.state = ACPI_STATE_D0;
> +
>  	*state = result;
>  
>   out: