Re: [PATCH 1/8] drm/i915/psr: Remove partial PSR support on multiple transcoders

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Thu, Mar 21, 2019 at 11:01:36AM -0700, José Roberto de Souza wrote:
> PSR is only support in eDP transcoder and there is only one instance
> of it, so lets drop all of this code.

One instance? Are you talking about PSR2?

Also the EDP transcoder is already doomed is it not?

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  17 +---
>  drivers/gpu/drm/i915/intel_psr.c | 147 ++++++++-----------------------
>  2 files changed, 42 insertions(+), 122 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index b46910453e61..d0ca6ef6d630 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4277,13 +4277,9 @@ 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(shift)			(1 << ((shift) + 2))
> -#define   EDP_PSR_POST_EXIT(shift)		(1 << ((shift) + 1))
> -#define   EDP_PSR_PRE_ENTRY(shift)		(1 << (shift))
> -#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_ERROR				(1 << 2)
> +#define   EDP_PSR_POST_EXIT			(1 << 1)
> +#define   EDP_PSR_PRE_ENTRY			(1 << 0)
>  
>  #define EDP_PSR_AUX_CTL				_MMIO(dev_priv->psr_mmio_base + 0x10)
>  #define   EDP_PSR_AUX_CTL_TIME_OUT_MASK		(3 << 26)
> @@ -4348,12 +4344,7 @@ enum {
>  #define   EDP_PSR2_IDLE_FRAME_MASK	0xf
>  #define   EDP_PSR2_IDLE_FRAME_SHIFT	0
>  
> -#define _PSR_EVENT_TRANS_A			0x60848
> -#define _PSR_EVENT_TRANS_B			0x61848
> -#define _PSR_EVENT_TRANS_C			0x62848
> -#define _PSR_EVENT_TRANS_D			0x63848
> -#define _PSR_EVENT_TRANS_EDP			0x6F848
> -#define PSR_EVENT(trans)			_MMIO_TRANS2(trans, _PSR_EVENT_TRANS_A)
> +#define PSR_EVENT				_MMIO(0x6F848)
>  #define  PSR_EVENT_PSR2_WD_TIMER_EXPIRE		(1 << 17)
>  #define  PSR_EVENT_PSR2_DISABLED		(1 << 16)
>  #define  PSR_EVENT_SU_DIRTY_FIFO_UNDERRUN	(1 << 15)
> diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c
> index 605fe8fc85cc..dc9fdb515a54 100644
> --- a/drivers/gpu/drm/i915/intel_psr.c
> +++ b/drivers/gpu/drm/i915/intel_psr.c
> @@ -84,46 +84,12 @@ 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);
> -
> -	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);
> -	}
> +	u32 mask = EDP_PSR_ERROR;
>  
>  	if (debug & I915_PSR_DEBUG_IRQ)
> -		mask |= debug_mask;
> +		mask |= EDP_PSR_POST_EXIT | EDP_PSR_PRE_ENTRY;
>  
>  	I915_WRITE(EDP_PSR_IMR, ~mask);
>  }
> @@ -167,62 +133,47 @@ static void psr_event_print(u32 val, bool psr2_enabled)
>  
>  void intel_psr_irq_handler(struct drm_i915_private *dev_priv, u32 psr_iir)
>  {
> -	u32 transcoders = BIT(TRANSCODER_EDP);
> -	enum transcoder cpu_transcoder;
> -	ktime_t time_ns =  ktime_get();
> -	u32 mask = 0;
> +	ktime_t time_ns = ktime_get();
>  
> -	if (INTEL_GEN(dev_priv) >= 8)
> -		transcoders |= BIT(TRANSCODER_A) |
> -			       BIT(TRANSCODER_B) |
> -			       BIT(TRANSCODER_C);
> -
> -	for_each_cpu_transcoder_masked(dev_priv, cpu_transcoder, transcoders) {
> -		int shift = edp_psr_shift(cpu_transcoder);
> -
> -		if (psr_iir & EDP_PSR_ERROR(shift)) {
> -			DRM_WARN("[transcoder %s] PSR aux error\n",
> -				 transcoder_name(cpu_transcoder));
> -
> -			dev_priv->psr.irq_aux_error = true;
> -
> -			/*
> -			 * If this interruption is not masked it will keep
> -			 * interrupting so fast that it prevents the scheduled
> -			 * work to run.
> -			 * Also after a PSR error, we don't want to arm PSR
> -			 * again so we don't care about unmask the interruption
> -			 * or unset irq_aux_error.
> -			 */
> -			mask |= EDP_PSR_ERROR(shift);
> -		}
> +	if (psr_iir & EDP_PSR_ERROR) {
> +		u32 mask;
>  
> -		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));
> -		}
> +		DRM_WARN("[transcoder %s] PSR aux error\n",
> +			 transcoder_name(TRANSCODER_EDP));
>  
> -		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));
> +		dev_priv->psr.irq_aux_error = true;
>  
> -			if (INTEL_GEN(dev_priv) >= 9) {
> -				u32 val = I915_READ(PSR_EVENT(cpu_transcoder));
> -				bool psr2_enabled = dev_priv->psr.psr2_enabled;
> +		/*
> +		 * If this interruption is not masked it will keep
> +		 * interrupting so fast that it prevents the scheduled
> +		 * work to run.
> +		 * Also after a PSR error, we don't want to arm PSR
> +		 * again so we don't care about unmask the interruption
> +		 * or unset irq_aux_error.
> +		 */
> +		mask = I915_READ(EDP_PSR_IMR) | EDP_PSR_ERROR;
> +		I915_WRITE(EDP_PSR_IMR, mask);
>  
> -				I915_WRITE(PSR_EVENT(cpu_transcoder), val);
> -				psr_event_print(val, psr2_enabled);
> -			}
> -		}
> +		schedule_work(&dev_priv->psr.work);
>  	}
>  
> -	if (mask) {
> -		mask |= I915_READ(EDP_PSR_IMR);
> -		I915_WRITE(EDP_PSR_IMR, mask);
> +	if (psr_iir & EDP_PSR_PRE_ENTRY) {
> +		dev_priv->psr.last_entry_attempt = time_ns;
> +		DRM_DEBUG_KMS("[transcoder %s] PSR entry attempt in 2 vblanks\n",
> +			      transcoder_name(TRANSCODER_EDP));
> +	}
>  
> -		schedule_work(&dev_priv->psr.work);
> +	if (psr_iir & EDP_PSR_POST_EXIT) {
> +		DRM_DEBUG_KMS("[transcoder %s] PSR exit completed\n",
> +			      transcoder_name(TRANSCODER_EDP));
> +
> +		if (INTEL_GEN(dev_priv) >= 9) {
> +			u32 val = I915_READ(PSR_EVENT);
> +			bool psr2_enabled = dev_priv->psr.psr2_enabled;
> +
> +			I915_WRITE(PSR_EVENT, val);
> +			psr_event_print(val, psr2_enabled);
> +		}
>  	}
>  }
>  
> @@ -672,30 +623,10 @@ static void intel_psr_activate(struct intel_dp *intel_dp)
>  	dev_priv->psr.active = true;
>  }
>  
> -static i915_reg_t gen9_chicken_trans_reg(struct drm_i915_private *dev_priv,
> -					 enum transcoder cpu_transcoder)
> -{
> -	static const i915_reg_t regs[] = {
> -		[TRANSCODER_A] = CHICKEN_TRANS_A,
> -		[TRANSCODER_B] = CHICKEN_TRANS_B,
> -		[TRANSCODER_C] = CHICKEN_TRANS_C,
> -		[TRANSCODER_EDP] = CHICKEN_TRANS_EDP,
> -	};
> -
> -	WARN_ON(INTEL_GEN(dev_priv) < 9);
> -
> -	if (WARN_ON(cpu_transcoder >= ARRAY_SIZE(regs) ||
> -		    !regs[cpu_transcoder].reg))
> -		cpu_transcoder = TRANSCODER_A;
> -
> -	return regs[cpu_transcoder];
> -}
> -
>  static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  				    const struct intel_crtc_state *crtc_state)
>  {
>  	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	u32 mask;
>  
>  	/* Only HSW and BDW have PSR AUX registers that need to be setup. SKL+
> @@ -706,13 +637,11 @@ static void intel_psr_enable_source(struct intel_dp *intel_dp,
>  
>  	if (dev_priv->psr.psr2_enabled && (IS_GEN(dev_priv, 9) &&
>  					   !IS_GEMINILAKE(dev_priv))) {
> -		i915_reg_t reg = gen9_chicken_trans_reg(dev_priv,
> -							cpu_transcoder);
> -		u32 chicken = I915_READ(reg);
> +		u32 chicken = I915_READ(CHICKEN_TRANS_EDP);
>  
>  		chicken |= PSR2_VSC_ENABLE_PROG_HEADER |
>  			   PSR2_ADD_VERTICAL_LINE_COUNT;
> -		I915_WRITE(reg, chicken);
> +		I915_WRITE(CHICKEN_TRANS_EDP, chicken);
>  	}
>  
>  	/*
> @@ -1225,7 +1154,7 @@ void intel_psr_init(struct drm_i915_private *dev_priv)
>  	 * to avoid any rendering problems.
>  	 */
>  	val = I915_READ(EDP_PSR_IIR);
> -	val &= EDP_PSR_ERROR(edp_psr_shift(TRANSCODER_EDP));
> +	val &= EDP_PSR_ERROR;
>  	if (val) {
>  		DRM_DEBUG_KMS("PSR interruption error set\n");
>  		dev_priv->psr.sink_not_reliable = true;
> -- 
> 2.21.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx