[Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

[Daniel Vetter]

On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kamble at intel.com wrote:
> From: Borun Fu <borun.fu at intel.com>
> 
> 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 at intel.com>
> Cc: Paulo Zanoni <paulo.r.zanoni at intel.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: Jani Nikula <jani.nikula at linux.intel.com>
> Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848
> Signed-off-by: Sagar Kamble <sagar.a.kamble at intel.com>

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