Re: [Intel-gfx] [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe
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 > > > > > > > > > > Add power well 5 to support 4th pipe and transcoder on TGL. > > > > > > > > > > Cc: James Ausmus > > > > > Cc: Imre Deak > > > > > Signed-off-by: Mika Kahola > > > > > Signed-off-by: Lucas De Marchi > > > > > --- > > > > > .../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 > > 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)
Re: [Intel-gfx] [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe
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 > > > > Add power well 5 to support 4th pipe and transcoder on TGL. > > > > Cc: James Ausmus > > Cc: Imre Deak > > Signed-off-by: Mika Kahola > > Signed-off-by: Lucas De Marchi > > --- > > .../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,
Re: [Intel-gfx] [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe
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 > > > > > > Add power well 5 to support 4th pipe and transcoder on TGL. > > > > > > Cc: James Ausmus > > > Cc: Imre Deak > > > Signed-off-by: Mika Kahola > > > Signed-off-by: Lucas De Marchi > > > --- > > > .../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
Re: [Intel-gfx] [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe
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 Add power well 5 to support 4th pipe and transcoder on TGL. Cc: James Ausmus Cc: Imre Deak Signed-off-by: Mika Kahola Signed-off-by: Lucas De Marchi --- .../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 86
Re: [Intel-gfx] [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe
On Mon, Jul 08, 2019 at 04:16:14PM -0700, Lucas De Marchi wrote: > From: Mika Kahola > > Add power well 5 to support 4th pipe and transcoder on TGL. > > Cc: James Ausmus > Cc: Imre Deak > Signed-off-by: Mika Kahola > Signed-off-by: Lucas De Marchi > --- > .../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_FITT