Quoting Imre Deak (2018-08-10 13:22:43) > On Fri, Aug 10, 2018 at 08:11:31AM +0100, Chris Wilson wrote: > > @@ -1474,6 +1476,8 @@ void i915_driver_unload(struct drm_device *dev) > > i915_driver_cleanup_mmio(dev_priv); > > > > intel_display_power_put(dev_priv, POWER_DOMAIN_INIT); > > + > > + WARN_ON(atomic_read(&dev_priv->runtime_pm.wakeref_count)); > > This probably WARNs atm at least due to having a low-level > pm_runtime_put_autosuspend() in intel_runtime_pm_enable(); but a > corresponding intel_runtime_pm_get() in intel_power_domains_fini_hw() (via > intel_display_set_init_power) which is i915 specific. The following > would fix that: It gets caught out by the very last display_set_init_power indeed. I mean to have a word with you about that function ;) The issue we have with intel_display_set_init_power() at the moment is that we don't always clean up it correctly as it not obvious who is responsible for it at any time. Would it be possible to make that into local POWER_DOMAIN_INIT grabs so the onion is always clear? (It wasn't obvious to me why ownership was being transferred fairly arbitrary.) > > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c > index e209edbc561d..1623c0d2cf57 100644 > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c > @@ -3806,6 +3806,10 @@ void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv) > */ > intel_display_set_init_power(dev_priv, true); In this case this would be much clearer as a raw intel_power_domain_get(POWER_DOMAIN_INIT) I think. -Chris _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
