[PATCH v6 06/26] drm/i915/psr: Split enabling sink for PSR and Panel Replay

[Manna, Animesh]

> -----Original Message-----
> From: Hogander, Jouni <jouni.hogander at intel.com>
> Sent: Wednesday, June 5, 2024 3:56 PM
> To: intel-gfx at lists.freedesktop.org
> Cc: Manna, Animesh <animesh.manna at intel.com>; Kahola, Mika
> <mika.kahola at intel.com>; Hogander, Jouni <jouni.hogander at intel.com>
> Subject: [PATCH v6 06/26] drm/i915/psr: Split enabling sink for PSR and Panel
> Replay
> 
> Current intel_psr_enable_sink is a mess due to partly reusing PSR bit
> definitions for Panel Replay. Even thought PSR and Panel Replay enable
> registers do have common bits they still have also different bits and same
> bits with different meaning. For sake of clarity split enabling sink to PSR and
> Panel Replay specific parts.
> 
> Also fix issue caused by using psr->panel_replay_enabled to early.
> 
> Fixes: 88ae6c65ecdb ("drm/i915/psr: Unify panel replay enable/disable sink")
> Signed-off-by: Jouni Högander <jouni.hogander at intel.com>

Reviewed-by: Animesh Manna <animesh.manna at intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_psr.c | 81 +++++++++++++-----------
>  1 file changed, 44 insertions(+), 37 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
> b/drivers/gpu/drm/i915/display/intel_psr.c
> index 4a4124a92a0d..3cc38ba2f954 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -686,56 +686,62 @@ static bool psr2_su_region_et_valid(struct intel_dp
> *intel_dp)
>  	return false;
>  }
> 
> -static unsigned int intel_psr_get_enable_sink_offset(struct intel_dp
> *intel_dp)
> +static void _panel_replay_enable_sink(struct intel_dp *intel_dp,
> +				     const struct intel_crtc_state *crtc_state)
>  {
> -	return intel_dp->psr.panel_replay_enabled ?
> -		PANEL_REPLAY_CONFIG : DP_PSR_EN_CFG;
> +	u8 val = DP_PANEL_REPLAY_ENABLE |
> +		DP_PANEL_REPLAY_VSC_SDP_CRC_EN |
> +		DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN |
> +		DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN |
> +		DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN;
> +
> +	if (crtc_state->has_sel_update)
> +		val |= DP_PANEL_REPLAY_SU_ENABLE;
> +
> +	if (crtc_state->enable_psr2_su_region_et)
> +		val |= DP_PANEL_REPLAY_ENABLE_SU_REGION_ET;
> +
> +	drm_dp_dpcd_writeb(&intel_dp->aux, PANEL_REPLAY_CONFIG, val);
>  }
> 
> -/*
> - * Note: Most of the bits are same in PANEL_REPLAY_CONFIG and
> DP_PSR_EN_CFG. We
> - * are relying on PSR definitions on these "common" bits.
> - */
> -void intel_psr_enable_sink(struct intel_dp *intel_dp,
> -			   const struct intel_crtc_state *crtc_state)
> +static void _psr_enable_sink(struct intel_dp *intel_dp,
> +			    const struct intel_crtc_state *crtc_state)
>  {
> -	struct drm_i915_private *dev_priv = dp_to_i915(intel_dp);
> -	u8 dpcd_val = DP_PSR_ENABLE;
> +	struct drm_i915_private *i915 = dp_to_i915(intel_dp);
> +	u8 val = DP_PSR_ENABLE;
> 
>  	if (crtc_state->has_sel_update) {
> -		/* Enable ALPM at sink for psr2 */
> -		if (!crtc_state->has_panel_replay) {
> -			drm_dp_dpcd_writeb(&intel_dp->aux,
> -					   DP_RECEIVER_ALPM_CONFIG,
> -					   DP_ALPM_ENABLE |
> -
> DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE);
> -
> -			if (crtc_state->enable_psr2_su_region_et)
> -				dpcd_val |=
> DP_PSR_ENABLE_SU_REGION_ET;
> -		}
> -
> -		dpcd_val |= DP_PSR_ENABLE_PSR2 |
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
> +		val |= DP_PSR_ENABLE_PSR2 |
> DP_PSR_IRQ_HPD_WITH_CRC_ERRORS;
>  	} else {
>  		if (intel_dp->psr.link_standby)
> -			dpcd_val |= DP_PSR_MAIN_LINK_ACTIVE;
> +			val |= DP_PSR_MAIN_LINK_ACTIVE;
> 
> -		if (!crtc_state->has_panel_replay && DISPLAY_VER(dev_priv)
> >= 8)
> -			dpcd_val |= DP_PSR_CRC_VERIFICATION;
> +		if (DISPLAY_VER(i915) >= 8)
> +			val |= DP_PSR_CRC_VERIFICATION;
>  	}
> 
> -	if (crtc_state->has_panel_replay)
> -		dpcd_val |=
> DP_PANEL_REPLAY_UNRECOVERABLE_ERROR_EN |
> -			DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN;
> -
> -	if (crtc_state->req_psr2_sdp_prior_scanline)
> -		dpcd_val |= DP_PSR_SU_REGION_SCANLINE_CAPTURE;
> +	if (crtc_state->enable_psr2_su_region_et)
> +		val |= DP_PANEL_REPLAY_ENABLE_SU_REGION_ET;
> 
>  	if (intel_dp->psr.entry_setup_frames > 0)
> -		dpcd_val |= DP_PSR_FRAME_CAPTURE;
> +		val |= DP_PSR_FRAME_CAPTURE;
> 
> -	drm_dp_dpcd_writeb(&intel_dp->aux,
> -			   intel_psr_get_enable_sink_offset(intel_dp),
> -			   dpcd_val);
> +	drm_dp_dpcd_writeb(&intel_dp->aux, DP_PSR_EN_CFG, val); }
> +
> +void intel_psr_enable_sink(struct intel_dp *intel_dp,
> +			   const struct intel_crtc_state *crtc_state) {
> +	/* Enable ALPM at sink for psr2 */
> +	if (!crtc_state->has_panel_replay && crtc_state->has_sel_update)
> +		drm_dp_dpcd_writeb(&intel_dp->aux,
> +				   DP_RECEIVER_ALPM_CONFIG,
> +				   DP_ALPM_ENABLE |
> +
> DP_ALPM_LOCK_ERROR_IRQ_HPD_ENABLE);
> +
> +	crtc_state->has_panel_replay ?
> +		_panel_replay_enable_sink(intel_dp, crtc_state) :
> +		_psr_enable_sink(intel_dp, crtc_state);
> 
>  	if (intel_dp_is_edp(intel_dp))
>  		drm_dp_dpcd_writeb(&intel_dp->aux, DP_SET_POWER,
> DP_SET_POWER_D0); @@ -1920,7 +1926,8 @@ static void
> intel_psr_disable_locked(struct intel_dp *intel_dp)
> 
>  	/* Disable PSR on Sink */
>  	drm_dp_dpcd_writeb(&intel_dp->aux,
> -			   intel_psr_get_enable_sink_offset(intel_dp), 0);
> +			   intel_dp->psr.panel_replay_enabled ?
> +			   PANEL_REPLAY_CONFIG : DP_PSR_EN_CFG, 0);
> 
>  	if (!intel_dp->psr.panel_replay_enabled &&
>  	    intel_dp->psr.sel_update_enabled)
> --
> 2.34.1