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

Kandpal, Suraj suraj.kandpal at intel.com
Tue Oct 22 11:49:59 UTC 2024



> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Jani
> Nikula
> Sent: Monday, September 30, 2024 7:24 PM
> To: intel-gfx at lists.freedesktop.org; intel-gvt-dev at lists.freedesktop.org
> Cc: Nikula, Jani <jani.nikula at intel.com>
> Subject: [RESEND] drm/i915/gvt: use macros from drm_dp.h instead of
> duplication
> 
> 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.

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

> 
> 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) {
> --
> 2.39.5



More information about the Intel-gfx mailing list