diff mbox

[2/3] drm/i915: Define VLV/CHV display power well domains properly

Message ID 1460977348-32260-3-git-send-email-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjala April 18, 2016, 11:02 a.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently we're using POWER_DOMAIN_MASK as the power domains for the
display power well on VLV/CHV. That includes all power domains even
though the disp2d/pipe-a power well is not needed for a lot of things.
Let's reduce these to what we actually need.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 42 +++++++++++++++++++++++++++++++--
 1 file changed, 40 insertions(+), 2 deletions(-)

Comments

Imre Deak April 19, 2016, 5:54 a.m. UTC | #1 
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 using POWER_DOMAIN_MASK as the power domains for the
> display power well on VLV/CHV. That includes all power domains even
> though the disp2d/pipe-a power well is not needed for a lot of
> things.
> Let's reduce these to what we actually need.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Ok, so this mostly removes domains that are not defined on the given
platform and so shouldn't be requested. (Btw, the always-on well will
be different now in this regard after patch 1, since for that we also
include the not-defined domains, but I think that's ok.) The modeset
domain is the exception which may power things atm unnecessarily. Looks
ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 42
> +++++++++++++++++++++++++++++++--
>  1 file changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index fe63d77b7dfb..585bbe7cefa6 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1615,7 +1615,23 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> \
>  	BIT(POWER_DOMAIN_INIT))
>  
> -#define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
> +#define VLV_DISPLAY_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PIPE_A) |		\
> +	BIT(POWER_DOMAIN_PIPE_B) |		\
> +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
> +	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> +	BIT(POWER_DOMAIN_PORT_DSI) |		\
> +	BIT(POWER_DOMAIN_PORT_CRT) |		\
> +	BIT(POWER_DOMAIN_VGA) |			\
> +	BIT(POWER_DOMAIN_AUDIO) |		\
> +	BIT(POWER_DOMAIN_AUX_B) |		\
> +	BIT(POWER_DOMAIN_AUX_C) |		\
> +	BIT(POWER_DOMAIN_GMBUS) |		\
> +	BIT(POWER_DOMAIN_INIT))
>  
>  #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
>  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> @@ -1645,6 +1661,28 @@ void intel_display_power_put(struct
> drm_i915_private *dev_priv,
>  	BIT(POWER_DOMAIN_AUX_C) |		\
>  	BIT(POWER_DOMAIN_INIT))
>  
> +#define CHV_DISPLAY_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_PIPE_A) |		\
> +	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_DSI) |		\
> +	BIT(POWER_DOMAIN_VGA) |			\
> +	BIT(POWER_DOMAIN_AUDIO) |		\
> +	BIT(POWER_DOMAIN_AUX_B) |		\
> +	BIT(POWER_DOMAIN_AUX_C) |		\
> +	BIT(POWER_DOMAIN_AUX_D) |		\
> +	BIT(POWER_DOMAIN_GMBUS) |		\
> +	BIT(POWER_DOMAIN_INIT))
> +
>  #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
>  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
>  	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> @@ -1829,7 +1867,7 @@ static struct i915_power_well chv_power_wells[]
> = {
>  		 * power wells don't actually exist. Pipe A power
> well is
>  		 * required for any pipe to work.
>  		 */
> -		.domains = VLV_DISPLAY_POWER_DOMAINS,
> +		.domains = CHV_DISPLAY_POWER_DOMAINS,
>  		.data = PIPE_A,
>  		.ops = &chv_pipe_power_well_ops,
>  	},
Ville Syrjala April 19, 2016, 12:04 p.m. UTC | #2 
On Tue, Apr 19, 2016 at 08:54:05AM +0300, Imre Deak wrote:
> 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 using POWER_DOMAIN_MASK as the power domains for the
> > display power well on VLV/CHV. That includes all power domains even
> > though the disp2d/pipe-a power well is not needed for a lot of
> > things.
> > Let's reduce these to what we actually need.
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Ok, so this mostly removes domains that are not defined on the given
> platform and so shouldn't be requested. (Btw, the always-on well will
> be different now in this regard after patch 1, since for that we also
> include the not-defined domains, but I think that's ok.) The modeset
> domain is the exception which may power things atm unnecessarily.

True. But probably not worth making an exception there.

> Looks
> ok:
> Reviewed-by: Imre Deak <imre.deak@intel.com>

Thanks. Entire series pushed to dinq.

> 
> > ---
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 42
> > +++++++++++++++++++++++++++++++--
> >  1 file changed, 40 insertions(+), 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index fe63d77b7dfb..585bbe7cefa6 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -1615,7 +1615,23 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv,
> >  	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	
> > \
> >  	BIT(POWER_DOMAIN_INIT))
> >  
> > -#define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
> > +#define VLV_DISPLAY_POWER_DOMAINS (		\
> > +	BIT(POWER_DOMAIN_PIPE_A) |		\
> > +	BIT(POWER_DOMAIN_PIPE_B) |		\
> > +	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
> > +	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> > +	BIT(POWER_DOMAIN_PORT_DSI) |		\
> > +	BIT(POWER_DOMAIN_PORT_CRT) |		\
> > +	BIT(POWER_DOMAIN_VGA) |			\
> > +	BIT(POWER_DOMAIN_AUDIO) |		\
> > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > +	BIT(POWER_DOMAIN_AUX_C) |		\
> > +	BIT(POWER_DOMAIN_GMBUS) |		\
> > +	BIT(POWER_DOMAIN_INIT))
> >  
> >  #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
> >  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> > @@ -1645,6 +1661,28 @@ void intel_display_power_put(struct
> > drm_i915_private *dev_priv,
> >  	BIT(POWER_DOMAIN_AUX_C) |		\
> >  	BIT(POWER_DOMAIN_INIT))
> >  
> > +#define CHV_DISPLAY_POWER_DOMAINS (		\
> > +	BIT(POWER_DOMAIN_PIPE_A) |		\
> > +	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_DSI) |		\
> > +	BIT(POWER_DOMAIN_VGA) |			\
> > +	BIT(POWER_DOMAIN_AUDIO) |		\
> > +	BIT(POWER_DOMAIN_AUX_B) |		\
> > +	BIT(POWER_DOMAIN_AUX_C) |		\
> > +	BIT(POWER_DOMAIN_AUX_D) |		\
> > +	BIT(POWER_DOMAIN_GMBUS) |		\
> > +	BIT(POWER_DOMAIN_INIT))
> > +
> >  #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
> >  	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
> >  	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
> > @@ -1829,7 +1867,7 @@ static struct i915_power_well chv_power_wells[]
> > = {
> >  		 * power wells don't actually exist. Pipe A power
> > well is
> >  		 * required for any pipe to work.
> >  		 */
> > -		.domains = VLV_DISPLAY_POWER_DOMAINS,
> > +		.domains = CHV_DISPLAY_POWER_DOMAINS,
> >  		.data = PIPE_A,
> >  		.ops = &chv_pipe_power_well_ops,
> >  	},
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index fe63d77b7dfb..585bbe7cefa6 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1615,7 +1615,23 @@  void intel_display_power_put(struct drm_i915_private *dev_priv,
 	(POWER_DOMAIN_MASK & ~BDW_ALWAYS_ON_POWER_DOMAINS) |	\
 	BIT(POWER_DOMAIN_INIT))
 
-#define VLV_DISPLAY_POWER_DOMAINS	POWER_DOMAIN_MASK
+#define VLV_DISPLAY_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PIPE_A) |		\
+	BIT(POWER_DOMAIN_PIPE_B) |		\
+	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_A) |	\
+	BIT(POWER_DOMAIN_TRANSCODER_B) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
+	BIT(POWER_DOMAIN_PORT_DSI) |		\
+	BIT(POWER_DOMAIN_PORT_CRT) |		\
+	BIT(POWER_DOMAIN_VGA) |			\
+	BIT(POWER_DOMAIN_AUDIO) |		\
+	BIT(POWER_DOMAIN_AUX_B) |		\
+	BIT(POWER_DOMAIN_AUX_C) |		\
+	BIT(POWER_DOMAIN_GMBUS) |		\
+	BIT(POWER_DOMAIN_INIT))
 
 #define VLV_DPIO_CMN_BC_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
@@ -1645,6 +1661,28 @@  void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_AUX_C) |		\
 	BIT(POWER_DOMAIN_INIT))
 
+#define CHV_DISPLAY_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_PIPE_A) |		\
+	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_DSI) |		\
+	BIT(POWER_DOMAIN_VGA) |			\
+	BIT(POWER_DOMAIN_AUDIO) |		\
+	BIT(POWER_DOMAIN_AUX_B) |		\
+	BIT(POWER_DOMAIN_AUX_C) |		\
+	BIT(POWER_DOMAIN_AUX_D) |		\
+	BIT(POWER_DOMAIN_GMBUS) |		\
+	BIT(POWER_DOMAIN_INIT))
+
 #define CHV_DPIO_CMN_BC_POWER_DOMAINS (		\
 	BIT(POWER_DOMAIN_PORT_DDI_B_LANES) |	\
 	BIT(POWER_DOMAIN_PORT_DDI_C_LANES) |	\
@@ -1829,7 +1867,7 @@  static struct i915_power_well chv_power_wells[] = {
 		 * power wells don't actually exist. Pipe A power well is
 		 * required for any pipe to work.
 		 */
-		.domains = VLV_DISPLAY_POWER_DOMAINS,
+		.domains = CHV_DISPLAY_POWER_DOMAINS,
 		.data = PIPE_A,
 		.ops = &chv_pipe_power_well_ops,
 	},