Re: [Nouveau] [PATCH 4/4] drm/nouveau/acpi: fix lockup with PCIe runtime PM

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi,

On 27-05-16 13:10, Peter Wu wrote:
On Wed, May 25, 2016 at 04:55:35PM +0300, Mika Westerberg wrote:
On Wed, May 25, 2016 at 12:53:01AM +0200, Peter Wu wrote:
Since "PCI: Add runtime PM support for PCIe ports", the parent PCIe port
can be runtime-suspended which disables power resources via ACPI. This
is incompatible with DSM, resulting in a GPU device which is still in D3
and locks up the kernel on resume.

Mirror the behavior of Windows 8 and newer[1] (as observed via an AMLi
debugger trace) and stop using the DSM functions for D3cold when power
resources are available on the parent PCIe port.

 [1]: https://msdn.microsoft.com/windows/hardware/drivers/bringup/firmware-requirements-for-d3cold

Signed-off-by: Peter Wu <peter@xxxxxxxxxxxxx>
---
 drivers/gpu/drm/nouveau/nouveau_acpi.c | 34 ++++++++++++++++++++++++++++++----
 1 file changed, 30 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c
index df9f73e..e469df7 100644
--- a/drivers/gpu/drm/nouveau/nouveau_acpi.c
+++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c
@@ -46,6 +46,7 @@ static struct nouveau_dsm_priv {
 	bool dsm_detected;
 	bool optimus_detected;
 	bool optimus_flags_detected;
+	bool optimus_skip_dsm;
 	acpi_handle dhandle;
 	acpi_handle rom_handle;
 } nouveau_dsm_priv;
@@ -212,8 +213,26 @@ static const struct vga_switcheroo_handler nouveau_dsm_handler = {
 	.get_client_id = nouveau_dsm_get_client_id,
 };

+/* Firmware supporting Windows 8 or later do not use _DSM to put the device into
+ * D3cold, they instead rely on disabling power resources on the parent. */
+static bool nouveau_pr3_present(struct pci_dev *pdev)
+{
+	struct pci_dev *parent_pdev = pci_upstream_bridge(pdev);
+	struct acpi_device *ad;

Nit: please call this adev instead of ad.

Will do.

+
+	if (!parent_pdev)
+		return false;
+
+	ad = ACPI_COMPANION(&parent_pdev->dev);
+	if (!ad)
+		return false;
+
+	return ad->power.flags.power_resources;

Is this sufficient to tell if the parent device has _PR3? I thought it
returns true if it has power resources in general, not necessarily _PR3.

Otherwise this looks okay to me.

It is indeed set whenever there is any _PRx method. I wonder if it is
appropriate to access fields directly like this, perhaps this would be
more accurate (based on device_pm.c):

    /* Check whether the _PR3 method is available. */
    return adev->power.states[ACPI_STATE_D3_COLD].flags.valid;

I am also considering adding a check in case the pcieport driver does
not support D3cold via runtime PM, what do you think of this?

    if (!parent_pdev)
        return false;
    /* If the PCIe port does not support D3cold via runtime PM, allow a
     * fallback to the Optimus DSM method to put the device in D3cold. */
    if (parent_pdev->no_d3cold)
        return false;

This is needed to avoid the regression reported in the cover letter, but
also allows pre-2015 systems to (still) have the D3cold possibility.


Out of curiosity I looked up an pre-2015 laptop (found Acer V5-573G,
apparently from November 2013, Windows 8.1) and extracted the ACPI
tables from the BIOS images. BIOS 2.28 (2014/05/13) introduces support
for power resources on the parent devicea(\_SB.PCI0.PEG0._PR3 and a
related NVP3 device) when _OSI("Windows 2013") is true. (This is added
as alternative for the old DSM interface.)

Maybe 2014 is also an appropriate cutoff date? I wonder if it is
feasible to detect firmware use of _OSI("Windows 2013") and use that
instead of the BIOS year.

It is definitely possible to check if the firmware uses _OSI("Windows 2013")
we do something similar to check for windows-8 ready laptops in the backlight
code, see acpi_osi_is_win8() in drivers/acpi/osl.c, or if you actually
want to test for Windows 8 or newer, just use acpi_osi_is_win8()  :)

Regards,

Hans


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



[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux