2014-04-24 18:55 GMT-03:00 Daniel Vetter <daniel.vetter@xxxxxxxx>: > Keeping track of the power domains is a bit messy since crtc->active > is currently updated by the platform hooks, but we need to be aware of > which state transition exactly is going on. Maybe we simply need to > shovel all the power domain handling down into platform code to > simplify this. But doing that requires some more auditing since > currently the ->mode_set callbacks still read some random registers > (to e.g. figure out the reference clocks). > > Also note that intel_crtc_update_dpms is always call first/last even > for encoders which have their own dpms functions. Hence we really only > need to update this place here. > > Being a quick "does it blow up?" run not really tested yet. Shouldn't we put some "if (is hsw or newer)" here? I was assuming your series did not care about the SNB code paths, and we do have SNB runtime PM support. I also agree with Jesse's plans to push things to crtc_enable/disable, then remove this code and the code from modeset_global_resources. But if that's complicated, for now the current solution is fine. > > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > --- > drivers/gpu/drm/i915/intel_display.c | 26 ++++++++++++++++++++++---- > 1 file changed, 22 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index e0bd0f94e43e..1b5d6b099b37 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4478,16 +4478,34 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) > { > struct drm_device *dev = crtc->dev; > struct drm_i915_private *dev_priv = dev->dev_private; > + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); > struct intel_encoder *intel_encoder; > + enum intel_display_power_domain domain; > + unsigned long domains; > bool enable = false; > > for_each_encoder_on_crtc(dev, crtc, intel_encoder) > enable |= intel_encoder->connectors_active; > > - if (enable) > - dev_priv->display.crtc_enable(crtc); > - else > - dev_priv->display.crtc_disable(crtc); > + 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; > + } > + } > > intel_crtc_update_sarea(crtc, enable); > } > -- > 1.8.1.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx