search for: nouveau_pr3_present

Replace nouveau_pr3_present() in favor of a more generic one, pci_pr3_present(). Also the presence of upstream bridge _PR3 doesn't need to go hand in hand with device's _DSM, so check _PR3 before _DSM. Signed-off-by: Kai-Heng Feng <kai.heng.feng at canonical.com> --- drivers/gpu/drm/nouveau/nouveau_acpi.c | 4...

...witcheroo_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; > + > + if (!parent_pdev) > + return false; > + > + ad = ACPI_COMPANION(&parent_pdev->dev); > + if (!ad)...

...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(stru...

...; .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; >> > + >> > + if (!parent_pdev) >> > + return false; >> > + >> > +...

...consequences. I have tested it on top of v4.7-rc5. To make patch four work properly, Lukas' RPM refcounting patches should be included. A similar (open/new) RPM refcounting issue in snd-hda-intel should also be fixed. Otherwise the bridge will not really sleep. There is another minor patch for nouveau_pr3_present, but it is not included here because it depends on visibility of pci_bridge_d3_possible(). I'll send a separate mail for this to linux-pci. Kind regards, Peter [1]: https://lists.freedesktop.org/archives/nouveau/2016-May/025116.html [2]: https://git.kernel.org/cgit/linux/kernel/git/helgaas/...

...13,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; + + if (!parent_pdev) + return false; + + ad = ACPI_COMPANION(&parent_pdev->dev); + if (!ad) + return false; + + return ad->power.flags.power_resources; +} + static void nouvea...

...13,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 *parent_adev; + + if (!parent_pdev) + return false; + + parent_adev = ACPI_COMPANION(&parent_pdev->dev); + if (!parent_adev) + return false; + + return acpi_has_method(parent_adev->h...

...ga_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. > + > + if (!parent_pdev) > + return false; > + > + ad = ACPI_COMPANION(&parent_pdev->dev); > + if (!ad)...

...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; > > + > > + if (!parent_pdev) > > + return false; > > + > > + ad = ACPI_COMPANION(&pare...

Hi, Here are two patches to fix an issue reported on kernel bugzilla (infinite loop due to unchecked function) and a more important fix to fix hanging Optimus machines when runtime PM is enabled (with pm/pci patches). An older (obsolete) patch for the first issue was tested by the reporter: https://bugzilla.kernel.org/show_bug.cgi?id=104791#c11 (it is replaced by "check for function 0x1B

...= 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; > >> > + > >> > + if (!parent_pdev) > >> > + return false; >...

...in particular I wonder why does everyone using dsm insists on saying Arg4 when they actually mean Arg3. ACPI numbers arguments from 0. So it's a bit ugly, and maybe worth fixing but unlikely to be an actual issue simply because we end up not using DSM in the end. Poking at the probing code in nouveau_pr3_present, I started to wonder: should I try to hack it to disable d3cold and pr3 and see what happens? > > > > [ 3.971745] nouveau 0000:01:00.0: enabling device (0006 -> 0007) > > > > [ 3.971923] nouveau 0000:01:00.0: NVIDIA GP107 (137000a1) > > > > [ 4.009...

....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) &gt...

...c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/drivers/gpu/drm/nouveau/nouveau_acpi.c b/drivers/gpu/drm/nouveau/nouveau_acpi.c index f2ad17a..dc57b62 100644 --- a/drivers/gpu/drm/nouveau/nouveau_acpi.c +++ b/drivers/gpu/drm/nouveau/nouveau_acpi.c @@ -225,6 +225,17 @@ static bool nouveau_pr3_present(struct pci_dev *pdev) if (!parent_pdev) return false; + if (!parent_pdev->bridge_d3) { + /* + * Parent PCI bridge is currently not power managed. + * Since userspace can change these afterwards to be on + * the safe side we stick with _DSM and prevent usage of + * _PR3 from the b...

...> using dsm insists on saying Arg4 > when they actually mean Arg3. ACPI numbers arguments from 0. > > So it's a bit ugly, and maybe worth fixing but unlikely to be > an actual issue simply because we end up not using DSM in the end. I agree. > Poking at the probing code in nouveau_pr3_present, I started to wonder: > should I try to hack it to disable d3cold and pr3 and see what > happens? I guess it is worth a try. You can do it from sysfs for the graphics PCI device there is an attribute d3cold_allowed that controls this. [snip] > > > 00:14.3 Network controller: Intel...

...gt; > when they actually mean Arg3. ACPI numbers arguments from 0. > > > > So it's a bit ugly, and maybe worth fixing but unlikely to be > > an actual issue simply because we end up not using DSM in the end. > > I agree. > > > Poking at the probing code in nouveau_pr3_present, I started to wonder: > > should I try to hack it to disable d3cold and pr3 and see what > > happens? > > I guess it is worth a try. You can do it from sysfs for the graphics > PCI device there is an attribute d3cold_allowed that controls this. > > [snip] But probably...

...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...

...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(stru...

...au_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. > > > > >...

...Arg3. ACPI numbers arguments from 0. > > > > > > So it's a bit ugly, and maybe worth fixing but unlikely to be > > > an actual issue simply because we end up not using DSM in the end. > > > > I agree. > > > > > Poking at the probing code in nouveau_pr3_present, I started to wonder: > > > should I try to hack it to disable d3cold and pr3 and see what > > > happens? > > > > I guess it is worth a try. You can do it from sysfs for the graphics > > PCI device there is an attribute d3cold_allowed that controls this. > &gt...