Re: [PATCH 3/8] drm/i915/psr: Compute Panel Replay/Adaptive coexistence behavior

[Imre Deak <imre.deak@intel.com>]

On Fri, Nov 21, 2025 at 01:16:50PM +0200, Jouni Högander wrote:
> Currently we are checking Panel Replay capability DPCD register in
> intel_alpm.c and writing PR_ALPM_CTL_ALLOW_LINK_OFF_BETWEEN_AS_SDP_AND_SU
> and PR_ALPM_CTL_AS_SDP_TRANSMISSION_IN_ACTIVE_DISABLE in PR_ALPM_CTL
> register base on the informaion. Instead of directly accessing
> intel_dp->pr_dpcd compute the behavior during psr_compute_config and store
> it in intel_crtc_state.
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c     |  6 ++---
>  .../drm/i915/display/intel_display_types.h    |  2 ++
>  drivers/gpu/drm/i915/display/intel_psr.c      | 22 +++++++++++++++----
>  3 files changed, 22 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 6372f533f65b5..7ce8c674bb030 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -326,11 +326,9 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  		if (intel_dp->as_sdp_supported) {
>  			u32 pr_alpm_ctl = PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T1;
>  
> -			if (intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> -			    DP_PANEL_REPLAY_LINK_OFF_SUPPORTED_IN_PR_AFTER_ADAPTIVE_SYNC_SDP)
> +			if (crtc_state->link_off_after_as_sdp_when_pr_active)
>  				pr_alpm_ctl |= PR_ALPM_CTL_ALLOW_LINK_OFF_BETWEEN_AS_SDP_AND_SU;
> -			if (!(intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> -						DP_PANEL_REPLAY_ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR))
> +			if (crtc_state->disable_as_sdp_when_pr_active)
>  				pr_alpm_ctl |= PR_ALPM_CTL_AS_SDP_TRANSMISSION_IN_ACTIVE_DISABLE;
>  
>  			intel_de_write(display, PR_ALPM_CTL(display, cpu_transcoder),
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index f39d62aa99246..d8a222689a35b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1161,6 +1161,8 @@ struct intel_crtc_state {
>  	bool enable_psr2_su_region_et;
>  	bool req_psr2_sdp_prior_scanline;
>  	bool has_panel_replay;
> +	bool link_off_after_as_sdp_when_pr_active;
> +	bool disable_as_sdp_when_pr_active;

I agree with Jani that moving the PSR/Panel Replay fields to a substruct
would make things clearer. That's a bigger change, so I presume it could
be also a follow-up.

>  	bool wm_level_disabled;
>  	bool pkg_c_latency_used;
>  	/* Only used for state verification. */
> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c b/drivers/gpu/drm/i915/display/intel_psr.c
> index 4c5883bed612b..9d2ba39423826 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -1715,10 +1715,21 @@ static bool _psr_compute_config(struct intel_dp *intel_dp,
>  	return true;
>  }
>  
> -static bool
> -_panel_replay_compute_config(struct intel_dp *intel_dp,
> -			     struct intel_crtc_state *crtc_state,
> -			     const struct drm_connector_state *conn_state)
> +static bool compute_link_off_after_as_sdp_when_pr_active(struct intel_dp *intel_dp)
> +{
> +	return (intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> +		DP_PANEL_REPLAY_LINK_OFF_SUPPORTED_IN_PR_AFTER_ADAPTIVE_SYNC_SDP);
> +}
> +
> +static bool compute_disable_as_sdp_when_pr_active(struct intel_dp *intel_dp)
> +{
> +	return !(intel_dp->pr_dpcd[INTEL_PR_DPCD_INDEX(DP_PANEL_REPLAY_CAP_CAPABILITY)] &
> +		 DP_PANEL_REPLAY_ASYNC_VIDEO_TIMING_NOT_SUPPORTED_IN_PR);
> +}

Nit: Aren't the above functions simple enough to inline them? Or do they
get more complicated later?

> +
> +static bool _panel_replay_compute_config(struct intel_dp *intel_dp,
> +					 struct intel_crtc_state *crtc_state,
> +					 const struct drm_connector_state *conn_state)

Nit: I'd drop intel_dp as mentioned earlier.

Regardless, the patch looks ok:
Reviewed-by: Imre Deak <imre.deak@intel.com>


>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	struct intel_connector *connector =
> @@ -1747,6 +1758,9 @@ _panel_replay_compute_config(struct intel_dp *intel_dp,
>  		return false;
>  	}
>  
> +	crtc_state->link_off_after_as_sdp_when_pr_active = compute_link_off_after_as_sdp_when_pr_active(intel_dp);
> +	crtc_state->disable_as_sdp_when_pr_active = compute_disable_as_sdp_when_pr_active(intel_dp);
> +
>  	if (!intel_dp_is_edp(intel_dp))
>  		return true;
>  
> -- 
> 2.43.0
>