search for: runtime_resum

...39;s > some kind of race between that resume and the reboot? I think we do in fact resume PCI devices before shutdown. Here's the path I'm looking at: device_shutdown pm_runtime_get_noresume pm_runtime_barrier dev->bus->shutdown pci_device_shutdown pm_runtime_resume __pm_runtime_resume(dev, 0) rpm_resume(dev, 0) __update_runtime_status(dev, RPM_RESUMING) callback = RPM_GET_CALLBACK(dev, runtime_resume) rpm_callback(callback, dev) __rpm_callback pci_pm_runtime_res...

...d the reboot? > > I think we do in fact resume PCI devices before shutdown. Here's the > path I'm looking at: > > device_shutdown > pm_runtime_get_noresume > pm_runtime_barrier > dev->bus->shutdown > pci_device_shutdown > pm_runtime_resume > __pm_runtime_resume(dev, 0) > rpm_resume(dev, 0) > __update_runtime_status(dev, RPM_RESUMING) > callback = RPM_GET_CALLBACK(dev, runtime_resume) > rpm_callback(callback, dev) > __rpm_callback >...

...n fact resume PCI devices before shutdown. Here's the > > path I'm looking at: > > > > device_shutdown > > pm_runtime_get_noresume > > pm_runtime_barrier > > dev->bus->shutdown > > pci_device_shutdown > > pm_runtime_resume > > __pm_runtime_resume(dev, 0) > > rpm_resume(dev, 0) > > __update_runtime_status(dev, RPM_RESUMING) > > callback = RPM_GET_CALLBACK(dev, runtime_resume) > > rpm_callback(callback, dev) > >...

...kernel to deadlock. > > With this, it should be safe to call the pm runtime functions in any > context in nouveau with one condition: any point in the driver that > calls pm_runtime_get*() cannot hold any locks owned by nouveau that > would be acquired anywhere inside nouveau_pmops_runtime_resume(). > This includes modesetting locks, i2c bus locks, etc. [snip] > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) > return -EBUSY; > } > > + dev->pow...

...ons(-) > >> > >> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > >> index 3bed6beda051..51b11cbd48f6 100644 > >> --- a/drivers/pci/pci-driver.c > >> +++ b/drivers/pci/pci-driver.c > >> @@ -1277,10 +1277,14 @@ static int pci_pm_runtime_resume(struct device *dev) > >> > >> /* > >> * If pci_dev->driver is not set (unbound), the device should > >> - * always remain in D0 regardless of the runtime PM status > >> + * always remain in D0 regardless of the runtime PM statu...

...1 + > 3 files changed, 8 insertions(+), 3 deletions(-) > > diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c > index 3bed6beda051..51b11cbd48f6 100644 > --- a/drivers/pci/pci-driver.c > +++ b/drivers/pci/pci-driver.c > @@ -1277,10 +1277,14 @@ static int pci_pm_runtime_resume(struct device *dev) > > /* > * If pci_dev->driver is not set (unbound), the device should > - * always remain in D0 regardless of the runtime PM status > + * always remain in D0 regardless of the runtime PM status. > + * But if its parent can go to D3cold, this devic...

...; With this, it should be safe to call the pm runtime functions in any > > > context in nouveau with one condition: any point in the driver that > > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that > > > would be acquired anywhere inside nouveau_pmops_runtime_resume(). > > > This includes modesetting locks, i2c bus locks, etc. > > > > [snip] > > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *d...

...e PM on the GPU: echo auto > /sys/bus/pci/devices/0000:01:00.0/power/control Wait for GPU to power off, then rebind it: echo 0000:01:00.0 > /sys/bus/pci/drivers/{nouveau,amdgpu,radeon}/bind Check dmesg for errors. If you see any then we may need to perform further actions in pci_pm_runtime_resume(), see patch [1/7]. Thanks, Lukas Lukas Wunner (7): PCI: Restore BARs on runtime resume despite being unbound PCI: Make pci_wakeup_bus() & pci_bus_set_current_state() public vga_switcheroo: Update PCI current_state on power change vga_switcheroo: Deduplicate power state tracking v...

...b/drivers/pci/pci.c @@ -860,6 +860,14 @@ static int pci_raw_set_power_state(struct pci_dev *dev, pci_power_t state) || (state == PCI_D2 && !dev->d2_support)) return -EIO; + /* + * Power management can be disabled for certain devices as they don't + * come back up later on runtime_resume. We rely on platform means to + * cut power consumption instead (e.g. ACPI). + */ + if (state != PCI_D0 && dev->parent_d3cold) + return 0; + pci_read_config_word(dev, dev->pm_cap + PCI_PM_CTRL, &pmcsr); if (pmcsr == (u16) ~0) { pci_err(dev, "can't change power...

...te: > On Wednesday, February 21, 2018 10:57:14 AM CET Rafael J. Wysocki wrote: > > So if pci_pm_runtime_suspend() is modified to call pci_save_state() > > before returning 0 in the !dev->driver case, we can just move the > > pci_restore_standard_config() invocation in pci_pm_runtime_resume() up > > to the very top and check dev->driver later. > > I mean something like the patch below, overall (untested). > > Tentatively-signed-off-by: Rafael J. Wysocki <rafael.j.wysocki at intel.com> Okay I've tested this successfully now. I'll have to respin t...

On Wed, 2019-04-24 at 13:59 -0500, Bjorn Helgaas wrote: > Not being a scheduled work expert, I was unsure if this experiment was > equivalent to what I proposed. > > I'm always suspicious of singleton solutions like this (using > schedule_work() in runtime_resume()) because usually they seem to be > solving a generic problem that should happen on many kinds of > hardware. The 0b2fe6594fa2 ("drm/nouveau: Queue hpd_work on (runtime) > resume") commit log says: > > We need to call drm_helper_hpd_irq_event() on resume to properly &...

...se, I could ,orthogonal to this, make the code more robust if we hit issues like this in the future. What I am mostly wondering about is, why pci core doesn't give up if the device doesn't come back from D3cold? It sounds like, that the most sane thing to do here is to just give up and fail runtime_resume and report errors back to userspace trying to make use of the devices. > > + * - sudden shutdowns, as the kernel identified an unrecoverable error after > > + * userspace tries to access the GPU. > > This doesn't fit with the others and more details might be > informa...

...when the bridge above it runtime suspends. + * Save its config space in case that happens. */ - if (!pci_dev->driver) + if (!pci_dev->driver) { + pci_save_state(pci_dev); return 0; + } if (!pm || !pm->runtime_suspend) return -ENOSYS; @@ -1276,16 +1279,18 @@ static int pci_pm_runtime_resume(struct device *dev) const struct dev_pm_ops *pm = dev->driver ? dev->driver->pm : NULL; /* - * If pci_dev->driver is not set (unbound), the device should - * always remain in D0 regardless of the runtime PM status + * Restoring config space is necessary even if the device is n...

...| 2 +- drivers/pci/pci.h | 1 + 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 3bed6beda051..51b11cbd48f6 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -1277,10 +1277,14 @@ static int pci_pm_runtime_resume(struct device *dev) /* * If pci_dev->driver is not set (unbound), the device should - * always remain in D0 regardless of the runtime PM status + * always remain in D0 regardless of the runtime PM status. + * But if its parent can go to D3cold, this device may have + * been in D3cold...

...should be safe to call the pm runtime functions in any > > > > context in nouveau with one condition: any point in the driver that > > > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that > > > > would be acquired anywhere inside nouveau_pmops_runtime_resume(). > > > > This includes modesetting locks, i2c bus locks, etc. > > > > > > [snip] > > > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > > > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > > > > @@ -835,6 +835,8 @@ nouveau_pmops_ru...

...it should be safe to call the pm runtime functions in any >> > > context in nouveau with one condition: any point in the driver that >> > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that >> > > would be acquired anywhere inside nouveau_pmops_runtime_resume(). >> > > This includes modesetting locks, i2c bus locks, etc. >> > >> > [snip] >> > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c >> > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c >> > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_s...

...e PM on the GPU: echo auto > /sys/bus/pci/devices/0000:01:00.0/power/control Wait for GPU to power off, then rebind it: echo 0000:01:00.0 > /sys/bus/pci/drivers/{nouveau,amdgpu,radeon}/bind Check dmesg for errors. If you see any then we may need to perform further actions in pci_pm_runtime_resume(), see patch [1/7]. Thanks, Lukas Lukas Wunner (6): PCI: Make pci_wakeup_bus() & pci_bus_set_current_state() public vga_switcheroo: Update PCI current_state on power change vga_switcheroo: Deduplicate power state tracking vga_switcheroo: Use device link for HDA controller vga_swi...

[+cc Rafael] On Wed, Mar 13, 2019 at 06:25:02PM -0400, Lyude Paul wrote: > On Fri, 2019-02-15 at 16:17 -0500, Lyude Paul wrote: > > On Thu, 2019-02-14 at 18:43 -0600, Bjorn Helgaas wrote: > > > On Tue, Feb 12, 2019 at 05:02:30PM -0500, Lyude Paul wrote: > > > > On a very specific subset of ThinkPad P50 SKUs, particularly > > > > ones that come with a

...ged, 8 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c >> index 3bed6beda051..51b11cbd48f6 100644 >> --- a/drivers/pci/pci-driver.c >> +++ b/drivers/pci/pci-driver.c >> @@ -1277,10 +1277,14 @@ static int pci_pm_runtime_resume(struct device *dev) >> >> /* >> * If pci_dev->driver is not set (unbound), the device should >> - * always remain in D0 regardless of the runtime PM status >> + * always remain in D0 regardless of the runtime PM status. >> + * But...

...> > > With this, it should be safe to call the pm runtime functions in any > > context in nouveau with one condition: any point in the driver that > > calls pm_runtime_get*() cannot hold any locks owned by nouveau that > > would be acquired anywhere inside nouveau_pmops_runtime_resume(). > > This includes modesetting locks, i2c bus locks, etc. > > [snip] > > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c > > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c > > @@ -835,6 +835,8 @@ nouveau_pmops_runtime_suspend(struct device *dev) > > return -EBUSY...