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

[Lucas De Marchi]

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,
>+								   POWER_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?

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
>