[PATCH v5 00/12] Add support for Common SDP Transmission Line

[PATCH v5 00/12] Add support for Common SDP Transmission Line

[Ankit Nautiyal]

Xe3p_lpd introduces new register bits to program a common SDP
Transmission line that will be used by the Hardware to position the
SDPs. Along with this, another new register is also added to stagger
the different SDPs.

This series adds support for programming the Common SDP Transmission Line
(CMN_SDP_TL) and its stagger control registers.

Rev2:
 - Add tracking for AS SDP Transmission line first. Instead of directly
   programming it in the register, compute the state during
   compute_config_late() and write the value from crtc_state.
   Readout from the register through a vrr helper, since the register is
   defined in context of VRR and will be used for both DP and HDMI VRR
   packets.
 - Add support to track other SDP transmission lines, instead of stagger
   values.
 - Compute the transmission lines in compute_config_late path and add
   readout.
 - Always write the Common SDP Transmission line and other stagger
   values, whether the packets are sent or not.
 - Dump SDP transmission lines in crtc_state.

Rev3:
 - Introduce new files for handling the Data Island Packets (DIPs)
   programming in one place. (Ville)
 - Extract the Transmission Line programming for
   HDMI Extended Metadata Packet (EMP) and Adaptive Sync SDP (AS SDP)
   i.e. EMP_AS_SDP_TL in the new file.
 - Add programming for Common Base SDP Transmission lines and other
   stagger offsets in the new file.
 - Add a #TODO to extract other helpers that deal with Data Island Packets
   and VIDEO_DIP registers.
 - Place the HAS_* Macros related to DIP in the intel_dip.h file
   instead of intel_display_devices.h (Jani) 
 - Make function names more descriptive and avoid overuse of multiple
   acronyms. (Jani)

Note:
 - Though I have fixed the naming as suggested in most places, I
   have retained HAS_EMP_AS_SDP_TL for the following reasons:
   1) Lack of an apt name that avoids the acronym for
      'Extended Metadata Packet Adaptive Sync Secondary Packet Transmission
      Line'.
   2) Keeping this name makes it easier to find and map with the exact name
      for such a complex register name.

   However, I am open to suggestions for a better name.

Rev4:
 - Drop the patch to check for DP encoder before writing EMP_AS_SDP_TL
   register. Instead reset the register to 0 for non DP encoder case.

Rev5:
 - Rebase
 - Address review comments from Suraj:
 - Drop some redundant checks, and fix few commit messages.

Ankit Nautiyal (11):
  drm/i915/dip: Add new file to handle Data Island Packet hardware
  drm/i915/vrr: Use the helper to write EMP_AS_SDP_TL register
  drm/i915/intel_dip: Add check for DP encoder
  drm/i915/dip: Add helper to get AS SDP Transmission Line
  drm/i915/display: Add crtc state for DIP transmission lines
  drm/i915/dip: Store and use AS SDP transmission line from crtc state
  drm/i915/dip: Add HAS_COMMON_SDP_TL macro
  drm/i915/dp: Store SDP transmission lines in crtc_state
  drm/i915/dp: Introduce helpers to enable/disable CMN SDP Transmission
    line
  drm/i915/dip: Enable Common SDP Transmission line
  drm/i915/display: Dump DIP Transmission lines

Arun R Murthy (1):
  drm/i915/dip_regs: Add register definitions for common SDP
    Transmission Line

 drivers/gpu/drm/i915/Makefile                 |   1 +
 .../drm/i915/display/intel_crtc_state_dump.c  |   9 +
 drivers/gpu/drm/i915/display/intel_ddi.c      |   5 +
 drivers/gpu/drm/i915/display/intel_dip.c      | 209 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dip.h      |  67 ++++++
 drivers/gpu/drm/i915/display/intel_dip_regs.h |  36 +++
 drivers/gpu/drm/i915/display/intel_display.c  |   7 +
 .../drm/i915/display/intel_display_types.h    |   3 +
 drivers/gpu/drm/i915/display/intel_dp.c       |   2 +
 drivers/gpu/drm/i915/display/intel_vrr.c      |  14 +-
 drivers/gpu/drm/i915/display/intel_vrr_regs.h |   6 -
 drivers/gpu/drm/xe/Makefile                   |   1 +
 12 files changed, 343 insertions(+), 17 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_dip.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_dip.h
 create mode 100644 drivers/gpu/drm/i915/display/intel_dip_regs.h

-- 
2.50.1

[PATCH v5 01/12] drm/i915/dip: Add new file to handle Data Island Packet hardware

[Ankit Nautiyal]

Add new files intel_dip.c, intel_dip.h, and intel_dip_regs.h to handle
low level hardware programming related to Data Island Packets.

Currently only programming of the Transmission Line for HDMI 2.1
Extended Metadata Packet (EMP) and DP Adaptive-Sync Secondary Data
Packet (SDP) is added (MMIO register EMP_AS_SDP_TL).

This will serve as a common place for DIP related code, which is currently
scattered across DP and HDMI files. A TODO has been added for extracting
the remaining DIP helpers.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/Makefile                 |  1 +
 drivers/gpu/drm/i915/display/intel_dip.c      | 39 +++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dip.h      | 38 ++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dip_regs.h | 17 ++++++++
 drivers/gpu/drm/i915/display/intel_vrr.c      |  1 +
 drivers/gpu/drm/i915/display/intel_vrr_regs.h |  6 ---
 drivers/gpu/drm/xe/Makefile                   |  1 +
 7 files changed, 97 insertions(+), 6 deletions(-)
 create mode 100644 drivers/gpu/drm/i915/display/intel_dip.c
 create mode 100644 drivers/gpu/drm/i915/display/intel_dip.h
 create mode 100644 drivers/gpu/drm/i915/display/intel_dip_regs.h

diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile
index a83fa8be0aba..d4e7764621e1 100644
--- a/drivers/gpu/drm/i915/Makefile
+++ b/drivers/gpu/drm/i915/Makefile
@@ -349,6 +349,7 @@ i915-y += \
 	display/intel_cx0_phy.o \
 	display/intel_ddi.o \
 	display/intel_ddi_buf_trans.o \
+	display/intel_dip.o \
 	display/intel_display_device.o \
 	display/intel_display_trace.o \
 	display/intel_dkl_phy.o \
diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
new file mode 100644
index 000000000000..2e2bdb2b199c
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dip.c
@@ -0,0 +1,39 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright © 2026 Intel Corporation
+ *
+ */
+
+#include "intel_de.h"
+#include "intel_dip.h"
+#include "intel_dip_regs.h"
+#include "intel_display_types.h"
+
+u16 intel_dip_read_emp_as_sdp_tl(const 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_EMP_AS_SDP_TL(display))
+		return 0;
+
+	val = intel_de_read(display, EMP_AS_SDP_TL(display, cpu_transcoder));
+	return REG_FIELD_GET(EMP_AS_SDP_DB_TL_MASK, val);
+}
+
+void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+
+	if (!HAS_EMP_AS_SDP_TL(display))
+		return;
+	/*
+	 * Since currently we support VRR only for DP/eDP, so this is programmed
+	 * only for Adaptive Sync SDP to Vsync start.
+	 */
+	intel_de_write(display,
+		       EMP_AS_SDP_TL(display, cpu_transcoder),
+		       EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dip.h b/drivers/gpu/drm/i915/display/intel_dip.h
new file mode 100644
index 000000000000..25bae4a04d6b
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dip.h
@@ -0,0 +1,38 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2026 Intel Corporation
+ */
+
+#ifndef __INTEL_DIP_H__
+#define __INTEL_DIP_H__
+
+#include "intel_display_device.h"
+
+/*
+ * Video DIP (Data Island Packet) helpers.
+ *
+ * This file contains helpers for programming video DIP related hardware.
+ *
+ * TODO: Currently, this is only used for programming EMP_AS_SDP_TL i.e. to
+ * program Transmission Line for HDMI 2.1 Extended Metadata Packet (EMP) and
+ * DP Adaptive Sync (AS) Secondary Data Packet (SDP). However, all low level
+ * DIP buffer read/write and related helpers should be extracted here later.
+ */
+
+struct intel_crtc_state;
+
+/*
+ * EMP AS SDP TL: Extended Metadata Packet (EMP) Adaptive Sync (AS)
+ * Secondary Data Packet (SDP) Transmission Line (TL).
+ *
+ * Starting with BMG (display ver 14.01) and LNL+ (display ver 20+),
+ * the AS SDP transmission line is programmable via the EMP AS SDP TL
+ * register.
+ */
+#define HAS_EMP_AS_SDP_TL(__display)	(DISPLAY_VERx100(__display) == 1401 || \
+					 DISPLAY_VER(__display) >= 20)
+
+u16 intel_dip_read_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
+void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
+
+#endif /* __INTEL_DIP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dip_regs.h b/drivers/gpu/drm/i915/display/intel_dip_regs.h
new file mode 100644
index 000000000000..85dcbf42e77d
--- /dev/null
+++ b/drivers/gpu/drm/i915/display/intel_dip_regs.h
@@ -0,0 +1,17 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2026 Intel Corporation
+ */
+
+#ifndef __INTEL_DIP_REGS_H__
+#define __INTEL_DIP_REGS_H__
+
+#include "intel_display_reg_defs.h"
+
+/* EMP (Extended Metadata Packet) AS (Adaptive Sync) SDP Transmission Line */
+#define _EMP_AS_SDP_TL_A			0x60204
+#define EMP_AS_SDP_TL(display, trans)		_MMIO_TRANS2((display), (trans), _EMP_AS_SDP_TL_A)
+#define   EMP_AS_SDP_DB_TL_MASK			REG_GENMASK(12, 0)
+#define   EMP_AS_SDP_DB_TL(db_transmit_line)	REG_FIELD_PREP(EMP_AS_SDP_DB_TL_MASK, (db_transmit_line))
+
+#endif /* __INTEL_DIP_REGS_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index e36db1174440..fd8b6f829cfe 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -17,6 +17,7 @@
 #include "intel_cmtg.h"
 #include "intel_crtc.h"
 #include "intel_de.h"
+#include "intel_dip_regs.h"
 #include "intel_display_limits.h"
 #include "intel_display_regs.h"
 #include "intel_display_types.h"
diff --git a/drivers/gpu/drm/i915/display/intel_vrr_regs.h b/drivers/gpu/drm/i915/display/intel_vrr_regs.h
index 9d4d6573a149..ba8631cbc672 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_vrr_regs.h
@@ -174,12 +174,6 @@
 #define   VRR_VSYNC_START_MASK			REG_GENMASK(12, 0)
 #define   VRR_VSYNC_START(vsync_start)		REG_FIELD_PREP(VRR_VSYNC_START_MASK, (vsync_start))
 
-/* Common register for HDMI EMP and DP AS SDP */
-#define _EMP_AS_SDP_TL_A			0x60204
-#define EMP_AS_SDP_TL(display, trans)		_MMIO_TRANS2((display), (trans), _EMP_AS_SDP_TL_A)
-#define   EMP_AS_SDP_DB_TL_MASK			REG_GENMASK(12, 0)
-#define   EMP_AS_SDP_DB_TL(db_transmit_line)	REG_FIELD_PREP(EMP_AS_SDP_DB_TL_MASK, (db_transmit_line))
-
 #define _TRANS_CMRR_M_LO_A			0x604F0
 #define TRANS_CMRR_M_LO(display, trans)		_MMIO_TRANS2((display), (trans), _TRANS_CMRR_M_LO_A)
 
diff --git a/drivers/gpu/drm/xe/Makefile b/drivers/gpu/drm/xe/Makefile
index 67b8b5477639..ba4404896f2f 100644
--- a/drivers/gpu/drm/xe/Makefile
+++ b/drivers/gpu/drm/xe/Makefile
@@ -259,6 +259,7 @@ xe-$(CONFIG_DRM_XE_DISPLAY) += \
 	i915-display/intel_ddi.o \
 	i915-display/intel_ddi_buf_trans.o \
 	i915-display/intel_de.o \
+	i915-display/intel_dip.o \
 	i915-display/intel_display.o \
 	i915-display/intel_display_conversion.o \
 	i915-display/intel_display_device.o \
-- 
2.50.1

[PATCH v5 02/12] drm/i915/vrr: Use the helper to write EMP_AS_SDP_TL register

[Ankit Nautiyal]

Now that intel_dip_write_emp_as_sdp_tl() exists, use it instead of
the open-coded EMP_AS_SDP_TL programming sequence in
intel_vrr_set_transcoder_timings().

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_vrr.c | 13 ++-----------
 1 file changed, 2 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
index fd8b6f829cfe..a516ba34e0d0 100644
--- a/drivers/gpu/drm/i915/display/intel_vrr.c
+++ b/drivers/gpu/drm/i915/display/intel_vrr.c
@@ -17,6 +17,7 @@
 #include "intel_cmtg.h"
 #include "intel_crtc.h"
 #include "intel_de.h"
+#include "intel_dip.h"
 #include "intel_dip_regs.h"
 #include "intel_display_limits.h"
 #include "intel_display_regs.h"
@@ -711,17 +712,7 @@ void intel_vrr_set_transcoder_timings(const struct intel_crtc_state *crtc_state)
 			       VRR_VSYNC_END(crtc_state->vrr.vsync_end) |
 			       VRR_VSYNC_START(crtc_state->vrr.vsync_start));
 
-	/*
-	 * For BMG and LNL+ onwards the EMP_AS_SDP_TL is used for programming
-	 * double buffering point and transmission line for VRR packets for
-	 * HDMI2.1/DP/eDP/DP->HDMI2.1 PCON.
-	 * Since currently we support VRR only for DP/eDP, so this is programmed
-	 * to for Adaptive Sync SDP to Vsync start.
-	 */
-	if (DISPLAY_VERx100(display) == 1401 || DISPLAY_VER(display) >= 20)
-		intel_de_write(display,
-			       EMP_AS_SDP_TL(display, cpu_transcoder),
-			       EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));
+	intel_dip_write_emp_as_sdp_tl(crtc_state);
 }
 
 void
-- 
2.50.1

[PATCH v5 03/12] drm/i915/intel_dip: Add check for DP encoder

[Ankit Nautiyal]

At the moment, the common register for programming Transmission line for
Extended Metadata Packet and Adaptive-Sync Secondary Data Packet
(EMP_AS_SDP_TL) is only used to program Adaptive-Sync SDP (AS SDP).

Since VRR and Video Timing Extended Metadat Packet (VTEMP) are not yet
implemented for HDMI, add an explicit check to write the register only
for DP encoders (that may use AS SDP) and reset the register for non-DP
encoders.

In subsequent changes, instead of directly writing the value,
appropriate helpers will be called, that will supply the transmission line
for these packets.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dip.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
index 2e2bdb2b199c..b28352a9e23f 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.c
+++ b/drivers/gpu/drm/i915/display/intel_dip.c
@@ -26,14 +26,19 @@ void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	u32 transmission_line = 0;
 
 	if (!HAS_EMP_AS_SDP_TL(display))
 		return;
 	/*
-	 * Since currently we support VRR only for DP/eDP, so this is programmed
-	 * only for Adaptive Sync SDP to Vsync start.
+	 * Since we currently support VRR only for DP/eDP, program the register
+	 * for Adaptive Sync SDP using vsync start. For non-DP encoders,
+	 * the register is reset to 0.
 	 */
+	if (intel_crtc_has_dp_encoder(crtc_state))
+		transmission_line = crtc_state->vrr.vsync_start;
+
 	intel_de_write(display,
 		       EMP_AS_SDP_TL(display, cpu_transcoder),
-		       EMP_AS_SDP_DB_TL(crtc_state->vrr.vsync_start));
+		       EMP_AS_SDP_DB_TL(transmission_line));
 }
-- 
2.50.1

[PATCH v5 04/12] drm/i915/dip: Add helper to get AS SDP Transmission Line

[Ankit Nautiyal]

Introduce a DIP helper to compute the Adaptive Sync SDP transmission line
and use it when programming the EMP_AS_SDP_TL register.

Currently the AS SDP transmission line is programmed to the T1 position.
This can be extended in the future to support programming the T2 position
as well.

While at it, improve the documentation: the AS SDP transmission line
corresponds to the T1 position, which maps to the start of the VSYNC
pulse.

v2:
- Move the helper into intel_dip.c and make it static, since intel_dip.c
  is its only caller.
- Drop the now unused prototype from intel_dp.h.
- Add the check HAS_EMP_AS_SDP_TL(). (Suraj)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com> (#v1)
---
 drivers/gpu/drm/i915/display/intel_dip.c | 19 ++++++++++++++++++-
 1 file changed, 18 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
index b28352a9e23f..0277b15e1c82 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.c
+++ b/drivers/gpu/drm/i915/display/intel_dip.c
@@ -9,6 +9,23 @@
 #include "intel_dip_regs.h"
 #include "intel_display_types.h"
 
+static int intel_dip_get_as_sdp_transmission_line(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+
+	if (!HAS_EMP_AS_SDP_TL(display))
+		return 0;
+
+	/*
+	 * EMP_AS_SDP_TL defines the T1 position as the default AS SDP
+	 * Transmission Line, which corresponds to the start of the
+	 * VSYNC pulse.
+	 *
+	 * Use the T1 position for now.
+	 */
+	return crtc_state->vrr.vsync_start;
+}
+
 u16 intel_dip_read_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
@@ -36,7 +53,7 @@ void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
 	 * the register is reset to 0.
 	 */
 	if (intel_crtc_has_dp_encoder(crtc_state))
-		transmission_line = crtc_state->vrr.vsync_start;
+		transmission_line = intel_dip_get_as_sdp_transmission_line(crtc_state);
 
 	intel_de_write(display,
 		       EMP_AS_SDP_TL(display, cpu_transcoder),
-- 
2.50.1

[PATCH v5 05/12] drm/i915/display: Add crtc state for DIP transmission lines

[Ankit Nautiyal]

The Adaptive Sync SDP is currently the only packet with a programmable
transmission line.

Make a structure struct intel_dip for Data Island Packets. Add a member to
track Adaptive-Sync SDP transmission line. Include the new member in the
pipe configuration comparison.

This will pave the way for supporting more packets' programmable
transmission lines, including the common base SDP transmission line
introduced with Xe3p_lpd.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dip.h           | 8 ++++++++
 drivers/gpu/drm/i915/display/intel_display.c       | 2 ++
 drivers/gpu/drm/i915/display/intel_display_types.h | 3 +++
 3 files changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dip.h b/drivers/gpu/drm/i915/display/intel_dip.h
index 25bae4a04d6b..37507ac3e645 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.h
+++ b/drivers/gpu/drm/i915/display/intel_dip.h
@@ -35,4 +35,12 @@ struct intel_crtc_state;
 u16 intel_dip_read_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
 void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
 
+struct intel_dip {
+	/*
+	 * DIP Transmission line, relative to the Vtotal.
+	 * The programmed transmit line is (Vtotal - value)
+	 */
+	u16 emp_as_sdp_tl;
+};
+
 #endif /* __INTEL_DIP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index fc30a455bed3..6dd2626256cf 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5603,6 +5603,8 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 
 	PIPE_CONF_CHECK_I(set_context_latency);
 
+	PIPE_CONF_CHECK_I(dip.emp_as_sdp_tl);
+
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
 #undef PIPE_CONF_CHECK_LLI
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9016be52c7ea..65f9a9745f53 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -46,6 +46,7 @@
 
 #include "i915_gtt_view_types.h"
 #include "intel_bios.h"
+#include "intel_dip.h"
 #include "intel_display.h"
 #include "intel_display_conversion.h"
 #include "intel_display_limits.h"
@@ -1312,6 +1313,8 @@ struct intel_crtc_state {
 		struct drm_dp_as_sdp as_sdp;
 	} infoframes;
 
+	struct intel_dip dip;
+
 	u8 eld[MAX_ELD_BYTES];
 
 	/* HDMI scrambling status */
-- 
2.50.1

[PATCH v5 06/12] drm/i915/dip: Store and use AS SDP transmission line from crtc state

[Ankit Nautiyal]

The driver currently computes the Adaptive Sync SDP transmission line
directly at programming time. Instead, compute and store the
AS SDP transmission line in the crtc state and use it when programming the
EMP_AS_SDP_TL register.

We get the clear picture about the SDPs and guardband only in
intel_dp_sdp_compute_config_late() therefore we must configure the
AS SDP transmission line at this point when AS SDP is enabled in
crtc_state.

This prepares the ground for supporting programmable transmission lines
for additional DP SDPs.

While moving the helper into intel_dip.c, drop the
intel_crtc_has_dp_encoder() check instead of relocating it. It was
needed in the old VRR write path shared by other encoderes as well, but
intel_dip_sdp_tl_compute_config_late() is only reached via DP, so HDMI
never sets crtc_state->dip.emp_as_sdp_tl and it stays 0 by default.

v2:
- Move the helper into intel_dip.c and drop the
  intel_crtc_has_dp_encoder() check.
- Drop the redundant checks. (Suraj)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c |  2 ++
 drivers/gpu/drm/i915/display/intel_dip.c | 20 +++++++++++---------
 drivers/gpu/drm/i915/display/intel_dip.h |  3 +++
 drivers/gpu/drm/i915/display/intel_dp.c  |  2 ++
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index 9b3b526e5e55..dacb4b7588a0 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -49,6 +49,7 @@
 #include "intel_ddi.h"
 #include "intel_ddi_buf_trans.h"
 #include "intel_de.h"
+#include "intel_dip.h"
 #include "intel_display_power.h"
 #include "intel_display_regs.h"
 #include "intel_display_types.h"
@@ -4235,6 +4236,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_dip_sdp_transmission_line_get_config(pipe_config);
 
 	intel_audio_codec_get_config(encoder, pipe_config);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
index 0277b15e1c82..d1acc7eb5a39 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.c
+++ b/drivers/gpu/drm/i915/display/intel_dip.c
@@ -43,19 +43,21 @@ void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
 {
 	struct intel_display *display = to_intel_display(crtc_state);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-	u32 transmission_line = 0;
 
 	if (!HAS_EMP_AS_SDP_TL(display))
 		return;
-	/*
-	 * Since we currently support VRR only for DP/eDP, program the register
-	 * for Adaptive Sync SDP using vsync start. For non-DP encoders,
-	 * the register is reset to 0.
-	 */
-	if (intel_crtc_has_dp_encoder(crtc_state))
-		transmission_line = intel_dip_get_as_sdp_transmission_line(crtc_state);
 
 	intel_de_write(display,
 		       EMP_AS_SDP_TL(display, cpu_transcoder),
-		       EMP_AS_SDP_DB_TL(transmission_line));
+		       EMP_AS_SDP_DB_TL(crtc_state->dip.emp_as_sdp_tl));
+}
+
+void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->dip.emp_as_sdp_tl = intel_dip_get_as_sdp_transmission_line(crtc_state);
+}
+
+void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
+{
+	crtc_state->dip.emp_as_sdp_tl = intel_dip_read_emp_as_sdp_tl(crtc_state);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dip.h b/drivers/gpu/drm/i915/display/intel_dip.h
index 37507ac3e645..600dabbf7372 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.h
+++ b/drivers/gpu/drm/i915/display/intel_dip.h
@@ -43,4 +43,7 @@ struct intel_dip {
 	u16 emp_as_sdp_tl;
 };
 
+void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state);
+void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state);
+
 #endif /* __INTEL_DIP_H__ */
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0cd5e6b5034c..eed4fca2b985 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -7324,6 +7324,8 @@ int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
 		return -EINVAL;
 	}
 
+	intel_dip_sdp_tl_compute_config_late(crtc_state);
+
 	return 0;
 }
 
-- 
2.50.1

[PATCH v5 07/12] drm/i915/dip_regs: Add register definitions for common SDP Transmission Line

[Ankit Nautiyal]

From: Arun R Murthy <arun.r.murthy@intel.com>

Add registers definitions for common SDP transmission line CMN_SDP_TL
and CMN_SDP_TL_STGR_CTL.

v2: Move all registers to intel_dip_regs.h (Ankit)

Bspec: 74384
Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dip_regs.h | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dip_regs.h b/drivers/gpu/drm/i915/display/intel_dip_regs.h
index 85dcbf42e77d..7e69c5cf63da 100644
--- a/drivers/gpu/drm/i915/display/intel_dip_regs.h
+++ b/drivers/gpu/drm/i915/display/intel_dip_regs.h
@@ -14,4 +14,23 @@
 #define   EMP_AS_SDP_DB_TL_MASK			REG_GENMASK(12, 0)
 #define   EMP_AS_SDP_DB_TL(db_transmit_line)	REG_FIELD_PREP(EMP_AS_SDP_DB_TL_MASK, (db_transmit_line))
 
+/* COMMON SDP TRANSMISSION LINE */
+#define _CMN_SDP_TL_A			0x6020c
+#define CMN_SDP_TL(display, trans)	_MMIO_TRANS2(display, (trans), _CMN_SDP_TL_A)
+#define  TRANSMISSION_LINE_ENABLE	REG_BIT(31)
+#define  BASE_TRANSMISSION_LINE_MASK	REG_GENMASK(12, 0)
+#define  BASE_TRANSMISSION_LINE(x)	REG_FIELD_PREP(BASE_TRANSMISSION_LINE_MASK, x)
+
+#define _CMN_SDP_TL_STGR_CTL_A			0x60214
+#define CMN_SDP_TL_STGR_CTL(display, trans)	_MMIO_TRANS2(display, (trans), _CMN_SDP_TL_STGR_CTL_A)
+#define  VSC_EXT_STAGGER_MASK			REG_GENMASK(11, 8)
+#define  VSC_EXT_STAGGER(x)			REG_FIELD_PREP(VSC_EXT_STAGGER_MASK, x)
+#define  VSC_EXT_STAGGER_DEFAULT		0x2
+#define  PPS_STAGGER_MASK			REG_GENMASK(7, 4)
+#define  PPS_STAGGER(x)				REG_FIELD_PREP(PPS_STAGGER_MASK, x)
+#define  PPS_STAGGER_DEFAULT			0x1
+#define  GMP_STAGGER_MASK			REG_GENMASK(3, 0)
+#define  GMP_STAGGER(x)				REG_FIELD_PREP(GMP_STAGGER_MASK, x)
+#define  GMP_STAGGER_DEFAULT			0x0
+
 #endif /* __INTEL_DIP_REGS_H__ */
-- 
2.50.1

[PATCH v5 08/12] drm/i915/dip: Add HAS_COMMON_SDP_TL macro

[Ankit Nautiyal]

Add a helper macro to detect CMN SDP TL support on platforms with display
version 35 and above.

v2: Use prefix drm/i915/dip. (Suraj)

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dip.h | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dip.h b/drivers/gpu/drm/i915/display/intel_dip.h
index 600dabbf7372..e9959356226e 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.h
+++ b/drivers/gpu/drm/i915/display/intel_dip.h
@@ -32,6 +32,16 @@ struct intel_crtc_state;
 #define HAS_EMP_AS_SDP_TL(__display)	(DISPLAY_VERx100(__display) == 1401 || \
 					 DISPLAY_VER(__display) >= 20)
 
+/*
+ * CMN SDP TL: Common Secondary Data Packet Transmission Line.
+ *
+ * Xe3p_lpd introduces new register CMN_SDP_TL to program a common SDP
+ * Transmission line that will be used by the Hardware to position the
+ * SDPs. Along with this, another new register CMN_SDP_TL_STGR_CTL is
+ * also added to stagger the different SDPs.
+ */
+#define HAS_COMMON_SDP_TL(__display)      (DISPLAY_VER(__display) >= 35)
+
 u16 intel_dip_read_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
 void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state);
 
-- 
2.50.1

[PATCH v5 09/12] drm/i915/dip: Store SDP transmission lines in crtc_state

[Ankit Nautiyal]

Currently the driver only programs the transmission line for the
Adaptive-Sync SDP, while the hardware controls the transmission lines for
other SDPs.

Starting with Xe3p_lpd, the hardware allows the driver to program
transmission lines for additional DP SDPs. Prepare for this by adding
fields to struct intel_crtc_state to store SDP transmission lines, and
include them in pipe config comparison.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dip.h     | 6 ++++++
 drivers/gpu/drm/i915/display/intel_display.c | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dip.h b/drivers/gpu/drm/i915/display/intel_dip.h
index e9959356226e..03ef749a79ca 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.h
+++ b/drivers/gpu/drm/i915/display/intel_dip.h
@@ -51,6 +51,12 @@ struct intel_dip {
 	 * The programmed transmit line is (Vtotal - value)
 	 */
 	u16 emp_as_sdp_tl;
+	u16 gmp_sdp_tl;
+	u16 pps_sdp_tl;
+	u16 vsc_sdp_tl;
+	u16 vsc_ext_sdp_tl;
+	/* Common SDP Base transmission line (Xe3p_lpd+) */
+	u16 cmn_sdp_tl;
 };
 
 void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
index 6dd2626256cf..9f4be9bf150e 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -5604,6 +5604,11 @@ intel_pipe_config_compare(const struct intel_crtc_state *current_config,
 	PIPE_CONF_CHECK_I(set_context_latency);
 
 	PIPE_CONF_CHECK_I(dip.emp_as_sdp_tl);
+	PIPE_CONF_CHECK_I(dip.gmp_sdp_tl);
+	PIPE_CONF_CHECK_I(dip.pps_sdp_tl);
+	PIPE_CONF_CHECK_I(dip.vsc_sdp_tl);
+	PIPE_CONF_CHECK_I(dip.vsc_ext_sdp_tl);
+	PIPE_CONF_CHECK_I(dip.cmn_sdp_tl);
 
 #undef PIPE_CONF_CHECK_X
 #undef PIPE_CONF_CHECK_I
-- 
2.50.1

[PATCH v5 10/12] drm/i915/dp: Introduce helpers to enable/disable CMN SDP Transmission line

[Ankit Nautiyal]

Introduce helpers to program or disable CMN_SDP_TL and stagger registers
using the state stored in crtc_state.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dip.c | 56 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_dip.h |  2 +
 2 files changed, 58 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
index d1acc7eb5a39..0c80d6305fec 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.c
+++ b/drivers/gpu/drm/i915/display/intel_dip.c
@@ -4,6 +4,8 @@
  *
  */
 
+#include <drm/drm_print.h>
+
 #include "intel_de.h"
 #include "intel_dip.h"
 #include "intel_dip_regs.h"
@@ -61,3 +63,57 @@ void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_st
 {
 	crtc_state->dip.emp_as_sdp_tl = intel_dip_read_emp_as_sdp_tl(crtc_state);
 }
+
+static int intel_dip_sdp_tl_to_stagger(const struct intel_crtc_state *crtc_state,
+				       u16 sdp_transmission_line)
+{
+	return sdp_transmission_line - crtc_state->dip.cmn_sdp_tl;
+}
+
+void intel_dip_cmn_sdp_transmission_line_enable(const struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+	int gmp_stagger;
+	int pps_stagger;
+	int vsc_ext_stagger;
+
+	if (!crtc_state->dip.cmn_sdp_tl)
+		return;
+
+	gmp_stagger = intel_dip_sdp_tl_to_stagger(crtc_state,
+						  crtc_state->dip.gmp_sdp_tl);
+
+	pps_stagger = intel_dip_sdp_tl_to_stagger(crtc_state,
+						  crtc_state->dip.pps_sdp_tl);
+
+	vsc_ext_stagger = intel_dip_sdp_tl_to_stagger(crtc_state,
+						      crtc_state->dip.vsc_ext_sdp_tl);
+
+	if (drm_WARN_ON(display->drm, gmp_stagger < 0))
+		return;
+	if (drm_WARN_ON(display->drm, pps_stagger < 0))
+		return;
+	if (drm_WARN_ON(display->drm, vsc_ext_stagger < 0))
+		return;
+
+	intel_de_write(display, CMN_SDP_TL_STGR_CTL(display, cpu_transcoder),
+		       GMP_STAGGER(gmp_stagger) |
+		       PPS_STAGGER(pps_stagger) |
+		       VSC_EXT_STAGGER(vsc_ext_stagger));
+
+	intel_de_write(display, CMN_SDP_TL(display, cpu_transcoder),
+		       TRANSMISSION_LINE_ENABLE |
+		       BASE_TRANSMISSION_LINE(crtc_state->dip.cmn_sdp_tl));
+}
+
+void intel_dip_cmn_sdp_transmission_line_disable(const struct intel_crtc_state *old_crtc_state)
+{
+	struct intel_display *display = to_intel_display(old_crtc_state);
+	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
+
+	if (!old_crtc_state->dip.cmn_sdp_tl)
+		return;
+
+	intel_de_write(display, CMN_SDP_TL(display, cpu_transcoder), 0);
+}
diff --git a/drivers/gpu/drm/i915/display/intel_dip.h b/drivers/gpu/drm/i915/display/intel_dip.h
index 03ef749a79ca..24d6228c2f8d 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.h
+++ b/drivers/gpu/drm/i915/display/intel_dip.h
@@ -61,5 +61,7 @@ struct intel_dip {
 
 void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state);
 void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state);
+void intel_dip_cmn_sdp_transmission_line_enable(const struct intel_crtc_state *crtc_state);
+void intel_dip_cmn_sdp_transmission_line_disable(const struct intel_crtc_state *old_crtc_state);
 
 #endif /* __INTEL_DIP_H__ */
-- 
2.50.1

[PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line

[Ankit Nautiyal]

Enable programming of the common SDP transmission line on platforms that
support it. Compute and program the common base transmission line and
per-SDP stagger values from the crtc state during modeset, and disable the
feature on pipe disable.

Currently, the stagger values are set as per the default policy of the
Hardware. This can be optimized later if we come up with a specific driver
policy to sequence the SDPs better.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_ddi.c |  3 +
 drivers/gpu/drm/i915/display/intel_dip.c | 90 ++++++++++++++++++++++++
 2 files changed, 93 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
index dacb4b7588a0..3cdb06e81130 100644
--- a/drivers/gpu/drm/i915/display/intel_ddi.c
+++ b/drivers/gpu/drm/i915/display/intel_ddi.c
@@ -2737,6 +2737,8 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
 	/* 6.o Configure and enable FEC if needed */
 	intel_ddi_enable_fec(encoder, crtc_state);
 
+	intel_dip_cmn_sdp_transmission_line_enable(crtc_state);
+
 	/* 7.a 128b/132b SST. */
 	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
 		/* VCPID 1, start slot 0 for 128b/132b, tu slots */
@@ -3124,6 +3126,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
 			     DP_TP_CTL_ENABLE, 0);
 	}
 
+	intel_dip_cmn_sdp_transmission_line_disable(crtc_state);
 	intel_ddi_disable_fec(encoder, crtc_state);
 
 	if (DISPLAY_VER(display) < 14)
diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
index 0c80d6305fec..007c8fb87ecd 100644
--- a/drivers/gpu/drm/i915/display/intel_dip.c
+++ b/drivers/gpu/drm/i915/display/intel_dip.c
@@ -10,6 +10,7 @@
 #include "intel_dip.h"
 #include "intel_dip_regs.h"
 #include "intel_display_types.h"
+#include "intel_hdmi.h"
 
 static int intel_dip_get_as_sdp_transmission_line(const struct intel_crtc_state *crtc_state)
 {
@@ -54,14 +55,103 @@ void intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
 		       EMP_AS_SDP_DB_TL(crtc_state->dip.emp_as_sdp_tl));
 }
 
+static int intel_dip_sdp_stagger_to_tl(struct intel_crtc_state *crtc_state,
+				       int stagger)
+{
+	return crtc_state->dip.cmn_sdp_tl + stagger;
+}
+
+static
+void intel_dip_cmn_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
+{
+	struct intel_display *display = to_intel_display(crtc_state);
+	bool as_sdp;
+
+	if (!HAS_COMMON_SDP_TL(display))
+		return;
+
+	as_sdp = crtc_state->infoframes.enable &
+		 intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
+	/*
+	 * When AS SDP is enabled :
+	 *  - The common SDP Transmission Line matches the EMP SDP Transmission Line.
+	 *
+	 * When AS SDP is disabled:
+	 *  - Bspec mentions the positions as lines of delayed vblank.
+	 *  - Guardband = 1st line of delayed vblank
+	 *  - Common SDP Transmission line is set to 2nd line of delayed vblank.
+	 */
+
+	if (as_sdp)
+		crtc_state->dip.cmn_sdp_tl = crtc_state->dip.emp_as_sdp_tl;
+	else
+		crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1;
+
+	/*
+	 * Currently we are programming the default stagger values, but these
+	 * can be optimized if required, based on number of SDPs enabled.
+	 *
+	 * Default values of the Transmission lines for SDPs other than AS SDP:
+	 * VSC : CMN SDP Transmission line
+	 * GMP : CMN SDP Transmission line
+	 * PPS : CMN SDP Transmission line + 1
+	 * VSC_EXT: CMN SDP Transmission line + 2
+	 */
+	crtc_state->dip.vsc_sdp_tl = crtc_state->dip.cmn_sdp_tl;
+	crtc_state->dip.gmp_sdp_tl =
+		intel_dip_sdp_stagger_to_tl(crtc_state, GMP_STAGGER_DEFAULT);
+	crtc_state->dip.pps_sdp_tl =
+		intel_dip_sdp_stagger_to_tl(crtc_state, PPS_STAGGER_DEFAULT);
+	crtc_state->dip.vsc_ext_sdp_tl =
+		intel_dip_sdp_stagger_to_tl(crtc_state, VSC_EXT_STAGGER_DEFAULT);
+}
+
 void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
 {
 	crtc_state->dip.emp_as_sdp_tl = intel_dip_get_as_sdp_transmission_line(crtc_state);
+
+	intel_dip_cmn_sdp_tl_compute_config_late(crtc_state);
+}
+
+static
+void intel_dip_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;
+	u16 vsc_ext_stagger, pps_stagger, gmp_stagger;
+	u32 val;
+
+	if (!HAS_COMMON_SDP_TL(display))
+		return;
+
+	val = intel_de_read(display, CMN_SDP_TL(display, cpu_transcoder));
+
+	if (!(val & TRANSMISSION_LINE_ENABLE))
+		return;
+
+	crtc_state->dip.cmn_sdp_tl = REG_FIELD_GET(BASE_TRANSMISSION_LINE_MASK, val);
+
+	/* SDP VSC uses same transmission line as CMN base transmission line */
+	crtc_state->dip.vsc_sdp_tl = crtc_state->dip.cmn_sdp_tl;
+
+	val = intel_de_read(display, CMN_SDP_TL_STGR_CTL(display, cpu_transcoder));
+
+	vsc_ext_stagger = REG_FIELD_GET(VSC_EXT_STAGGER_MASK, val);
+	pps_stagger = REG_FIELD_GET(PPS_STAGGER_MASK, val);
+	gmp_stagger = REG_FIELD_GET(GMP_STAGGER_MASK, val);
+
+	crtc_state->dip.vsc_ext_sdp_tl =
+		intel_dip_sdp_stagger_to_tl(crtc_state, vsc_ext_stagger);
+	crtc_state->dip.pps_sdp_tl =
+		intel_dip_sdp_stagger_to_tl(crtc_state, pps_stagger);
+	crtc_state->dip.gmp_sdp_tl =
+		intel_dip_sdp_stagger_to_tl(crtc_state, gmp_stagger);
 }
 
 void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
 {
 	crtc_state->dip.emp_as_sdp_tl = intel_dip_read_emp_as_sdp_tl(crtc_state);
+	intel_dip_cmn_sdp_transmission_line_get_config(crtc_state);
 }
 
 static int intel_dip_sdp_tl_to_stagger(const struct intel_crtc_state *crtc_state,
-- 
2.50.1

[PATCH v5 12/12] drm/i915/display: Dump DIP Transmission lines

[Ankit Nautiyal]

Add DIP transmission lines to the CRTC state dump.

Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
---
 drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
index ad4f362e0c09..92675a88b969 100644
--- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
+++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
@@ -251,6 +251,15 @@ void intel_crtc_state_dump(const struct intel_crtc_state *pipe_config,
 			   str_enabled_disabled(pipe_config->has_panel_replay),
 			   str_enabled_disabled(pipe_config->enable_psr2_sel_fetch));
 		drm_printf(&p, "minimum hblank: %d\n", pipe_config->min_hblank);
+
+		drm_printf(&p, "DIP Transmission Lines: EMP/AS SDP: %u\n",
+			   pipe_config->dip.emp_as_sdp_tl);
+		drm_printf(&p, "DIP Transmission Lines: Common Base SDP: %u, GMP SDP: %u, PPS SDP: %u, VSC SDP: %u, VSC_EXT SDP: %u\n",
+			   pipe_config->dip.cmn_sdp_tl,
+			   pipe_config->dip.gmp_sdp_tl,
+			   pipe_config->dip.pps_sdp_tl,
+			   pipe_config->dip.vsc_sdp_tl,
+			   pipe_config->dip.vsc_ext_sdp_tl);
 	}
 
 	drm_printf(&p, "audio: %i, infoframes: %i, infoframes enabled: 0x%x\n",
-- 
2.50.1

Re: [PATCH v5 06/12] drm/i915/dip: Store and use AS SDP transmission line from crtc state

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Spurious state mismatch warnings for dip.emp_as_sdp_tl due to unconditional hardware readout of stale register values.
- [Medium] Unconditional fastset check for `dip.emp_as_sdp_tl` breaks seamless Link Refresh Rate (LRR) updates, forcing heavy full modesets.
--

commit fdc12bb4795c43a7d04cbec4746e152e2a99a6d8
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/dip: Store and use AS SDP transmission line from crtc state

This commit moves the computation of the Adaptive Sync SDP transmission
line from programming time to the crtc state computation phase. It stores
the value in the crtc state to be used when programming the EMP_AS_SDP_TL
register, preparing for programmable transmission lines for other DP SDPs.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
[ ... ]
> @@ -4235,6 +4236,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_dip_sdp_transmission_line_get_config(pipe_config);

[Severity: Medium]
Could this unconditional hardware readout capture stale values and cause
spurious state mismatch warnings?

intel_dip_sdp_transmission_line_get_config() unconditionally reads the
EMP_AS_SDP_TL hardware register. If the encoder is HDMI, or if VRR was
disabled, the computed software state leaves dip.emp_as_sdp_tl = 0.

However, when VRR is disabled, intel_vrr_set_transcoder_timings() in
drivers/gpu/drm/i915/display/intel_vrr.c returns early and never clears
EMP_AS_SDP_TL. This leaves a stale non-zero value in the hardware register
from a previous DP modeset.

Could this mismatch cause intel_pipe_config_compare() to fail, incorrectly
forcing an unnecessary full modeset instead of a fastset?

> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
> --- a/drivers/gpu/drm/i915/display/intel_dip.c
> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
[ ... ]
> +void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
> +{
> +	crtc_state->dip.emp_as_sdp_tl = intel_dip_get_as_sdp_transmission_line(crtc_state);
> +}

[Severity: Medium]
Does computing and checking this transmission line state break seamless
Link Refresh Rate (LRR) updates?

LRR is designed to perform seamless refresh rate transitions without screen
blanking. The intel_pipe_config_compare() function in intel_display.c
ignores vrr.vsync_start differences during a fastset because it is
explicitly guarded by an if (!fastset) condition.

Because dip.emp_as_sdp_tl is computed directly from vrr.vsync_start, and
its corresponding state check (PIPE_CONF_CHECK_I) runs outside the
!fastset block, an LRR transition that naturally alters the vtotal and
vsync_start will produce a mismatch.

Will this erroneously fail the fastset check and force a heavy full modeset
instead of a seamless transition?

> +
> +void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
> +{
> +	crtc_state->dip.emp_as_sdp_tl = intel_dip_read_emp_as_sdp_tl(crtc_state);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908062224.2897987-1-ankit.k.nautiyal@intel.com?part=6

Re: [PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Integer underflow when computing cmn_sdp_tl with guardband=0 causes a kernel warning, and a valid transmission line of 0 is incorrectly treated as 'disabled'.
- [Medium] Per-transcoder SDP transmission line programming is incorrectly placed in port-level enable/disable paths, breaking DP MST secondary streams.
--

commit dcefae9ad5800e7d0f8f1d55a58dfb400880ed28
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/dip: Enable Common SDP Transmission line

This commit enables programming of the common SDP transmission line on
platforms that support it. It computes and programs the common base
transmission line and per-SDP stagger values from the crtc state during
modeset, and disables the feature on pipe disable.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2737,6 +2737,8 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>  	/* 6.o Configure and enable FEC if needed */
>  	intel_ddi_enable_fec(encoder, crtc_state);
>  
> +	intel_dip_cmn_sdp_transmission_line_enable(crtc_state);
> +

[Severity: Medium]
Since mtl_ddi_pre_enable_dp() acts at the port level and only executes for
the first stream in a DP MST topology, will secondary streams' transcoders
miss having their SDP transmission line initialized?

It seems MST secondary streams will bypass this hook entirely. Furthermore,
mst_stream_compute_config_late() in intel_dp_mst.c doesn't appear to call
intel_dip_cmn_sdp_tl_compute_config_late(), meaning the values aren't
computed for MST streams.

>  	/* 7.a 128b/132b SST. */
>  	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
>  		/* VCPID 1, start slot 0 for 128b/132b, tu slots */
[ ... ]
> @@ -3124,6 +3126,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
>  			     DP_TP_CTL_ENABLE, 0);
>  	}
>  
> +	intel_dip_cmn_sdp_transmission_line_disable(crtc_state);

[Severity: Medium]
Similarly, since this port-level disable hook only executes when the last
stream is disabled, does this leave the transmission line registers active
for other transcoders in an MST setup when they are individually disabled?

>  	intel_ddi_disable_fec(encoder, crtc_state);
>  
>  	if (DISPLAY_VER(display) < 14)
> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
> --- a/drivers/gpu/drm/i915/display/intel_dip.c
> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
[ ... ]
> +static
> +void intel_dip_cmn_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
> +{
[ ... ]
> +	if (as_sdp)
> +		crtc_state->dip.cmn_sdp_tl = crtc_state->dip.emp_as_sdp_tl;
> +	else
> +		crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1;

[Severity: High]
If crtc_state->vrr.guardband is 0 (for example, when VRR is not used), will
this subtraction underflow and set the u16 cmn_sdp_tl to 65535?

If it underflows to 65535, pps_stagger evaluates to a negative value 
(0 - 65535) in intel_dip_cmn_sdp_transmission_line_enable(). Will this trigger
the drm_WARN_ON(display->drm, pps_stagger < 0) and abort enabling the
transmission line?

Also, if guardband evaluates to 1, cmn_sdp_tl will be 0. Does the
!crtc_state->dip.cmn_sdp_tl check in intel_dip_cmn_sdp_transmission_line_enable()
and disable() incorrectly treat a valid transmission line value of 0 as a
sentinel for 'feature disabled', silently skipping the hardware enablement?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908062224.2897987-1-ankit.k.nautiyal@intel.com?part=11

Re: [PATCH v5 10/12] drm/i915/dp: Introduce helpers to enable/disable CMN SDP Transmission line

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Using cmn_sdp_tl == 0 as a condition to skip hardware programming incorrectly disables valid configurations where the base transmission line is exactly 0.
--

commit add857e2d6bd573b97121b3d862f0f6816f3b50e
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/dp: Introduce helpers to enable/disable CMN SDP Transmission line

This commit introduces helper functions to enable and disable the Common SDP
Transmission Line (CMN_SDP_TL) and its stagger registers based on the
configuration stored in crtc_state.

> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
> index d1acc7eb5a391..0c80d6305feca 100644
> --- a/drivers/gpu/drm/i915/display/intel_dip.c
> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
[ ... ]
> @@ -61,3 +63,57 @@ void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_st
> +void intel_dip_cmn_sdp_transmission_line_enable(const struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> +	int gmp_stagger;
> +	int pps_stagger;
> +	int vsc_ext_stagger;
> +
> +	if (!crtc_state->dip.cmn_sdp_tl)
> +		return;

[Severity: High]
Does this incorrectly skip hardware programming for a valid configuration?

Since the transmission line can be computed as crtc_state->vrr.guardband - 1,
if guardband evaluates to 1, cmn_sdp_tl becomes 0. Since 0 is a
mathematically valid 0-based register index, this early return will
incorrectly treat a valid base transmission line of 0 as disabled, skipping
the register programming entirely.

[ ... ]
> +void intel_dip_cmn_sdp_transmission_line_disable(const struct intel_crtc_state *old_crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(old_crtc_state);
> +	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
> +
> +	if (!old_crtc_state->dip.cmn_sdp_tl)
> +		return;

[Severity: High]
Similarly, does this skip disabling the transmission line if the valid
configured base transmission line was exactly 0? 

If the base transmission line was initially programmed as 0, this early
return prevents the register from being cleared on disable.

> +
> +	intel_de_write(display, CMN_SDP_TL(display, cpu_transcoder), 0);
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908062224.2897987-1-ankit.k.nautiyal@intel.com?part=10

RE: [PATCH v5 07/12] drm/i915/dip_regs: Add register definitions for common SDP Transmission Line

[Kandpal, Suraj]

> Subject: [PATCH v5 07/12] drm/i915/dip_regs: Add register definitions for
> common SDP Transmission Line
> 
> From: Arun R Murthy <arun.r.murthy@intel.com>
> 
> Add registers definitions for common SDP transmission line CMN_SDP_TL and
> CMN_SDP_TL_STGR_CTL.
> 
> v2: Move all registers to intel_dip_regs.h (Ankit)
> 
> Bspec: 74384
> Signed-off-by: Arun R Murthy <arun.r.murthy@intel.com>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dip_regs.h | 19 +++++++++++++++++++
>  1 file changed, 19 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dip_regs.h
> b/drivers/gpu/drm/i915/display/intel_dip_regs.h
> index 85dcbf42e77d..7e69c5cf63da 100644
> --- a/drivers/gpu/drm/i915/display/intel_dip_regs.h
> +++ b/drivers/gpu/drm/i915/display/intel_dip_regs.h
> @@ -14,4 +14,23 @@
>  #define   EMP_AS_SDP_DB_TL_MASK			REG_GENMASK(12, 0)
>  #define   EMP_AS_SDP_DB_TL(db_transmit_line)
> 	REG_FIELD_PREP(EMP_AS_SDP_DB_TL_MASK, (db_transmit_line))
> 
> +/* COMMON SDP TRANSMISSION LINE */
> +#define _CMN_SDP_TL_A			0x6020c
> +#define CMN_SDP_TL(display, trans)	_MMIO_TRANS2(display, (trans),
> _CMN_SDP_TL_A)
> +#define  TRANSMISSION_LINE_ENABLE	REG_BIT(31)
> +#define  BASE_TRANSMISSION_LINE_MASK	REG_GENMASK(12, 0)
> +#define  BASE_TRANSMISSION_LINE(x)
> 	REG_FIELD_PREP(BASE_TRANSMISSION_LINE_MASK, x)
> +
> +#define _CMN_SDP_TL_STGR_CTL_A			0x60214
> +#define CMN_SDP_TL_STGR_CTL(display, trans)
> 	_MMIO_TRANS2(display, (trans), _CMN_SDP_TL_STGR_CTL_A)
> +#define  VSC_EXT_STAGGER_MASK			REG_GENMASK(11, 8)
> +#define  VSC_EXT_STAGGER(x)
> 	REG_FIELD_PREP(VSC_EXT_STAGGER_MASK, x)
> +#define  VSC_EXT_STAGGER_DEFAULT		0x2
> +#define  PPS_STAGGER_MASK			REG_GENMASK(7, 4)
> +#define  PPS_STAGGER(x)
> 	REG_FIELD_PREP(PPS_STAGGER_MASK, x)
> +#define  PPS_STAGGER_DEFAULT			0x1
> +#define  GMP_STAGGER_MASK			REG_GENMASK(3, 0)
> +#define  GMP_STAGGER(x)
> 	REG_FIELD_PREP(GMP_STAGGER_MASK, x)
> +#define  GMP_STAGGER_DEFAULT			0x0
> +
>  #endif /* __INTEL_DIP_REGS_H__ */
> --
> 2.50.1

RE: [PATCH v5 09/12] drm/i915/dip: Store SDP transmission lines in crtc_state

[Kandpal, Suraj]

> Subject: [PATCH v5 09/12] drm/i915/dip: Store SDP transmission lines in
> crtc_state
> 
> Currently the driver only programs the transmission line for the Adaptive-Sync
> SDP, while the hardware controls the transmission lines for other SDPs.
> 
> Starting with Xe3p_lpd, the hardware allows the driver to program transmission
> lines for additional DP SDPs. Prepare for this by adding fields to struct
> intel_crtc_state to store SDP transmission lines, and include them in pipe config
> comparison.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_dip.h     | 6 ++++++
>  drivers/gpu/drm/i915/display/intel_display.c | 5 +++++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dip.h
> b/drivers/gpu/drm/i915/display/intel_dip.h
> index e9959356226e..03ef749a79ca 100644
> --- a/drivers/gpu/drm/i915/display/intel_dip.h
> +++ b/drivers/gpu/drm/i915/display/intel_dip.h
> @@ -51,6 +51,12 @@ struct intel_dip {
>  	 * The programmed transmit line is (Vtotal - value)
>  	 */
>  	u16 emp_as_sdp_tl;
> +	u16 gmp_sdp_tl;
> +	u16 pps_sdp_tl;
> +	u16 vsc_sdp_tl;
> +	u16 vsc_ext_sdp_tl;
> +	/* Common SDP Base transmission line (Xe3p_lpd+) */
> +	u16 cmn_sdp_tl;
>  };
> 
>  void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state);
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c
> b/drivers/gpu/drm/i915/display/intel_display.c
> index 6dd2626256cf..9f4be9bf150e 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -5604,6 +5604,11 @@ intel_pipe_config_compare(const struct
> intel_crtc_state *current_config,
>  	PIPE_CONF_CHECK_I(set_context_latency);
> 
>  	PIPE_CONF_CHECK_I(dip.emp_as_sdp_tl);
> +	PIPE_CONF_CHECK_I(dip.gmp_sdp_tl);
> +	PIPE_CONF_CHECK_I(dip.pps_sdp_tl);
> +	PIPE_CONF_CHECK_I(dip.vsc_sdp_tl);
> +	PIPE_CONF_CHECK_I(dip.vsc_ext_sdp_tl);
> +	PIPE_CONF_CHECK_I(dip.cmn_sdp_tl);
> 
>  #undef PIPE_CONF_CHECK_X
>  #undef PIPE_CONF_CHECK_I
> --
> 2.50.1

RE: [PATCH v5 12/12] drm/i915/display: Dump DIP Transmission lines

[Kandpal, Suraj]

> Subject: [PATCH v5 12/12] drm/i915/display: Dump DIP Transmission lines
> 
> Add DIP transmission lines to the CRTC state dump.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

LGTM,
Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_crtc_state_dump.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> index ad4f362e0c09..92675a88b969 100644
> --- a/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> +++ b/drivers/gpu/drm/i915/display/intel_crtc_state_dump.c
> @@ -251,6 +251,15 @@ void intel_crtc_state_dump(const struct
> intel_crtc_state *pipe_config,
>  			   str_enabled_disabled(pipe_config-
> >has_panel_replay),
>  			   str_enabled_disabled(pipe_config-
> >enable_psr2_sel_fetch));
>  		drm_printf(&p, "minimum hblank: %d\n", pipe_config-
> >min_hblank);
> +
> +		drm_printf(&p, "DIP Transmission Lines: EMP/AS SDP: %u\n",
> +			   pipe_config->dip.emp_as_sdp_tl);
> +		drm_printf(&p, "DIP Transmission Lines: Common Base SDP:
> %u, GMP SDP: %u, PPS SDP: %u, VSC SDP: %u, VSC_EXT SDP: %u\n",
> +			   pipe_config->dip.cmn_sdp_tl,
> +			   pipe_config->dip.gmp_sdp_tl,
> +			   pipe_config->dip.pps_sdp_tl,
> +			   pipe_config->dip.vsc_sdp_tl,
> +			   pipe_config->dip.vsc_ext_sdp_tl);
>  	}
> 
>  	drm_printf(&p, "audio: %i, infoframes: %i, infoframes enabled:
> 0x%x\n",
> --
> 2.50.1

RE: [PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line

[Kandpal, Suraj]

> Subject: [PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line
> 
> Enable programming of the common SDP transmission line on platforms that
> support it. Compute and program the common base transmission line and per-
> SDP stagger values from the crtc state during modeset, and disable the feature
> on pipe disable.
> 
> Currently, the stagger values are set as per the default policy of the Hardware.
> This can be optimized later if we come up with a specific driver policy to
> sequence the SDPs better.
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_ddi.c |  3 +
> drivers/gpu/drm/i915/display/intel_dip.c | 90 ++++++++++++++++++++++++
>  2 files changed, 93 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
> b/drivers/gpu/drm/i915/display/intel_ddi.c
> index dacb4b7588a0..3cdb06e81130 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> @@ -2737,6 +2737,8 @@ static void mtl_ddi_pre_enable_dp(struct
> intel_atomic_state *state,
>  	/* 6.o Configure and enable FEC if needed */
>  	intel_ddi_enable_fec(encoder, crtc_state);
> 
> +	intel_dip_cmn_sdp_transmission_line_enable(crtc_state);
> +
>  	/* 7.a 128b/132b SST. */
>  	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
>  		/* VCPID 1, start slot 0 for 128b/132b, tu slots */ @@ -3124,6
> +3126,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
>  			     DP_TP_CTL_ENABLE, 0);
>  	}
> 
> +	intel_dip_cmn_sdp_transmission_line_disable(crtc_state);
>  	intel_ddi_disable_fec(encoder, crtc_state);
> 
>  	if (DISPLAY_VER(display) < 14)
> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c
> b/drivers/gpu/drm/i915/display/intel_dip.c
> index 0c80d6305fec..007c8fb87ecd 100644
> --- a/drivers/gpu/drm/i915/display/intel_dip.c
> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
> @@ -10,6 +10,7 @@
>  #include "intel_dip.h"
>  #include "intel_dip_regs.h"
>  #include "intel_display_types.h"
> +#include "intel_hdmi.h"
> 
>  static int intel_dip_get_as_sdp_transmission_line(const struct intel_crtc_state
> *crtc_state)  { @@ -54,14 +55,103 @@ void
> intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
>  		       EMP_AS_SDP_DB_TL(crtc_state->dip.emp_as_sdp_tl));
>  }
> 
> +static int intel_dip_sdp_stagger_to_tl(struct intel_crtc_state *crtc_state,
> +				       int stagger)
> +{
> +	return crtc_state->dip.cmn_sdp_tl + stagger; }
> +
> +static
> +void intel_dip_cmn_sdp_tl_compute_config_late(struct intel_crtc_state
> +*crtc_state) {
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	bool as_sdp;
> +
> +	if (!HAS_COMMON_SDP_TL(display))
> +		return;
> +
> +	as_sdp = crtc_state->infoframes.enable &

Since as_sdp is bool does this need to be "&&"

> +		 intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
> +	/*
> +	 * When AS SDP is enabled :
> +	 *  - The common SDP Transmission Line matches the EMP SDP
> Transmission Line.
> +	 *
> +	 * When AS SDP is disabled:
> +	 *  - Bspec mentions the positions as lines of delayed vblank.
> +	 *  - Guardband = 1st line of delayed vblank
> +	 *  - Common SDP Transmission line is set to 2nd line of delayed vblank.
> +	 */
> +
> +	if (as_sdp)
> +		crtc_state->dip.cmn_sdp_tl = crtc_state->dip.emp_as_sdp_tl;
> +	else
> +		crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1;
> +

According to Bspec 74384
The transmission line for each SDP type must not be within the V. Active region and should be positioned at least one line into the V. Blank (see "Transcoder VRR Function" for definition of Vmin)
(V. Active + 1) <= (Vmin - SDP TL)

When Panel Replay and AS SDPs are enabled, the eDP/DP standard requires the VSC SDP indicating PR Active to be sent out before the AS SDP. Therefore, the following programming restriction is required for this configuration:
CMN_SDP_TL >= EMP_AS_SDP_TL

If PSR1 is going to be used with the Sink, then Software must position CMN_SDP_TL on the second line of the VRR Guardband (i.e. CMN_SDP_TL = (VRR Guardband-1))

Don't we need to check these restrictions

Also according to Bspec " crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1 " only when using PSR1

Regards,
Suraj Kandpal

> +	/*
> +	 * Currently we are programming the default stagger values, but these
> +	 * can be optimized if required, based on number of SDPs enabled.
> +	 *
> +	 * Default values of the Transmission lines for SDPs other than AS SDP:
> +	 * VSC : CMN SDP Transmission line
> +	 * GMP : CMN SDP Transmission line
> +	 * PPS : CMN SDP Transmission line + 1
> +	 * VSC_EXT: CMN SDP Transmission line + 2
> +	 */
> +	crtc_state->dip.vsc_sdp_tl = crtc_state->dip.cmn_sdp_tl;
> +	crtc_state->dip.gmp_sdp_tl =
> +		intel_dip_sdp_stagger_to_tl(crtc_state,
> GMP_STAGGER_DEFAULT);
> +	crtc_state->dip.pps_sdp_tl =
> +		intel_dip_sdp_stagger_to_tl(crtc_state,
> PPS_STAGGER_DEFAULT);
> +	crtc_state->dip.vsc_ext_sdp_tl =
> +		intel_dip_sdp_stagger_to_tl(crtc_state,
> VSC_EXT_STAGGER_DEFAULT); }
> +
>  void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
> {
>  	crtc_state->dip.emp_as_sdp_tl =
> intel_dip_get_as_sdp_transmission_line(crtc_state);
> +
> +	intel_dip_cmn_sdp_tl_compute_config_late(crtc_state);
> +}
> +
> +static
> +void intel_dip_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;
> +	u16 vsc_ext_stagger, pps_stagger, gmp_stagger;
> +	u32 val;
> +
> +	if (!HAS_COMMON_SDP_TL(display))
> +		return;
> +
> +	val = intel_de_read(display, CMN_SDP_TL(display, cpu_transcoder));
> +
> +	if (!(val & TRANSMISSION_LINE_ENABLE))
> +		return;
> +
> +	crtc_state->dip.cmn_sdp_tl =
> +REG_FIELD_GET(BASE_TRANSMISSION_LINE_MASK, val);
> +
> +	/* SDP VSC uses same transmission line as CMN base transmission line
> */
> +	crtc_state->dip.vsc_sdp_tl = crtc_state->dip.cmn_sdp_tl;
> +
> +	val = intel_de_read(display, CMN_SDP_TL_STGR_CTL(display,
> +cpu_transcoder));
> +
> +	vsc_ext_stagger = REG_FIELD_GET(VSC_EXT_STAGGER_MASK, val);
> +	pps_stagger = REG_FIELD_GET(PPS_STAGGER_MASK, val);
> +	gmp_stagger = REG_FIELD_GET(GMP_STAGGER_MASK, val);
> +
> +	crtc_state->dip.vsc_ext_sdp_tl =
> +		intel_dip_sdp_stagger_to_tl(crtc_state, vsc_ext_stagger);
> +	crtc_state->dip.pps_sdp_tl =
> +		intel_dip_sdp_stagger_to_tl(crtc_state, pps_stagger);
> +	crtc_state->dip.gmp_sdp_tl =
> +		intel_dip_sdp_stagger_to_tl(crtc_state, gmp_stagger);
>  }
> 
>  void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state
> *crtc_state)  {
>  	crtc_state->dip.emp_as_sdp_tl =
> intel_dip_read_emp_as_sdp_tl(crtc_state);
> +	intel_dip_cmn_sdp_transmission_line_get_config(crtc_state);
>  }
> 
>  static int intel_dip_sdp_tl_to_stagger(const struct intel_crtc_state *crtc_state,
> --
> 2.50.1

✗ i915.CI.BAT: failure for Add support for Common SDP Transmission Line (rev5)

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 1797 bytes --]

== Series Details ==

Series: Add support for Common SDP Transmission Line (rev5)
URL   : https://patchwork.freedesktop.org/series/162622/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_19102 -> Patchwork_162622v5
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_162622v5 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_162622v5, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162622v5/index.html

Participating hosts (39 -> 37)
------------------------------

  Missing    (2): bat-dg2-13 bat-adls-6 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_162622v5:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-apl-1:          [PASS][1] -> [DMESG-WARN][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_19102/bat-apl-1/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162622v5/bat-apl-1/igt@i915_module_load@load.html

  


Build changes
-------------

  * Linux: CI_DRM_19102 -> Patchwork_162622v5

  CI-20190529: 20190529
  CI_DRM_19102: 06602b51786b6c2a0f97e58f0e0f168be1ada27e @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_9085: 9085
  Patchwork_162622v5: 06602b51786b6c2a0f97e58f0e0f168be1ada27e @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_162622v5/index.html

[-- Attachment #2: Type: text/html, Size: 2399 bytes --]

Re: [PATCH v5 06/12] drm/i915/dip: Store and use AS SDP transmission line from crtc state

[Nautiyal, Ankit K]

On 9/8/2026 12:39 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [Medium] Spurious state mismatch warnings for dip.emp_as_sdp_tl due to unconditional hardware readout of stale register values.
> - [Medium] Unconditional fastset check for `dip.emp_as_sdp_tl` breaks seamless Link Refresh Rate (LRR) updates, forcing heavy full modesets.
> --
>
> commit fdc12bb4795c43a7d04cbec4746e152e2a99a6d8
> Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>
> drm/i915/dip: Store and use AS SDP transmission line from crtc state
>
> This commit moves the computation of the Adaptive Sync SDP transmission
> line from programming time to the crtc state computation phase. It stores
> the value in the crtc state to be used when programming the EMP_AS_SDP_TL
> register, preparing for programmable transmission lines for other DP SDPs.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
> [ ... ]
>> @@ -4235,6 +4236,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_dip_sdp_transmission_line_get_config(pipe_config);
> [Severity: Medium]
> Could this unconditional hardware readout capture stale values and cause
> spurious state mismatch warnings?
>
> intel_dip_sdp_transmission_line_get_config() unconditionally reads the
> EMP_AS_SDP_TL hardware register. If the encoder is HDMI, or if VRR was
> disabled, the computed software state leaves dip.emp_as_sdp_tl = 0.
>
> However, when VRR is disabled, intel_vrr_set_transcoder_timings() in
> drivers/gpu/drm/i915/display/intel_vrr.c returns early and never clears
> EMP_AS_SDP_TL. This leaves a stale non-zero value in the hardware register
> from a previous DP modeset.
>
> Could this mismatch cause intel_pipe_config_compare() to fail, incorrectly
> forcing an unnecessary full modeset instead of a fastset?

intel_vrr_possible() (vrr.flipline) can only be 0 via !HAS_VRR(display) 
or an interlaced mode.

On every HAS_EMP_AS_SDP_TL-capable platform, intel_vrr_possible() is 
true since:

- HAS_VRR() is true for those platforms

- interlace modes are not supported on those platforms.

flipline gets set (non-zero) in both the VRR and fixed-refresh-rate 
branches of intel_vrr_compute_config(), so it's non-zero regardless of 
encoder type or whether VRR is actually enabled.

The early-return path is unreachable, so no fix is needed here.


>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
>> --- a/drivers/gpu/drm/i915/display/intel_dip.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
> [ ... ]
>> +void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
>> +{
>> +	crtc_state->dip.emp_as_sdp_tl = intel_dip_get_as_sdp_transmission_line(crtc_state);
>> +}
> [Severity: Medium]
> Does computing and checking this transmission line state break seamless
> Link Refresh Rate (LRR) updates?
>
> LRR is designed to perform seamless refresh rate transitions without screen
> blanking. The intel_pipe_config_compare() function in intel_display.c
> ignores vrr.vsync_start differences during a fastset because it is
> explicitly guarded by an if (!fastset) condition.
>
> Because dip.emp_as_sdp_tl is computed directly from vrr.vsync_start, and
> its corresponding state check (PIPE_CONF_CHECK_I) runs outside the
> !fastset block, an LRR transition that naturally alters the vtotal and
> vsync_start will produce a mismatch.
>
> Will this erroneously fail the fastset check and force a heavy full modeset
> instead of a seamless transition?


hmm the dip.emp_as_sdp_tl and other cmn_sdp_tl can change on the fly, so 
makes sense to put them under !fastset check.


Thanks & Regards,

Ankit


>
>> +
>> +void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_state)
>> +{
>> +	crtc_state->dip.emp_as_sdp_tl = intel_dip_read_emp_as_sdp_tl(crtc_state);
>>   }

Re: [PATCH v5 10/12] drm/i915/dp: Introduce helpers to enable/disable CMN SDP Transmission line

[Nautiyal, Ankit K]

On 9/8/2026 12:42 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [High] Using cmn_sdp_tl == 0 as a condition to skip hardware programming incorrectly disables valid configurations where the base transmission line is exactly 0.
> --
>
> commit add857e2d6bd573b97121b3d862f0f6816f3b50e
> Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>
> drm/i915/dp: Introduce helpers to enable/disable CMN SDP Transmission line
>
> This commit introduces helper functions to enable and disable the Common SDP
> Transmission Line (CMN_SDP_TL) and its stagger registers based on the
> configuration stored in crtc_state.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
>> index d1acc7eb5a391..0c80d6305feca 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dip.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
> [ ... ]
>> @@ -61,3 +63,57 @@ void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state *crtc_st
>> +void intel_dip_cmn_sdp_transmission_line_enable(const struct intel_crtc_state *crtc_state)
>> +{
>> +	struct intel_display *display = to_intel_display(crtc_state);
>> +	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>> +	int gmp_stagger;
>> +	int pps_stagger;
>> +	int vsc_ext_stagger;
>> +
>> +	if (!crtc_state->dip.cmn_sdp_tl)
>> +		return;
> [Severity: High]
> Does this incorrectly skip hardware programming for a valid configuration?
>
> Since the transmission line can be computed as crtc_state->vrr.guardband - 1,
> if guardband evaluates to 1, cmn_sdp_tl becomes 0. Since 0 is a
> mathematically valid 0-based register index, this early return will
> incorrectly treat a valid base transmission line of 0 as disabled, skipping
> the register programming entirely.
>
> [ ... ]
>> +void intel_dip_cmn_sdp_transmission_line_disable(const struct intel_crtc_state *old_crtc_state)
>> +{
>> +	struct intel_display *display = to_intel_display(old_crtc_state);
>> +	enum transcoder cpu_transcoder = old_crtc_state->cpu_transcoder;
>> +
>> +	if (!old_crtc_state->dip.cmn_sdp_tl)
>> +		return;
> [Severity: High]
> Similarly, does this skip disabling the transmission line if the valid
> configured base transmission line was exactly 0?
>
> If the base transmission line was initially programmed as 0, this early
> return prevents the register from being cleared on disable.


Thanks for pointing this out. Makes sense to gate on 
HAS_COMMON_SDP_TL(display) instead of checking 
crtc_state->dip.cmn_sdp_tl in both places, since 0 is a valid 
transmission line value.

Will fix in next version.


Thanks & Regards,

Ankit

>
>> +
>> +	intel_de_write(display, CMN_SDP_TL(display, cpu_transcoder), 0);
>> +}

Re: [PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line

[Nautiyal, Ankit K]

On 9/8/2026 12:41 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
> - [High] Integer underflow when computing cmn_sdp_tl with guardband=0 causes a kernel warning, and a valid transmission line of 0 is incorrectly treated as 'disabled'.
> - [Medium] Per-transcoder SDP transmission line programming is incorrectly placed in port-level enable/disable paths, breaking DP MST secondary streams.
> --
>
> commit dcefae9ad5800e7d0f8f1d55a58dfb400880ed28
> Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>
> drm/i915/dip: Enable Common SDP Transmission line
>
> This commit enables programming of the common SDP transmission line on
> platforms that support it. It computes and programs the common base
> transmission line and per-SDP stagger values from the crtc state during
> modeset, and disables the feature on pipe disable.
>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2737,6 +2737,8 @@ static void mtl_ddi_pre_enable_dp(struct intel_atomic_state *state,
>>   	/* 6.o Configure and enable FEC if needed */
>>   	intel_ddi_enable_fec(encoder, crtc_state);
>>   
>> +	intel_dip_cmn_sdp_transmission_line_enable(crtc_state);
>> +
> [Severity: Medium]
> Since mtl_ddi_pre_enable_dp() acts at the port level and only executes for
> the first stream in a DP MST topology, will secondary streams' transcoders
> miss having their SDP transmission line initialized?
>
> It seems MST secondary streams will bypass this hook entirely. Furthermore,
> mst_stream_compute_config_late() in intel_dp_mst.c doesn't appear to call
> intel_dip_cmn_sdp_tl_compute_config_late(), meaning the values aren't
> computed for MST streams.

VRR is not set for MST yet so we are covered.


>
>>   	/* 7.a 128b/132b SST. */
>>   	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
>>   		/* VCPID 1, start slot 0 for 128b/132b, tu slots */
> [ ... ]
>> @@ -3124,6 +3126,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
>>   			     DP_TP_CTL_ENABLE, 0);
>>   	}
>>   
>> +	intel_dip_cmn_sdp_transmission_line_disable(crtc_state);
> [Severity: Medium]
> Similarly, since this port-level disable hook only executes when the last
> stream is disabled, does this leave the transmission line registers active
> for other transcoders in an MST setup when they are individually disabled?
>
>>   	intel_ddi_disable_fec(encoder, crtc_state);
>>   
>>   	if (DISPLAY_VER(display) < 14)
>> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
>> --- a/drivers/gpu/drm/i915/display/intel_dip.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
> [ ... ]
>> +static
>> +void intel_dip_cmn_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
>> +{
> [ ... ]
>> +	if (as_sdp)
>> +		crtc_state->dip.cmn_sdp_tl = crtc_state->dip.emp_as_sdp_tl;
>> +	else
>> +		crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1;
> [Severity: High]
> If crtc_state->vrr.guardband is 0 (for example, when VRR is not used), will
> this subtraction underflow and set the u16 cmn_sdp_tl to 65535?
>
> If it underflows to 65535, pps_stagger evaluates to a negative value
> (0 - 65535) in intel_dip_cmn_sdp_transmission_line_enable(). Will this trigger
> the drm_WARN_ON(display->drm, pps_stagger < 0) and abort enabling the
> transmission line?
>
> Also, if guardband evaluates to 1, cmn_sdp_tl will be 0. Does the
> !crtc_state->dip.cmn_sdp_tl check in intel_dip_cmn_sdp_transmission_line_enable()
> and disable() incorrectly treat a valid transmission line value of 0 as a
> sentinel for 'feature disabled', silently skipping the hardware enablement?


guardband can't be 0 or 1 here. DP always computes it via the optimized 
guardband path (intel_vrr_compute_optimized_guardband()), which calls 
intel_dp_sdp_min_guardband(crtc_state, true) with assume_all_enabled = true.

That guarantees a floor of at least 8 (GMP's contribution), regardless 
of which SDPs are actually active, so no underflow is possible.

That said, this does need to change once CMN_SDP_TL is set: with 
CMN_SDP_TL enabled, the per-packet minimum SDP guardband requirement no 
longer applies.

I'll set a 5-line floor for the guardband in that case, 4 lines to cover 
the packet types plus 1 extra.

Thanks & Regards,

Ankit

>

Re: [PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line

[Nautiyal, Ankit K]

On 9/8/2026 1:40 PM, Kandpal, Suraj wrote:
>> Subject: [PATCH v5 11/12] drm/i915/dip: Enable Common SDP Transmission line
>>
>> Enable programming of the common SDP transmission line on platforms that
>> support it. Compute and program the common base transmission line and per-
>> SDP stagger values from the crtc state during modeset, and disable the feature
>> on pipe disable.
>>
>> Currently, the stagger values are set as per the default policy of the Hardware.
>> This can be optimized later if we come up with a specific driver policy to
>> sequence the SDPs better.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_ddi.c |  3 +
>> drivers/gpu/drm/i915/display/intel_dip.c | 90 ++++++++++++++++++++++++
>>   2 files changed, 93 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c
>> b/drivers/gpu/drm/i915/display/intel_ddi.c
>> index dacb4b7588a0..3cdb06e81130 100644
>> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
>> @@ -2737,6 +2737,8 @@ static void mtl_ddi_pre_enable_dp(struct
>> intel_atomic_state *state,
>>   	/* 6.o Configure and enable FEC if needed */
>>   	intel_ddi_enable_fec(encoder, crtc_state);
>>
>> +	intel_dip_cmn_sdp_transmission_line_enable(crtc_state);
>> +
>>   	/* 7.a 128b/132b SST. */
>>   	if (!is_mst && intel_dp_is_uhbr(crtc_state)) {
>>   		/* VCPID 1, start slot 0 for 128b/132b, tu slots */ @@ -3124,6
>> +3126,7 @@ static void intel_ddi_buf_disable(struct intel_encoder *encoder,
>>   			     DP_TP_CTL_ENABLE, 0);
>>   	}
>>
>> +	intel_dip_cmn_sdp_transmission_line_disable(crtc_state);
>>   	intel_ddi_disable_fec(encoder, crtc_state);
>>
>>   	if (DISPLAY_VER(display) < 14)
>> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c
>> b/drivers/gpu/drm/i915/display/intel_dip.c
>> index 0c80d6305fec..007c8fb87ecd 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dip.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
>> @@ -10,6 +10,7 @@
>>   #include "intel_dip.h"
>>   #include "intel_dip_regs.h"
>>   #include "intel_display_types.h"
>> +#include "intel_hdmi.h"
>>
>>   static int intel_dip_get_as_sdp_transmission_line(const struct intel_crtc_state
>> *crtc_state)  { @@ -54,14 +55,103 @@ void
>> intel_dip_write_emp_as_sdp_tl(const struct intel_crtc_state *crtc_state)
>>   		       EMP_AS_SDP_DB_TL(crtc_state->dip.emp_as_sdp_tl));
>>   }
>>
>> +static int intel_dip_sdp_stagger_to_tl(struct intel_crtc_state *crtc_state,
>> +				       int stagger)
>> +{
>> +	return crtc_state->dip.cmn_sdp_tl + stagger; }
>> +
>> +static
>> +void intel_dip_cmn_sdp_tl_compute_config_late(struct intel_crtc_state
>> +*crtc_state) {
>> +	struct intel_display *display = to_intel_display(crtc_state);
>> +	bool as_sdp;
>> +
>> +	if (!HAS_COMMON_SDP_TL(display))
>> +		return;
>> +
>> +	as_sdp = crtc_state->infoframes.enable &
> Since as_sdp is bool does this need to be "&&"
>
>> +		 intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
>> +	/*
>> +	 * When AS SDP is enabled :
>> +	 *  - The common SDP Transmission Line matches the EMP SDP
>> Transmission Line.
>> +	 *
>> +	 * When AS SDP is disabled:
>> +	 *  - Bspec mentions the positions as lines of delayed vblank.
>> +	 *  - Guardband = 1st line of delayed vblank
>> +	 *  - Common SDP Transmission line is set to 2nd line of delayed vblank.
>> +	 */
>> +
>> +	if (as_sdp)
>> +		crtc_state->dip.cmn_sdp_tl = crtc_state->dip.emp_as_sdp_tl;
>> +	else
>> +		crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1;
>> +
> According to Bspec 74384
> The transmission line for each SDP type must not be within the V. Active region and should be positioned at least one line into the V. Blank (see "Transcoder VRR Function" for definition of Vmin)
> (V. Active + 1) <= (Vmin - SDP TL)
>
> When Panel Replay and AS SDPs are enabled, the eDP/DP standard requires the VSC SDP indicating PR Active to be sent out before the AS SDP. Therefore, the following programming restriction is required for this configuration:
> CMN_SDP_TL >= EMP_AS_SDP_TL
>
> If PSR1 is going to be used with the Sink, then Software must position CMN_SDP_TL on the second line of the VRR Guardband (i.e. CMN_SDP_TL = (VRR Guardband-1))
>
> Don't we need to check these restrictions
>
> Also according to Bspec " crtc_state->dip.cmn_sdp_tl = crtc_state->vrr.guardband - 1 " only when using PSR1

CMN_SDP_TL = (VRR Guardband - 1) only for PSR1 this formula isn't 
PSR1-exclusiveit's our general strategy based on Bspec: 68921 of 
replicating the disabled-mode default SDP position (2nd line of 
guardband) whenever AS SDP isn't active.

It happens to satisfy the PSR1-specific bspec requirement, but it isn't 
gated on PSR1 specifically since it produces the correct/expected 
position regardless of PSR mode.

But I agree we should have some checks for cmn sdp tl.

The guardband must be sized so all CMN SDP TL positions can actually be 
transmitted.


I'll address this in two ways in the next version

- Floor the guardband at 5 lines in intel_dp_sdp_min_guardband() once 
CMN_SDP_TL is in use. (As mentioned in response to earlier sashiko comment.)

- In compute_config_late, add a check that cmn_sdp_tl (measured from 
V.Blank end) stays less than guardband + SCL (also measured from V.Blank 
end)i.e., within the SCL + Guardband region.


Regards,

Ankit


>
> Regards,
> Suraj Kandpal
>
>> +	/*
>> +	 * Currently we are programming the default stagger values, but these
>> +	 * can be optimized if required, based on number of SDPs enabled.
>> +	 *
>> +	 * Default values of the Transmission lines for SDPs other than AS SDP:
>> +	 * VSC : CMN SDP Transmission line
>> +	 * GMP : CMN SDP Transmission line
>> +	 * PPS : CMN SDP Transmission line + 1
>> +	 * VSC_EXT: CMN SDP Transmission line + 2
>> +	 */
>> +	crtc_state->dip.vsc_sdp_tl = crtc_state->dip.cmn_sdp_tl;
>> +	crtc_state->dip.gmp_sdp_tl =
>> +		intel_dip_sdp_stagger_to_tl(crtc_state,
>> GMP_STAGGER_DEFAULT);
>> +	crtc_state->dip.pps_sdp_tl =
>> +		intel_dip_sdp_stagger_to_tl(crtc_state,
>> PPS_STAGGER_DEFAULT);
>> +	crtc_state->dip.vsc_ext_sdp_tl =
>> +		intel_dip_sdp_stagger_to_tl(crtc_state,
>> VSC_EXT_STAGGER_DEFAULT); }
>> +
>>   void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
>> {
>>   	crtc_state->dip.emp_as_sdp_tl =
>> intel_dip_get_as_sdp_transmission_line(crtc_state);
>> +
>> +	intel_dip_cmn_sdp_tl_compute_config_late(crtc_state);
>> +}
>> +
>> +static
>> +void intel_dip_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;
>> +	u16 vsc_ext_stagger, pps_stagger, gmp_stagger;
>> +	u32 val;
>> +
>> +	if (!HAS_COMMON_SDP_TL(display))
>> +		return;
>> +
>> +	val = intel_de_read(display, CMN_SDP_TL(display, cpu_transcoder));
>> +
>> +	if (!(val & TRANSMISSION_LINE_ENABLE))
>> +		return;
>> +
>> +	crtc_state->dip.cmn_sdp_tl =
>> +REG_FIELD_GET(BASE_TRANSMISSION_LINE_MASK, val);
>> +
>> +	/* SDP VSC uses same transmission line as CMN base transmission line
>> */
>> +	crtc_state->dip.vsc_sdp_tl = crtc_state->dip.cmn_sdp_tl;
>> +
>> +	val = intel_de_read(display, CMN_SDP_TL_STGR_CTL(display,
>> +cpu_transcoder));
>> +
>> +	vsc_ext_stagger = REG_FIELD_GET(VSC_EXT_STAGGER_MASK, val);
>> +	pps_stagger = REG_FIELD_GET(PPS_STAGGER_MASK, val);
>> +	gmp_stagger = REG_FIELD_GET(GMP_STAGGER_MASK, val);
>> +
>> +	crtc_state->dip.vsc_ext_sdp_tl =
>> +		intel_dip_sdp_stagger_to_tl(crtc_state, vsc_ext_stagger);
>> +	crtc_state->dip.pps_sdp_tl =
>> +		intel_dip_sdp_stagger_to_tl(crtc_state, pps_stagger);
>> +	crtc_state->dip.gmp_sdp_tl =
>> +		intel_dip_sdp_stagger_to_tl(crtc_state, gmp_stagger);
>>   }
>>
>>   void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state
>> *crtc_state)  {
>>   	crtc_state->dip.emp_as_sdp_tl =
>> intel_dip_read_emp_as_sdp_tl(crtc_state);
>> +	intel_dip_cmn_sdp_transmission_line_get_config(crtc_state);
>>   }
>>
>>   static int intel_dip_sdp_tl_to_stagger(const struct intel_crtc_state *crtc_state,
>> --
>> 2.50.1