[PATCH v2] drm/nouveau/acpi: fix check for power resources support

[PATCH v2] drm/nouveau/acpi: fix check for power resources support

[Peter Wu]

Check whether the kernel really supports power resources for a device,
otherwise the power might not be removed when the device is runtime
suspended (DSM should still work in these cases where PR does not).

This is a workaround for a problem where ACPICA and Windows 10 differ in
behavior. ACPICA does not correctly enumerate power resources within a
conditional block (due to delayed execution of such blocks) and as a
result power_resources is set to false even if _PR3 exists.

Fixes: 692a17dcc292 ("drm/nouveau/acpi: fix lockup with PCIe runtime PM")
Link: https://bugs.freedesktop.org/show_bug.cgi?id=98398
Reported-and-tested-by: Rick Kerkhof <rick.2889@gmail.com>
Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
Signed-off-by: Peter Wu <peter@lekensteyn.nl>
---
 v2: collected tags from Rick and Mika; added ACPICA note as requested by Mika

I suggest Cc: stable (if the maintainer is OK with that?)
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index dc57b62..193573d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -240,7 +240,8 @@ static bool nouveau_pr3_present(struct pci_dev *pdev)
 	if (!parent_adev)
 		return false;
 
-	return acpi_has_method(parent_adev->handle, "_PR3");
+	return parent_adev->power.flags.power_resources &&
+		acpi_has_method(parent_adev->handle, "_PR3");
 }
 
 static void nouveau_dsm_pci_probe(struct pci_dev *pdev, acpi_handle *dhandle_out,
-- 
2.10.1

_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2] drm/nouveau/acpi: fix check for power resources support

[Dave Airlie]

On 1 November 2016 at 08:48, Peter Wu <peter@lekensteyn.nl> wrote:
> Check whether the kernel really supports power resources for a device,
> otherwise the power might not be removed when the device is runtime
> suspended (DSM should still work in these cases where PR does not).
>
> This is a workaround for a problem where ACPICA and Windows 10 differ in
> behavior. ACPICA does not correctly enumerate power resources within a
> conditional block (due to delayed execution of such blocks) and as a
> result power_resources is set to false even if _PR3 exists.
>
> Fixes: 692a17dcc292 ("drm/nouveau/acpi: fix lockup with PCIe runtime PM")
> Link: https://bugs.freedesktop.org/show_bug.cgi?id=98398
> Reported-and-tested-by: Rick Kerkhof <rick.2889@gmail.com>
> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> Signed-off-by: Peter Wu <peter@lekensteyn.nl>

I've appled it this and cc'ed stable to drm-fixes.

Are we going to get ACPICA fixed?

Dave.
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2] drm/nouveau/acpi: fix check for power resources support

[Alex Deucher]

On Tue, Nov 1, 2016 at 12:55 AM, Dave Airlie <airlied@gmail.com> wrote:
> On 1 November 2016 at 08:48, Peter Wu <peter@lekensteyn.nl> wrote:
>> Check whether the kernel really supports power resources for a device,
>> otherwise the power might not be removed when the device is runtime
>> suspended (DSM should still work in these cases where PR does not).
>>
>> This is a workaround for a problem where ACPICA and Windows 10 differ in
>> behavior. ACPICA does not correctly enumerate power resources within a
>> conditional block (due to delayed execution of such blocks) and as a
>> result power_resources is set to false even if _PR3 exists.
>>
>> Fixes: 692a17dcc292 ("drm/nouveau/acpi: fix lockup with PCIe runtime PM")
>> Link: https://bugs.freedesktop.org/show_bug.cgi?id=98398
>> Reported-and-tested-by: Rick Kerkhof <rick.2889@gmail.com>
>> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
>
> I've appled it this and cc'ed stable to drm-fixes.
>
> Are we going to get ACPICA fixed?

Looks like we may have hit this on radeon/amdgpu as well:
https://bugs.freedesktop.org/show_bug.cgi?id=98505

Alex

>
> Dave.
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm/nouveau/acpi: fix check for power resources support

[Peter Wu]

On Tue, Nov 01, 2016 at 09:24:23AM -0400, Alex Deucher wrote:
> On Tue, Nov 1, 2016 at 12:55 AM, Dave Airlie <airlied@gmail.com> wrote:
> > On 1 November 2016 at 08:48, Peter Wu <peter@lekensteyn.nl> wrote:
> >> Check whether the kernel really supports power resources for a device,
> >> otherwise the power might not be removed when the device is runtime
> >> suspended (DSM should still work in these cases where PR does not).
> >>
> >> This is a workaround for a problem where ACPICA and Windows 10 differ in
> >> behavior. ACPICA does not correctly enumerate power resources within a
> >> conditional block (due to delayed execution of such blocks) and as a
> >> result power_resources is set to false even if _PR3 exists.
> >>
> >> Fixes: 692a17dcc292 ("drm/nouveau/acpi: fix lockup with PCIe runtime PM")
> >> Link: https://bugs.freedesktop.org/show_bug.cgi?id=98398
> >> Reported-and-tested-by: Rick Kerkhof <rick.2889@gmail.com>
> >> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
> >> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
> >
> > I've appled it this and cc'ed stable to drm-fixes.
> >
> > Are we going to get ACPICA fixed?

The ACPI folks are aware of the problem, see this thread and its
follow-ups:
http://www.spinics.net/lists/linux-acpi/msg70050.html

> Looks like we may have hit this on radeon/amdgpu as well:
> https://bugs.freedesktop.org/show_bug.cgi?id=98505
> 
> Alex

That log seems to suggest that resume fails while this nouveau issue is
related to suspend not powering off a device. The root cause might be
different.
-- 
Kind regards,
Peter Wu
https://lekensteyn.nl
_______________________________________________
Nouveau mailing list
Nouveau@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/nouveau

Re: [PATCH v2] drm/nouveau/acpi: fix check for power resources support

[Alex Deucher]

On Tue, Nov 1, 2016 at 3:00 PM, Peter Wu <peter@lekensteyn.nl> wrote:
> On Tue, Nov 01, 2016 at 09:24:23AM -0400, Alex Deucher wrote:
>> On Tue, Nov 1, 2016 at 12:55 AM, Dave Airlie <airlied@gmail.com> wrote:
>> > On 1 November 2016 at 08:48, Peter Wu <peter@lekensteyn.nl> wrote:
>> >> Check whether the kernel really supports power resources for a device,
>> >> otherwise the power might not be removed when the device is runtime
>> >> suspended (DSM should still work in these cases where PR does not).
>> >>
>> >> This is a workaround for a problem where ACPICA and Windows 10 differ in
>> >> behavior. ACPICA does not correctly enumerate power resources within a
>> >> conditional block (due to delayed execution of such blocks) and as a
>> >> result power_resources is set to false even if _PR3 exists.
>> >>
>> >> Fixes: 692a17dcc292 ("drm/nouveau/acpi: fix lockup with PCIe runtime PM")
>> >> Link: https://bugs.freedesktop.org/show_bug.cgi?id=98398
>> >> Reported-and-tested-by: Rick Kerkhof <rick.2889@gmail.com>
>> >> Reviewed-by: Mika Westerberg <mika.westerberg@linux.intel.com>
>> >> Signed-off-by: Peter Wu <peter@lekensteyn.nl>
>> >
>> > I've appled it this and cc'ed stable to drm-fixes.
>> >
>> > Are we going to get ACPICA fixed?
>
> The ACPI folks are aware of the problem, see this thread and its
> follow-ups:
> http://www.spinics.net/lists/linux-acpi/msg70050.html
>
>> Looks like we may have hit this on radeon/amdgpu as well:
>> https://bugs.freedesktop.org/show_bug.cgi?id=98505
>>
>> Alex
>
> That log seems to suggest that resume fails while this nouveau issue is
> related to suspend not powering off a device. The root cause might be
> different.

Resume (from runtime suspend) is failing because the device is not
getting powered off.  Reverting the patch that enables the use of PR3
to power off the dGPU and falls back to the old ATPX power off method
fixes the issue.  So it would appear that PR3 is either not getting
enabled for this system (perhaps due to the date check in the pci
code) or due to some problem in the ACPICA.  Note that on A+I or A+A
systems, if PR3 is available (determined by the OSI setting), it
should be used so the date shouldn't really matter in the case of
those systems.  In practice ATPX generally still works on those
systems as well, but it's not a requirement so OEMs/ODMs may not
always make it available if PR3 is exposed.

Alex
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel