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@xxxxxxxx> 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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx