Re: [Intel-gfx] [PATCH 1/2] drm/i915: Get transcoder power domain before reading its register
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 > > Signed-off-by: José Roberto de Souza > > --- > > 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 > > 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/2] drm/i915: Get transcoder power domain before reading its register
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 Signed-off-by: José Roberto de Souza --- 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 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 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Get transcoder power domain before reading its register
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 Signed-off-by: José Roberto de Souza --- 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)); + 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@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx