| Message ID | 20190625175437.14840-13-lucas.demarchi@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Initial support for Tiger Lake | expand |
On Tue, Jun 25, 2019 at 10:54:21AM -0700, Lucas De Marchi wrote: > From: José Roberto de Souza <jose.souza@intel.com> > > On TGL the special EDP transcoder is gone and it should be handled by > transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > distinction clear and update vdsc code path. > > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> Looks good to me. Reviewed-by: Manasi Navare <manasi.d.navare@intel.com> Manasi > --- > drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- > 3 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 0c7d4a363deb..15582841fefc 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, > return "TRANSCODER_EDP"; > case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > return "TRANSCODER_EDP_VDSC"; > + case POWER_DOMAIN_TRANSCODER_A_VDSC: > + return "TRANSCODER_A_VDSC"; > case POWER_DOMAIN_TRANSCODER_DSI_A: > return "TRANSCODER_DSI_A"; > case POWER_DOMAIN_TRANSCODER_DSI_C: > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 79262a5bceb4..7761b493608a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -29,6 +29,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_TRANSCODER_D, > POWER_DOMAIN_TRANSCODER_EDP, > POWER_DOMAIN_TRANSCODER_EDP_VDSC, > + POWER_DOMAIN_TRANSCODER_A_VDSC, > POWER_DOMAIN_TRANSCODER_DSI_A, > POWER_DOMAIN_TRANSCODER_DSI_C, > POWER_DOMAIN_PORT_DDI_A_LANES, > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c > index ffec807b8960..0c75b408d6ba 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, > enum intel_display_power_domain > intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > { > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.state->dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > /* > - * On ICL VDSC/joining for eDP transcoder uses a separate power well PW2 > - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. > + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate power well > + * PW2. This requires > + * POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power > + * domain. > * For any other transcoder, VDSC/joining uses the power well associated > * with the pipe/transcoder in use. Hence another reference on the > * transcoder power domain will suffice. > */ > - if (cpu_transcoder == TRANSCODER_EDP) > + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == TRANSCODER_A) > + return POWER_DOMAIN_TRANSCODER_A_VDSC; > + else if (cpu_transcoder == TRANSCODER_EDP) > return POWER_DOMAIN_TRANSCODER_EDP_VDSC; > else > return POWER_DOMAIN_TRANSCODER(cpu_transcoder); > -- > 2.21.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, 2019-06-25 at 10:54 -0700, Lucas De Marchi wrote: > From: José Roberto de Souza <jose.souza@intel.com> > > On TGL the special EDP transcoder is gone and it should be handled by > transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > distinction clear and update vdsc code path. > > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- > 3 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > b/drivers/gpu/drm/i915/display/intel_display_power.c > index 0c7d4a363deb..15582841fefc 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct > drm_i915_private *i915, > return "TRANSCODER_EDP"; > case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > return "TRANSCODER_EDP_VDSC"; > + case POWER_DOMAIN_TRANSCODER_A_VDSC: > + return "TRANSCODER_A_VDSC"; > case POWER_DOMAIN_TRANSCODER_DSI_A: > return "TRANSCODER_DSI_A"; > case POWER_DOMAIN_TRANSCODER_DSI_C: > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > b/drivers/gpu/drm/i915/display/intel_display_power.h > index 79262a5bceb4..7761b493608a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -29,6 +29,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_TRANSCODER_D, > POWER_DOMAIN_TRANSCODER_EDP, > POWER_DOMAIN_TRANSCODER_EDP_VDSC, > + POWER_DOMAIN_TRANSCODER_A_VDSC, > POWER_DOMAIN_TRANSCODER_DSI_A, > POWER_DOMAIN_TRANSCODER_DSI_C, > POWER_DOMAIN_PORT_DDI_A_LANES, > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c > b/drivers/gpu/drm/i915/display/intel_vdsc.c > index ffec807b8960..0c75b408d6ba 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp > *intel_dp, > enum intel_display_power_domain > intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > { > + struct drm_i915_private *dev_priv = to_i915(crtc_state- > >base.state->dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > /* > - * On ICL VDSC/joining for eDP transcoder uses a separate power > well PW2 > - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. > + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate > power well > + * PW2. This requires > + * > POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power > + * domain. > * For any other transcoder, VDSC/joining uses the power well > associated > * with the pipe/transcoder in use. Hence another reference on > the > * transcoder power domain will suffice. > */ > - if (cpu_transcoder == TRANSCODER_EDP) > + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == > TRANSCODER_A) > + return POWER_DOMAIN_TRANSCODER_A_VDSC; > + else if (cpu_transcoder == TRANSCODER_EDP) > return POWER_DOMAIN_TRANSCODER_EDP_VDSC; > else > return POWER_DOMAIN_TRANSCODER(cpu_transcoder); This is missing the change adding POWER_DOMAIN_TRANSCODER_EDP_VDSC to TGL_PW_2_POWER_DOMAINS.
On Thu, 2019-06-27 at 12:28 -0700, José Roberto de Souza wrote: > On Tue, 2019-06-25 at 10:54 -0700, Lucas De Marchi wrote: > > From: José Roberto de Souza <jose.souza@intel.com> > > > > On TGL the special EDP transcoder is gone and it should be handled > > by > > transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > > distinction clear and update vdsc code path. > > > > Cc: Imre Deak <imre.deak@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > > drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++ > > --- > > 3 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 0c7d4a363deb..15582841fefc 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct > > drm_i915_private *i915, > > return "TRANSCODER_EDP"; > > case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > > return "TRANSCODER_EDP_VDSC"; > > + case POWER_DOMAIN_TRANSCODER_A_VDSC: > > + return "TRANSCODER_A_VDSC"; > > case POWER_DOMAIN_TRANSCODER_DSI_A: > > return "TRANSCODER_DSI_A"; > > case POWER_DOMAIN_TRANSCODER_DSI_C: > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > b/drivers/gpu/drm/i915/display/intel_display_power.h > > index 79262a5bceb4..7761b493608a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > @@ -29,6 +29,7 @@ enum intel_display_power_domain { > > POWER_DOMAIN_TRANSCODER_D, > > POWER_DOMAIN_TRANSCODER_EDP, > > POWER_DOMAIN_TRANSCODER_EDP_VDSC, > > + POWER_DOMAIN_TRANSCODER_A_VDSC, > > POWER_DOMAIN_TRANSCODER_DSI_A, > > POWER_DOMAIN_TRANSCODER_DSI_C, > > POWER_DOMAIN_PORT_DDI_A_LANES, > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c > > b/drivers/gpu/drm/i915/display/intel_vdsc.c > > index ffec807b8960..0c75b408d6ba 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > > @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct > > intel_dp > > *intel_dp, > > enum intel_display_power_domain > > intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > > { > > + struct drm_i915_private *dev_priv = to_i915(crtc_state- > > > base.state->dev); > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > > > /* > > - * On ICL VDSC/joining for eDP transcoder uses a separate power > > well PW2 > > - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. > > + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate > > power well > > + * PW2. This requires > > + * > > POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC > > power > > + * domain. > > * For any other transcoder, VDSC/joining uses the power well > > associated > > * with the pipe/transcoder in use. Hence another reference on > > the > > * transcoder power domain will suffice. > > */ > > - if (cpu_transcoder == TRANSCODER_EDP) > > + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == > > TRANSCODER_A) > > + return POWER_DOMAIN_TRANSCODER_A_VDSC; > > + else if (cpu_transcoder == TRANSCODER_EDP) > > return POWER_DOMAIN_TRANSCODER_EDP_VDSC; > > else > > return POWER_DOMAIN_TRANSCODER(cpu_transcoder); > > This is missing the change adding POWER_DOMAIN_TRANSCODER_EDP_VDSC to > TGL_PW_2_POWER_DOMAINS. * TRANSCODER_A_VDSC * Missing the change adding TRANSCODER_A_VDSC to TGL_PW_2_POWER_DOMAINS
On Thu, Jun 27, 2019 at 07:28:15PM +0000, Souza, Jose wrote: > On Tue, 2019-06-25 at 10:54 -0700, Lucas De Marchi wrote: > > From: José Roberto de Souza <jose.souza@intel.com> > > > > On TGL the special EDP transcoder is gone and it should be handled by > > transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > > distinction clear and update vdsc code path. > > > > Cc: Imre Deak <imre.deak@intel.com> > > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > --- > > drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > > drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- > > 3 files changed, 11 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c > > b/drivers/gpu/drm/i915/display/intel_display_power.c > > index 0c7d4a363deb..15582841fefc 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct > > drm_i915_private *i915, > > return "TRANSCODER_EDP"; > > case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > > return "TRANSCODER_EDP_VDSC"; > > + case POWER_DOMAIN_TRANSCODER_A_VDSC: > > + return "TRANSCODER_A_VDSC"; > > case POWER_DOMAIN_TRANSCODER_DSI_A: > > return "TRANSCODER_DSI_A"; > > case POWER_DOMAIN_TRANSCODER_DSI_C: > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h > > b/drivers/gpu/drm/i915/display/intel_display_power.h > > index 79262a5bceb4..7761b493608a 100644 > > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > @@ -29,6 +29,7 @@ enum intel_display_power_domain { > > POWER_DOMAIN_TRANSCODER_D, > > POWER_DOMAIN_TRANSCODER_EDP, > > POWER_DOMAIN_TRANSCODER_EDP_VDSC, > > + POWER_DOMAIN_TRANSCODER_A_VDSC, > > POWER_DOMAIN_TRANSCODER_DSI_A, > > POWER_DOMAIN_TRANSCODER_DSI_C, > > POWER_DOMAIN_PORT_DDI_A_LANES, > > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c > > b/drivers/gpu/drm/i915/display/intel_vdsc.c > > index ffec807b8960..0c75b408d6ba 100644 > > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > > @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp > > *intel_dp, > > enum intel_display_power_domain > > intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > > { > > + struct drm_i915_private *dev_priv = to_i915(crtc_state- > > >base.state->dev); > > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > > > /* > > - * On ICL VDSC/joining for eDP transcoder uses a separate power > > well PW2 > > - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. > > + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate > > power well > > + * PW2. This requires > > + * > > POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power > > + * domain. > > * For any other transcoder, VDSC/joining uses the power well > > associated > > * with the pipe/transcoder in use. Hence another reference on > > the > > * transcoder power domain will suffice. > > */ > > - if (cpu_transcoder == TRANSCODER_EDP) > > + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == > > TRANSCODER_A) > > + return POWER_DOMAIN_TRANSCODER_A_VDSC; > > + else if (cpu_transcoder == TRANSCODER_EDP) > > return POWER_DOMAIN_TRANSCODER_EDP_VDSC; > > else > > return POWER_DOMAIN_TRANSCODER(cpu_transcoder); > > > This is missing the change adding POWER_DOMAIN_TRANSCODER_EDP_VDSC to > TGL_PW_2_POWER_DOMAINS. You mean POWER_DOMAIN_TRANSCODER_A_VDSC? Manasi > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Tue, Jun 25, 2019 at 10:54:21AM -0700, Lucas De Marchi wrote: > From: José Roberto de Souza <jose.souza@intel.com> > > On TGL the special EDP transcoder is gone and it should be handled by > transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > distinction clear and update vdsc code path. > > Cc: Imre Deak <imre.deak@intel.com> > Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > --- > drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- > 3 files changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > index 0c7d4a363deb..15582841fefc 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.c > +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, > return "TRANSCODER_EDP"; > case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > return "TRANSCODER_EDP_VDSC"; > + case POWER_DOMAIN_TRANSCODER_A_VDSC: > + return "TRANSCODER_A_VDSC"; > case POWER_DOMAIN_TRANSCODER_DSI_A: > return "TRANSCODER_DSI_A"; > case POWER_DOMAIN_TRANSCODER_DSI_C: > diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > index 79262a5bceb4..7761b493608a 100644 > --- a/drivers/gpu/drm/i915/display/intel_display_power.h > +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > @@ -29,6 +29,7 @@ enum intel_display_power_domain { > POWER_DOMAIN_TRANSCODER_D, > POWER_DOMAIN_TRANSCODER_EDP, > POWER_DOMAIN_TRANSCODER_EDP_VDSC, > + POWER_DOMAIN_TRANSCODER_A_VDSC, Two power domains for essentially the same thing seems a bit wasteful. > POWER_DOMAIN_TRANSCODER_DSI_A, > POWER_DOMAIN_TRANSCODER_DSI_C, > POWER_DOMAIN_PORT_DDI_A_LANES, > diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c > index ffec807b8960..0c75b408d6ba 100644 > --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, > enum intel_display_power_domain > intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > { > + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.state->dev); > enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > > /* > - * On ICL VDSC/joining for eDP transcoder uses a separate power well PW2 > - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. > + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate power well > + * PW2. This requires > + * POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power > + * domain. > * For any other transcoder, VDSC/joining uses the power well associated > * with the pipe/transcoder in use. Hence another reference on the > * transcoder power domain will suffice. > */ > - if (cpu_transcoder == TRANSCODER_EDP) > + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == TRANSCODER_A) > + return POWER_DOMAIN_TRANSCODER_A_VDSC; > + else if (cpu_transcoder == TRANSCODER_EDP) > return POWER_DOMAIN_TRANSCODER_EDP_VDSC; > else > return POWER_DOMAIN_TRANSCODER(cpu_transcoder); > -- > 2.21.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, Jun 28, 2019 at 12:55:17PM +0300, Ville Syrjälä wrote: >On Tue, Jun 25, 2019 at 10:54:21AM -0700, Lucas De Marchi wrote: >> From: José Roberto de Souza <jose.souza@intel.com> >> >> On TGL the special EDP transcoder is gone and it should be handled by >> transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this >> distinction clear and update vdsc code path. >> >> Cc: Imre Deak <imre.deak@intel.com> >> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ >> drivers/gpu/drm/i915/display/intel_display_power.h | 1 + >> drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- >> 3 files changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c >> index 0c7d4a363deb..15582841fefc 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c >> @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, >> return "TRANSCODER_EDP"; >> case POWER_DOMAIN_TRANSCODER_EDP_VDSC: >> return "TRANSCODER_EDP_VDSC"; >> + case POWER_DOMAIN_TRANSCODER_A_VDSC: >> + return "TRANSCODER_A_VDSC"; >> case POWER_DOMAIN_TRANSCODER_DSI_A: >> return "TRANSCODER_DSI_A"; >> case POWER_DOMAIN_TRANSCODER_DSI_C: >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h >> index 79262a5bceb4..7761b493608a 100644 >> --- a/drivers/gpu/drm/i915/display/intel_display_power.h >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h >> @@ -29,6 +29,7 @@ enum intel_display_power_domain { >> POWER_DOMAIN_TRANSCODER_D, >> POWER_DOMAIN_TRANSCODER_EDP, >> POWER_DOMAIN_TRANSCODER_EDP_VDSC, >> + POWER_DOMAIN_TRANSCODER_A_VDSC, > >Two power domains for essentially the same thing seems a bit wasteful. just reuse the name then? and on gen12+ check for TRANSCODER_A like below? Lucas De Marchi > >> POWER_DOMAIN_TRANSCODER_DSI_A, >> POWER_DOMAIN_TRANSCODER_DSI_C, >> POWER_DOMAIN_PORT_DDI_A_LANES, >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c >> index ffec807b8960..0c75b408d6ba 100644 >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c >> @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, >> enum intel_display_power_domain >> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) >> { >> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.state->dev); >> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; >> >> /* >> - * On ICL VDSC/joining for eDP transcoder uses a separate power well PW2 >> - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. >> + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate power well >> + * PW2. This requires >> + * POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power >> + * domain. >> * For any other transcoder, VDSC/joining uses the power well associated >> * with the pipe/transcoder in use. Hence another reference on the >> * transcoder power domain will suffice. >> */ >> - if (cpu_transcoder == TRANSCODER_EDP) >> + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == TRANSCODER_A) >> + return POWER_DOMAIN_TRANSCODER_A_VDSC; >> + else if (cpu_transcoder == TRANSCODER_EDP) >> return POWER_DOMAIN_TRANSCODER_EDP_VDSC; >> else >> return POWER_DOMAIN_TRANSCODER(cpu_transcoder); >> -- >> 2.21.0 >> >> _______________________________________________ >> Intel-gfx mailing list >> Intel-gfx@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > >-- >Ville Syrjälä >Intel
On Fri, Jun 28, 2019 at 09:31:04AM -0700, Lucas De Marchi wrote: > On Fri, Jun 28, 2019 at 12:55:17PM +0300, Ville Syrjälä wrote: > >On Tue, Jun 25, 2019 at 10:54:21AM -0700, Lucas De Marchi wrote: > >> From: José Roberto de Souza <jose.souza@intel.com> > >> > >> On TGL the special EDP transcoder is gone and it should be handled by > >> transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > >> distinction clear and update vdsc code path. > >> > >> Cc: Imre Deak <imre.deak@intel.com> > >> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > >> drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > >> drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- > >> 3 files changed, 11 insertions(+), 3 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > >> index 0c7d4a363deb..15582841fefc 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c > >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > >> @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, > >> return "TRANSCODER_EDP"; > >> case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > >> return "TRANSCODER_EDP_VDSC"; > >> + case POWER_DOMAIN_TRANSCODER_A_VDSC: > >> + return "TRANSCODER_A_VDSC"; > >> case POWER_DOMAIN_TRANSCODER_DSI_A: > >> return "TRANSCODER_DSI_A"; > >> case POWER_DOMAIN_TRANSCODER_DSI_C: > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > >> index 79262a5bceb4..7761b493608a 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_display_power.h > >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > >> @@ -29,6 +29,7 @@ enum intel_display_power_domain { > >> POWER_DOMAIN_TRANSCODER_D, > >> POWER_DOMAIN_TRANSCODER_EDP, > >> POWER_DOMAIN_TRANSCODER_EDP_VDSC, > >> + POWER_DOMAIN_TRANSCODER_A_VDSC, > > > >Two power domains for essentially the same thing seems a bit wasteful. > > just reuse the name then? > > and on gen12+ check for TRANSCODER_A like below? That was my initial idea yes. In theory it would be nice to have fully abstracted power domains but that would lead to a lot of bits getting used. I suspect we might have to switch to using the kernel bitmask stuff in that case. Not sure how many bits we have free ATM. > > Lucas De Marchi > > > > >> POWER_DOMAIN_TRANSCODER_DSI_A, > >> POWER_DOMAIN_TRANSCODER_DSI_C, > >> POWER_DOMAIN_PORT_DDI_A_LANES, > >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c > >> index ffec807b8960..0c75b408d6ba 100644 > >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c > >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c > >> @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, > >> enum intel_display_power_domain > >> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) > >> { > >> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.state->dev); > >> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; > >> > >> /* > >> - * On ICL VDSC/joining for eDP transcoder uses a separate power well PW2 > >> - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. > >> + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate power well > >> + * PW2. This requires > >> + * POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power > >> + * domain. > >> * For any other transcoder, VDSC/joining uses the power well associated > >> * with the pipe/transcoder in use. Hence another reference on the > >> * transcoder power domain will suffice. > >> */ > >> - if (cpu_transcoder == TRANSCODER_EDP) > >> + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == TRANSCODER_A) > >> + return POWER_DOMAIN_TRANSCODER_A_VDSC; > >> + else if (cpu_transcoder == TRANSCODER_EDP) > >> return POWER_DOMAIN_TRANSCODER_EDP_VDSC; > >> else > >> return POWER_DOMAIN_TRANSCODER(cpu_transcoder); > >> -- > >> 2.21.0 > >> > >> _______________________________________________ > >> Intel-gfx mailing list > >> Intel-gfx@lists.freedesktop.org > >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > >-- > >Ville Syrjälä > >Intel
On Mon, Jul 01, 2019 at 08:32:59PM +0300, Ville Syrjälä wrote: > On Fri, Jun 28, 2019 at 09:31:04AM -0700, Lucas De Marchi wrote: > > On Fri, Jun 28, 2019 at 12:55:17PM +0300, Ville Syrjälä wrote: > > >On Tue, Jun 25, 2019 at 10:54:21AM -0700, Lucas De Marchi wrote: > > >> From: José Roberto de Souza <jose.souza@intel.com> > > >> > > >> On TGL the special EDP transcoder is gone and it should be handled by > > >> transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this > > >> distinction clear and update vdsc code path. > > >> > > >> Cc: Imre Deak <imre.deak@intel.com> > > >> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> > > >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> > > >> --- > > >> drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ > > >> drivers/gpu/drm/i915/display/intel_display_power.h | 1 + > > >> drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- > > >> 3 files changed, 11 insertions(+), 3 deletions(-) > > >> > > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c > > >> index 0c7d4a363deb..15582841fefc 100644 > > >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c > > >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c > > >> @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, > > >> return "TRANSCODER_EDP"; > > >> case POWER_DOMAIN_TRANSCODER_EDP_VDSC: > > >> return "TRANSCODER_EDP_VDSC"; > > >> + case POWER_DOMAIN_TRANSCODER_A_VDSC: > > >> + return "TRANSCODER_A_VDSC"; > > >> case POWER_DOMAIN_TRANSCODER_DSI_A: > > >> return "TRANSCODER_DSI_A"; > > >> case POWER_DOMAIN_TRANSCODER_DSI_C: > > >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h > > >> index 79262a5bceb4..7761b493608a 100644 > > >> --- a/drivers/gpu/drm/i915/display/intel_display_power.h > > >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h > > >> @@ -29,6 +29,7 @@ enum intel_display_power_domain { > > >> POWER_DOMAIN_TRANSCODER_D, > > >> POWER_DOMAIN_TRANSCODER_EDP, > > >> POWER_DOMAIN_TRANSCODER_EDP_VDSC, > > >> + POWER_DOMAIN_TRANSCODER_A_VDSC, > > > > > >Two power domains for essentially the same thing seems a bit wasteful. > > > > just reuse the name then? > > > > and on gen12+ check for TRANSCODER_A like below? > > That was my initial idea yes. In theory it would be nice to have fully > abstracted power domains but that would lead to a lot of bits getting > used. I suspect we might have to switch to using the kernel bitmask > stuff in that case. Not sure how many bits we have free ATM. I've also pondered about shaving off a few bits by special casing the HSW panel fitter stuff in a similar fashion.
On Mon, Jul 01, 2019 at 08:32:59PM +0300, Ville Syrjälä wrote: >On Fri, Jun 28, 2019 at 09:31:04AM -0700, Lucas De Marchi wrote: >> On Fri, Jun 28, 2019 at 12:55:17PM +0300, Ville Syrjälä wrote: >> >On Tue, Jun 25, 2019 at 10:54:21AM -0700, Lucas De Marchi wrote: >> >> From: José Roberto de Souza <jose.souza@intel.com> >> >> >> >> On TGL the special EDP transcoder is gone and it should be handled by >> >> transcoder A. Add POWER_DOMAIN_TRANSCODER_A_VDSC to make this >> >> distinction clear and update vdsc code path. >> >> >> >> Cc: Imre Deak <imre.deak@intel.com> >> >> Signed-off-by: José Roberto de Souza <jose.souza@intel.com> >> >> Signed-off-by: Lucas De Marchi <lucas.demarchi@intel.com> >> >> --- >> >> drivers/gpu/drm/i915/display/intel_display_power.c | 2 ++ >> >> drivers/gpu/drm/i915/display/intel_display_power.h | 1 + >> >> drivers/gpu/drm/i915/display/intel_vdsc.c | 11 ++++++++--- >> >> 3 files changed, 11 insertions(+), 3 deletions(-) >> >> >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c >> >> index 0c7d4a363deb..15582841fefc 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_display_power.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.c >> >> @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, >> >> return "TRANSCODER_EDP"; >> >> case POWER_DOMAIN_TRANSCODER_EDP_VDSC: >> >> return "TRANSCODER_EDP_VDSC"; >> >> + case POWER_DOMAIN_TRANSCODER_A_VDSC: >> >> + return "TRANSCODER_A_VDSC"; >> >> case POWER_DOMAIN_TRANSCODER_DSI_A: >> >> return "TRANSCODER_DSI_A"; >> >> case POWER_DOMAIN_TRANSCODER_DSI_C: >> >> diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h >> >> index 79262a5bceb4..7761b493608a 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_display_power.h >> >> +++ b/drivers/gpu/drm/i915/display/intel_display_power.h >> >> @@ -29,6 +29,7 @@ enum intel_display_power_domain { >> >> POWER_DOMAIN_TRANSCODER_D, >> >> POWER_DOMAIN_TRANSCODER_EDP, >> >> POWER_DOMAIN_TRANSCODER_EDP_VDSC, >> >> + POWER_DOMAIN_TRANSCODER_A_VDSC, >> > >> >Two power domains for essentially the same thing seems a bit wasteful. >> >> just reuse the name then? >> >> and on gen12+ check for TRANSCODER_A like below? > >That was my initial idea yes. In theory it would be nice to have fully >abstracted power domains but that would lead to a lot of bits getting >used. I suspect we might have to switch to using the kernel bitmask >stuff in that case. Not sure how many bits we have free ATM. we are currently using 45 and with TGL we will jump to 60. 4 more until we have to swap out that logic in favor of bitmap.h Lucas De Marchi > >> >> Lucas De Marchi >> >> > >> >> POWER_DOMAIN_TRANSCODER_DSI_A, >> >> POWER_DOMAIN_TRANSCODER_DSI_C, >> >> POWER_DOMAIN_PORT_DDI_A_LANES, >> >> diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c >> >> index ffec807b8960..0c75b408d6ba 100644 >> >> --- a/drivers/gpu/drm/i915/display/intel_vdsc.c >> >> +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c >> >> @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, >> >> enum intel_display_power_domain >> >> intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) >> >> { >> >> + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.state->dev); >> >> enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; >> >> >> >> /* >> >> - * On ICL VDSC/joining for eDP transcoder uses a separate power well PW2 >> >> - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. >> >> + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate power well >> >> + * PW2. This requires >> >> + * POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power >> >> + * domain. >> >> * For any other transcoder, VDSC/joining uses the power well associated >> >> * with the pipe/transcoder in use. Hence another reference on the >> >> * transcoder power domain will suffice. >> >> */ >> >> - if (cpu_transcoder == TRANSCODER_EDP) >> >> + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == TRANSCODER_A) >> >> + return POWER_DOMAIN_TRANSCODER_A_VDSC; >> >> + else if (cpu_transcoder == TRANSCODER_EDP) >> >> return POWER_DOMAIN_TRANSCODER_EDP_VDSC; >> >> else >> >> return POWER_DOMAIN_TRANSCODER(cpu_transcoder); >> >> -- >> >> 2.21.0 >> >> >> >> _______________________________________________ >> >> Intel-gfx mailing list >> >> Intel-gfx@lists.freedesktop.org >> >> https://lists.freedesktop.org/mailman/listinfo/intel-gfx >> > >> >-- >> >Ville Syrjälä >> >Intel > >-- >Ville Syrjälä >Intel
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c index 0c7d4a363deb..15582841fefc 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.c +++ b/drivers/gpu/drm/i915/display/intel_display_power.c @@ -58,6 +58,8 @@ intel_display_power_domain_str(struct drm_i915_private *i915, return "TRANSCODER_EDP"; case POWER_DOMAIN_TRANSCODER_EDP_VDSC: return "TRANSCODER_EDP_VDSC"; + case POWER_DOMAIN_TRANSCODER_A_VDSC: + return "TRANSCODER_A_VDSC"; case POWER_DOMAIN_TRANSCODER_DSI_A: return "TRANSCODER_DSI_A"; case POWER_DOMAIN_TRANSCODER_DSI_C: diff --git a/drivers/gpu/drm/i915/display/intel_display_power.h b/drivers/gpu/drm/i915/display/intel_display_power.h index 79262a5bceb4..7761b493608a 100644 --- a/drivers/gpu/drm/i915/display/intel_display_power.h +++ b/drivers/gpu/drm/i915/display/intel_display_power.h @@ -29,6 +29,7 @@ enum intel_display_power_domain { POWER_DOMAIN_TRANSCODER_D, POWER_DOMAIN_TRANSCODER_EDP, POWER_DOMAIN_TRANSCODER_EDP_VDSC, + POWER_DOMAIN_TRANSCODER_A_VDSC, POWER_DOMAIN_TRANSCODER_DSI_A, POWER_DOMAIN_TRANSCODER_DSI_C, POWER_DOMAIN_PORT_DDI_A_LANES, diff --git a/drivers/gpu/drm/i915/display/intel_vdsc.c b/drivers/gpu/drm/i915/display/intel_vdsc.c index ffec807b8960..0c75b408d6ba 100644 --- a/drivers/gpu/drm/i915/display/intel_vdsc.c +++ b/drivers/gpu/drm/i915/display/intel_vdsc.c @@ -459,16 +459,21 @@ int intel_dp_compute_dsc_params(struct intel_dp *intel_dp, enum intel_display_power_domain intel_dsc_power_domain(const struct intel_crtc_state *crtc_state) { + struct drm_i915_private *dev_priv = to_i915(crtc_state->base.state->dev); enum transcoder cpu_transcoder = crtc_state->cpu_transcoder; /* - * On ICL VDSC/joining for eDP transcoder uses a separate power well PW2 - * This requires POWER_DOMAIN_TRANSCODER_EDP_VDSC power domain. + * On ICL+ VDSC/joining for eDP/A transcoder uses a separate power well + * PW2. This requires + * POWER_DOMAIN_TRANSCODER_EDP_VDSC/POWER_DOMAIN_TRANSCODER_A_VDSC power + * domain. * For any other transcoder, VDSC/joining uses the power well associated * with the pipe/transcoder in use. Hence another reference on the * transcoder power domain will suffice. */ - if (cpu_transcoder == TRANSCODER_EDP) + if (INTEL_GEN(dev_priv) >= 12 && cpu_transcoder == TRANSCODER_A) + return POWER_DOMAIN_TRANSCODER_A_VDSC; + else if (cpu_transcoder == TRANSCODER_EDP) return POWER_DOMAIN_TRANSCODER_EDP_VDSC; else return POWER_DOMAIN_TRANSCODER(cpu_transcoder);