[Intel-gfx] [PATCH 17/21] drm/i915: Track the wakeref used to initialise display power domains

[Chris Wilson]

Quoting John Harrison (2019-01-10 23:15:31)
> On 1/10/2019 02:11, Chris Wilson wrote:
> > @@ -4054,18 +4055,20 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
> >        * resources powered until display HW readout is complete. We drop
> >        * this reference in intel_power_domains_enable().
> >        */
> > -     intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> > +     power_domains->wakeref =
> > +             intel_display_power_get(i915, POWER_DOMAIN_INIT);
> > +
> >       /* Disable power support if the user asked so. */
> >       if (!i915_modparams.disable_power_well)
> > -             intel_display_power_get(dev_priv, POWER_DOMAIN_INIT);
> > -     intel_power_domains_sync_hw(dev_priv);
> > +             intel_display_power_get(i915, POWER_DOMAIN_INIT);
> Why not save this cookie away somewhere too, e.g. 
> power_domains->wakeref_disable? That way you can also get rid of the 
> put_unchecked calls below.

Just seemed like a bit more hassle for the case where rpm was
intentionally disabled by the user. The system is going to leak the
wakerefs, tracking seemed pointless, so I just threw my hands up in the
air and gave up.

> > +     intel_power_domains_sync_hw(i915);
> >   
> >       power_domains->initializing = false;
> >   }
> >   
> >   /**
> >    * intel_power_domains_fini_hw - deinitialize hw power domain state
> > - * @dev_priv: i915 device instance
> > + * @i915: i915 device instance
> >    *
> >    * De-initializes the display power domain HW state. It also ensures that the
> >    * device stays powered up so that the driver can be reloaded.
> > @@ -4074,21 +4077,24 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv, bool resume)
> >    * intel_power_domains_disable()) and must be paired with
> >    * intel_power_domains_init_hw().
> >    */
> > -void intel_power_domains_fini_hw(struct drm_i915_private *dev_priv)
> > +void intel_power_domains_fini_hw(struct drm_i915_private *i915)
> >   {
> > -     /* Keep the power well enabled, but cancel its rpm wakeref. */
> > -     intel_runtime_pm_put_unchecked(dev_priv);
> > +     intel_wakeref_t wakeref __maybe_unused =
> > +             fetch_and_zero(&i915->power_domains.wakeref);
> Why the '__maybe_unused'? The call to put is unconditional. Or is it 
> about keeping the compiler happy in the case where the wakeref tracking 
> is disabled? Although I don't recall seeing any 'maybe's in the earlier 
> patches.

Just about keeping the compiler happy when the compiler complained.
-Chris