[Intel-gfx] [PATCH v2 10/25] drm/i915/tgl: Add power well to support 4th pipe
Lucas De Marchi
lucas.demarchi at intel.com
Wed Jul 10 16:02:22 UTC 2019
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 at intel.com>
>> > >
>> > > Add power well 5 to support 4th pipe and transcoder on TGL.
>> > >
>> > > Cc: James Ausmus <james.ausmus at intel.com>
>> > > Cc: Imre Deak <imre.deak at intel.com>
>> > > Signed-off-by: Mika Kahola <mika.kahola at intel.com>
>> > > Signed-off-by: Lucas De Marchi <lucas.demarchi at 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 at lists.freedesktop.org
>> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list