diff mbox series

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

Message ID 20190708231629.9296-11-lucas.demarchi@intel.com (mailing list archive)
State New, archived
Headers show
Series [v2,01/25] drm/i915: Add 4th pipe and transcoder | expand

Commit Message

Lucas De Marchi July 8, 2019, 11:16 p.m. UTC 
From: Mika Kahola <mika.kahola@intel.com>

Add power well 5 to support 4th pipe and transcoder on TGL.

Cc: James Ausmus <james.ausmus@intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Mika Kahola <mika.kahola@intel.com>
Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
---
 .../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(-)

Comments

Rodrigo Vivi July 9, 2019, 11:57 a.m. UTC | #1 
On Mon, Jul 08, 2019 at 04:16:14PM -0700, Lucas De Marchi wrote:
> From: Mika Kahola <mika.kahola@intel.com>
> 
> Add power well 5 to support 4th pipe and transcoder on TGL.
> 
> Cc: James Ausmus <james.ausmus@intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> ---
>  .../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?

>  	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?

>  		.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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lucas De Marchi July 9, 2019, 4:20 p.m. UTC | #2 
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@intel.com>
>>
>> Add power well 5 to support 4th pipe and transcoder on TGL.
>>
>> Cc: James Ausmus <james.ausmus@intel.com>
>> Cc: Imre Deak <imre.deak@intel.com>
>> Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> ---
>>  .../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?

>
>>  	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@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi July 10, 2019, 11:04 a.m. UTC | #3 
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@intel.com>
> > > 
> > > Add power well 5 to support 4th pipe and transcoder on TGL.
> > > 
> > > Cc: James Ausmus <james.ausmus@intel.com>
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > ---
> > >  .../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

> 
> > 
> > >  	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@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Lucas De Marchi July 10, 2019, 4:02 p.m. UTC | #4 
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@intel.com>
>> > >
>> > > Add power well 5 to support 4th pipe and transcoder on TGL.
>> > >
>> > > Cc: James Ausmus <james.ausmus@intel.com>
>> > > Cc: Imre Deak <imre.deak@intel.com>
>> > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
>> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
>> > > ---
>> > >  .../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.

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@lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Rodrigo Vivi July 10, 2019, 4:42 p.m. UTC | #5 
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@intel.com>
> > > > >
> > > > > Add power well 5 to support 4th pipe and transcoder on TGL.
> > > > >
> > > > > Cc: James Ausmus <james.ausmus@intel.com>
> > > > > Cc: Imre Deak <imre.deak@intel.com>
> > > > > Signed-off-by: Mika Kahola <mika.kahola@intel.com>
> > > > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com>
> > > > > ---
> > > > >  .../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@intel.com>

> 
> 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@lists.freedesktop.org
> > > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox series

Patch

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 |			\
 	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,
 		.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