diff mbox

Acer Aspire V7-582PG (Haswell, GTX 750M) fails to power off GPU via Power Resources

Message ID 20161027095508.GA1476@lahna.fi.intel.com (mailing list archive)
State New, archived
Delegated to: Bjorn Helgaas
Headers show

Commit Message

Mika Westerberg Oct. 27, 2016, 9:55 a.m. UTC 
On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
>    No, there are not. Here is the recursive directory listing:

Are you able to try the following patch and send me dmesg (or attach it
to that bug)? It should show if the ACPI core even tries to add those
power resources.

--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Peter Wu Oct. 27, 2016, 4:06 p.m. UTC | #1 
On Thu, Oct 27, 2016 at 12:55:08PM +0300, Mika Westerberg wrote:
> On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
> >    No, there are not. Here is the recursive directory listing:
> 
> Are you able to try the following patch and send me dmesg (or attach it
> to that bug)? It should show if the ACPI core even tries to add those
> power resources.

So Rick has tested this patch now on top of 4.8.4 (mainline fails to
boot due to a kbuild issue which I reported elsewhere), but the output
is empty. That seems to indicate that flags.power_resources is unset.

Given that _PS3 exists and is indeed a package with some elements, it
seems that acpi_extract_power_resources is failing. Note that in the
SSDT, the power resource NVP3 was referenced before it was defined,
could that result in this enumeration failure? Relevant SSDT excerpt:

    Scope (\_SB.PCI0.RP05)
    {
        Name (_PR3, Package (0x01)  // _PR3: Power Resources for D3hot
        {
            NVP3
        })
        // ...
    }

    PowerResource (NVP3, 0x00, 0x0000)

Kind regards,
Peter

> diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
> index fcd4ce6f78d5..af9c3e15dd74 100644
> --- a/drivers/acpi/power.c
> +++ b/drivers/acpi/power.c
> @@ -444,6 +444,9 @@ void acpi_power_add_remove_device(struct acpi_device *adev, bool add)
>  	if (!adev->power.flags.power_resources)
>  		return;
>  
> +	acpi_handle_info(adev->handle, "Adding power resources for %s\n",
> +			 dev_name(&adev->dev));
> +
>  	for (state = ACPI_STATE_D0; state <= ACPI_STATE_D3_HOT; state++)
>  		acpi_power_expose_hide(adev,
>  				       &adev->power.states[state].resources,
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Oct. 28, 2016, 8:56 a.m. UTC | #2 
On Thu, Oct 27, 2016 at 06:06:48PM +0200, Peter Wu wrote:
> On Thu, Oct 27, 2016 at 12:55:08PM +0300, Mika Westerberg wrote:
> > On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
> > >    No, there are not. Here is the recursive directory listing:
> > 
> > Are you able to try the following patch and send me dmesg (or attach it
> > to that bug)? It should show if the ACPI core even tries to add those
> > power resources.
> 
> So Rick has tested this patch now on top of 4.8.4 (mainline fails to
> boot due to a kbuild issue which I reported elsewhere), but the output
> is empty. That seems to indicate that flags.power_resources is unset.

Is it completely empty or is it empty just for RP05? It should print out
all devices with power resources.

> Given that _PS3 exists and is indeed a package with some elements, it
> seems that acpi_extract_power_resources is failing. Note that in the
> SSDT, the power resource NVP3 was referenced before it was defined,
> could that result in this enumeration failure? Relevant SSDT excerpt:
> 
>     Scope (\_SB.PCI0.RP05)
>     {
>         Name (_PR3, Package (0x01)  // _PR3: Power Resources for D3hot
>         {
>             NVP3
>         })
>         // ...
>     }
> 
>     PowerResource (NVP3, 0x00, 0x0000)

That and the fact that they come from an SSDT instead of DSDT may cause
this. However, I'm not expert in ACPICA so adding Bob and Lv if they
have ideas.

Bob, Lv, the bug in question is: https://bugs.freedesktop.org/show_bug.cgi?id=98398
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Wu Oct. 28, 2016, 11:09 a.m. UTC | #3 
On Fri, Oct 28, 2016 at 11:56:30AM +0300, Mika Westerberg wrote:
> On Thu, Oct 27, 2016 at 06:06:48PM +0200, Peter Wu wrote:
> > On Thu, Oct 27, 2016 at 12:55:08PM +0300, Mika Westerberg wrote:
> > > On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
> > > >    No, there are not. Here is the recursive directory listing:
> > > 
> > > Are you able to try the following patch and send me dmesg (or attach it
> > > to that bug)? It should show if the ACPI core even tries to add those
> > > power resources.
> > 
> > So Rick has tested this patch now on top of 4.8.4 (mainline fails to
> > boot due to a kbuild issue which I reported elsewhere), but the output
> > is empty. That seems to indicate that flags.power_resources is unset.
> 
> Is it completely empty or is it empty just for RP05? It should print out
> all devices with power resources.

\NVP2 and \NVP3 are the only power resources under RP05 and defined in
SSDT1, there are no others.

Kind regards,
Peter

> > Given that _PS3 exists and is indeed a package with some elements, it
> > seems that acpi_extract_power_resources is failing. Note that in the
> > SSDT, the power resource NVP3 was referenced before it was defined,
> > could that result in this enumeration failure? Relevant SSDT excerpt:
> > 
> >     Scope (\_SB.PCI0.RP05)
> >     {
> >         Name (_PR3, Package (0x01)  // _PR3: Power Resources for D3hot
> >         {
> >             NVP3
> >         })
> >         // ...
> >     }
> > 
> >     PowerResource (NVP3, 0x00, 0x0000)
> 
> That and the fact that they come from an SSDT instead of DSDT may cause
> this. However, I'm not expert in ACPICA so adding Bob and Lv if they
> have ideas.
> 
> Bob, Lv, the bug in question is: https://bugs.freedesktop.org/show_bug.cgi?id=98398
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Mika Westerberg Oct. 28, 2016, 11:19 a.m. UTC | #4 
On Fri, Oct 28, 2016 at 01:09:30PM +0200, Peter Wu wrote:
> On Fri, Oct 28, 2016 at 11:56:30AM +0300, Mika Westerberg wrote:
> > On Thu, Oct 27, 2016 at 06:06:48PM +0200, Peter Wu wrote:
> > > On Thu, Oct 27, 2016 at 12:55:08PM +0300, Mika Westerberg wrote:
> > > > On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
> > > > >    No, there are not. Here is the recursive directory listing:
> > > > 
> > > > Are you able to try the following patch and send me dmesg (or attach it
> > > > to that bug)? It should show if the ACPI core even tries to add those
> > > > power resources.
> > > 
> > > So Rick has tested this patch now on top of 4.8.4 (mainline fails to
> > > boot due to a kbuild issue which I reported elsewhere), but the output
> > > is empty. That seems to indicate that flags.power_resources is unset.
> > 
> > Is it completely empty or is it empty just for RP05? It should print out
> > all devices with power resources.
> 
> \NVP2 and \NVP3 are the only power resources under RP05 and defined in
> SSDT1, there are no others.

We should probably add a debug print before checking
flags.power_resources just to be sure the patch is correctly applied.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lv Zheng Oct. 29, 2016, 12:42 a.m. UTC | #5 
Hi, Mika

> From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com]
> Subject: Re: Acer Aspire V7-582PG (Haswell, GTX 750M) fails to power off GPU via Power Resources
> 
> On Thu, Oct 27, 2016 at 06:06:48PM +0200, Peter Wu wrote:
> > On Thu, Oct 27, 2016 at 12:55:08PM +0300, Mika Westerberg wrote:
> > > On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
> > > >    No, there are not. Here is the recursive directory listing:
> > >
> > > Are you able to try the following patch and send me dmesg (or attach it
> > > to that bug)? It should show if the ACPI core even tries to add those
> > > power resources.
> >
> > So Rick has tested this patch now on top of 4.8.4 (mainline fails to
> > boot due to a kbuild issue which I reported elsewhere), but the output
> > is empty. That seems to indicate that flags.power_resources is unset.
> 
> Is it completely empty or is it empty just for RP05? It should print out
> all devices with power resources.
> 
> > Given that _PS3 exists and is indeed a package with some elements, it
> > seems that acpi_extract_power_resources is failing. Note that in the
> > SSDT, the power resource NVP3 was referenced before it was defined,
> > could that result in this enumeration failure? Relevant SSDT excerpt:
> >
> >     Scope (\_SB.PCI0.RP05)
> >     {
> >         Name (_PR3, Package (0x01)  // _PR3: Power Resources for D3hot
> >         {
> >             NVP3
> >         })
> >         // ...
> >     }
> >
> >     PowerResource (NVP3, 0x00, 0x0000)

Looks wrong order to me.

However, _PR3 is a package, for AML opcode that contains PkgLength grammar primitive, forward reference may be OK (for example Method).
DefPackage := PackageOp PkgLength NumElements PackageElementList
DefMethod := MethodOp PkgLength NameString MethodFlags TermList
As when it is encountered, AML interpreter may only saves entire packaged object.

I need to test behaviors around Package with qemu but I don't have environment now.
Can you help to give it a try?
By adding customize an ssdt with the above code putting under root scope,
DefinitionBlock ("ssdt-package.aml", "SSDT", 2, "INTEL ", "PACKAGE ", 0x00003000)
{
    Scope (\)
    {
        Name (_PR3, Package (0x01) { NVP3 })
    }
    PowerResource (NVP3, 0x00, 0x0000) {}
}
Running Windows images on qemu with this ssdt appended "-acpitable file=ssdt-package.aml".
To see if blue screen can be resulted.

Thanks
Lv

> 
> That and the fact that they come from an SSDT instead of DSDT may cause
> this. However, I'm not expert in ACPICA so adding Bob and Lv if they
> have ideas.
> 
> Bob, Lv, the bug in question is: https://bugs.freedesktop.org/show_bug.cgi?id=98398
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Peter Wu Oct. 30, 2016, 11:08 a.m. UTC | #6 
On Sat, Oct 29, 2016 at 12:42:34AM +0000, Zheng, Lv wrote:
> Hi, Mika
> 
> > From: Mika Westerberg [mailto:mika.westerberg@linux.intel.com]
> > Subject: Re: Acer Aspire V7-582PG (Haswell, GTX 750M) fails to power off GPU via Power Resources
> > 
> > On Thu, Oct 27, 2016 at 06:06:48PM +0200, Peter Wu wrote:
> > > On Thu, Oct 27, 2016 at 12:55:08PM +0300, Mika Westerberg wrote:
> > > > On Thu, Oct 27, 2016 at 09:42:28AM +0000, Rick Kerkhof wrote:
> > > > >    No, there are not. Here is the recursive directory listing:
> > > >
> > > > Are you able to try the following patch and send me dmesg (or attach it
> > > > to that bug)? It should show if the ACPI core even tries to add those
> > > > power resources.
> > >
> > > So Rick has tested this patch now on top of 4.8.4 (mainline fails to
> > > boot due to a kbuild issue which I reported elsewhere), but the output
> > > is empty. That seems to indicate that flags.power_resources is unset.
> > 
> > Is it completely empty or is it empty just for RP05? It should print out
> > all devices with power resources.
> > 
> > > Given that _PS3 exists and is indeed a package with some elements, it
> > > seems that acpi_extract_power_resources is failing. Note that in the
> > > SSDT, the power resource NVP3 was referenced before it was defined,
> > > could that result in this enumeration failure? Relevant SSDT excerpt:
> > >
> > >     Scope (\_SB.PCI0.RP05)
> > >     {
> > >         Name (_PR3, Package (0x01)  // _PR3: Power Resources for D3hot
> > >         {
> > >             NVP3
> > >         })
> > >         // ...
> > >     }
> > >
> > >     PowerResource (NVP3, 0x00, 0x0000)
> 
> Looks wrong order to me.
> 
> However, _PR3 is a package, for AML opcode that contains PkgLength grammar primitive, forward reference may be OK (for example Method).
> DefPackage := PackageOp PkgLength NumElements PackageElementList
> DefMethod := MethodOp PkgLength NameString MethodFlags TermList
> As when it is encountered, AML interpreter may only saves entire packaged object.
> 
> I need to test behaviors around Package with qemu but I don't have environment now.
> Can you help to give it a try?
> By adding customize an ssdt with the above code putting under root scope,
> DefinitionBlock ("ssdt-package.aml", "SSDT", 2, "INTEL ", "PACKAGE ", 0x00003000)
> {
>     Scope (\)
>     {
>         Name (_PR3, Package (0x01) { NVP3 })
>     }
>     PowerResource (NVP3, 0x00, 0x0000) {}
> }
> Running Windows images on qemu with this ssdt appended "-acpitable file=ssdt-package.aml".
> To see if blue screen can be resulted.
> 
> Thanks
> Lv

Testing this code with Windows 10 gives a BSOD, changing the order of
the PowerResource and Scope does not make a difference. If I take my
previous SSDT and change the ordering of NVP3 definition vs _PR3 use,
there is no change.

Kind regards,
Peter

> > 
> > That and the fact that they come from an SSDT instead of DSDT may cause
> > this. However, I'm not expert in ACPICA so adding Bob and Lv if they
> > have ideas.
> > 
> > Bob, Lv, the bug in question is: https://bugs.freedesktop.org/show_bug.cgi?id=98398
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/drivers/acpi/power.c b/drivers/acpi/power.c
index fcd4ce6f78d5..af9c3e15dd74 100644
--- a/drivers/acpi/power.c
+++ b/drivers/acpi/power.c
@@ -444,6 +444,9 @@  void acpi_power_add_remove_device(struct acpi_device *adev, bool add)
 	if (!adev->power.flags.power_resources)
 		return;
 
+	acpi_handle_info(adev->handle, "Adding power resources for %s\n",
+			 dev_name(&adev->dev));
+
 	for (state = ACPI_STATE_D0; state <= ACPI_STATE_D3_HOT; state++)
 		acpi_power_expose_hide(adev,
 				       &adev->power.states[state].resources,