Re: [PATCH 3/3] drm/i915: Define HSW/BDW display power domains the right way up

[Imre Deak <imre.deak@intel.com>]

On Mon, 2016-04-18 at 14:02 +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Currently we're trying to define HSW/BDW power wells by what's not
> included. Let's do it the other way around, so that you can actually
> tell when the power well would get enabled. This will also allow us
> to
> add new power domains without accidentally adding it to the HSW/BDW
> display power domains.
> 
> The current set of domains looks rather buggy even:
> - POWER_DOMAIN_MODESET is included in the display power well
> needlessly
> - DDI-B to DDI-E were not part of the display power well when they
>   should be
> 
> So let's fix that up while at it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Yes, much clearer without negations and indirections. We discussed this
already in person, but for reference: not including the DDI domains in
the display power well happened not to cause a problem, since they are
always requested along with their respective pipe and transcoder
domains. Nice work,
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 45 +++++++++++++++++++--
> ------------
>  1 file changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 585bbe7cefa6..2cead6cb95a1 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1589,30 +1589,37 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	intel_runtime_pm_put(dev_priv);
>  }
>  
> -#define HSW_ALWAYS_ON_POWER_DOMAINS (			\
> -	BIT(POWER_DOMAIN_PIPE_A) |			\
> -	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> -	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> +#define HSW_DISPLAY_POWER_DOMAINS (			\
> +	BIT(POWER_DOMAIN_PIPE_B) |			\
> +	BIT(POWER_DOMAIN_PIPE_C) |			\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> -	BIT(POWER_DOMAIN_PORT_CRT) |			\
> -	BIT(POWER_DOMAIN_PLLS) |			\
> -	BIT(POWER_DOMAIN_AUX_A) |			\
> -	BIT(POWER_DOMAIN_AUX_B) |			\
> -	BIT(POWER_DOMAIN_AUX_C) |			\
> -	BIT(POWER_DOMAIN_AUX_D) |			\
> -	BIT(POWER_DOMAIN_GMBUS) |			\
> -	BIT(POWER_DOMAIN_INIT))
> -#define HSW_DISPLAY_POWER_DOMAINS (				\
> -	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	
> \
> +	BIT(POWER_DOMAIN_PORT_CRT) | /* DDI E */	\
> +	BIT(POWER_DOMAIN_VGA) |				\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  
> -#define BDW_ALWAYS_ON_POWER_DOMAINS (			\
> -	HSW_ALWAYS_ON_POWER_DOMAINS |			\
> -	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER))
> -#define BDW_DISPLAY_POWER_DOMAINS (				\
> -	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> \
> +#define BDW_DISPLAY_POWER_DOMAINS (			\
> +	BIT(POWER_DOMAIN_PIPE_B) |			\
> +	BIT(POWER_DOMAIN_PIPE_C) |			\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_C) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> +	BIT(POWER_DOMAIN_PORT_CRT) | /* DDI E */	\
> +	BIT(POWER_DOMAIN_VGA) |				\
> +	BIT(POWER_DOMAIN_AUDIO) |			\
>  	BIT(POWER_DOMAIN_INIT))
>  
>  #define VLV_DISPLAY_POWER_DOMAINS (		\
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx