search for: pci_set_power_state

...t we call when setting up the DRM device, then store the result of that into drm->runpm_dsm instead of changing it every time we runtime suspend? > ret = nouveau_do_suspend(drm_dev, true); > pci_save_state(pdev); > pci_disable_device(pdev); > pci_ignore_hotplug(pdev); > - pci_set_power_state(pdev, PCI_D3cold); > + if (drm->runpm_dsm) > + pci_set_power_state(pdev, PCI_D3cold); > + > drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; > return ret; > } > @@ -925,7 +929,8 @@ nouveau_pmops_runtime_resume(struct device *dev) > { > struct pci...

..._pmops_suspend(struct device *dev) >> return 0; >> } >> >> +static int >> +nouveau_set_power_state_D0(struct pci_dev *pdev) >> +{ >> + struct nouveau_drm *drm = nouveau_drm(pci_get_drvdata(pdev)); >> + int ret; >> + >> + pci_set_power_state(pdev, PCI_D0); >> + /* abort if anything went wrong */ >> + if (pdev->current_state != PCI_D0) { >> + NV_ERROR(drm, "couldn't wake up GPU!\n"); >> + return -EBUSY; >> + } > > Looks to me like the more idiomatic...

...b4f7 100644 --- a/drm/nouveau/nouveau_drm.c +++ b/drm/nouveau/nouveau_drm.c @@ -792,6 +792,27 @@ nouveau_pmops_suspend(struct device *dev) return 0; } +static int +nouveau_set_power_state_D0(struct pci_dev *pdev) +{ + struct nouveau_drm *drm = nouveau_drm(pci_get_drvdata(pdev)); + int ret; + + pci_set_power_state(pdev, PCI_D0); + /* abort if anything went wrong */ + if (pdev->current_state != PCI_D0) { + NV_ERROR(drm, "couldn't wake up GPU!\n"); + return -EBUSY; + } + pci_restore_state(pdev); + ret = pci_enable_device(pdev); + if (ret) + return ret; + + pci_set_master(pdev); + return 0;...

In the autumn and winter it's considered bad form to set power state to D3cold, it might cause the device to freeze to death. This is also the case in the other seasons, so any device in the southern hemisphere is affected too. D3cold is not a valid state in a call to pci_set_power_state, only up to D3hot is allowed. Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com> --- diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c index 89efeff..566e544 100644 --- a/drivers/gpu/drm/nouveau/nouveau_drm.c +++ b/drivers/gpu/drm/n...

...return -EBUSY; } - nouveau_switcheroo_optimus_dsm(); + drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; + drm->runpm_dsm = nouveau_switcheroo_optimus_dsm(); ret = nouveau_do_suspend(drm_dev, true); pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev); - pci_set_power_state(pdev, PCI_D3cold); + if (drm->runpm_dsm) + pci_set_power_state(pdev, PCI_D3cold); + drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; return ret; } @@ -925,7 +929,8 @@ nouveau_pmops_runtime_resume(struct device *dev) { struct pci_dev *pdev = to_pci_dev(dev); struct drm_de...

...@@ nouveau_pmops_runtime_suspend(struct device *dev) return -EBUSY; } + drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; nouveau_switcheroo_optimus_dsm(); ret = nouveau_do_suspend(drm_dev, true); pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev); - pci_set_power_state(pdev, PCI_D3cold); + if (drm->runpm_dsm) + pci_set_power_state(pdev, PCI_D3cold); + drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; return ret; } @@ -925,7 +930,8 @@ nouveau_pmops_runtime_resume(struct device *dev) { struct pci_dev *pdev = to_pci_dev(dev); struct drm_de...

...@@ nouveau_pmops_runtime_suspend(struct device *dev) return -EBUSY; } + drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; nouveau_switcheroo_optimus_dsm(); ret = nouveau_do_suspend(drm_dev, true); pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev); - pci_set_power_state(pdev, PCI_D3cold); + if (drm->runpm_dsm) + pci_set_power_state(pdev, PCI_D3cold); + drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; return ret; } @@ -941,7 +946,9 @@ nouveau_pmops_runtime_resume(struct device *dev) return -EBUSY; } - pci_set_power_state(pdev, PCI_D0);...

...@@ nouveau_pmops_runtime_suspend(struct device *dev) return -EBUSY; } + drm_dev->switch_power_state = DRM_SWITCH_POWER_CHANGING; nouveau_switcheroo_optimus_dsm(); ret = nouveau_do_suspend(drm_dev, true); pci_save_state(pdev); pci_disable_device(pdev); pci_ignore_hotplug(pdev); - pci_set_power_state(pdev, PCI_D3cold); + if (drm->runpm_dsm) + pci_set_power_state(pdev, PCI_D3cold); + drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; return ret; } @@ -941,7 +946,9 @@ nouveau_pmops_runtime_resume(struct device *dev) return -EBUSY; } - pci_set_power_state(pdev, PCI_D0);...

...every time we runtime suspend? > heh? and I ignore anybody telling me to use u8 instead of bools :p > > ret = nouveau_do_suspend(drm_dev, true); > > pci_save_state(pdev); > > pci_disable_device(pdev); > > pci_ignore_hotplug(pdev); > > - pci_set_power_state(pdev, PCI_D3cold); > > + if (drm->runpm_dsm) > > + pci_set_power_state(pdev, PCI_D3cold); > > + > > drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; > > return ret; > > } > > @@ -925,7 +929,8 @@ nouveau_pmops_ru...

...return 0; > >> } > >> > >> +static int > >> +nouveau_set_power_state_D0(struct pci_dev *pdev) > >> +{ > >> + struct nouveau_drm *drm = nouveau_drm(pci_get_drvdata(pdev)); > >> + int ret; > >> + > >> + pci_set_power_state(pdev, PCI_D0); > >> + /* abort if anything went wrong */ > >> + if (pdev->current_state != PCI_D0) { > >> + NV_ERROR(drm, "couldn't wake up GPU!\n"); > >> + return -EBUSY; > >> + } > > > >...

...drm->rpm_task = current; > + > nouveau_switcheroo_optimus_dsm(); > ret = nouveau_do_suspend(drm_dev, true); > pci_save_state(pdev); > @@ -873,6 +876,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) > pci_ignore_hotplug(pdev); > pci_set_power_state(pdev, PCI_D3cold); > drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; > + > + drm->rpm_task = NULL; > return ret; > } > > @@ -881,6 +886,7 @@ nouveau_pmops_runtime_resume(struct device *dev) > { > struct pci_dev *pdev = to_...

While investigating the runpm issues on my GP107 I noticed that something inside devinit makes runpm break. If Nouveau loads up to the point right before doing devinit, runpm works without any issues, if devinit is ran, not anymore. Out of curiousity I even tried to "bisect" devinit by not running it on vbios provided signed PMU image, but on the devinit parser we have inside Nouveau.

...ouveau/nouveau_drm.c > @@ -792,6 +792,27 @@ nouveau_pmops_suspend(struct device *dev) > return 0; > } > > +static int > +nouveau_set_power_state_D0(struct pci_dev *pdev) > +{ > + struct nouveau_drm *drm = nouveau_drm(pci_get_drvdata(pdev)); > + int ret; > + > + pci_set_power_state(pdev, PCI_D0); > + /* abort if anything went wrong */ > + if (pdev->current_state != PCI_D0) { > + NV_ERROR(drm, "couldn't wake up GPU!\n"); > + return -EBUSY; > + } Looks to me like the more idiomatic way to do this is: ret = pci_set_power_state(pdev, PCI_D0);...

...d(struct device *dev) return -EBUSY; } + dev->power.disable_depth++; + drm_kms_helper_poll_disable(drm_dev); nouveau_switcheroo_optimus_dsm(); ret = nouveau_do_suspend(drm_dev, true); @@ -843,6 +845,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) pci_ignore_hotplug(pdev); pci_set_power_state(pdev, PCI_D3cold); drm_dev->switch_power_state = DRM_SWITCH_POWER_DYNAMIC_OFF; + + dev->power.disable_depth--; return ret; } @@ -859,11 +863,13 @@ nouveau_pmops_runtime_resume(struct device *dev) return -EBUSY; } + dev->power.disable_depth++; + pci_set_power_state(pdev, PCI...

CCing linux-pci and Bjorn Helgaas. Maybe we could get better insights on how a reasonable fix would look like. Anyway, to me this entire issue looks like something which has to be fixed on a PCI level instead of inside a driver, so it makes sense to ask the pci folks if they have any better suggestions. Original cover letter: While investigating the runpm issues on my GP107 I noticed that

...ruct pci_dev *pdev, struct drm_device *drm_dev = pci_get_drvdata(pdev); int ret; - if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF || - drm_dev->switch_power_state == DRM_SWITCH_POWER_DYNAMIC_OFF) + if (drm_dev->switch_power_state == DRM_SWITCH_POWER_OFF) return 0; pci_set_power_state(pdev, PCI_D0); -- 2.10.1

There are PCI devices which are power-manageable by a nonstandard means, such as a custom ACPI method. One example are discrete GPUs in hybrid graphics laptops, another are Thunderbolt controllers in Macs. Such devices can't be put into D3cold with pci_set_power_state() because pci_platform_power_transition() fails with -ENODEV. Instead they're put into D3hot by pci_set_power_state() and subsequently into D3cold by invoking the nonstandard means. However as a consequence the cached current_state is incorrectly left at D3hot. What we need to do is walk the...

...ML does something which > cannot be done to a device in a PCI low-power state. BTW, I'm wondering if anyone has tried to skip the AML instead of skipping the PCI PM in this case (as of 5.4-rc that would be a similar patch to skip the invocations of __pci_start/complete_power_transition() in pci_set_power_state() for the affected device).

Here's some fixes for the less important DP AUX issues I mentioned a while back. Lyude Paul (2): drm/dp: Add ->pre/post_transfer() hooks for drm_dp_aux drm/nouveau: Grab an rpm reference before/after DP AUX transactions drivers/gpu/drm/drm_dp_helper.c | 5 ++ drivers/gpu/drm/nouveau/nouveau_connector.c | 36 ++++++++ drivers/gpu/drm/nouveau/nouveau_drm.c | 12 ++-

...clared for it and _EJ0 is also declared for the slot then nobody is going to set current_state = PCI_D0 for this device. This is what I think it is happening: pci_enable_device | __pci_enable_device_flags /* here we do not set current_state because !pm_cap */ | do_pci_enable_device | pci_set_power_state | __pci_start_power_transition | pci_platform_power_transition /* platform_pci_power_manageable() calls acpi_pci_power_manageable that * returns true */ | platform_pci_set_power_state /* acpi_pci_set_power_state gets called and does nothing because the * acpi device has _EJ0, see the...