On Sat, 17 May 2014 00:19:09 +0200
Daniel Vetter <[email protected]> wrote:

> On Fri, May 16, 2014 at 02:48:27PM -0700, Jesse Barnes wrote:
> > On Thu, 24 Apr 2014 23:55:42 +0200
> > Daniel Vetter <[email protected]> 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.

Yeah no doubt, not suggesting you change it as part of this series...
but overall it's something to consider for a future rewrite of our
power well code. :)

-- 
Jesse Barnes, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to