On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kamble@xxxxxxxxx wrote: > From: Borun Fu <borun.fu@xxxxxxxxx> > > On VLV, after i915_pm_suspend display power wells are staying > power ungated. So, after initiating mem sleep "echo mem > /sys/power/state" > Display is staing D0 State. There might be better way/place to power gate > these wells. Also, we need to make sure that if wells are power gated due to > DPMS OFF sequence, they need not be turned off by i915_pm_suspend again. > > v2: Extracted helper for intel_crtc_disable and power gating CRTC power wells. > [Daniel] > > Cc: Imre Deak <imre.deak@xxxxxxxxx> > Cc: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx> > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > Cc: Jani Nikula <jani.nikula@xxxxxxxxxxxxxxx> > Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848 > Signed-off-by: Sagar Kamble <sagar.a.kamble@xxxxxxxxx> intel_crtc_control looks like a neat idea - I've started also thinking about the enable side and noticed that we'll have similar issues there. But complicated with modeset_global_resources and the differences between the dpms paths and modeset paths. But that's work for another day. Queued for -next, thanks for the patch. -Daniel > --- > drivers/gpu/drm/i915/i915_drv.c | 7 +++---- > drivers/gpu/drm/i915/i915_drv.h | 4 ++++ > drivers/gpu/drm/i915/intel_display.c | 30 +++++++++++++++++------------- > drivers/gpu/drm/i915/intel_drv.h | 1 + > 4 files changed, 25 insertions(+), 17 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 83cb43a..5e4fefd 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -524,12 +524,11 @@ static int i915_drm_freeze(struct drm_device *dev) > > /* > * Disable CRTCs directly since we want to preserve sw state > - * for _thaw. > + * for _thaw. Also, power gate the CRTC power wells. > */ > drm_modeset_lock_all(dev); > - for_each_crtc(dev, crtc) { > - dev_priv->display.crtc_disable(crtc); > - } > + for_each_crtc(dev, crtc) > + intel_crtc_control(crtc, false); > drm_modeset_unlock_all(dev); > > intel_modeset_suspend_hw(dev); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index a89c912..54f98d3 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -179,6 +179,10 @@ enum hpd_pin { > list_for_each_entry((intel_connector), &(dev)->mode_config.connector_list, base.head) \ > if ((intel_connector)->base.encoder == (__encoder)) > > +#define for_each_power_domain(domain, mask) \ > + for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ > + if ((1 << (domain)) & (mask)) > + > struct drm_i915_private; > struct i915_mmu_object; > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index fe6f1db..7a1f14f 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -4300,10 +4300,6 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc) > I915_WRITE(BCLRPAT(crtc->pipe), 0); > } > > -#define for_each_power_domain(domain, mask) \ > - for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++) \ > - if ((1 << (domain)) & (mask)) > - > enum intel_display_power_domain > intel_display_port_power_domain(struct intel_encoder *intel_encoder) > { > @@ -4872,21 +4868,14 @@ static void intel_crtc_update_sarea(struct drm_crtc *crtc, > } > } > > -/** > - * Sets the power management mode of the pipe and plane. > - */ > -void intel_crtc_update_dpms(struct drm_crtc *crtc) > +/* Master function to enable/disable CRTC and corresponding power wells */ > +void intel_crtc_control(struct drm_crtc *crtc, bool enable) > { > 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) { > if (!intel_crtc->active) { > @@ -4907,6 +4896,21 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc) > intel_crtc->enabled_power_domains = 0; > } > } > +} > + > +/** > + * Sets the power management mode of the pipe and plane. > + */ > +void intel_crtc_update_dpms(struct drm_crtc *crtc) > +{ > + struct drm_device *dev = crtc->dev; > + struct intel_encoder *intel_encoder; > + bool enable = false; > + > + for_each_encoder_on_crtc(dev, crtc, intel_encoder) > + enable |= intel_encoder->connectors_active; > + > + intel_crtc_control(crtc, enable); > > intel_crtc_update_sarea(crtc, enable); > } > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 016d894..4c24f88 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -755,6 +755,7 @@ void intel_frontbuffer_flip(struct drm_device *dev, > void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire); > void intel_mark_idle(struct drm_device *dev); > void intel_crtc_restore_mode(struct drm_crtc *crtc); > +void intel_crtc_control(struct drm_crtc *crtc, bool enable); > void intel_crtc_update_dpms(struct drm_crtc *crtc); > void intel_encoder_destroy(struct drm_encoder *encoder); > void intel_connector_dpms(struct drm_connector *, int mode); > -- > 1.8.5 > -- 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