[Intel-gfx] [PATCH 1/2] drm/i915: Get transcoder power domain before reading its register

[Souza, Jose]

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 at intel.com>
> > Signed-off-by: José Roberto de Souza <jose.souza at intel.com>
> > ---
> > 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 at intel.com>
> 
> 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
> >