Re: [PATCH 66/66] drm/i915: runtime PM support for DPMS

Daniel Vetter
2014-05-16T22:19:13Z

On Fri, May 16, 2014 at 02:48:27PM -0700, Jesse Barnes wrote:
> On Thu, 24 Apr 2014 23:55:42 +0200
> Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>
> > + if (enable) {
> > + if (!intel_crtc->active) {
> > + domains = get_crtc_power_domains(crtc);
> > + for_each_power_domain(domain, domains)
> > + intel_display_power_get(dev_priv, domain);
> > + intel_crtc->enabled_power_domains = domains;
> > +
> > + dev_priv->display.crtc_enable(crtc);
> > + }
> > + } else {
> > + if (intel_crtc->active) {
> > + dev_priv->display.crtc_disable(crtc);
> > +
> > + domains = intel_crtc->enabled_power_domains;
> > + for_each_power_domain(domain, domains)
> > + intel_display_power_put(dev_priv, domain);
> > + intel_crtc->enabled_power_domains = 0;
> > + }
> > + }
>
> These branches could probably be cleaned up a bit.
>
> But the power domain bits here got me thinking that maybe we can push
> them down into the crtc_enable/disable functions instead. That would
> let us do the right thing per-platform and save us the
> "get_crtc_power_domains" call which may not make sense on all platforms.
>
> I haven't thought it through for the other power wells, but that type
> of approach may make more sense than trying to abstract the wells at
> the high level we're doing today, especially since things are likely to
> get finer grained over time rather than coarser.

Had the same idea but then things get ugly since since the power domain
grabbing in the modeset sequence is a bit convoluted (for historical
reasons). So would require a bit of unwinding.

Also this gives us a much clearer bisect point imo.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch