Re: [PATCH 13/14] drm/i915/display: Add member to store AS SDP transmission time

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

On Mon, Feb 23, 2026 at 07:14:30PM +0530, Ankit Nautiyal wrote:
> AS SDP can be sent at two different positions T1 and T2.
> These depend on the Panel Replay configuration and Adaptive Sync SDP
> modes as per DP v2.1.
> Currently we have configurations where SDP needs to be sent at T1 only.
> However, to make way for supporting more PR and AS SDP configurations,
> add a new member to store AS SDP transmission time in crtc_state.
> 
> This is filled with T1 for now during panel_replay_compute_config() and
> is used to set the MMIO register PR_ALPM and DPCD Panel_Replay_Config3
> DPCD offsets.
> 
> readout for this new member needs to be added along with other related
> members.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c     | 21 ++++++++++++++++++-
>  .../drm/i915/display/intel_display_types.h    |  1 +
>  drivers/gpu/drm/i915/display/intel_psr.c      |  7 +++++++
>  3 files changed, 28 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c b/drivers/gpu/drm/i915/display/intel_alpm.c
> index b3334bc4d0f9..6eecd5ce4fad 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -365,6 +365,25 @@ void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp,
>  	crtc_state->has_lobf = true;
>  }
>  
> +static int get_pr_alpm_as_sdp_transmission_time(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +
> +	switch (crtc_state->pr_as_sdp_transmission) {
> +	case AS_SDP_SETUP_TIME_T1:
> +		return PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T1;
> +	case AS_SDP_SETUP_TIME_DYNAMIC:
> +		return PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T1_OR_T2;
> +	case AS_SDP_SETUP_TIME_T2:
> +		return PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T2;
> +	default:
> +		drm_dbg_kms(display->drm,
> +			    "Missing case %d for AS SDP Position, going with T1 by default\n",
> +			    crtc_state->pr_as_sdp_transmission);

MISSING_CASE()

There's no point in adding eloquent debug messages/etc into dead code.

> +		return AS_SDP_SETUP_TIME_T1;
> +	}
> +}
> +
>  static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  			       const struct intel_crtc_state *crtc_state)
>  {
> @@ -388,7 +407,7 @@ static void lnl_alpm_configure(struct intel_dp *intel_dp,
>  			ALPM_CTL_AUX_LESS_WAKE_TIME(crtc_state->alpm_state.aux_less_wake_lines);
>  
>  		if (intel_dp->as_sdp_supported) {
> -			u32 pr_alpm_ctl = PR_ALPM_CTL_ADAPTIVE_SYNC_SDP_POSITION_T1;
> +			u32 pr_alpm_ctl = get_pr_alpm_as_sdp_transmission_time(crtc_state);
>  
>  			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;
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index e8e4af03a6a6..9065bf8bd6a5 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1181,6 +1181,7 @@ struct intel_crtc_state {
>  	bool has_panel_replay;
>  	bool link_off_after_as_sdp_when_pr_active;
>  	bool disable_as_sdp_when_pr_active;
> +	int pr_as_sdp_transmission;

I don't see much point of adding that if you only ever set
it to one fixed value. Though I suppose the point was to make
sure the DPCD and PR_ALPM_CTL register values stay in sync.
We could also achieve that by replacing that crtc_state member
with eg. just a simple function.

>  	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 e0e6ddbfaa2d..74242c93db87 100644
> --- a/drivers/gpu/drm/i915/display/intel_psr.c
> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
> @@ -779,6 +779,7 @@ static void _panel_replay_enable_sink(struct intel_dp *intel_dp,
>  		DP_PANEL_REPLAY_RFB_STORAGE_ERROR_EN |
>  		DP_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR_EN;
>  	u8 panel_replay_config2 = DP_PANEL_REPLAY_CRC_VERIFICATION;
> +	u8 panel_replay_config3 = crtc_state->pr_as_sdp_transmission;
>  
>  	if (crtc_state->has_sel_update)
>  		val |= DP_PANEL_REPLAY_SU_ENABLE;
> @@ -794,6 +795,9 @@ static void _panel_replay_enable_sink(struct intel_dp *intel_dp,
>  
>  	drm_dp_dpcd_writeb(&intel_dp->aux, PANEL_REPLAY_CONFIG2,
>  			   panel_replay_config2);
> +
> +	drm_dp_dpcd_writeb(&intel_dp->aux, PANEL_REPLAY_CONFIG3,
> +			   panel_replay_config3);

This is starting to look like someone should make this a burst write.

>  }
>  
>  static void _psr_enable_sink(struct intel_dp *intel_dp,
> @@ -1771,6 +1775,9 @@ static bool _panel_replay_compute_config(struct intel_crtc_state *crtc_state,
>  	crtc_state->link_off_after_as_sdp_when_pr_active = compute_link_off_after_as_sdp_when_pr_active(connector);
>  	crtc_state->disable_as_sdp_when_pr_active = compute_disable_as_sdp_when_pr_active(connector);
>  
> +	/* For now we use T1 as the transmission time */
> +	crtc_state->pr_as_sdp_transmission = AS_SDP_SETUP_TIME_T1;
> +
>  	if (!intel_dp_is_edp(intel_dp))
>  		return true;
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel