On Thursday, September 17, 2015 02:23:32 PM Jesse Barnes wrote: > According to the PCI docs and Rafael, we need to be using > pm_runtime_put_noidle() and pm_runtime_get_noresume() in our init and > teardown routines, rather than using a direct enable/disable pair (and > we didn't even have the enable side, so never autosuspended after an > unload). > > This fixes one failure of the basic-pci-d3-state test on my BYT. I'm > still debugging why the device never autosuspends. > > Signed-off-by: Jesse Barnes <jbarnes@xxxxxxxxxxxxxxxx> > --- > drivers/gpu/drm/i915/intel_runtime_pm.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index 85c35fd..1addb8a 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -1822,7 +1822,7 @@ static void intel_runtime_pm_disable(struct drm_i915_private *dev_priv) > > /* Make sure we're not suspended first. */ > pm_runtime_get_sync(device); > - pm_runtime_disable(device); That is correct IMO. If you ensure that the usage counter is always above 0 and the device is "on", you don't need to disable runtime PM in addition to that. > + pm_runtime_get_noresume(device); I'm not sure that this is needed. You've already bumped up the usage counter. Are you going to drop it going forward? > } > > /** > @@ -2114,8 +2114,6 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) > if (!HAS_RUNTIME_PM(dev)) > return; > > - pm_runtime_set_active(device); > - > /* > * RPM depends on RC6 to save restore the GT HW context, so make RC6 a > * requirement. > @@ -2130,5 +2128,6 @@ void intel_runtime_pm_enable(struct drm_i915_private *dev_priv) > pm_runtime_use_autosuspend(device); > > pm_runtime_put_autosuspend(device); > + pm_runtime_put_noidle(device); That shouldn't be necessary. The "put" is already done in pm_runtime_put_autosuspend(). > } > > The rule is that .probe() should do one extra decrementation of the usage counter and .remove() should do one extra incrementation of it. Enable/disable should never be necessary. Thanks, Rafael _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
