[Intel-gfx] [PATCH 0/3] Enable Adaptive Sync SDP Support for DP

[Intel-gfx] [PATCH 0/3] Enable Adaptive Sync SDP Support for DP

[Mitul Golani]

An Adaptive Sync SDP allows a DP protocol converter to
forward Adaptive Sync video with minimal buffering overhead
within the converter. An Adaptive-Sync-capable DP protocol
converter indicates its support by setting the related bit
in the DPCD register.

Computes AS SDP values based on the display configuration,
ensuring proper handling of Variable Refresh Rate (VRR)
in the context of Adaptive Sync.

Mitul Golani (3):
  drm: Add Adaptive Sync SDP logging
  drm/i915/display/: Add Read/Write support for Adaptive Sync SDP
  drm/i915/display/:Compute and enable daptive Sync SDP

 drivers/gpu/drm/display/drm_dp_helper.c       |  15 ++
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 139 +++++++++++++++++-
 drivers/gpu/drm/i915/display/intel_hdmi.c     |  11 +-
 drivers/gpu/drm/i915/i915_reg.h               |   6 +
 include/drm/display/drm_dp.h                  |   1 +
 include/drm/display/drm_dp_helper.h           |  30 ++++
 7 files changed, 196 insertions(+), 7 deletions(-)

-- 
2.25.1

[Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging

[Mitul Golani]

Add structure representing Adaptive Sync Secondary Data
Packet (AS SDP). Also, add Adaptive Sync SDP logging in
drm_dp_helper.c to facilitate debugging.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 drivers/gpu/drm/display/drm_dp_helper.c | 15 +++++++++++++
 include/drm/display/drm_dp.h            |  1 +
 include/drm/display/drm_dp_helper.h     | 30 +++++++++++++++++++++++++
 3 files changed, 46 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index d72b6f9a352c..a205e14a6681 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -2917,6 +2917,21 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
 }
 EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
 
+void drm_dp_as_sdp_log(const char *level, struct device *dev,
+		       const struct drm_dp_as_sdp *async)
+{
+#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
+	DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
+		   async->revision, async->length);
+	DP_SDP_LOG("	vmin: %d vmax: %d\n", async->vmin, async->vmax);
+	DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
+	DP_SDP_LOG("    duration_incr_ms: %u\n", async->duration_incr_ms);
+	DP_SDP_LOG("    duration_decr_ms: %u\n", async->duration_decr_ms);
+	DP_SDP_LOG("    operation_mode: %u\n", async->operation_mode);
+#undef DP_SDP_LOG
+}
+EXPORT_SYMBOL(drm_dp_as_sdp_log);
+
 /**
  * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON
  * @dpcd: DisplayPort configuration data
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index 83d2039c018b..0575ab8ea088 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -1578,6 +1578,7 @@ enum drm_dp_phy {
 #define DP_SDP_PPS			0x10 /* DP 1.4 */
 #define DP_SDP_VSC_EXT_VESA		0x20 /* DP 1.4 */
 #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
+#define DP_SDP_ADAPTIVE_SYNC	0x22 /* DP 1.4 */
 /* 0x80+ CEA-861 infoframe types */
 
 #define DP_SDP_AUDIO_INFOFRAME_HB2	0x1b
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 863b2e7add29..63b6bef3f21d 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp {
 	enum dp_content_type content_type;
 };
 
+/**
+ * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP
+ *
+ * This structure represents a DP AS SDP of drm
+ * It is based on DP 2.1 spec [Table 2-126:  Adaptive-Sync SDP Header Bytes] and
+ * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8]
+ *
+ * @sdp_type: secondary-data packet type
+ * @length: number of valid data bytes
+ * @vmin: minimum vtotal
+ * @vmax: maximum vtotal
+ * @duration_incr_ms: Successive frame duration increase
+ * @duration_decr_ms: Successive frame duration decrease
+ * @operation_mode: Adaptive Sync Operation Mode
+ */
+
+struct drm_dp_as_sdp {
+	unsigned char sdp_type;
+	unsigned char revision;
+	unsigned char length;
+	u16 vmin, vmax;
+	u16 target_rr;
+	u8 duration_incr_ms;
+	u8 duration_decr_ms;
+	u8 operation_mode;
+};
+
+void drm_dp_as_sdp_log(const char *level, struct device *dev,
+		       const struct drm_dp_as_sdp *async);
+
 void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
 			const struct drm_dp_vsc_sdp *vsc);
 
-- 
2.25.1

[Intel-gfx] [PATCH 2/3] drm/i915/display/: Add Read/Write support for Adaptive Sync SDP

[Mitul Golani]

Add the necessary structures and functions to handle reading and
unpacking Adaptive Sync Secondary Data Packets. Also add support
to write and pack AS SDP.

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 .../drm/i915/display/intel_display_types.h    |   1 +
 drivers/gpu/drm/i915/display/intel_dp.c       | 118 +++++++++++++++++-
 2 files changed, 114 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 9a44350ba05d..7d87923f63af 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1325,6 +1325,7 @@ struct intel_crtc_state {
 		union hdmi_infoframe hdmi;
 		union hdmi_infoframe drm;
 		struct drm_dp_vsc_sdp vsc;
+		struct drm_dp_as_sdp  async;
 	} infoframes;
 
 	u8 eld[MAX_ELD_BYTES];
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 1422c2370269..39624746d612 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -94,6 +94,8 @@
 #define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
 #define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
 
+#define AS_SDP_ENABLE					REG_BIT(2)
+#define AS_SDP_OP_MODE					REG_GENMASK(1, 0)
 
 /* Constants for DP DSC configurations */
 static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15};
@@ -4113,6 +4115,42 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
 	return false;
 }
 
+static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *async,
+				    struct dp_sdp *sdp, size_t size)
+{
+	size_t length = sizeof(struct dp_sdp);
+
+	if (size < length)
+		return -ENOSPC;
+
+	memset(sdp, 0, size);
+
+	/* Prepare AS (Adaptive Sync) VSC Header */
+	sdp->sdp_header.HB0 = 0;
+	sdp->sdp_header.HB1 = async->sdp_type;
+	sdp->sdp_header.HB2 = 0x02;
+	sdp->sdp_header.HB3 = async->length;
+
+	/* Fill AS (Adaptive Sync) SDP Payload */
+	if ((sdp->db[0] & 0x03) == 0) {
+		sdp->db[3] = 0;
+		sdp->db[4] &= 0xFC;
+	}
+
+	sdp->db[1] = async->vmin & 0xFF;
+	sdp->db[2] = (async->vmin >> 8) & 0xF;
+	sdp->db[17] = (async->vmin >> 8) & 0xFF;
+	sdp->db[18] = async->vmax & 0xFF;
+	sdp->db[19] = (async->vmax >> 8) & 0xFF;
+	sdp->db[20] = async->target_rr & 0xFF;
+	sdp->db[21] = (async->target_rr >> 8) & 0xFF;
+	sdp->db[22] = async->duration_incr_ms;
+	sdp->db[23] = async->duration_decr_ms;
+	sdp->db[24] = async->operation_mode;
+
+	return length;
+}
+
 static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
 				     struct dp_sdp *sdp, size_t size)
 {
@@ -4280,6 +4318,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
 							       &crtc_state->infoframes.drm.drm,
 							       &sdp, sizeof(sdp));
 		break;
+	case DP_SDP_ADAPTIVE_SYNC:
+		len = intel_dp_as_sdp_pack(&crtc_state->infoframes.async, &sdp,
+					   sizeof(sdp));
+		break;
 	default:
 		MISSING_CASE(type);
 		return;
@@ -4342,6 +4384,44 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder,
 	intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA);
 }
 
+/*
+ * This function is to unpack AS SDP Packet
+ */
+static
+int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *async,
+			   const void *buffer, size_t size)
+{
+	const struct dp_sdp *sdp = buffer;
+
+	if (size < sizeof(struct dp_sdp))
+		return -EINVAL;
+
+	memset(async, 0, sizeof(*async));
+
+	if (sdp->sdp_header.HB0 != 0)
+		return -EINVAL;
+
+	if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC)
+		return -EINVAL;
+
+	if (sdp->sdp_header.HB2 != 0x02)
+		return -EINVAL;
+
+	if ((sdp->sdp_header.HB3 & 0x3F) != 9)
+		return -EINVAL;
+
+	if (sdp->db[0] != (AS_SDP_ENABLE | AS_SDP_OP_MODE))
+		return -EINVAL;
+
+	async->vmin = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1];
+	async->vmax = 0;
+	async->target_rr = 0;
+	async->duration_incr_ms = 0;
+	async->duration_decr_ms = 0;
+
+	return 0;
+}
+
 static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
 				   const void *buffer, size_t size)
 {
@@ -4412,12 +4492,35 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
 	return 0;
 }
 
+/*
+ * This function to read registers to fetch packets
+ */
+static int
+intel_read_dp_as_metadata_infoframe_sdp(struct intel_encoder *encoder,
+					struct intel_crtc_state *crtc_state,
+					struct drm_dp_as_sdp *async)
+{
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
+	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
+	unsigned int type = DP_SDP_ADAPTIVE_SYNC;
+	struct dp_sdp sdp = {};
+	int ret;
+
+	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
+				 sizeof(sdp));
+
+	ret = intel_dp_as_sdp_unpack(async, &sdp, sizeof(sdp));
+	if (ret)
+		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP AS SDP\n");
+
+	return ret;
+}
+
 static int
 intel_dp_hdr_metadata_infoframe_sdp_unpack(struct hdmi_drm_infoframe *drm_infoframe,
 					   const void *buffer, size_t size)
 {
 	int ret;
-
 	const struct dp_sdp *sdp = buffer;
 
 	if (size < sizeof(struct dp_sdp))
@@ -4484,9 +4587,10 @@ static void intel_read_dp_vsc_sdp(struct intel_encoder *encoder,
 		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP VSC SDP\n");
 }
 
-static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
-						     struct intel_crtc_state *crtc_state,
-						     struct hdmi_drm_infoframe *drm_infoframe)
+static void
+intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
+					 struct intel_crtc_state *crtc_state,
+					 struct hdmi_drm_infoframe *drm_infoframe)
 {
 	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
@@ -4495,7 +4599,7 @@ static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encod
 	int ret;
 
 	if ((crtc_state->infoframes.enable &
-	    intel_hdmi_infoframe_enable(type)) == 0)
+	     intel_hdmi_infoframe_enable(type)) == 0)
 		return;
 
 	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
@@ -4522,6 +4626,10 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
 		intel_read_dp_hdr_metadata_infoframe_sdp(encoder, crtc_state,
 							 &crtc_state->infoframes.drm.drm);
 		break;
+	case DP_SDP_ADAPTIVE_SYNC:
+		intel_read_dp_as_metadata_infoframe_sdp(encoder, crtc_state,
+							&crtc_state->infoframes.async);
+	break;
 	default:
 		MISSING_CASE(type);
 		break;
-- 
2.25.1

[Intel-gfx] [PATCH 3/3] drm/i915/display/:Compute and enable daptive Sync SDP

[Mitul Golani]

Add necessary functions and register definitions to enable
and compute AS SDP data. The new `intel_dp_compute_as_sdp`
function computes AS SDP values based on the display
configuration, ensuring proper handling of Variable Refresh
Rate (VRR).

Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp.c   | 21 +++++++++++++++++++++
 drivers/gpu/drm/i915/display/intel_hdmi.c | 11 +++++++++--
 drivers/gpu/drm/i915/i915_reg.h           |  6 ++++++
 3 files changed, 36 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 39624746d612..b3eb2d342a99 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -2629,6 +2629,26 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
 					 &crtc_state->infoframes.vsc);
 }
 
+static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
+				    struct intel_crtc_state *crtc_state,
+				     const struct drm_connector_state *conn_state)
+{
+	struct drm_dp_as_sdp *async = &crtc_state->infoframes.async;
+	struct intel_connector *connector = intel_dp->attached_connector;
+	int vrefresh = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode);
+
+	if (intel_vrr_is_in_range(connector, vrefresh))
+		return;
+
+	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);
+	async->sdp_type = DP_SDP_ADAPTIVE_SYNC;
+	async->length = 0x9;
+	async->vmin = crtc_state->vrr.vmin;
+	async->vmax = crtc_state->vrr.vmax;
+	async->target_rr = 0;
+	async->operation_mode = 0x0;
+}
+
 void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
 				  const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state,
@@ -2965,6 +2985,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
 	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
 	intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16);
 	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
+	intel_dp_compute_as_sdp(intel_dp, pipe_config, conn_state);
 	intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);
 
 	return 0;
diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
index ab18cfc19c0a..abea359985ce 100644
--- a/drivers/gpu/drm/i915/display/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
@@ -136,6 +136,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
 		return VIDEO_DIP_ENABLE_GMP_HSW;
 	case DP_SDP_VSC:
 		return VIDEO_DIP_ENABLE_VSC_HSW;
+	case DP_SDP_ADAPTIVE_SYNC:
+		return VIDEO_DIP_ENABLE_AS_HSW;
 	case DP_SDP_PPS:
 		return VDIP_ENABLE_PPS;
 	case HDMI_INFOFRAME_TYPE_AVI:
@@ -163,6 +165,8 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
 		return HSW_TVIDEO_DIP_GMP_DATA(cpu_transcoder, i);
 	case DP_SDP_VSC:
 		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
+	case DP_SDP_ADAPTIVE_SYNC:
+		return HSW_TVIDEO_DIP_ASYNC_DATA(cpu_transcoder, i);
 	case DP_SDP_PPS:
 		return ICL_VIDEO_DIP_PPS_DATA(cpu_transcoder, i);
 	case HDMI_INFOFRAME_TYPE_AVI:
@@ -185,6 +189,8 @@ static int hsw_dip_data_size(struct drm_i915_private *dev_priv,
 	switch (type) {
 	case DP_SDP_VSC:
 		return VIDEO_DIP_VSC_DATA_SIZE;
+	case DP_SDP_ADAPTIVE_SYNC:
+		return VIDEO_DIP_ASYNC_DATA_SIZE;
 	case DP_SDP_PPS:
 		return VIDEO_DIP_PPS_DATA_SIZE;
 	case HDMI_PACKET_TYPE_GAMUT_METADATA:
@@ -555,7 +561,8 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
 
 	mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
 		VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
-		VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
+		VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
+		VIDEO_DIP_ENABLE_AS_HSW);
 
 	if (DISPLAY_VER(dev_priv) >= 10)
 		mask |= VIDEO_DIP_ENABLE_DRM_GLK;
@@ -1209,7 +1216,7 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
 	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
 		 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
 		 VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
-		 VIDEO_DIP_ENABLE_DRM_GLK);
+		 VIDEO_DIP_ENABLE_DRM_GLK | VIDEO_DIP_ENABLE_AS_HSW);
 
 	if (!enable) {
 		intel_de_write(dev_priv, reg, val);
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 27dc903f0553..81d64c428693 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -2312,6 +2312,7 @@
  * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
  * of the infoframe structure specified by CEA-861. */
 #define   VIDEO_DIP_DATA_SIZE	32
+#define   VIDEO_DIP_ASYNC_DATA_SIZE	32
 #define   VIDEO_DIP_GMP_DATA_SIZE	36
 #define   VIDEO_DIP_VSC_DATA_SIZE	36
 #define   VIDEO_DIP_PPS_DATA_SIZE	132
@@ -2344,6 +2345,7 @@
 #define   VSC_DIP_HW_DATA_SW_HEA	(2 << 25)
 #define   VSC_DIP_SW_HEA_DATA		(3 << 25)
 #define   VDIP_ENABLE_PPS		(1 << 24)
+#define   VIDEO_DIP_ENABLE_AS_HSW	REG_BIT(23)
 #define   VIDEO_DIP_ENABLE_VSC_HSW	(1 << 20)
 #define   VIDEO_DIP_ENABLE_GCP_HSW	(1 << 16)
 #define   VIDEO_DIP_ENABLE_AVI_HSW	(1 << 12)
@@ -5038,6 +5040,7 @@
 #define _HSW_VIDEO_DIP_SPD_DATA_A	0x602A0
 #define _HSW_VIDEO_DIP_GMP_DATA_A	0x602E0
 #define _HSW_VIDEO_DIP_VSC_DATA_A	0x60320
+#define	_HSW_VIDEO_DIP_ASYNC_DATA_A	0x60484
 #define _GLK_VIDEO_DIP_DRM_DATA_A	0x60440
 #define _HSW_VIDEO_DIP_AVI_ECC_A	0x60240
 #define _HSW_VIDEO_DIP_VS_ECC_A		0x60280
@@ -5052,6 +5055,7 @@
 #define _HSW_VIDEO_DIP_SPD_DATA_B	0x612A0
 #define _HSW_VIDEO_DIP_GMP_DATA_B	0x612E0
 #define _HSW_VIDEO_DIP_VSC_DATA_B	0x61320
+#define	_HSW_VIDEO_DIP_ASYNC_DATA_B	0x61484
 #define _GLK_VIDEO_DIP_DRM_DATA_B	0x61440
 #define _HSW_VIDEO_DIP_BVI_ECC_B	0x61240
 #define _HSW_VIDEO_DIP_VS_ECC_B		0x61280
@@ -5078,6 +5082,8 @@
 #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_GMP_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4)
 #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
+#define HSW_TVIDEO_DIP_ASYNC_DATA(trans, i)	_MMIO_TRANS2(trans,\
+							     _HSW_VIDEO_DIP_ASYNC_DATA_A + (i) * 4)
 #define GLK_TVIDEO_DIP_DRM_DATA(trans, i)	_MMIO_TRANS2(trans, _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_DATA(trans, i)	_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
 #define ICL_VIDEO_DIP_PPS_ECC(trans, i)		_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)
-- 
2.25.1

Re: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging

[Nautiyal, Ankit K]

On 11/23/2023 7:32 PM, Mitul Golani wrote:
> Add structure representing Adaptive Sync Secondary Data
> Packet (AS SDP). Also, add Adaptive Sync SDP logging in
> drm_dp_helper.c to facilitate debugging.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/display/drm_dp_helper.c | 15 +++++++++++++
>   include/drm/display/drm_dp.h            |  1 +
>   include/drm/display/drm_dp_helper.h     | 30 +++++++++++++++++++++++++
>   3 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index d72b6f9a352c..a205e14a6681 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2917,6 +2917,21 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
>   }
>   EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>   
> +void drm_dp_as_sdp_log(const char *level, struct device *dev,
> +		       const struct drm_dp_as_sdp *async)

Perhaps as_sdp, instead of async to avoid confusion?


> +{
> +#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
> +	DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",

I think you mean AS_SDP here.

Also, you need to send this to dri-devel as well.

Regards,

Ankit


> +		   async->revision, async->length);
> +	DP_SDP_LOG("	vmin: %d vmax: %d\n", async->vmin, async->vmax);
> +	DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
> +	DP_SDP_LOG("    duration_incr_ms: %u\n", async->duration_incr_ms);
> +	DP_SDP_LOG("    duration_decr_ms: %u\n", async->duration_decr_ms);
> +	DP_SDP_LOG("    operation_mode: %u\n", async->operation_mode);
> +#undef DP_SDP_LOG
> +}
> +EXPORT_SYMBOL(drm_dp_as_sdp_log);
> +
>   /**
>    * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON
>    * @dpcd: DisplayPort configuration data
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index 83d2039c018b..0575ab8ea088 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -1578,6 +1578,7 @@ enum drm_dp_phy {
>   #define DP_SDP_PPS			0x10 /* DP 1.4 */
>   #define DP_SDP_VSC_EXT_VESA		0x20 /* DP 1.4 */
>   #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
> +#define DP_SDP_ADAPTIVE_SYNC	0x22 /* DP 1.4 */
>   /* 0x80+ CEA-861 infoframe types */
>   
>   #define DP_SDP_AUDIO_INFOFRAME_HB2	0x1b
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index 863b2e7add29..63b6bef3f21d 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp {
>   	enum dp_content_type content_type;
>   };
>   
> +/**
> + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP
> + *
> + * This structure represents a DP AS SDP of drm
> + * It is based on DP 2.1 spec [Table 2-126:  Adaptive-Sync SDP Header Bytes] and
> + * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8]
> + *
> + * @sdp_type: secondary-data packet type
> + * @length: number of valid data bytes
> + * @vmin: minimum vtotal
> + * @vmax: maximum vtotal
> + * @duration_incr_ms: Successive frame duration increase
> + * @duration_decr_ms: Successive frame duration decrease
> + * @operation_mode: Adaptive Sync Operation Mode
> + */
> +
> +struct drm_dp_as_sdp {
> +	unsigned char sdp_type;
> +	unsigned char revision;
> +	unsigned char length;
> +	u16 vmin, vmax;
> +	u16 target_rr;
> +	u8 duration_incr_ms;
> +	u8 duration_decr_ms;
> +	u8 operation_mode;
> +};
> +
> +void drm_dp_as_sdp_log(const char *level, struct device *dev,
> +		       const struct drm_dp_as_sdp *async);
> +
>   void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
>   			const struct drm_dp_vsc_sdp *vsc);
>   

[Intel-gfx] ✗ Fi.CI.SPARSE: warning for Enable Adaptive Sync SDP Support for DP

[Patchwork]

== Series Details ==

Series: Enable Adaptive Sync SDP Support for DP
URL   : https://patchwork.freedesktop.org/series/126829/
State : warning

== Summary ==

Error: dim sparse failed
Sparse version: v0.6.2
Fast mode used, each commit won't be checked separately.

[Intel-gfx] ✗ Fi.CI.BAT: failure for Enable Adaptive Sync SDP Support for DP

[Patchwork]

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

== Series Details ==

Series: Enable Adaptive Sync SDP Support for DP
URL   : https://patchwork.freedesktop.org/series/126829/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_13917 -> Patchwork_126829v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_126829v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_126829v1, please notify your bug team (lgci.bug.filing@intel.com) 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_126829v1/index.html

Participating hosts (37 -> 36)
------------------------------

  Additional (1): fi-pnv-d510 
  Missing    (2): bat-kbl-2 fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@client:
    - bat-mtlp-6:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13917/bat-mtlp-6/igt@i915_selftest@live@client.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-mtlp-6/igt@i915_selftest@live@client.html

  * igt@i915_selftest@live@gt_timelines:
    - bat-atsm-1:         [PASS][3] -> [TIMEOUT][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13917/bat-atsm-1/igt@i915_selftest@live@gt_timelines.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-atsm-1/igt@i915_selftest@live@gt_timelines.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - bat-atsm-1:         [PASS][5] -> [WARN][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13917/bat-atsm-1/igt@i915_suspend@basic-s2idle-without-i915.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-atsm-1/igt@i915_suspend@basic-s2idle-without-i915.html

  
Known issues
------------

  Here are the changes found in Patchwork_126829v1 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_lmem_swapping@basic:
    - fi-pnv-d510:        NOTRUN -> [SKIP][7] ([fdo#109271]) +25 other tests skip
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/fi-pnv-d510/igt@gem_lmem_swapping@basic.html

  * igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence:
    - bat-dg2-11:         NOTRUN -> [SKIP][8] ([i915#1845] / [i915#9197]) +3 other tests skip
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-dg2-11/igt@kms_pipe_crc_basic@nonblocking-crc-frame-sequence.html

  * igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1:
    - bat-rplp-1:         [PASS][9] -> [ABORT][10] ([i915#8668])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13917/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-rplp-1/igt@kms_pipe_crc_basic@read-crc-frame-sequence@pipe-d-edp-1.html

  
#### Possible fixes ####

  * igt@gem_exec_suspend@basic-s0@lmem0:
    - bat-dg2-9:          [INCOMPLETE][11] ([i915#9275]) -> [PASS][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13917/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-dg2-9/igt@gem_exec_suspend@basic-s0@lmem0.html

  * igt@kms_flip@basic-flip-vs-wf_vblank@d-dp5:
    - bat-adlp-11:        [DMESG-WARN][13] ([i915#6868]) -> [PASS][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13917/bat-adlp-11/igt@kms_flip@basic-flip-vs-wf_vblank@d-dp5.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_126829v1/bat-adlp-11/igt@kms_flip@basic-flip-vs-wf_vblank@d-dp5.html

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [i915#1845]: https://gitlab.freedesktop.org/drm/intel/issues/1845
  [i915#6868]: https://gitlab.freedesktop.org/drm/intel/issues/6868
  [i915#8668]: https://gitlab.freedesktop.org/drm/intel/issues/8668
  [i915#9197]: https://gitlab.freedesktop.org/drm/intel/issues/9197
  [i915#9275]: https://gitlab.freedesktop.org/drm/intel/issues/9275


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

  * Linux: CI_DRM_13917 -> Patchwork_126829v1

  CI-20190529: 20190529
  CI_DRM_13917: e8f02d065ade740b7f56a134f6113e2777b5d11a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_7600: b889fd01780dc79f6fcc8545346c81f5c79f5efb @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_126829v1: e8f02d065ade740b7f56a134f6113e2777b5d11a @ git://anongit.freedesktop.org/gfx-ci/linux


### Linux commits

3abbe93e5b93 drm/i915/display/:Compute and enable daptive Sync SDP
2f4eb53e3034 drm/i915/display/: Add Read/Write support for Adaptive Sync SDP
09febf83bd92 drm: Add Adaptive Sync SDP logging

== Logs ==

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

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

Re: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging

[kernel test robot]

Hi Mitul,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-tip/drm-tip]

url:    https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-Add-Adaptive-Sync-SDP-logging/20231123-220931
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:    https://lore.kernel.org/r/20231123140244.4183869-2-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging
config: x86_64-defconfig (https://download.01.org/0day-ci/archive/20231124/202311241136.tl5STYqx-lkp@intel.com/config)
compiler: gcc-11 (Debian 11.3.0-12) 11.3.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/202311241136.tl5STYqx-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311241136.tl5STYqx-lkp@intel.com/

All warnings (new ones prefixed by >>):

   In file included from include/linux/device.h:15,
                    from include/linux/backlight.h:12,
                    from drivers/gpu/drm/display/drm_dp_helper.c:23:
   drivers/gpu/drm/display/drm_dp_helper.c: In function 'drm_dp_as_sdp_log':
>> drivers/gpu/drm/display/drm_dp_helper.c:2927:20: warning: format '%s' expects argument of type 'char *', but argument 4 has type 'int' [-Wformat=]
    2927 |         DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
         |                    ^~~~~~~~~~~~~~~~~~~~~  ~~~~~~~~~~~~~~~~
         |                                                |
         |                                                int
   include/linux/dev_printk.h:129:41: note: in definition of macro 'dev_printk'
     129 |                 _dev_printk(level, dev, fmt, ##__VA_ARGS__);            \
         |                                         ^~~
   drivers/gpu/drm/display/drm_dp_helper.c:2927:9: note: in expansion of macro 'DP_SDP_LOG'
    2927 |         DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
         |         ^~~~~~~~~~
   drivers/gpu/drm/display/drm_dp_helper.c:2927:37: note: format string is defined here
    2927 |         DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
         |                                    ~^
         |                                     |
         |                                     char *
         |                                    %d


vim +2927 drivers/gpu/drm/display/drm_dp_helper.c

  2919	
  2920	void drm_dp_as_sdp_log(const char *level, struct device *dev,
  2921			       const struct drm_dp_as_sdp *async)
  2922	{
  2923	#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
  2924		DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
  2925			   async->revision, async->length);
  2926		DP_SDP_LOG("	vmin: %d vmax: %d\n", async->vmin, async->vmax);
> 2927		DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
  2928		DP_SDP_LOG("    duration_incr_ms: %u\n", async->duration_incr_ms);
  2929		DP_SDP_LOG("    duration_decr_ms: %u\n", async->duration_decr_ms);
  2930		DP_SDP_LOG("    operation_mode: %u\n", async->operation_mode);
  2931	#undef DP_SDP_LOG
  2932	}
  2933	EXPORT_SYMBOL(drm_dp_as_sdp_log);
  2934	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging

[kernel test robot]

Hi Mitul,

kernel test robot noticed the following build warnings:

[auto build test WARNING on drm-tip/drm-tip]

url:    https://github.com/intel-lab-lkp/linux/commits/Mitul-Golani/drm-Add-Adaptive-Sync-SDP-logging/20231123-220931
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
patch link:    https://lore.kernel.org/r/20231123140244.4183869-2-mitulkumar.ajitkumar.golani%40intel.com
patch subject: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging
config: x86_64-allyesconfig (https://download.01.org/0day-ci/archive/20231124/202311241251.ZXIZrJRp-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20231124/202311241251.ZXIZrJRp-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202311241251.ZXIZrJRp-lkp@intel.com/

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/display/drm_dp_helper.c:2927:36: warning: format specifies type 'char *' but the argument has type 'u16' (aka 'unsigned short') [-Wformat]
           DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
                                      ~~     ^~~~~~~~~~~~~~~~
                                      %hu
   drivers/gpu/drm/display/drm_dp_helper.c:2923:60: note: expanded from macro 'DP_SDP_LOG'
   #define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
                                                       ~~~    ^~~~~~~~~~~
   include/linux/dev_printk.h:129:34: note: expanded from macro 'dev_printk'
                   _dev_printk(level, dev, fmt, ##__VA_ARGS__);            \
                                           ~~~    ^~~~~~~~~~~
   1 warning generated.


vim +2927 drivers/gpu/drm/display/drm_dp_helper.c

  2919	
  2920	void drm_dp_as_sdp_log(const char *level, struct device *dev,
  2921			       const struct drm_dp_as_sdp *async)
  2922	{
  2923	#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
  2924		DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
  2925			   async->revision, async->length);
  2926		DP_SDP_LOG("	vmin: %d vmax: %d\n", async->vmin, async->vmax);
> 2927		DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
  2928		DP_SDP_LOG("    duration_incr_ms: %u\n", async->duration_incr_ms);
  2929		DP_SDP_LOG("    duration_decr_ms: %u\n", async->duration_decr_ms);
  2930		DP_SDP_LOG("    operation_mode: %u\n", async->operation_mode);
  2931	#undef DP_SDP_LOG
  2932	}
  2933	EXPORT_SYMBOL(drm_dp_as_sdp_log);
  2934	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [Intel-gfx] [PATCH 1/3] drm: Add Adaptive Sync SDP logging

[Jani Nikula]

On Thu, 23 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add structure representing Adaptive Sync Secondary Data
> Packet (AS SDP). Also, add Adaptive Sync SDP logging in
> drm_dp_helper.c to facilitate debugging.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  drivers/gpu/drm/display/drm_dp_helper.c | 15 +++++++++++++
>  include/drm/display/drm_dp.h            |  1 +
>  include/drm/display/drm_dp_helper.h     | 30 +++++++++++++++++++++++++
>  3 files changed, 46 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index d72b6f9a352c..a205e14a6681 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -2917,6 +2917,21 @@ void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
>  }
>  EXPORT_SYMBOL(drm_dp_vsc_sdp_log);
>  
> +void drm_dp_as_sdp_log(const char *level, struct device *dev,
> +		       const struct drm_dp_as_sdp *async)
> +{
> +#define DP_SDP_LOG(fmt, ...) dev_printk(level, dev, fmt, ##__VA_ARGS__)
> +	DP_SDP_LOG("DP SDP: %s, revision %u, length %u\n", "VSC",
> +		   async->revision, async->length);
> +	DP_SDP_LOG("	vmin: %d vmax: %d\n", async->vmin, async->vmax);
> +	DP_SDP_LOG("    target_rr: %s\n", async->target_rr);
> +	DP_SDP_LOG("    duration_incr_ms: %u\n", async->duration_incr_ms);
> +	DP_SDP_LOG("    duration_decr_ms: %u\n", async->duration_decr_ms);
> +	DP_SDP_LOG("    operation_mode: %u\n", async->operation_mode);
> +#undef DP_SDP_LOG
> +}
> +EXPORT_SYMBOL(drm_dp_as_sdp_log);

This inspired me to resurrect a drm logging series I've had [1], in
particular patch [2]. Please let's do this properly.

Also, throughout the series, please don't use "async" for "adaptive
sync". It's bound to confuse people. Async means asynchronous, period.

[1] https://patchwork.freedesktop.org/series/126873/
[2] https://patchwork.freedesktop.org/patch/msgid/95f1e3981fa3c5304f3c74e82330f12983d35735.1700829750.git.jani.nikula@intel.com

> +
>  /**
>   * drm_dp_get_pcon_max_frl_bw() - maximum frl supported by PCON
>   * @dpcd: DisplayPort configuration data
> diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
> index 83d2039c018b..0575ab8ea088 100644
> --- a/include/drm/display/drm_dp.h
> +++ b/include/drm/display/drm_dp.h
> @@ -1578,6 +1578,7 @@ enum drm_dp_phy {
>  #define DP_SDP_PPS			0x10 /* DP 1.4 */
>  #define DP_SDP_VSC_EXT_VESA		0x20 /* DP 1.4 */
>  #define DP_SDP_VSC_EXT_CEA		0x21 /* DP 1.4 */
> +#define DP_SDP_ADAPTIVE_SYNC	0x22 /* DP 1.4 */

This is completely unrelated to everything else in the patch.

>  /* 0x80+ CEA-861 infoframe types */
>  
>  #define DP_SDP_AUDIO_INFOFRAME_HB2	0x1b
> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
> index 863b2e7add29..63b6bef3f21d 100644
> --- a/include/drm/display/drm_dp_helper.h
> +++ b/include/drm/display/drm_dp_helper.h
> @@ -98,6 +98,36 @@ struct drm_dp_vsc_sdp {
>  	enum dp_content_type content_type;
>  };
>  
> +/**
> + * struct drm_dp_as_sdp - drm DP Adaptive Sync SDP
> + *
> + * This structure represents a DP AS SDP of drm
> + * It is based on DP 2.1 spec [Table 2-126:  Adaptive-Sync SDP Header Bytes] and
> + * [Table 2-127: Adaptive-Sync SDP Payload for DB0 through DB8]
> + *
> + * @sdp_type: secondary-data packet type
> + * @length: number of valid data bytes
> + * @vmin: minimum vtotal
> + * @vmax: maximum vtotal
> + * @duration_incr_ms: Successive frame duration increase
> + * @duration_decr_ms: Successive frame duration decrease
> + * @operation_mode: Adaptive Sync Operation Mode
> + */
> +
> +struct drm_dp_as_sdp {
> +	unsigned char sdp_type;
> +	unsigned char revision;
> +	unsigned char length;

Why unsigned char here...

> +	u16 vmin, vmax;
> +	u16 target_rr;
> +	u8 duration_incr_ms;
> +	u8 duration_decr_ms;
> +	u8 operation_mode;

...but u8 here?

> +};
> +
> +void drm_dp_as_sdp_log(const char *level, struct device *dev,
> +		       const struct drm_dp_as_sdp *async);
> +
>  void drm_dp_vsc_sdp_log(const char *level, struct device *dev,
>  			const struct drm_dp_vsc_sdp *vsc);

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH 2/3] drm/i915/display/: Add Read/Write support for Adaptive Sync SDP

[Jani Nikula]

On Thu, 23 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add the necessary structures and functions to handle reading and
> unpacking Adaptive Sync Secondary Data Packets. Also add support
> to write and pack AS SDP.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |   1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 118 +++++++++++++++++-
>  2 files changed, 114 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9a44350ba05d..7d87923f63af 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1325,6 +1325,7 @@ struct intel_crtc_state {
>  		union hdmi_infoframe hdmi;
>  		union hdmi_infoframe drm;
>  		struct drm_dp_vsc_sdp vsc;
> +		struct drm_dp_as_sdp  async;
>  	} infoframes;
>  
>  	u8 eld[MAX_ELD_BYTES];
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1422c2370269..39624746d612 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -94,6 +94,8 @@
>  #define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>  #define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>  
> +#define AS_SDP_ENABLE					REG_BIT(2)
> +#define AS_SDP_OP_MODE					REG_GENMASK(1, 0)

REG_BIT and REG_GENMASK are for i915_reg_t.

Moreover, these are stuff inside the SDP, so these should be somewhere
near struct drm_dp_as_sdp definition, otherwise people are just going to
duplicate them.

>  
>  /* Constants for DP DSC configurations */
>  static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15};
> @@ -4113,6 +4115,42 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
>  	return false;
>  }
>  
> +static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *async,

Again, please don't use async for adaptive sync.

> +				    struct dp_sdp *sdp, size_t size)
> +{
> +	size_t length = sizeof(struct dp_sdp);
> +
> +	if (size < length)
> +		return -ENOSPC;
> +
> +	memset(sdp, 0, size);
> +
> +	/* Prepare AS (Adaptive Sync) VSC Header */
> +	sdp->sdp_header.HB0 = 0;
> +	sdp->sdp_header.HB1 = async->sdp_type;
> +	sdp->sdp_header.HB2 = 0x02;
> +	sdp->sdp_header.HB3 = async->length;
> +
> +	/* Fill AS (Adaptive Sync) SDP Payload */
> +	if ((sdp->db[0] & 0x03) == 0) {
> +		sdp->db[3] = 0;
> +		sdp->db[4] &= 0xFC;
> +	}
> +
> +	sdp->db[1] = async->vmin & 0xFF;
> +	sdp->db[2] = (async->vmin >> 8) & 0xF;
> +	sdp->db[17] = (async->vmin >> 8) & 0xFF;
> +	sdp->db[18] = async->vmax & 0xFF;
> +	sdp->db[19] = (async->vmax >> 8) & 0xFF;
> +	sdp->db[20] = async->target_rr & 0xFF;
> +	sdp->db[21] = (async->target_rr >> 8) & 0xFF;
> +	sdp->db[22] = async->duration_incr_ms;
> +	sdp->db[23] = async->duration_decr_ms;
> +	sdp->db[24] = async->operation_mode;
> +
> +	return length;
> +}
> +
>  static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
>  				     struct dp_sdp *sdp, size_t size)
>  {
> @@ -4280,6 +4318,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>  							       &crtc_state->infoframes.drm.drm,
>  							       &sdp, sizeof(sdp));
>  		break;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		len = intel_dp_as_sdp_pack(&crtc_state->infoframes.async, &sdp,
> +					   sizeof(sdp));
> +		break;
>  	default:
>  		MISSING_CASE(type);
>  		return;
> @@ -4342,6 +4384,44 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder,
>  	intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA);
>  }
>  
> +/*
> + * This function is to unpack AS SDP Packet
> + */

What value does this comment add? It says exactly the same thing as the
function name.

> +static
> +int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *async,
> +			   const void *buffer, size_t size)
> +{
> +	const struct dp_sdp *sdp = buffer;
> +
> +	if (size < sizeof(struct dp_sdp))
> +		return -EINVAL;
> +
> +	memset(async, 0, sizeof(*async));
> +
> +	if (sdp->sdp_header.HB0 != 0)
> +		return -EINVAL;
> +
> +	if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC)
> +		return -EINVAL;
> +
> +	if (sdp->sdp_header.HB2 != 0x02)
> +		return -EINVAL;
> +
> +	if ((sdp->sdp_header.HB3 & 0x3F) != 9)
> +		return -EINVAL;
> +
> +	if (sdp->db[0] != (AS_SDP_ENABLE | AS_SDP_OP_MODE))
> +		return -EINVAL;
> +
> +	async->vmin = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1];
> +	async->vmax = 0;
> +	async->target_rr = 0;
> +	async->duration_incr_ms = 0;
> +	async->duration_decr_ms = 0;
> +
> +	return 0;
> +}
> +
>  static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
>  				   const void *buffer, size_t size)
>  {
> @@ -4412,12 +4492,35 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
>  	return 0;
>  }
>  
> +/*
> + * This function to read registers to fetch packets
> + */

Unnecessary comment.

> +static int
> +intel_read_dp_as_metadata_infoframe_sdp(struct intel_encoder *encoder,
> +					struct intel_crtc_state *crtc_state,
> +					struct drm_dp_as_sdp *async)
> +{
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	unsigned int type = DP_SDP_ADAPTIVE_SYNC;
> +	struct dp_sdp sdp = {};
> +	int ret;
> +
> +	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> +				 sizeof(sdp));
> +
> +	ret = intel_dp_as_sdp_unpack(async, &sdp, sizeof(sdp));
> +	if (ret)
> +		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP AS SDP\n");
> +
> +	return ret;
> +}
> +
>  static int
>  intel_dp_hdr_metadata_infoframe_sdp_unpack(struct hdmi_drm_infoframe *drm_infoframe,
>  					   const void *buffer, size_t size)
>  {
>  	int ret;
> -

Unrelated change.

>  	const struct dp_sdp *sdp = buffer;
>  
>  	if (size < sizeof(struct dp_sdp))
> @@ -4484,9 +4587,10 @@ static void intel_read_dp_vsc_sdp(struct intel_encoder *encoder,
>  		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP VSC SDP\n");
>  }
>  
> -static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
> -						     struct intel_crtc_state *crtc_state,
> -						     struct hdmi_drm_infoframe *drm_infoframe)
> +static void
> +intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
> +					 struct intel_crtc_state *crtc_state,
> +					 struct hdmi_drm_infoframe *drm_infoframe)

Unrelated change.

>  {
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -4495,7 +4599,7 @@ static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encod
>  	int ret;
>  
>  	if ((crtc_state->infoframes.enable &
> -	    intel_hdmi_infoframe_enable(type)) == 0)
> +	     intel_hdmi_infoframe_enable(type)) == 0)

Unrelated change.

>  		return;
>  
>  	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> @@ -4522,6 +4626,10 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
>  		intel_read_dp_hdr_metadata_infoframe_sdp(encoder, crtc_state,
>  							 &crtc_state->infoframes.drm.drm);
>  		break;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		intel_read_dp_as_metadata_infoframe_sdp(encoder, crtc_state,
> +							&crtc_state->infoframes.async);
> +	break;

Wrong indent, please run checkpatch before posting.

>  	default:
>  		MISSING_CASE(type);
>  		break;

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH 2/3] drm/i915/display/: Add Read/Write support for Adaptive Sync SDP

[Jani Nikula]

On Thu, 23 Nov 2023, Mitul Golani <mitulkumar.ajitkumar.golani@intel.com> wrote:
> Add the necessary structures and functions to handle reading and
> unpacking Adaptive Sync Secondary Data Packets. Also add support
> to write and pack AS SDP.

Also, subject prefix maybe "drm/i915/dp: ".

>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>  .../drm/i915/display/intel_display_types.h    |   1 +
>  drivers/gpu/drm/i915/display/intel_dp.c       | 118 +++++++++++++++++-
>  2 files changed, 114 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9a44350ba05d..7d87923f63af 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1325,6 +1325,7 @@ struct intel_crtc_state {
>  		union hdmi_infoframe hdmi;
>  		union hdmi_infoframe drm;
>  		struct drm_dp_vsc_sdp vsc;
> +		struct drm_dp_as_sdp  async;
>  	} infoframes;
>  
>  	u8 eld[MAX_ELD_BYTES];
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1422c2370269..39624746d612 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -94,6 +94,8 @@
>  #define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>  #define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>  
> +#define AS_SDP_ENABLE					REG_BIT(2)
> +#define AS_SDP_OP_MODE					REG_GENMASK(1, 0)
>  
>  /* Constants for DP DSC configurations */
>  static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15};
> @@ -4113,6 +4115,42 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
>  	return false;
>  }
>  
> +static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *async,
> +				    struct dp_sdp *sdp, size_t size)
> +{
> +	size_t length = sizeof(struct dp_sdp);
> +
> +	if (size < length)
> +		return -ENOSPC;
> +
> +	memset(sdp, 0, size);
> +
> +	/* Prepare AS (Adaptive Sync) VSC Header */
> +	sdp->sdp_header.HB0 = 0;
> +	sdp->sdp_header.HB1 = async->sdp_type;
> +	sdp->sdp_header.HB2 = 0x02;
> +	sdp->sdp_header.HB3 = async->length;
> +
> +	/* Fill AS (Adaptive Sync) SDP Payload */
> +	if ((sdp->db[0] & 0x03) == 0) {
> +		sdp->db[3] = 0;
> +		sdp->db[4] &= 0xFC;
> +	}
> +
> +	sdp->db[1] = async->vmin & 0xFF;
> +	sdp->db[2] = (async->vmin >> 8) & 0xF;
> +	sdp->db[17] = (async->vmin >> 8) & 0xFF;
> +	sdp->db[18] = async->vmax & 0xFF;
> +	sdp->db[19] = (async->vmax >> 8) & 0xFF;
> +	sdp->db[20] = async->target_rr & 0xFF;
> +	sdp->db[21] = (async->target_rr >> 8) & 0xFF;
> +	sdp->db[22] = async->duration_incr_ms;
> +	sdp->db[23] = async->duration_decr_ms;
> +	sdp->db[24] = async->operation_mode;
> +
> +	return length;
> +}
> +
>  static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
>  				     struct dp_sdp *sdp, size_t size)
>  {
> @@ -4280,6 +4318,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>  							       &crtc_state->infoframes.drm.drm,
>  							       &sdp, sizeof(sdp));
>  		break;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		len = intel_dp_as_sdp_pack(&crtc_state->infoframes.async, &sdp,
> +					   sizeof(sdp));
> +		break;
>  	default:
>  		MISSING_CASE(type);
>  		return;
> @@ -4342,6 +4384,44 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder,
>  	intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA);
>  }
>  
> +/*
> + * This function is to unpack AS SDP Packet
> + */
> +static
> +int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *async,
> +			   const void *buffer, size_t size)
> +{
> +	const struct dp_sdp *sdp = buffer;
> +
> +	if (size < sizeof(struct dp_sdp))
> +		return -EINVAL;
> +
> +	memset(async, 0, sizeof(*async));
> +
> +	if (sdp->sdp_header.HB0 != 0)
> +		return -EINVAL;
> +
> +	if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC)
> +		return -EINVAL;
> +
> +	if (sdp->sdp_header.HB2 != 0x02)
> +		return -EINVAL;
> +
> +	if ((sdp->sdp_header.HB3 & 0x3F) != 9)
> +		return -EINVAL;
> +
> +	if (sdp->db[0] != (AS_SDP_ENABLE | AS_SDP_OP_MODE))
> +		return -EINVAL;
> +
> +	async->vmin = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1];
> +	async->vmax = 0;
> +	async->target_rr = 0;
> +	async->duration_incr_ms = 0;
> +	async->duration_decr_ms = 0;
> +
> +	return 0;
> +}
> +
>  static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
>  				   const void *buffer, size_t size)
>  {
> @@ -4412,12 +4492,35 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
>  	return 0;
>  }
>  
> +/*
> + * This function to read registers to fetch packets
> + */
> +static int
> +intel_read_dp_as_metadata_infoframe_sdp(struct intel_encoder *encoder,
> +					struct intel_crtc_state *crtc_state,
> +					struct drm_dp_as_sdp *async)
> +{
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	unsigned int type = DP_SDP_ADAPTIVE_SYNC;
> +	struct dp_sdp sdp = {};
> +	int ret;
> +
> +	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> +				 sizeof(sdp));
> +
> +	ret = intel_dp_as_sdp_unpack(async, &sdp, sizeof(sdp));
> +	if (ret)
> +		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP AS SDP\n");
> +
> +	return ret;
> +}
> +
>  static int
>  intel_dp_hdr_metadata_infoframe_sdp_unpack(struct hdmi_drm_infoframe *drm_infoframe,
>  					   const void *buffer, size_t size)
>  {
>  	int ret;
> -
>  	const struct dp_sdp *sdp = buffer;
>  
>  	if (size < sizeof(struct dp_sdp))
> @@ -4484,9 +4587,10 @@ static void intel_read_dp_vsc_sdp(struct intel_encoder *encoder,
>  		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP VSC SDP\n");
>  }
>  
> -static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
> -						     struct intel_crtc_state *crtc_state,
> -						     struct hdmi_drm_infoframe *drm_infoframe)
> +static void
> +intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
> +					 struct intel_crtc_state *crtc_state,
> +					 struct hdmi_drm_infoframe *drm_infoframe)
>  {
>  	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -4495,7 +4599,7 @@ static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encod
>  	int ret;
>  
>  	if ((crtc_state->infoframes.enable &
> -	    intel_hdmi_infoframe_enable(type)) == 0)
> +	     intel_hdmi_infoframe_enable(type)) == 0)
>  		return;
>  
>  	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> @@ -4522,6 +4626,10 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
>  		intel_read_dp_hdr_metadata_infoframe_sdp(encoder, crtc_state,
>  							 &crtc_state->infoframes.drm.drm);
>  		break;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		intel_read_dp_as_metadata_infoframe_sdp(encoder, crtc_state,
> +							&crtc_state->infoframes.async);
> +	break;
>  	default:
>  		MISSING_CASE(type);
>  		break;

-- 
Jani Nikula, Intel

Re: [Intel-gfx] [PATCH 2/3] drm/i915/display/: Add Read/Write support for Adaptive Sync SDP

[Nautiyal, Ankit K]

On 11/23/2023 7:32 PM, Mitul Golani wrote:
> Add the necessary structures and functions to handle reading and
> unpacking Adaptive Sync Secondary Data Packets. Also add support
> to write and pack AS SDP.
>
> Signed-off-by: Mitul Golani <mitulkumar.ajitkumar.golani@intel.com>
> ---
>   .../drm/i915/display/intel_display_types.h    |   1 +
>   drivers/gpu/drm/i915/display/intel_dp.c       | 118 +++++++++++++++++-
>   2 files changed, 114 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
> index 9a44350ba05d..7d87923f63af 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_types.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_types.h
> @@ -1325,6 +1325,7 @@ struct intel_crtc_state {
>   		union hdmi_infoframe hdmi;
>   		union hdmi_infoframe drm;
>   		struct drm_dp_vsc_sdp vsc;
> +		struct drm_dp_as_sdp  async;
>   	} infoframes;
>   
>   	u8 eld[MAX_ELD_BYTES];
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 1422c2370269..39624746d612 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -94,6 +94,8 @@
>   #define INTEL_DP_RESOLUTION_STANDARD	(2 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>   #define INTEL_DP_RESOLUTION_FAILSAFE	(3 << INTEL_DP_RESOLUTION_SHIFT_MASK)
>   
> +#define AS_SDP_ENABLE					REG_BIT(2)
> +#define AS_SDP_OP_MODE					REG_GENMASK(1, 0)
>   
>   /* Constants for DP DSC configurations */
>   static const u8 valid_dsc_bpp[] = {6, 8, 10, 12, 15};
> @@ -4113,6 +4115,42 @@ intel_dp_needs_vsc_sdp(const struct intel_crtc_state *crtc_state,
>   	return false;
>   }
>   
> +static ssize_t intel_dp_as_sdp_pack(const struct drm_dp_as_sdp *async,
> +				    struct dp_sdp *sdp, size_t size)
> +{
> +	size_t length = sizeof(struct dp_sdp);
> +
> +	if (size < length)
> +		return -ENOSPC;
> +
> +	memset(sdp, 0, size);
> +
> +	/* Prepare AS (Adaptive Sync) VSC Header */
> +	sdp->sdp_header.HB0 = 0;
> +	sdp->sdp_header.HB1 = async->sdp_type;
> +	sdp->sdp_header.HB2 = 0x02;
> +	sdp->sdp_header.HB3 = async->length;
> +
> +	/* Fill AS (Adaptive Sync) SDP Payload */
> +	if ((sdp->db[0] & 0x03) == 0) {
> +		sdp->db[3] = 0;
> +		sdp->db[4] &= 0xFC;
> +	}
> +
> +	sdp->db[1] = async->vmin & 0xFF;
> +	sdp->db[2] = (async->vmin >> 8) & 0xF;
> +	sdp->db[17] = (async->vmin >> 8) & 0xFF;
> +	sdp->db[18] = async->vmax & 0xFF;
> +	sdp->db[19] = (async->vmax >> 8) & 0xFF;
> +	sdp->db[20] = async->target_rr & 0xFF;
> +	sdp->db[21] = (async->target_rr >> 8) & 0xFF;
> +	sdp->db[22] = async->duration_incr_ms;
> +	sdp->db[23] = async->duration_decr_ms;
> +	sdp->db[24] = async->operation_mode;

This doesnt look correct, DB9-31 are supposed to be 0, am  I missing 
something? Can you re-check this?


> +
> +	return length;
> +}
> +
>   static ssize_t intel_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc,
>   				     struct dp_sdp *sdp, size_t size)
>   {
> @@ -4280,6 +4318,10 @@ static void intel_write_dp_sdp(struct intel_encoder *encoder,
>   							       &crtc_state->infoframes.drm.drm,
>   							       &sdp, sizeof(sdp));
>   		break;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		len = intel_dp_as_sdp_pack(&crtc_state->infoframes.async, &sdp,
> +					   sizeof(sdp));
> +		break;

The function intel_write_dp_sdp with type DP_SDP_ADAPTIVE_SYNC needs to 
be called from intel_dp_set_infoframes.

I see this is missing, perhaps to be added as last patch.


>   	default:
>   		MISSING_CASE(type);
>   		return;
> @@ -4342,6 +4384,44 @@ void intel_dp_set_infoframes(struct intel_encoder *encoder,
>   	intel_write_dp_sdp(encoder, crtc_state, HDMI_PACKET_TYPE_GAMUT_METADATA);
>   }
>   
> +/*
> + * This function is to unpack AS SDP Packet
> + */
> +static
> +int intel_dp_as_sdp_unpack(struct drm_dp_as_sdp *async,
> +			   const void *buffer, size_t size)
> +{
> +	const struct dp_sdp *sdp = buffer;
> +
> +	if (size < sizeof(struct dp_sdp))
> +		return -EINVAL;
> +
> +	memset(async, 0, sizeof(*async));
> +
> +	if (sdp->sdp_header.HB0 != 0)
> +		return -EINVAL;
> +
> +	if (sdp->sdp_header.HB1 != DP_SDP_ADAPTIVE_SYNC)
> +		return -EINVAL;
> +
> +	if (sdp->sdp_header.HB2 != 0x02)
> +		return -EINVAL;
> +
> +	if ((sdp->sdp_header.HB3 & 0x3F) != 9)
> +		return -EINVAL;
> +
> +	if (sdp->db[0] != (AS_SDP_ENABLE | AS_SDP_OP_MODE))
> +		return -EINVAL;
> +
> +	async->vmin = ((u64)sdp->db[2] << 32) | (u64)sdp->db[1];
> +	async->vmax = 0;
> +	async->target_rr = 0;
> +	async->duration_incr_ms = 0;
> +	async->duration_decr_ms = 0;
> +
> +	return 0;
> +}
> +
>   static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
>   				   const void *buffer, size_t size)
>   {
> @@ -4412,12 +4492,35 @@ static int intel_dp_vsc_sdp_unpack(struct drm_dp_vsc_sdp *vsc,
>   	return 0;
>   }
>   
> +/*
> + * This function to read registers to fetch packets
> + */
> +static int
> +intel_read_dp_as_metadata_infoframe_sdp(struct intel_encoder *encoder,
> +					struct intel_crtc_state *crtc_state,
> +					struct drm_dp_as_sdp *async)
> +{
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> +	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> +	unsigned int type = DP_SDP_ADAPTIVE_SYNC;
> +	struct dp_sdp sdp = {};
> +	int ret;
> +
> +	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> +				 sizeof(sdp));
> +
> +	ret = intel_dp_as_sdp_unpack(async, &sdp, sizeof(sdp));
> +	if (ret)
> +		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP AS SDP\n");
> +
> +	return ret;
> +}
> +
>   static int
>   intel_dp_hdr_metadata_infoframe_sdp_unpack(struct hdmi_drm_infoframe *drm_infoframe,
>   					   const void *buffer, size_t size)
>   {
>   	int ret;
> -
>   	const struct dp_sdp *sdp = buffer;
>   
>   	if (size < sizeof(struct dp_sdp))
> @@ -4484,9 +4587,10 @@ static void intel_read_dp_vsc_sdp(struct intel_encoder *encoder,
>   		drm_dbg_kms(&dev_priv->drm, "Failed to unpack DP VSC SDP\n");
>   }
>   
> -static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
> -						     struct intel_crtc_state *crtc_state,
> -						     struct hdmi_drm_infoframe *drm_infoframe)
> +static void
> +intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encoder,
> +					 struct intel_crtc_state *crtc_state,
> +					 struct hdmi_drm_infoframe *drm_infoframe)
>   {
>   	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>   	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> @@ -4495,7 +4599,7 @@ static void intel_read_dp_hdr_metadata_infoframe_sdp(struct intel_encoder *encod
>   	int ret;
>   
>   	if ((crtc_state->infoframes.enable &
> -	    intel_hdmi_infoframe_enable(type)) == 0)
> +	     intel_hdmi_infoframe_enable(type)) == 0)
>   		return;
>   
>   	dig_port->read_infoframe(encoder, crtc_state, type, &sdp,
> @@ -4522,6 +4626,10 @@ void intel_read_dp_sdp(struct intel_encoder *encoder,
>   		intel_read_dp_hdr_metadata_infoframe_sdp(encoder, crtc_state,
>   							 &crtc_state->infoframes.drm.drm);
>   		break;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		intel_read_dp_as_metadata_infoframe_sdp(encoder, crtc_state,
> +							&crtc_state->infoframes.async);
> +	break;


Similar to the write case, call to intel_read_dp_sdp with 
DP_SDP_ADAPTIVE_SYNC is missing from intel_ddi_get_config.

So need a patch to actually call read and writes.

Regards,

Ankit

>   	default:
>   		MISSING_CASE(type);
>   		break;

Re: [Intel-gfx] [PATCH 3/3] drm/i915/display/:Compute and enable daptive Sync SDP

[Nautiyal, Ankit K]

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

Typo in the Subject: s/daptive/adaptive/


On 11/23/2023 7:32 PM, Mitul Golani wrote:
> Add necessary functions and register definitions to enable
> and compute AS SDP data. The new `intel_dp_compute_as_sdp`
> function computes AS SDP values based on the display
> configuration, ensuring proper handling of Variable Refresh
> Rate (VRR).
>
> Signed-off-by: Mitul Golani<mitulkumar.ajitkumar.golani@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp.c   | 21 +++++++++++++++++++++
>   drivers/gpu/drm/i915/display/intel_hdmi.c | 11 +++++++++--
>   drivers/gpu/drm/i915/i915_reg.h           |  6 ++++++
>   3 files changed, 36 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 39624746d612..b3eb2d342a99 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -2629,6 +2629,26 @@ static void intel_dp_compute_vsc_sdp(struct intel_dp *intel_dp,
>   					 &crtc_state->infoframes.vsc);
>   }
>   
> +static void intel_dp_compute_as_sdp(struct intel_dp *intel_dp,
> +				    struct intel_crtc_state *crtc_state,
> +				     const struct drm_connector_state *conn_state)
> +{
> +	struct drm_dp_as_sdp *async = &crtc_state->infoframes.async;
> +	struct intel_connector *connector = intel_dp->attached_connector;
> +	int vrefresh = drm_mode_vrefresh(&crtc_state->hw.adjusted_mode);
> +
> +	if (intel_vrr_is_in_range(connector, vrefresh))
> +		return;
> +
> +	crtc_state->infoframes.enable |= intel_hdmi_infoframe_enable(DP_SDP_ADAPTIVE_SYNC);

To make this work, need to add DP_SDP_ADAPTIVE_SYNC to 
infoframe_type_to_idx().


> +	async->sdp_type = DP_SDP_ADAPTIVE_SYNC;
> +	async->length = 0x9;
> +	async->vmin = crtc_state->vrr.vmin;
> +	async->vmax = crtc_state->vrr.vmax;
> +	async->target_rr = 0;
> +	async->operation_mode = 0x0;
> +}
> +
>   void intel_dp_compute_psr_vsc_sdp(struct intel_dp *intel_dp,
>   				  const struct intel_crtc_state *crtc_state,
>   				  const struct drm_connector_state *conn_state,
> @@ -2965,6 +2985,7 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>   	intel_psr_compute_config(intel_dp, pipe_config, conn_state);
>   	intel_dp_drrs_compute_config(connector, pipe_config, link_bpp_x16);
>   	intel_dp_compute_vsc_sdp(intel_dp, pipe_config, conn_state);
> +	intel_dp_compute_as_sdp(intel_dp, pipe_config, conn_state);


IMHO, This compute part and read and write calls to 
intel_read/write_dp_sdp should be in separate patch

>   	intel_dp_compute_hdr_metadata_infoframe_sdp(intel_dp, pipe_config, conn_state);
>   
>   	return 0;
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index ab18cfc19c0a..abea359985ce 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
> @@ -136,6 +136,8 @@ static u32 hsw_infoframe_enable(unsigned int type)
>   		return VIDEO_DIP_ENABLE_GMP_HSW;
>   	case DP_SDP_VSC:
>   		return VIDEO_DIP_ENABLE_VSC_HSW;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		return VIDEO_DIP_ENABLE_AS_HSW;
>   	case DP_SDP_PPS:
>   		return VDIP_ENABLE_PPS;
>   	case HDMI_INFOFRAME_TYPE_AVI:
> @@ -163,6 +165,8 @@ hsw_dip_data_reg(struct drm_i915_private *dev_priv,
>   		return HSW_TVIDEO_DIP_GMP_DATA(cpu_transcoder, i);
>   	case DP_SDP_VSC:
>   		return HSW_TVIDEO_DIP_VSC_DATA(cpu_transcoder, i);
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		return HSW_TVIDEO_DIP_ASYNC_DATA(cpu_transcoder, i);
>   	case DP_SDP_PPS:
>   		return ICL_VIDEO_DIP_PPS_DATA(cpu_transcoder, i);
>   	case HDMI_INFOFRAME_TYPE_AVI:
> @@ -185,6 +189,8 @@ static int hsw_dip_data_size(struct drm_i915_private *dev_priv,
>   	switch (type) {
>   	case DP_SDP_VSC:
>   		return VIDEO_DIP_VSC_DATA_SIZE;
> +	case DP_SDP_ADAPTIVE_SYNC:
> +		return VIDEO_DIP_ASYNC_DATA_SIZE;
>   	case DP_SDP_PPS:
>   		return VIDEO_DIP_PPS_DATA_SIZE;
>   	case HDMI_PACKET_TYPE_GAMUT_METADATA:
> @@ -555,7 +561,8 @@ static u32 hsw_infoframes_enabled(struct intel_encoder *encoder,
>   
>   	mask = (VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
>   		VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
> -		VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW);
> +		VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
> +		VIDEO_DIP_ENABLE_AS_HSW);
>   
>   	if (DISPLAY_VER(dev_priv) >= 10)
>   		mask |= VIDEO_DIP_ENABLE_DRM_GLK;
> @@ -1209,7 +1216,7 @@ static void hsw_set_infoframes(struct intel_encoder *encoder,
>   	val &= ~(VIDEO_DIP_ENABLE_VSC_HSW | VIDEO_DIP_ENABLE_AVI_HSW |
>   		 VIDEO_DIP_ENABLE_GCP_HSW | VIDEO_DIP_ENABLE_VS_HSW |
>   		 VIDEO_DIP_ENABLE_GMP_HSW | VIDEO_DIP_ENABLE_SPD_HSW |
> -		 VIDEO_DIP_ENABLE_DRM_GLK);
> +		 VIDEO_DIP_ENABLE_DRM_GLK | VIDEO_DIP_ENABLE_AS_HSW);
>   
>   	if (!enable) {
>   		intel_de_write(dev_priv, reg, val);
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 27dc903f0553..81d64c428693 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -2312,6 +2312,7 @@
>    * (Haswell and newer) to see which VIDEO_DIP_DATA byte corresponds to each byte
>    * of the infoframe structure specified by CEA-861. */
>   #define   VIDEO_DIP_DATA_SIZE	32
> +#define   VIDEO_DIP_ASYNC_DATA_SIZE	32
>   #define   VIDEO_DIP_GMP_DATA_SIZE	36
>   #define   VIDEO_DIP_VSC_DATA_SIZE	36
>   #define   VIDEO_DIP_PPS_DATA_SIZE	132
> @@ -2344,6 +2345,7 @@
>   #define   VSC_DIP_HW_DATA_SW_HEA	(2 << 25)
>   #define   VSC_DIP_SW_HEA_DATA		(3 << 25)
>   #define   VDIP_ENABLE_PPS		(1 << 24)
> +#define   VIDEO_DIP_ENABLE_AS_HSW	REG_BIT(23)
This is defined for ADLP+
>   #define   VIDEO_DIP_ENABLE_VSC_HSW	(1 << 20)
>   #define   VIDEO_DIP_ENABLE_GCP_HSW	(1 << 16)
>   #define   VIDEO_DIP_ENABLE_AVI_HSW	(1 << 12)
> @@ -5038,6 +5040,7 @@
>   #define _HSW_VIDEO_DIP_SPD_DATA_A	0x602A0
>   #define _HSW_VIDEO_DIP_GMP_DATA_A	0x602E0
>   #define _HSW_VIDEO_DIP_VSC_DATA_A	0x60320
> +#define	_HSW_VIDEO_DIP_ASYNC_DATA_A	0x60484

These are defined from ADL onwards.

Also indentation seems to be off.


Regards,

Ankit

>   #define _GLK_VIDEO_DIP_DRM_DATA_A	0x60440
>   #define _HSW_VIDEO_DIP_AVI_ECC_A	0x60240
>   #define _HSW_VIDEO_DIP_VS_ECC_A		0x60280
> @@ -5052,6 +5055,7 @@
>   #define _HSW_VIDEO_DIP_SPD_DATA_B	0x612A0
>   #define _HSW_VIDEO_DIP_GMP_DATA_B	0x612E0
>   #define _HSW_VIDEO_DIP_VSC_DATA_B	0x61320
> +#define	_HSW_VIDEO_DIP_ASYNC_DATA_B	0x61484
>   #define _GLK_VIDEO_DIP_DRM_DATA_B	0x61440
>   #define _HSW_VIDEO_DIP_BVI_ECC_B	0x61240
>   #define _HSW_VIDEO_DIP_VS_ECC_B		0x61280
> @@ -5078,6 +5082,8 @@
>   #define HSW_TVIDEO_DIP_SPD_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_SPD_DATA_A + (i) * 4)
>   #define HSW_TVIDEO_DIP_GMP_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_GMP_DATA_A + (i) * 4)
>   #define HSW_TVIDEO_DIP_VSC_DATA(trans, i)	_MMIO_TRANS2(trans, _HSW_VIDEO_DIP_VSC_DATA_A + (i) * 4)
> +#define HSW_TVIDEO_DIP_ASYNC_DATA(trans, i)	_MMIO_TRANS2(trans,\
> +							     _HSW_VIDEO_DIP_ASYNC_DATA_A + (i) * 4)
>   #define GLK_TVIDEO_DIP_DRM_DATA(trans, i)	_MMIO_TRANS2(trans, _GLK_VIDEO_DIP_DRM_DATA_A + (i) * 4)
>   #define ICL_VIDEO_DIP_PPS_DATA(trans, i)	_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_DATA_A + (i) * 4)
>   #define ICL_VIDEO_DIP_PPS_ECC(trans, i)		_MMIO_TRANS2(trans, _ICL_VIDEO_DIP_PPS_ECC_A + (i) * 4)

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