Reviewed-by: Zhi Wang <[email protected]>
Sorry for the late reply as I was on vacation. On Tue, Oct 22, 2024, 2:50 PM Kandpal, Suraj <[email protected]> wrote: > > > > -----Original Message----- > > From: Intel-gfx <[email protected]> On Behalf Of > Jani > > Nikula > > Sent: Monday, September 30, 2024 7:24 PM > > To: [email protected]; [email protected] > > Cc: Nikula, Jani <[email protected]> > > 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 <[email protected]> > > > > > Signed-off-by: Jani Nikula <[email protected]> > > --- > > 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 > >
