On Mon, Nov 19, 2018 at 10:46:37PM +0200, Imre Deak wrote:
> Depending on the transcoder enum values to translate from transcoder
> to EDP PSR flags can easily break if we add a new transcoder. So remove
> the dependency by using an explicit mapping.
>
> While at it also add a WARN for unexpected trancoders.
>
> v2:
> - Simplify things by defining flag shift values instead of indices.
> - s/trans/cpu_transcoder/ (Ville)
>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Cc: Mika Kahola <mika.kahola@xxxxxxxxx>
> Signed-off-by: Imre Deak <imre.deak@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_reg.h | 10 +++++---
> drivers/gpu/drm/i915/intel_psr.c | 55 +++++++++++++++++++++++++++-------------
> 2 files changed, 44 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index edb58af1e903..f80b16e3ff33 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4150,9 +4150,13 @@ enum {
> /* Bspec claims those aren't shifted but stay at 0x64800 */
> #define EDP_PSR_IMR _MMIO(0x64834)
> #define EDP_PSR_IIR _MMIO(0x64838)
> -#define EDP_PSR_ERROR(trans) (1 << (((trans) * 8 + 10) & 31))
> -#define EDP_PSR_POST_EXIT(trans) (1 << (((trans) * 8 + 9) & 31))
> -#define EDP_PSR_PRE_ENTRY(trans) (1 << (((trans) * 8 + 8) & 31))
> +#define EDP_PSR_ERROR(shift) (4 << (shift))
> +#define EDP_PSR_POST_EXIT(shift) (2 << (shift))
> +#define EDP_PSR_PRE_ENTRY(shift) (1 << (shift))
The 1,2,4 suggest that these are values for the same bitfield, which
they are not. So I would prefer 1<<((shift)+n) etc. instead.
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> +#define EDP_PSR_TRANSCODER_C_SHIFT 24
> +#define EDP_PSR_TRANSCODER_B_SHIFT 16
> +#define EDP_PSR_TRANSCODER_A_SHIFT 8
> +#define EDP_PSR_TRANSCODER_EDP_SHIFT 0
>
> #define EDP_PSR_AUX_CTL _MMIO(dev_priv->psr_mmio_base + 0x10)
> #define EDP_PSR_AUX_CTL_TIME_OUT_MASK (3 << 26)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 48df16a02fac..26292961d693 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -83,25 +83,42 @@ static bool intel_psr2_enabled(struct drm_i915_private *dev_priv,
> }
> }
>
> +static int edp_psr_shift(enum transcoder cpu_transcoder)
> +{
> + switch (cpu_transcoder) {
> + case TRANSCODER_A:
> + return EDP_PSR_TRANSCODER_A_SHIFT;
> + case TRANSCODER_B:
> + return EDP_PSR_TRANSCODER_B_SHIFT;
> + case TRANSCODER_C:
> + return EDP_PSR_TRANSCODER_C_SHIFT;
> + default:
> + MISSING_CASE(cpu_transcoder);
> + /* fallthrough */
> + case TRANSCODER_EDP:
> + return EDP_PSR_TRANSCODER_EDP_SHIFT;
> + }
> +}
> +
> void intel_psr_irq_control(struct drm_i915_private *dev_priv, u32 debug)
> {
> u32 debug_mask, mask;
> + enum transcoder cpu_transcoder;
> + u32 transcoders = BIT(TRANSCODER_EDP);
>
> - mask = EDP_PSR_ERROR(TRANSCODER_EDP);
> - debug_mask = EDP_PSR_POST_EXIT(TRANSCODER_EDP) |
> - EDP_PSR_PRE_ENTRY(TRANSCODER_EDP);
> -
> - if (INTEL_GEN(dev_priv) >= 8) {
> - mask |= EDP_PSR_ERROR(TRANSCODER_A) |
> - EDP_PSR_ERROR(TRANSCODER_B) |
> - EDP_PSR_ERROR(TRANSCODER_C);
> -
> - debug_mask |= EDP_PSR_POST_EXIT(TRANSCODER_A) |
> - EDP_PSR_PRE_ENTRY(TRANSCODER_A) |
> - EDP_PSR_POST_EXIT(TRANSCODER_B) |
> - EDP_PSR_PRE_ENTRY(TRANSCODER_B) |
> - EDP_PSR_POST_EXIT(TRANSCODER_C) |
> - EDP_PSR_PRE_ENTRY(TRANSCODER_C);
> + if (INTEL_GEN(dev_priv) >= 8)
> + transcoders |= BIT(TRANSCODER_A) |
> + BIT(TRANSCODER_B) |
> + BIT(TRANSCODER_C);
> +
> + debug_mask = 0;
> + mask = 0;
> + for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> + int shift = edp_psr_shift(cpu_transcoder);
> +
> + mask |= EDP_PSR_ERROR(shift);
> + debug_mask |= EDP_PSR_POST_EXIT(shift) |
> + EDP_PSR_PRE_ENTRY(shift);
> }
>
> if (debug & I915_PSR_DEBUG_IRQ)
> @@ -159,18 +176,20 @@ void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
> BIT(TRANSCODER_C);
>
> for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> + int shift = edp_psr_shift(cpu_transcoder);
> +
> /* FIXME: Exit PSR and link train manually when this happens. */
> - if (psr_iir & EDP_PSR_ERROR(cpu_transcoder))
> + if (psr_iir & EDP_PSR_ERROR(shift))
> DRM_DEBUG_KMS("[transcoder %s] PSR aux error\n",
> transcoder_name(cpu_transcoder));
>
> - if (psr_iir & EDP_PSR_PRE_ENTRY(cpu_transcoder)) {
> + if (psr_iir & EDP_PSR_PRE_ENTRY(shift)) {
> dev_priv->psr.last_entry_attempt = time_ns;
> DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
> transcoder_name(cpu_transcoder));
> }
>
> - if (psr_iir & EDP_PSR_POST_EXIT(cpu_transcoder)) {
> + if (psr_iir & EDP_PSR_POST_EXIT(shift)) {
> dev_priv->psr.last_exit = time_ns;
> DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> transcoder_name(cpu_transcoder));
> --
> 2.13.2
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
