On Thu, 2019-08-01 at 17:41 -0700, Lucas De Marchi wrote:
> On Thu, Aug 01, 2019 at 04:28:11PM -0700, Jose Souza wrote:
> > When getting the pipes attached to encoder if it is not a eDP
> > encoder
> > it iterates over all pipes and read a transcoder register.
> > But it should not read a transcoder register before get its power
> > domain.
> >
> > It was not a issue in gens older than 12 because if it only had
> > port A connected it would be attached to EDP and it would skip all
> > the transcoders readout, if it had more than one port connected,
> > pipe B would cause PG3 to be on and it contains all other
> > transcoders.
> >
> > But on gen 12 there is no EDP transcoder so it is always iterating
> > over all pipes and if only one sink is connected, PG3 is kept off
> > and reading other transcoders registers would cause a
> > unclaimed read warning.
> >
> > So here getting the power domain of the transcoder only if it is
> > enabled, otherwise it is not connected to the DDI.
> >
> > Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> > Signed-off-by: José Roberto de Souza <jose.souza@xxxxxxxxx>
> > ---
> > drivers/gpu/drm/i915/display/intel_ddi.c | 8 ++++++++
> > 1 file changed, 8 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> > b/drivers/gpu/drm/i915/display/intel_ddi.c
> > index fb58845020dc..660bb001be35 100644
> > --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> > @@ -2015,6 +2015,12 @@ static void
> > intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
> > for_each_pipe(dev_priv, p) {
> > enum transcoder cpu_transcoder = (enum transcoder)p;
> > unsigned int port_mask, ddi_select;
> > + intel_wakeref_t trans_wakeref;
> > +
> > + trans_wakeref =
> > intel_display_power_get_if_enabled(dev_priv,
> > + POWE
> > R_DOMAIN_TRANSCODER(cpu_transcoder));
>
> And on Tiger Lake POWER_DOMAIN_TRANSCODER_B,
> POWER_DOMAIN_TRANSCODER_C
> and POWER_DOMAIN_TRANSCODER_D are on PW3. POWER_DOMAIN_TRANSCODER_A
> is
> on PW1.
>
> Looks correct.
>
> Reviewed-by: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
>
> Are the warnings now fixed?
With only eDP connected yes, we still have a few with eDP+HDMI.
>
> thanks
> Lucas De Marchi
>
>
>
>
> > + if (!trans_wakeref)
> > + continue;
> >
> > if (INTEL_GEN(dev_priv) >= 12) {
> > port_mask = TGL_TRANS_DDI_PORT_MASK;
> > @@ -2025,6 +2031,8 @@ static void
> > intel_ddi_get_encoder_pipes(struct intel_encoder *encoder,
> > }
> >
> > tmp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> > + intel_display_power_put(dev_priv,
> > POWER_DOMAIN_TRANSCODER(cpu_transcoder),
> > + trans_wakeref);
> >
> > if ((tmp & port_mask) != ddi_select)
> > continue;
> > --
> > 2.22.0
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
