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

2019-07-10 Thread Rodrigo Vivi
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

2019-07-10 Thread Lucas De Marchi

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

2019-07-10 Thread Rodrigo Vivi
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

2019-07-09 Thread Lucas De Marchi

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

2019-07-09 Thread Rodrigo Vivi
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