Re: [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Jul 10, 2019 at 09:02:22AM -0700, Lucas De Marchi wrote:
> On Wed, Jul 10, 2019 at 04:04:29AM -0700, Rodrigo Vivi wrote:
> > On Tue, Jul 09, 2019 at 09:20:42AM -0700, Lucas De Marchi wrote:
> > > On Tue, Jul 09, 2019 at 04:57:32AM -0700, Rodrigo Vivi wrote:
> > > > On Mon, Jul 08, 2019 at 04:16:14PM -0700, Lucas De Marchi wrote:
> > > > > From: Mika Kahola <mika.kahola@xxxxxxxxx>
> > > > >
> > > > > Add power well 5 to support 4th pipe and transcoder on TGL.
> > > > >
> > > > > Cc: James Ausmus <james.ausmus@xxxxxxxxx>
> > > > > Cc: Imre Deak <imre.deak@xxxxxxxxx>
> > > > > Signed-off-by: Mika Kahola <mika.kahola@xxxxxxxxx>
> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > > > > ---
> > > > >  .../drm/i915/display/intel_display_power.c    | 30 ++++++++++++++++---
> > > > >  .../drm/i915/display/intel_display_power.h    |  3 ++
> > > > >  drivers/gpu/drm/i915/i915_reg.h               |  3 +-
> > > > >  3 files changed, 31 insertions(+), 5 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > index c3f42169831f..455f9aab188d 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c
> > > > > @@ -37,18 +37,24 @@ intel_display_power_domain_str(struct drm_i915_private *i915,
> > > > >  		return "PIPE_B";
> > > > >  	case POWER_DOMAIN_PIPE_C:
> > > > >  		return "PIPE_C";
> > > > > +	case POWER_DOMAIN_PIPE_D:
> > > > > +		return "PIPE_D";
> > > > >  	case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
> > > > >  		return "PIPE_A_PANEL_FITTER";
> > > > >  	case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
> > > > >  		return "PIPE_B_PANEL_FITTER";
> > > > >  	case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
> > > > >  		return "PIPE_C_PANEL_FITTER";
> > > > > +	case POWER_DOMAIN_PIPE_D_PANEL_FITTER:
> > > > > +		return "PIPE_D_PANEL_FITTER";
> > > > >  	case POWER_DOMAIN_TRANSCODER_A:
> > > > >  		return "TRANSCODER_A";
> > > > >  	case POWER_DOMAIN_TRANSCODER_B:
> > > > >  		return "TRANSCODER_B";
> > > > >  	case POWER_DOMAIN_TRANSCODER_C:
> > > > >  		return "TRANSCODER_C";
> > > > > +	case POWER_DOMAIN_TRANSCODER_D:
> > > > > +		return "TRANSCODER_D";
> > > > >  	case POWER_DOMAIN_TRANSCODER_EDP:
> > > > >  		return "TRANSCODER_EDP";
> > > > >  	case POWER_DOMAIN_TRANSCODER_EDP_VDSC:
> > > > > @@ -2451,7 +2457,6 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > >   * - DDI_A
> > > > >   * - FBC
> > > > >   */
> > > > > -/* TODO: TGL_PW_5_POWER_DOMAINS: PIPE_D */
> > > > >  #define ICL_PW_4_POWER_DOMAINS (			\
> > > > >  	BIT_ULL(POWER_DOMAIN_PIPE_C) |			\
> > > > >  	BIT_ULL(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |	\
> > > > > @@ -2539,7 +2544,13 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > >  #define ICL_AUX_TBT4_IO_POWER_DOMAINS (			\
> > > > >  	BIT_ULL(POWER_DOMAIN_AUX_TBT4))
> > > > >
> > > > > +#define TGL_PW_5_POWER_DOMAINS (			\
> > > > > +	BIT_ULL(POWER_DOMAIN_PIPE_D) |			\
> > > > > +	BIT_ULL(POWER_DOMAIN_PIPE_D_PANEL_FITTER) |     \
> > > > > +	BIT_ULL(POWER_DOMAIN_INIT))
> > > > > +
> > > > >  #define TGL_PW_4_POWER_DOMAINS (			\
> > > > > +	TGL_PW_5_POWER_DOMAINS |			\
> > > >
> > > > why?
> > > 
> > > not sure I understand this one. Are you saying we shouldn't have a new
> > > power well for pipe d? How would we handle the different ctl?
> > 
> > We should have a new one. The above block who adds PW5 domains is okay.
> > What I didn't understand is why to add pipe D also on PW4
> 
> as we chated on IRC, because there's this dependency on the enabling
> sequence:
> 
> PG0 -> PG1 -> PG2 -> PG3 -> PG4 -> PG5
> 
> So to enable PG5 I need to enable all the previous power wells. When we
> lookup, say, POWER_DOMAIN_PIPE_D, the bit will be set on all the
> power wells, which makes this happen.

Thanks for the info here and on irc and sorry for my inverted confusion there ;)

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx>

> 
> Lucas De Marchi
> 
> > 
> > > 
> > > >
> > > > >  	BIT_ULL(POWER_DOMAIN_PIPE_C) |			\
> > > > >  	BIT_ULL(POWER_DOMAIN_PIPE_C_PANEL_FITTER) |	\
> > > > >  	BIT_ULL(POWER_DOMAIN_INIT))
> > > > > @@ -2549,7 +2560,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
> > > > >  	BIT_ULL(POWER_DOMAIN_PIPE_B) |			\
> > > > >  	BIT_ULL(POWER_DOMAIN_TRANSCODER_B) |		\
> > > > >  	BIT_ULL(POWER_DOMAIN_TRANSCODER_C) |		\
> > > > > -	/* TODO: TRANSCODER_D */			\
> > > > > +	BIT_ULL(POWER_DOMAIN_TRANSCODER_D) |		\
> > > > >  	BIT_ULL(POWER_DOMAIN_PIPE_B_PANEL_FITTER) |	\
> > > > >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_TC1_LANES) |	\
> > > > >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_TC1_IO) |		\
> > > > > @@ -3882,7 +3893,7 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> > > > >  	},
> > > > >  	{
> > > > >  		.name = "power well 4",
> > > > > -		.domains = ICL_PW_4_POWER_DOMAINS,
> > > > > +		.domains = TGL_PW_4_POWER_DOMAINS,
> > > >
> > > > why?
> > > 
> > > this is a leftover from v1 and should be squashed on previous patch, my
> > > bad. In v1 we were reusing the ICL definitions. I changed in this
> > > revision and forgot to squash this change there. I will send a new
> > > version
> > > 
> > > thanks
> > > 
> > > Lucas De Marchi
> > > 
> > > >
> > > > >  		.ops = &hsw_power_well_ops,
> > > > >  		.id = DISP_PW_ID_NONE,
> > > > >  		{
> > > > > @@ -3892,7 +3903,18 @@ static const struct i915_power_well_desc tgl_power_wells[] = {
> > > > >  			.hsw.irq_pipe_mask = BIT(PIPE_C),
> > > > >  		}
> > > > >  	},
> > > > > -	/* TODO: power well 5 for pipe D */
> > > > > +	{
> > > > > +		.name = "power well 5",
> > > > > +		.domains = TGL_PW_5_POWER_DOMAINS,
> > > > > +		.ops = &hsw_power_well_ops,
> > > > > +		.id = DISP_PW_ID_NONE,
> > > > > +		{
> > > > > +			.hsw.regs = &hsw_power_well_regs,
> > > > > +			.hsw.idx = TGL_PW_CTL_IDX_PW_5,
> > > > > +			.hsw.has_fuses = true,
> > > > > +			.hsw.irq_pipe_mask = BIT(PIPE_D),
> > > > > +		},
> > > > > +	},
> > > > >  };
> > > > >
> > > > >  static int
> > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > > index 86afd70c1fb2..ebb397e330ea 100644
> > > > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h
> > > > > @@ -18,12 +18,15 @@ enum intel_display_power_domain {
> > > > >  	POWER_DOMAIN_PIPE_A,
> > > > >  	POWER_DOMAIN_PIPE_B,
> > > > >  	POWER_DOMAIN_PIPE_C,
> > > > > +	POWER_DOMAIN_PIPE_D,
> > > > >  	POWER_DOMAIN_PIPE_A_PANEL_FITTER,
> > > > >  	POWER_DOMAIN_PIPE_B_PANEL_FITTER,
> > > > >  	POWER_DOMAIN_PIPE_C_PANEL_FITTER,
> > > > > +	POWER_DOMAIN_PIPE_D_PANEL_FITTER,
> > > > >  	POWER_DOMAIN_TRANSCODER_A,
> > > > >  	POWER_DOMAIN_TRANSCODER_B,
> > > > >  	POWER_DOMAIN_TRANSCODER_C,
> > > > > +	POWER_DOMAIN_TRANSCODER_D,
> > > > >  	POWER_DOMAIN_TRANSCODER_EDP,
> > > > >  	POWER_DOMAIN_TRANSCODER_EDP_VDSC,
> > > > >  	POWER_DOMAIN_TRANSCODER_DSI_A,
> > > > > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > > > > index f59cb5c45c34..5ca74eca05a4 100644
> > > > > --- a/drivers/gpu/drm/i915/i915_reg.h
> > > > > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > > > > @@ -9147,7 +9147,8 @@ enum {
> > > > >  #define   GLK_PW_CTL_IDX_DDI_A			1
> > > > >  #define   SKL_PW_CTL_IDX_MISC_IO		0
> > > > >
> > > > > -/* ICL - power wells */
> > > > > +/* ICL/TGL - power wells */
> > > > > +#define   TGL_PW_CTL_IDX_PW_5			4
> > > > >  #define   ICL_PW_CTL_IDX_PW_4			3
> > > > >  #define   ICL_PW_CTL_IDX_PW_3			2
> > > > >  #define   ICL_PW_CTL_IDX_PW_2			1
> > > > > --
> > > > > 2.21.0
> > > > >
> > > > > _______________________________________________
> > > > > Intel-gfx mailing list
> > > > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux