Re: [PATCH] drm/i915/gvt: use macros from drm_dp.h instead of duplication

Zhi Wang zhi.wang.linux at gmail.com
Thu Sep 19 22:25:56 UTC 2024



On September 17, 2024 12:52:26 PM GMT+02:00, Jani Nikula <jani.nikula at intel.com> wrote:
>Use the existing macros in drm_dp.h for DPCD and DP AUX instead of
>duplicating. Remove unused macros, as well as the duplicate definition
>of DPCD_SIZE.
>
>AUX_NATIVE_REPLY_NAK is left unchanged, as it does not match
>DP_AUX_NATIVE_REPLY_NACK, and I'm not sure what the right thing to do is
>here.
>

Guess it must be the emulation routines from the older platforms e.g IVB or HSW and forgot to update when moving to SKL and later...I can't access the GPU spec now. Will talk to Zhenyu when I met him on the weekend in the summit. 

>Signed-off-by: Jani Nikula <jani.nikula at intel.com>
>---
> drivers/gpu/drm/i915/gvt/display.c  |  4 ++-
> drivers/gpu/drm/i915/gvt/display.h  | 42 ----------------------------
> drivers/gpu/drm/i915/gvt/edid.c     | 12 ++++----
> drivers/gpu/drm/i915/gvt/edid.h     |  8 ------
> drivers/gpu/drm/i915/gvt/handlers.c | 43 +++++++++++++++++------------
> 5 files changed, 36 insertions(+), 73 deletions(-)
>
>diff --git a/drivers/gpu/drm/i915/gvt/display.c b/drivers/gpu/drm/i915/gvt/display.c
>index c66d6d3177c8..17f74cb244bb 100644
>--- a/drivers/gpu/drm/i915/gvt/display.c
>+++ b/drivers/gpu/drm/i915/gvt/display.c
>@@ -32,6 +32,8 @@
>  *
>  */
> 
>+#include <drm/display/drm_dp.h>
>+
> #include "i915_drv.h"
> #include "i915_reg.h"
> #include "gvt.h"
>@@ -568,7 +570,7 @@ static int setup_virtual_dp_monitor(struct intel_vgpu *vgpu, int port_num,
> 
> 	memcpy(port->dpcd->data, dpcd_fix_data, DPCD_HEADER_SIZE);
> 	port->dpcd->data_valid = true;
>-	port->dpcd->data[DPCD_SINK_COUNT] = 0x1;
>+	port->dpcd->data[DP_SINK_COUNT] = 0x1;
> 	port->type = type;
> 	port->id = resolution;
> 	port->vrefresh_k = GVT_DEFAULT_REFRESH_RATE * MSEC_PER_SEC;
>diff --git a/drivers/gpu/drm/i915/gvt/display.h b/drivers/gpu/drm/i915/gvt/display.h
>index f5616f99ef2f..8090bc53c7e1 100644
>--- a/drivers/gpu/drm/i915/gvt/display.h
>+++ b/drivers/gpu/drm/i915/gvt/display.h
>@@ -59,52 +59,10 @@ struct intel_vgpu;
> 
> #define INTEL_GVT_MAX_UEVENT_VARS	3
> 
>-/* DPCD start */
>-#define DPCD_SIZE	0x700
>-
>-/* DPCD */
>-#define DP_SET_POWER            0x600
>-#define DP_SET_POWER_D0         0x1
>-#define AUX_NATIVE_WRITE        0x8
>-#define AUX_NATIVE_READ         0x9
>-
>-#define AUX_NATIVE_REPLY_MASK   (0x3 << 4)
>-#define AUX_NATIVE_REPLY_ACK    (0x0 << 4)
> #define AUX_NATIVE_REPLY_NAK    (0x1 << 4)
>-#define AUX_NATIVE_REPLY_DEFER  (0x2 << 4)
> 
> #define AUX_BURST_SIZE          20
> 
>-/* DPCD addresses */
>-#define DPCD_REV			0x000
>-#define DPCD_MAX_LINK_RATE		0x001
>-#define DPCD_MAX_LANE_COUNT		0x002
>-
>-#define DPCD_TRAINING_PATTERN_SET	0x102
>-#define	DPCD_SINK_COUNT			0x200
>-#define DPCD_LANE0_1_STATUS		0x202
>-#define DPCD_LANE2_3_STATUS		0x203
>-#define DPCD_LANE_ALIGN_STATUS_UPDATED	0x204
>-#define DPCD_SINK_STATUS		0x205
>-
>-/* link training */
>-#define DPCD_TRAINING_PATTERN_SET_MASK	0x03
>-#define DPCD_LINK_TRAINING_DISABLED	0x00
>-#define DPCD_TRAINING_PATTERN_1		0x01
>-#define DPCD_TRAINING_PATTERN_2		0x02
>-
>-#define DPCD_CP_READY_MASK		(1 << 6)
>-
>-/* lane status */
>-#define DPCD_LANES_CR_DONE		0x11
>-#define DPCD_LANES_EQ_DONE		0x22
>-#define DPCD_SYMBOL_LOCKED		0x44
>-
>-#define DPCD_INTERLANE_ALIGN_DONE	0x01
>-
>-#define DPCD_SINK_IN_SYNC		0x03
>-/* DPCD end */
>-
> #define SBI_RESPONSE_MASK               0x3
> #define SBI_RESPONSE_SHIFT              0x1
> #define SBI_STAT_MASK                   0x1
>diff --git a/drivers/gpu/drm/i915/gvt/edid.c b/drivers/gpu/drm/i915/gvt/edid.c
>index c022dc736045..0a357ca42db1 100644
>--- a/drivers/gpu/drm/i915/gvt/edid.c
>+++ b/drivers/gpu/drm/i915/gvt/edid.c
>@@ -32,6 +32,8 @@
>  *
>  */
> 
>+#include <drm/display/drm_dp.h>
>+
> #include "display/intel_dp_aux_regs.h"
> #include "display/intel_gmbus_regs.h"
> #include "gvt.h"
>@@ -504,13 +506,13 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu *vgpu,
> 	}
> 
> 	/* Always set the wanted value for vms. */
>-	ret_msg_size = (((op & 0x1) == GVT_AUX_I2C_READ) ? 2 : 1);
>+	ret_msg_size = (((op & 0x1) == DP_AUX_I2C_READ) ? 2 : 1);
> 	vgpu_vreg(vgpu, offset) =
> 		DP_AUX_CH_CTL_DONE |
> 		DP_AUX_CH_CTL_MESSAGE_SIZE(ret_msg_size);
> 
> 	if (msg_length == 3) {
>-		if (!(op & GVT_AUX_I2C_MOT)) {
>+		if (!(op & DP_AUX_I2C_MOT)) {
> 			/* stop */
> 			intel_vgpu_init_i2c_edid(vgpu);
> 		} else {
>@@ -530,7 +532,7 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu *vgpu,
> 					i2c_edid->edid_available = true;
> 			}
> 		}
>-	} else if ((op & 0x1) == GVT_AUX_I2C_WRITE) {
>+	} else if ((op & 0x1) == DP_AUX_I2C_WRITE) {
> 		/* TODO
> 		 * We only support EDID reading from I2C_over_AUX. And
> 		 * we do not expect the index mode to be used. Right now
>@@ -538,7 +540,7 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu *vgpu,
> 		 * support the gfx driver to do EDID access.
> 		 */
> 	} else {
>-		if (drm_WARN_ON(&i915->drm, (op & 0x1) != GVT_AUX_I2C_READ))
>+		if (drm_WARN_ON(&i915->drm, (op & 0x1) != DP_AUX_I2C_READ))
> 			return;
> 		if (drm_WARN_ON(&i915->drm, msg_length != 4))
> 			return;
>@@ -553,7 +555,7 @@ void intel_gvt_i2c_handle_aux_ch_write(struct intel_vgpu *vgpu,
> 	 * ACK of I2C_WRITE
> 	 * returned byte if it is READ
> 	 */
>-	aux_data_for_write |= GVT_AUX_I2C_REPLY_ACK << 24;
>+	aux_data_for_write |= DP_AUX_I2C_REPLY_ACK << 24;
> 	vgpu_vreg(vgpu, offset + 4) = aux_data_for_write;
> }
> 
>diff --git a/drivers/gpu/drm/i915/gvt/edid.h b/drivers/gpu/drm/i915/gvt/edid.h
>index c3b5a55aecb3..13fd06590929 100644
>--- a/drivers/gpu/drm/i915/gvt/edid.h
>+++ b/drivers/gpu/drm/i915/gvt/edid.h
>@@ -42,14 +42,6 @@ struct intel_vgpu;
> #define EDID_SIZE		128
> #define EDID_ADDR		0x50 /* Linux hvm EDID addr */
> 
>-#define GVT_AUX_NATIVE_WRITE			0x8
>-#define GVT_AUX_NATIVE_READ			0x9
>-#define GVT_AUX_I2C_WRITE			0x0
>-#define GVT_AUX_I2C_READ			0x1
>-#define GVT_AUX_I2C_STATUS			0x2
>-#define GVT_AUX_I2C_MOT				0x4
>-#define GVT_AUX_I2C_REPLY_ACK			0x0
>-
> struct intel_vgpu_edid_data {
> 	bool data_valid;
> 	unsigned char edid_block[EDID_SIZE];
>diff --git a/drivers/gpu/drm/i915/gvt/handlers.c b/drivers/gpu/drm/i915/gvt/handlers.c
>index 0f09344d3c20..9494d812c00a 100644
>--- a/drivers/gpu/drm/i915/gvt/handlers.c
>+++ b/drivers/gpu/drm/i915/gvt/handlers.c
>@@ -36,6 +36,8 @@
> 
>  */
> 
>+#include <drm/display/drm_dp.h>
>+
> #include "i915_drv.h"
> #include "i915_reg.h"
> #include "gvt.h"
>@@ -1129,29 +1131,36 @@ static int dp_aux_ch_ctl_trans_done(struct intel_vgpu *vgpu, u32 value,
> static void dp_aux_ch_ctl_link_training(struct intel_vgpu_dpcd_data *dpcd,
> 		u8 t)
> {
>-	if ((t & DPCD_TRAINING_PATTERN_SET_MASK) == DPCD_TRAINING_PATTERN_1) {
>+	if ((t & DP_TRAINING_PATTERN_MASK) == DP_TRAINING_PATTERN_1) {
> 		/* training pattern 1 for CR */
> 		/* set LANE0_CR_DONE, LANE1_CR_DONE */
>-		dpcd->data[DPCD_LANE0_1_STATUS] |= DPCD_LANES_CR_DONE;
>+		dpcd->data[DP_LANE0_1_STATUS] |= DP_LANE_CR_DONE |
>+			DP_LANE_CR_DONE << 4;
> 		/* set LANE2_CR_DONE, LANE3_CR_DONE */
>-		dpcd->data[DPCD_LANE2_3_STATUS] |= DPCD_LANES_CR_DONE;
>-	} else if ((t & DPCD_TRAINING_PATTERN_SET_MASK) ==
>-			DPCD_TRAINING_PATTERN_2) {
>+		dpcd->data[DP_LANE2_3_STATUS] |= DP_LANE_CR_DONE |
>+			DP_LANE_CR_DONE << 4;
>+	} else if ((t & DP_TRAINING_PATTERN_MASK) ==
>+			DP_TRAINING_PATTERN_2) {
> 		/* training pattern 2 for EQ */
> 		/* Set CHANNEL_EQ_DONE and  SYMBOL_LOCKED for Lane0_1 */
>-		dpcd->data[DPCD_LANE0_1_STATUS] |= DPCD_LANES_EQ_DONE;
>-		dpcd->data[DPCD_LANE0_1_STATUS] |= DPCD_SYMBOL_LOCKED;
>+		dpcd->data[DP_LANE0_1_STATUS] |= DP_LANE_CHANNEL_EQ_DONE |
>+			DP_LANE_CHANNEL_EQ_DONE << 4;
>+		dpcd->data[DP_LANE0_1_STATUS] |= DP_LANE_SYMBOL_LOCKED |
>+			DP_LANE_SYMBOL_LOCKED << 4;
> 		/* Set CHANNEL_EQ_DONE and  SYMBOL_LOCKED for Lane2_3 */
>-		dpcd->data[DPCD_LANE2_3_STATUS] |= DPCD_LANES_EQ_DONE;
>-		dpcd->data[DPCD_LANE2_3_STATUS] |= DPCD_SYMBOL_LOCKED;
>+		dpcd->data[DP_LANE2_3_STATUS] |= DP_LANE_CHANNEL_EQ_DONE |
>+			DP_LANE_CHANNEL_EQ_DONE << 4;
>+		dpcd->data[DP_LANE2_3_STATUS] |= DP_LANE_SYMBOL_LOCKED |
>+			DP_LANE_SYMBOL_LOCKED << 4;
> 		/* set INTERLANE_ALIGN_DONE */
>-		dpcd->data[DPCD_LANE_ALIGN_STATUS_UPDATED] |=
>-			DPCD_INTERLANE_ALIGN_DONE;
>-	} else if ((t & DPCD_TRAINING_PATTERN_SET_MASK) ==
>-			DPCD_LINK_TRAINING_DISABLED) {
>+		dpcd->data[DP_LANE_ALIGN_STATUS_UPDATED] |=
>+			DP_INTERLANE_ALIGN_DONE;
>+	} else if ((t & DP_TRAINING_PATTERN_MASK) ==
>+			DP_TRAINING_PATTERN_DISABLE) {
> 		/* finish link training */
> 		/* set sink status as synchronized */
>-		dpcd->data[DPCD_SINK_STATUS] = DPCD_SINK_IN_SYNC;
>+		dpcd->data[DP_SINK_STATUS] = DP_RECEIVE_PORT_0_STATUS |
>+			DP_RECEIVE_PORT_1_STATUS;
> 	}
> }
> 
>@@ -1206,7 +1215,7 @@ static int dp_aux_ch_ctl_mmio_write(struct intel_vgpu *vgpu,
> 	len = msg & 0xff;
> 	op = ctrl >> 4;
> 
>-	if (op == GVT_AUX_NATIVE_WRITE) {
>+	if (op == DP_AUX_NATIVE_WRITE) {
> 		int t;
> 		u8 buf[16];
> 
>@@ -1252,7 +1261,7 @@ static int dp_aux_ch_ctl_mmio_write(struct intel_vgpu *vgpu,
> 
> 				dpcd->data[p] = buf[t];
> 				/* check for link training */
>-				if (p == DPCD_TRAINING_PATTERN_SET)
>+				if (p == DP_TRAINING_PATTERN_SET)
> 					dp_aux_ch_ctl_link_training(dpcd,
> 							buf[t]);
> 			}
>@@ -1265,7 +1274,7 @@ static int dp_aux_ch_ctl_mmio_write(struct intel_vgpu *vgpu,
> 		return 0;
> 	}
> 
>-	if (op == GVT_AUX_NATIVE_READ) {
>+	if (op == DP_AUX_NATIVE_READ) {
> 		int idx, i, ret = 0;
> 
> 		if ((addr + len + 1) >= DPCD_SIZE) {


More information about the intel-gvt-dev mailing list