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 <[email protected]>
> ---
> 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
