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