Re: [PATCH 2/6] drm/i915: factor out is_always_on_domain

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Wed, 16 Oct 2013 17:25:49 +0300
Imre Deak <imre.deak@intel.com> wrote:

> It is just cleaner this way and makes it easier to add support for
> other HW generations with always-on power wells powering a different
> set of domains.
> 
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h |  8 ++++
>  drivers/gpu/drm/i915/intel_pm.c | 84 +++++++++++++++--------------------------
>  2 files changed, 38 insertions(+), 54 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 9b04d05..ca05f3a 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -100,8 +100,12 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_TRANSCODER_C,
>  	POWER_DOMAIN_TRANSCODER_EDP,
>  	POWER_DOMAIN_VGA,
> +
> +	POWER_DOMAIN_NUM,
>  };
>  
> +#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
> +
>  #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
>  #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \
>  		((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER)
> @@ -109,6 +113,10 @@ enum intel_display_power_domain {
>  	((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \
>  	 (tran) + POWER_DOMAIN_TRANSCODER_A)
>  
> +#define HSW_ALWAYS_ON_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PIPE_A) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_EDP))
> +
>  enum hpd_pin {
>  	HPD_NONE = 0,
>  	HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3d3658c..57d08a2 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5367,6 +5367,23 @@ void intel_suspend_hw(struct drm_device *dev)
>  		lpt_suspend_hw(dev);
>  }
>  
> +static bool is_always_on_power_domain(struct drm_device *dev,
> +				      enum intel_display_power_domain domain)
> +{
> +	unsigned long always_on_domains;
> +
> +	BUG_ON(BIT(domain) & ~POWER_DOMAIN_MASK);
> +
> +	if (IS_HASWELL(dev)) {
> +		always_on_domains = HSW_ALWAYS_ON_POWER_DOMAINS;
> +	} else {
> +		WARN_ON(1);
> +		return true;
> +	}
> +
> +	return BIT(domain) & always_on_domains;
> +}
> +
>  /**
>   * We should only use the power well if we explicitly asked the hardware to
>   * enable it, so check if it's enabled and also check if we've requested it to
> @@ -5380,24 +5397,11 @@ bool intel_display_power_enabled(struct drm_device *dev,
>  	if (!HAS_POWER_WELL(dev))
>  		return true;
>  
> -	switch (domain) {
> -	case POWER_DOMAIN_PIPE_A:
> -	case POWER_DOMAIN_TRANSCODER_EDP:
> +	if (is_always_on_power_domain(dev, domain))
>  		return true;
> -	case POWER_DOMAIN_VGA:
> -	case POWER_DOMAIN_PIPE_B:
> -	case POWER_DOMAIN_PIPE_C:
> -	case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
> -	case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
> -	case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
> -	case POWER_DOMAIN_TRANSCODER_A:
> -	case POWER_DOMAIN_TRANSCODER_B:
> -	case POWER_DOMAIN_TRANSCODER_C:
> -		return I915_READ(HSW_PWR_WELL_DRIVER) ==
> +
> +	return I915_READ(HSW_PWR_WELL_DRIVER) ==
>  		     (HSW_PWR_WELL_ENABLE_REQUEST | HSW_PWR_WELL_STATE_ENABLED);
> -	default:
> -		BUG();
> -	}
>  }
>  
>  static void __intel_set_power_well(struct drm_device *dev, bool enable)
> @@ -5469,26 +5473,12 @@ void intel_display_power_get(struct drm_device *dev,
>  	if (!HAS_POWER_WELL(dev))
>  		return;
>  
> -	switch (domain) {
> -	case POWER_DOMAIN_PIPE_A:
> -	case POWER_DOMAIN_TRANSCODER_EDP:
> +	if (is_always_on_power_domain(dev, domain))
>  		return;
> -	case POWER_DOMAIN_VGA:
> -	case POWER_DOMAIN_PIPE_B:
> -	case POWER_DOMAIN_PIPE_C:
> -	case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
> -	case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
> -	case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
> -	case POWER_DOMAIN_TRANSCODER_A:
> -	case POWER_DOMAIN_TRANSCODER_B:
> -	case POWER_DOMAIN_TRANSCODER_C:
> -		spin_lock_irq(&power_well->lock);
> -		__intel_power_well_get(power_well);
> -		spin_unlock_irq(&power_well->lock);
> -		return;
> -	default:
> -		BUG();
> -	}
> +
> +	spin_lock_irq(&power_well->lock);
> +	__intel_power_well_get(power_well);
> +	spin_unlock_irq(&power_well->lock);
>  }
>  
>  void intel_display_power_put(struct drm_device *dev,
> @@ -5500,26 +5490,12 @@ void intel_display_power_put(struct drm_device *dev,
>  	if (!HAS_POWER_WELL(dev))
>  		return;
>  
> -	switch (domain) {
> -	case POWER_DOMAIN_PIPE_A:
> -	case POWER_DOMAIN_TRANSCODER_EDP:
> +	if (is_always_on_power_domain(dev, domain))
>  		return;
> -	case POWER_DOMAIN_VGA:
> -	case POWER_DOMAIN_PIPE_B:
> -	case POWER_DOMAIN_PIPE_C:
> -	case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
> -	case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
> -	case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
> -	case POWER_DOMAIN_TRANSCODER_A:
> -	case POWER_DOMAIN_TRANSCODER_B:
> -	case POWER_DOMAIN_TRANSCODER_C:
> -		spin_lock_irq(&power_well->lock);
> -		__intel_power_well_put(power_well);
> -		spin_unlock_irq(&power_well->lock);
> -		return;
> -	default:
> -		BUG();
> -	}
> +
> +	spin_lock_irq(&power_well->lock);
> +	__intel_power_well_put(power_well);
> +	spin_unlock_irq(&power_well->lock);
>  }
>  
>  static struct i915_power_well *hsw_pwr;

Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center