On 3/6/2026 5:25 PM, Ville Syrjälä wrote:
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.
Hmm I was wondering that since at this point in the commit we have not
written these fields, the readout will also be 0 and will always match.
However I now realize that GOP might have programmed these and perhaps
we will immediately start seeing mismatch.
In general when we add a new thing, that we want to track in crtc state,
what should be the best ordering of the patches, does the below makes sense?
- Introduce the new registers
- Add the new members to crtc_state. In the same patch add functions
to read from HW and fill the new member, and also add function to write
into the HW from the crtc_state. (Since at this point of time, nobody is
setting them, so there is effectively no writing yet).
- Write functions that configure the new member of crtc_state in the
(should be part of atomic check phase). Also add PIPE CONF check in same
patch.
+
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?
Makes sense, we can drop enable and just track the base transmission line.
Regards,
Ankit
+ 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
