From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend Date: Sat, 12 Jul 2014 11:50:28 +0200 Message-ID: <20140712095028.GP17271@phenom.ffwll.local> References: <20140711161407.GG17271@phenom.ffwll.local> <1405139547-13043-1-git-send-email-sagar.a.kamble@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f182.google.com (mail-we0-f182.google.com [74.125.82.182]) by gabe.freedesktop.org (Postfix) with ESMTP id CE3C889F85 for ; Sat, 12 Jul 2014 02:50:17 -0700 (PDT) Received: by mail-we0-f182.google.com with SMTP id q59so2127867wes.13 for ; Sat, 12 Jul 2014 02:50:16 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1405139547-13043-1-git-send-email-sagar.a.kamble@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: sagar.a.kamble@intel.com Cc: Paulo Zanoni , Daniel Vetter , intel-gfx@lists.freedesktop.org, Borun Fu List-Id: intel-gfx@lists.freedesktop.org On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kamble@intel.com wrote: > From: Borun Fu > > 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 > Cc: Paulo Zanoni > Cc: Daniel Vetter > Cc: Jani Nikula > Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848 > Signed-off-by: Sagar Kamble 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