linux-acpi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
@ 2012-08-27  7:38 Aaron Lu
  2012-09-06 21:45 ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Lu @ 2012-08-27  7:38 UTC (permalink / raw)
  To: Len Brown, Rafael J. Wysocki; +Cc: linux-acpi, linux-pm, Aaron Lu, Aaron Lu

Currently, when we are trying to get the power state of an acpi device,
we will do the following:
If device is not power manageable, init its power state as its parent or
if it does not have a parent, init as D0;
If device is power manageable, evaluate _PSC and then refine with
acpi_power_get_inferred_state.

But there exist some devices with _PSx defined, but no _PSC or _PRx.
It is power manageable, but the above method to get power state does
not cover this case and its power state will be UNKNOWN(255).

So change the check of power manageable to whether _PSC and _PRx
defined.

Signed-off-by: Aaron Lu <aaron.lu@intel.com>
---
 drivers/acpi/bus.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index 9628652..b564e6d 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -207,7 +207,8 @@ static int __acpi_bus_get_power(struct acpi_device *device, int *state)
 	if (!device || !state)
 		return -EINVAL;
 
-	if (!device->flags.power_manageable) {
+	if (!device->power.flags.explicit_get &&
+			!device->power.flags.power_resources) {
 		/* TBD: Non-recursive algorithm for walking up hierarchy. */
 		*state = device->parent ?
 			device->parent->power.state : ACPI_STATE_D0;
-- 
1.7.12.21.g871e293


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-08-27  7:38 [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case Aaron Lu
@ 2012-09-06 21:45 ` Rafael J. Wysocki
  2012-09-06 21:53   ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-06 21:45 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Monday, August 27, 2012, Aaron Lu wrote:
> Currently, when we are trying to get the power state of an acpi device,
> we will do the following:
> If device is not power manageable, init its power state as its parent or
> if it does not have a parent, init as D0;
> If device is power manageable, evaluate _PSC and then refine with
> acpi_power_get_inferred_state.
> 
> But there exist some devices with _PSx defined, but no _PSC or _PRx.
> It is power manageable, but the above method to get power state does
> not cover this case and its power state will be UNKNOWN(255).
> 
> So change the check of power manageable to whether _PSC and _PRx
> defined.
> 
> Signed-off-by: Aaron Lu <aaron.lu@intel.com>

Applied to the linux-next branch of the linux-pm.git tree.

I think it should go to -stable too and therefore it would be good to have
it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections.

Thanks,
Rafael


> ---
>  drivers/acpi/bus.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> index 9628652..b564e6d 100644
> --- a/drivers/acpi/bus.c
> +++ b/drivers/acpi/bus.c
> @@ -207,7 +207,8 @@ static int __acpi_bus_get_power(struct acpi_device *device, int *state)
>  	if (!device || !state)
>  		return -EINVAL;
>  
> -	if (!device->flags.power_manageable) {
> +	if (!device->power.flags.explicit_get &&
> +			!device->power.flags.power_resources) {
>  		/* TBD: Non-recursive algorithm for walking up hierarchy. */
>  		*state = device->parent ?
>  			device->parent->power.state : ACPI_STATE_D0;
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-06 21:45 ` Rafael J. Wysocki
@ 2012-09-06 21:53   ` Rafael J. Wysocki
  2012-09-07  0:35     ` Aaron Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-06 21:53 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Thursday, September 06, 2012, Rafael J. Wysocki wrote:
> On Monday, August 27, 2012, Aaron Lu wrote:
> > Currently, when we are trying to get the power state of an acpi device,
> > we will do the following:
> > If device is not power manageable, init its power state as its parent or
> > if it does not have a parent, init as D0;
> > If device is power manageable, evaluate _PSC and then refine with
> > acpi_power_get_inferred_state.
> > 
> > But there exist some devices with _PSx defined, but no _PSC or _PRx.
> > It is power manageable, but the above method to get power state does
> > not cover this case and its power state will be UNKNOWN(255).
> > 
> > So change the check of power manageable to whether _PSC and _PRx
> > defined.
> > 
> > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> 
> Applied to the linux-next branch of the linux-pm.git tree.
> 
> I think it should go to -stable too and therefore it would be good to have
> it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections.

On a second thought, perhaps the initial power state of those devices
_should_ be "unknown", hmm?

After all, we don't know what power state the device is in.

Is there any practical user-visible problem this causes to happen?

Rafael


> > ---
> >  drivers/acpi/bus.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
> > index 9628652..b564e6d 100644
> > --- a/drivers/acpi/bus.c
> > +++ b/drivers/acpi/bus.c
> > @@ -207,7 +207,8 @@ static int __acpi_bus_get_power(struct acpi_device *device, int *state)
> >  	if (!device || !state)
> >  		return -EINVAL;
> >  
> > -	if (!device->flags.power_manageable) {
> > +	if (!device->power.flags.explicit_get &&
> > +			!device->power.flags.power_resources) {
> >  		/* TBD: Non-recursive algorithm for walking up hierarchy. */
> >  		*state = device->parent ?
> >  			device->parent->power.state : ACPI_STATE_D0;
> > 
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-06 21:53   ` Rafael J. Wysocki
@ 2012-09-07  0:35     ` Aaron Lu
  2012-09-07 11:46       ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Lu @ 2012-09-07  0:35 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Thu, Sep 06, 2012 at 11:53:34PM +0200, Rafael J. Wysocki wrote:
> On Thursday, September 06, 2012, Rafael J. Wysocki wrote:
> > On Monday, August 27, 2012, Aaron Lu wrote:
> > > Currently, when we are trying to get the power state of an acpi device,
> > > we will do the following:
> > > If device is not power manageable, init its power state as its parent or
> > > if it does not have a parent, init as D0;
> > > If device is power manageable, evaluate _PSC and then refine with
> > > acpi_power_get_inferred_state.
> > > 
> > > But there exist some devices with _PSx defined, but no _PSC or _PRx.
> > > It is power manageable, but the above method to get power state does
> > > not cover this case and its power state will be UNKNOWN(255).
> > > 
> > > So change the check of power manageable to whether _PSC and _PRx
> > > defined.
> > > 
> > > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > 
> > Applied to the linux-next branch of the linux-pm.git tree.
> > 
> > I think it should go to -stable too and therefore it would be good to have
> > it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections.
> 
> On a second thought, perhaps the initial power state of those devices
> _should_ be "unknown", hmm?
> 
> After all, we don't know what power state the device is in.

Agree here but I think this is a safe assumption that on initial system
boot, all devices should be at D0. We have already assumed this fact if
the device is not power manageable and has no parent.

And this patch just changed the condition to not judging if power
manageable but if it has _PSC or _PRx, since we will use that to know
the device's power state, so I think using _PSC or _PRx as the
condition check is more precise.

> 
> Is there any practical user-visible problem this causes to happen?

Yes, on a test system, when I try to put a device into D3 cold and ACPI
will complain that I can't due to its parent is in a even lower power
state UNKNOWN(255), this parent device is power manageable but has no
_PSC and _PRx defined.

Thanks,
Aaron

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-07  0:35     ` Aaron Lu
@ 2012-09-07 11:46       ` Rafael J. Wysocki
  2012-09-07 14:32         ` Aaron Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-07 11:46 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Friday, September 07, 2012, Aaron Lu wrote:
> On Thu, Sep 06, 2012 at 11:53:34PM +0200, Rafael J. Wysocki wrote:
> > On Thursday, September 06, 2012, Rafael J. Wysocki wrote:
> > > On Monday, August 27, 2012, Aaron Lu wrote:
> > > > Currently, when we are trying to get the power state of an acpi device,
> > > > we will do the following:
> > > > If device is not power manageable, init its power state as its parent or
> > > > if it does not have a parent, init as D0;
> > > > If device is power manageable, evaluate _PSC and then refine with
> > > > acpi_power_get_inferred_state.
> > > > 
> > > > But there exist some devices with _PSx defined, but no _PSC or _PRx.
> > > > It is power manageable, but the above method to get power state does
> > > > not cover this case and its power state will be UNKNOWN(255).
> > > > 
> > > > So change the check of power manageable to whether _PSC and _PRx
> > > > defined.
> > > > 
> > > > Signed-off-by: Aaron Lu <aaron.lu@intel.com>
> > > 
> > > Applied to the linux-next branch of the linux-pm.git tree.
> > > 
> > > I think it should go to -stable too and therefore it would be good to have
> > > it in v3.6, so I'd like to push it to Linus for -rc5, if Len has no objections.
> > 
> > On a second thought, perhaps the initial power state of those devices
> > _should_ be "unknown", hmm?
> > 
> > After all, we don't know what power state the device is in.
> 
> Agree here but I think this is a safe assumption that on initial system
> boot, all devices should be at D0. We have already assumed this fact if
> the device is not power manageable and has no parent.

That is mandated by the spec, though.

> And this patch just changed the condition to not judging if power
> manageable but if it has _PSC or _PRx, since we will use that to know
> the device's power state, so I think using _PSC or _PRx as the
> condition check is more precise.

It is not clear if we can assume anything about the initial power states
of devices having _PSx defined if they cannot be determined through
_PSC or power resources.

> > Is there any practical user-visible problem this causes to happen?
> 
> Yes, on a test system, when I try to put a device into D3 cold and ACPI
> will complain that I can't due to its parent is in a even lower power
> state UNKNOWN(255), this parent device is power manageable but has no
> _PSC and _PRx defined.

Perhaps we can force _PS0 for such devices to start with, so that we know
for sure that the initial state is D0?

Rafael

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-07 11:46       ` Rafael J. Wysocki
@ 2012-09-07 14:32         ` Aaron Lu
  2012-09-07 18:32           ` Rafael J. Wysocki
  0 siblings, 1 reply; 14+ messages in thread
From: Aaron Lu @ 2012-09-07 14:32 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On 09/07/2012 07:46 PM, Rafael J. Wysocki wrote:
>> Yes, on a test system, when I try to put a device into D3 cold and ACPI
>> will complain that I can't due to its parent is in a even lower power
>> state UNKNOWN(255), this parent device is power manageable but has no
>> _PSC and _PRx defined.
>
> Perhaps we can force _PS0 for such devices to start with, so that we know
> for sure that the initial state is D0?

Sounds good, I'll update the patch, thanks for the advice.

-Aaron

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-07 14:32         ` Aaron Lu
@ 2012-09-07 18:32           ` Rafael J. Wysocki
  2012-09-10  0:38             ` Aaron Lu
  0 siblings, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-07 18:32 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Friday, September 07, 2012, Aaron Lu wrote:
> On 09/07/2012 07:46 PM, Rafael J. Wysocki wrote:
> >> Yes, on a test system, when I try to put a device into D3 cold and ACPI
> >> will complain that I can't due to its parent is in a even lower power
> >> state UNKNOWN(255), this parent device is power manageable but has no
> >> _PSC and _PRx defined.
> >
> > Perhaps we can force _PS0 for such devices to start with, so that we know
> > for sure that the initial state is D0?
> 
> Sounds good, I'll update the patch, thanks for the advice.

Actually, I suppose we can do something like the appended patch instead.

I wonder if it works around the particular problem you're seeing?

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
@@ -229,7 +229,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);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-07 18:32           ` Rafael J. Wysocki
@ 2012-09-10  0:38             ` Aaron Lu
  2012-09-10 19:48               ` Rafael J. Wysocki
  2012-09-10 19:50               ` [PATCH] ACPI / PM: Infer parent power state from child if unknown Rafael J. Wysocki
  0 siblings, 2 replies; 14+ messages in thread
From: Aaron Lu @ 2012-09-10  0:38 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Fri, Sep 07, 2012 at 08:32:55PM +0200, Rafael J. Wysocki wrote:
> On Friday, September 07, 2012, Aaron Lu wrote:
> > On 09/07/2012 07:46 PM, Rafael J. Wysocki wrote:
> > >> Yes, on a test system, when I try to put a device into D3 cold and ACPI
> > >> will complain that I can't due to its parent is in a even lower power
> > >> state UNKNOWN(255), this parent device is power manageable but has no
> > >> _PSC and _PRx defined.
> > >
> > > Perhaps we can force _PS0 for such devices to start with, so that we know
> > > for sure that the initial state is D0?
> > 
> > Sounds good, I'll update the patch, thanks for the advice.
> 
> Actually, I suppose we can do something like the appended patch instead.
> 
> I wonder if it works around the particular problem you're seeing?

Yes, thanks.

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

-Aaron

> 
> 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
> @@ -229,7 +229,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);

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case
  2012-09-10  0:38             ` Aaron Lu
@ 2012-09-10 19:48               ` Rafael J. Wysocki
  2012-09-10 19:50               ` [PATCH] ACPI / PM: Infer parent power state from child if unknown Rafael J. Wysocki
  1 sibling, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-10 19:48 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu

On Monday, September 10, 2012, Aaron Lu wrote:
> On Fri, Sep 07, 2012 at 08:32:55PM +0200, Rafael J. Wysocki wrote:
> > On Friday, September 07, 2012, Aaron Lu wrote:
> > > On 09/07/2012 07:46 PM, Rafael J. Wysocki wrote:
> > > >> Yes, on a test system, when I try to put a device into D3 cold and ACPI
> > > >> will complain that I can't due to its parent is in a even lower power
> > > >> state UNKNOWN(255), this parent device is power manageable but has no
> > > >> _PSC and _PRx defined.
> > > >
> > > > Perhaps we can force _PS0 for such devices to start with, so that we know
> > > > for sure that the initial state is D0?
> > > 
> > > Sounds good, I'll update the patch, thanks for the advice.
> > 
> > Actually, I suppose we can do something like the appended patch instead.
> > 
> > I wonder if it works around the particular problem you're seeing?
> 
> Yes, thanks.
> 
> Reviewed-by: Aaron Lu <aaron.lu@intel.com>

Cool, thanks for testing!

I'll resend it shortly with a proper changelog.

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
> > @@ -229,7 +229,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);
> 
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] ACPI / PM: Infer parent power state from child if unknown
  2012-09-10  0:38             ` Aaron Lu
  2012-09-10 19:48               ` Rafael J. Wysocki
@ 2012-09-10 19:50               ` Rafael J. Wysocki
  2012-09-11  5:33                 ` Aaron Lu
  1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-10 19:50 UTC (permalink / raw)
  To: Aaron Lu, Len Brown; +Cc: linux-acpi, linux-pm, Aaron Lu, LKML

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ACPI / PM: Infer parent power state from child if unknown
  2012-09-10 19:50               ` [PATCH] ACPI / PM: Infer parent power state from child if unknown Rafael J. Wysocki
@ 2012-09-11  5:33                 ` Aaron Lu
  2012-09-11 20:36                   ` Rafael J. Wysocki
  2012-09-11 20:38                   ` [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2 Rafael J. Wysocki
  0 siblings, 2 replies; 14+ messages in thread
From: Aaron Lu @ 2012-09-11  5:33 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu, LKML

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ACPI / PM: Infer parent power state from child if unknown
  2012-09-11  5:33                 ` Aaron Lu
@ 2012-09-11 20:36                   ` Rafael J. Wysocki
  2012-09-11 20:38                   ` [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2 Rafael J. Wysocki
  1 sibling, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-11 20:36 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu, LKML

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2
  2012-09-11  5:33                 ` Aaron Lu
  2012-09-11 20:36                   ` Rafael J. Wysocki
@ 2012-09-11 20:38                   ` Rafael J. Wysocki
  2012-09-12  6:59                     ` Aaron Lu
  1 sibling, 1 reply; 14+ messages in thread
From: Rafael J. Wysocki @ 2012-09-11 20:38 UTC (permalink / raw)
  To: Aaron Lu; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu, LKML

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:

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2
  2012-09-11 20:38                   ` [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2 Rafael J. Wysocki
@ 2012-09-12  6:59                     ` Aaron Lu
  0 siblings, 0 replies; 14+ messages in thread
From: Aaron Lu @ 2012-09-12  6:59 UTC (permalink / raw)
  To: Rafael J. Wysocki; +Cc: Len Brown, linux-acpi, linux-pm, Aaron Lu, LKML

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:

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2012-09-12  6:59 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-27  7:38 [PATCH] acpi: bus: handle power manageable but no _PSC/_PRx case Aaron Lu
2012-09-06 21:45 ` Rafael J. Wysocki
2012-09-06 21:53   ` Rafael J. Wysocki
2012-09-07  0:35     ` Aaron Lu
2012-09-07 11:46       ` Rafael J. Wysocki
2012-09-07 14:32         ` Aaron Lu
2012-09-07 18:32           ` Rafael J. Wysocki
2012-09-10  0:38             ` Aaron Lu
2012-09-10 19:48               ` Rafael J. Wysocki
2012-09-10 19:50               ` [PATCH] ACPI / PM: Infer parent power state from child if unknown Rafael J. Wysocki
2012-09-11  5:33                 ` Aaron Lu
2012-09-11 20:36                   ` Rafael J. Wysocki
2012-09-11 20:38                   ` [PATCH] ACPI / PM: Infer parent power state from child if unknown, v2 Rafael J. Wysocki
2012-09-12  6:59                     ` Aaron Lu

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).