Re: [PATCH 2/5] drm/i915/dp: Add fields to store CMN_SDP_TL register state in crtc_state

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

On Thu, Mar 05, 2026 at 09:31:15AM +0530, Ankit Nautiyal wrote:
> Xe3p_lpd introduces new register bits to program a common SDP
> Transmission Line, which the hardware uses to position various
> SDPs. It also adds a separate control register to stagger the different
> SDPs (VSC EXT, PPS, GMP).
> 
> Add fields in struct intel_crtc_state to store the state of these new
> registers. Add register readback and pipe config comparison for the new
> fields.
> 
> Also add a display version check (HAS_CMN_SDP_TL) to gate access to the
> new registers.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c      |  1 +
>  drivers/gpu/drm/i915/display/intel_display.c  |  6 ++++++
>  .../drm/i915/display/intel_display_device.h   |  1 +
>  .../drm/i915/display/intel_display_types.h    |  9 ++++++++
>  drivers/gpu/drm/i915/display/intel_dp.c       | 21 +++++++++++++++++++
>  drivers/gpu/drm/i915/display/intel_dp.h       |  2 ++
>  6 files changed, 40 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 94ae583e907f..bdbd89600bee 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -4217,6 +4217,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>  	intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA);
>  	intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC);
>  	intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC);
> +	intel_dp_cmn_sdp_transmission_line_get_config(pipe_config);
>  
>  	intel_audio_codec_get_config(encoder, pipe_config);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index 27354585ba92..76eea9d23766 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5461,6 +5461,12 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
>  	}
>  	PIPE_CONF_CHECK_DP_VSC_SDP(vsc);
>  
> +	PIPE_CONF_CHECK_BOOL(cmn_sdp_tl.enable);
> +	PIPE_CONF_CHECK_I(cmn_sdp_tl.transmission_line);
> +	PIPE_CONF_CHECK_I(cmn_sdp_tl.vsc_ext_stagger);
> +	PIPE_CONF_CHECK_I(cmn_sdp_tl.pps_stagger);
> +	PIPE_CONF_CHECK_I(cmn_sdp_tl.gmp_stagger);

This will make things fail every time until you add the actual
code to program these. Ie. you are intentionally introducing
broken bisection steps here.

> +
>  	PIPE_CONF_CHECK_X(sync_mode_slaves_mask);
>  	PIPE_CONF_CHECK_I(master_transcoder);
>  	PIPE_CONF_CHECK_X(joiner_pipes);
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index e84c190dcc4f..43e259761048 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -154,6 +154,7 @@ struct intel_display_platforms {
>  #define HAS_CASF(__display)		(DISPLAY_VER(__display) >= 20)
>  #define HAS_CDCLK_CRAWL(__display)	(DISPLAY_INFO(__display)->has_cdclk_crawl)
>  #define HAS_CDCLK_SQUASH(__display)	(DISPLAY_INFO(__display)->has_cdclk_squash)
> +#define HAS_CMN_SDP_TL(__display)	(DISPLAY_VER(__display) >= 35)
>  #define HAS_CMRR(__display)		(DISPLAY_VER(__display) >= 20)
>  #define HAS_CMTG(__display)		(!(__display)->platform.dg2 && DISPLAY_VER(__display) >= 13)
>  #define HAS_CUR_FBC(__display)		(!HAS_GMCH(__display) && IS_DISPLAY_VER(__display, 7, 13))
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 8a2b37c7bccf..474d6e2ae34b 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1298,6 +1298,15 @@ struct intel_crtc_state {
>  		struct drm_dp_as_sdp as_sdp;
>  	} infoframes;
>  
> +	struct {
> +		/* Common SDP Transmission line */
> +		bool enable;
> +		int transmission_line;
> +		int vsc_ext_stagger;
> +		int pps_stagger;
> +		int gmp_stagger;
> +	} cmn_sdp_tl;
> +
>  	u8 eld[MAX_ELD_BYTES];
>  
>  	/* HDMI scrambling status */
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 025e906b63a9..86390553800d 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7287,3 +7287,24 @@ bool intel_dp_joiner_candidate_valid(struct intel_connector *connector,
>  
>  	return true;
>  }
> +
> +void intel_dp_cmn_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	u32 val;
> +
> +	if (!HAS_CMN_SDP_TL(display))
> +		return;
> +
> +	val = intel_de_read(display, CMN_SDP_TL(display, cpu_transcoder));
> +
> +	crtc_state->cmn_sdp_tl.enable = val & TRANSMISSION_LINE_ENABLE;

Tracking the enable bit seems fairly pointless. If it's not set we could
just skip the readout. I don't think we should ever want TL==0?

> +	crtc_state->cmn_sdp_tl.transmission_line = REG_FIELD_GET(BASE_TRANSMISSION_LINE_MASK, val);
> +
> +	val = intel_de_read(display, CMN_SDP_TL_STGR_CTL(display, cpu_transcoder));
> +
> +	crtc_state->cmn_sdp_tl.vsc_ext_stagger = REG_FIELD_GET(VSC_EXT_STAGGER_MASK, val);
> +	crtc_state->cmn_sdp_tl.pps_stagger = REG_FIELD_GET(PPS_STAGGER_MASK, val);
> +	crtc_state->cmn_sdp_tl.gmp_stagger = REG_FIELD_GET(GMP_STAGGER_MASK, val);
> +}
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.h b/drivers/gpu/drm/i915/display/intel_dp.h
> index b0bbd5981f57..24df234a43d3 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.h
> +++ b/drivers/gpu/drm/i915/display/intel_dp.h
> @@ -237,4 +237,6 @@ bool intel_dp_joiner_candidate_valid(struct intel_connector *connector,
>  	for ((__num_joined_pipes) = 1; (__num_joined_pipes) <= (I915_MAX_PIPES); (__num_joined_pipes)++) \
>  		for_each_if(intel_dp_joiner_candidate_valid(__connector, (__mode)->hdisplay, __num_joined_pipes))
>  
> +void intel_dp_cmn_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state);
> +
>  #endif /* __INTEL_DP_H__ */
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel