[Intel-gfx] [PATCH 6/7] drm/i915/psr: Remove partial PSR support on multiple transcoders

Rodrigo Vivi rodrigo.vivi at intel.com
Thu Apr 4 00:31:25 UTC 2019


On Wed, Apr 03, 2019 at 04:35:38PM -0700, José Roberto de Souza wrote:
> PSR is only supported in eDP transcoder and there is only one
> instance of it, so lets drop all of this code.

Is this sentence true? I mean, in the way it is written it
seems like HW doesn't actually support it...
Or should we re-phrase for we are not really enabling support
for other transcoders than eDP and we do not have plans to do
it so soon so let's clean the code...
or something like that?

> 
> Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan at intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Signed-off-by: José Roberto de Souza <jose.souza at 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 c59cfa83dbaf..18e2991b376d 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -4241,13 +4241,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)
> @@ -4312,12 +4308,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 bb97c1657493..b984e005b72e 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);
> +		}
>  	}
>  }
>  
> @@ -669,30 +620,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+
> @@ -703,13 +634,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);
>  	}
>  
>  	/*
> @@ -1222,7 +1151,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
> 


More information about the Intel-gfx mailing list