This series adds debugfs support for the zynqmp_dp driver. The intent is
to enable compliance testing or to help debug signal-integrity issues.
The first three patches are general improvements (and could be applied
independently), while the last three add debugfs support.
Sean Anderson (6):
drm: zynqmp_dp: Downgrade log level for aux retries message
drm: zynqmp_dp: Adjust training values per-lane
drm: zynqmp_dp: Add locking
drm: zynqmp_dp: Split off several helper functions
drm: zynqmp_dp: Optionally ignore DPCD errors
drm: zynqmp_dp: Add debugfs interface for compliance testing
drivers/gpu/drm/xlnx/zynqmp_dp.c | 749 ++++++++++++++++++++++++++++---
1 file changed, 691 insertions(+), 58 deletions(-)
--
2.35.1.1320.gc452695387.dirty
Enable this message for verbose debugging only as it is otherwise
printed after every AUX message, quickly filling the log buffer.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index a0606fab0e22..98a32e6a0459 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -1006,7 +1006,7 @@ zynqmp_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
msg->buffer, msg->size,
&msg->reply);
if (!ret) {
- dev_dbg(dp->dev, "aux %d retries\n", i);
+ dev_vdbg(dp->dev, "aux %d retries\n", i);
return msg->size;
}
--
2.35.1.1320.gc452695387.dirty
The feedback we get from the DPRX is per-lane. Make changes using this
information, instead of picking the maximum values from all lanes. This
results in more-consistent training on marginal links.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++---------------
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 98a32e6a0459..8635b5673386 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
u8 link_status[DP_LINK_STATUS_SIZE])
{
u8 *train_set = dp->train_set;
- u8 voltage = 0, preemphasis = 0;
u8 i;
for (i = 0; i < dp->mode.lane_cnt; i++) {
- u8 v = drm_dp_get_adjust_request_voltage(link_status, i);
- u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i);
+ u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i);
+ u8 preemphasis =
+ drm_dp_get_adjust_request_pre_emphasis(link_status, i);
- if (v > voltage)
- voltage = v;
+ if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3)
+ voltage |= DP_TRAIN_MAX_SWING_REACHED;
- if (p > preemphasis)
- preemphasis = p;
- }
+ if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2)
+ preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
- if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3)
- voltage |= DP_TRAIN_MAX_SWING_REACHED;
-
- if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2)
- preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
-
- for (i = 0; i < dp->mode.lane_cnt; i++)
train_set[i] = voltage | preemphasis;
+ }
}
/**
--
2.35.1.1320.gc452695387.dirty
In preparation for supporting compliance testing, split off several
helper functions. No functional change intended.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 49 +++++++++++++++++++++-----------
1 file changed, 33 insertions(+), 16 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index d2dee58e7bf2..24043847dab4 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -627,6 +627,7 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
/**
* zynqmp_dp_update_vs_emph - Update the training values
* @dp: DisplayPort IP core structure
+ * @train_set: A set of training values
*
* Update the training values based on the request from sink. The mapped values
* are predefined, and values(vs, pe, pc) are from the device manual.
@@ -634,12 +635,12 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
* Return: 0 if vs and emph are updated successfully, or the error code returned
* by drm_dp_dpcd_write().
*/
-static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp)
+static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set)
{
unsigned int i;
int ret;
- ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, dp->train_set,
+ ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set,
dp->mode.lane_cnt);
if (ret < 0)
return ret;
@@ -647,7 +648,7 @@ static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp)
for (i = 0; i < dp->mode.lane_cnt; i++) {
u32 reg = ZYNQMP_DP_SUB_TX_PHY_PRECURSOR_LANE_0 + i * 4;
union phy_configure_opts opts = { 0 };
- u8 train = dp->train_set[i];
+ u8 train = train_set[i];
opts.dp.voltage[0] = (train & DP_TRAIN_VOLTAGE_SWING_MASK)
>> DP_TRAIN_VOLTAGE_SWING_SHIFT;
@@ -691,7 +692,7 @@ static int zynqmp_dp_link_train_cr(struct zynqmp_dp *dp)
* So, This loop should exit before 512 iterations
*/
for (max_tries = 0; max_tries < 512; max_tries++) {
- ret = zynqmp_dp_update_vs_emph(dp);
+ ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
if (ret)
return ret;
@@ -756,7 +757,7 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
return ret;
for (tries = 0; tries < DP_MAX_TRAINING_TRIES; tries++) {
- ret = zynqmp_dp_update_vs_emph(dp);
+ ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
if (ret)
return ret;
@@ -779,28 +780,28 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
}
/**
- * zynqmp_dp_train - Train the link
- * @dp: DisplayPort IP core structure
+ * zynqmp_dp_setup() - Set up major link parameters
+ * @bw_code: The link bandwidth as a multiple of 270 MHz
+ * @lane_cnt: The number of lanes to use
+ * @enhanced: Use enhanced framing
+ * @downspread: Enable spread-spectrum clocking
*
- * Return: 0 if all trains are done successfully, or corresponding error code.
+ * Return: 0 on success, or -errno on failure
*/
-static int zynqmp_dp_train(struct zynqmp_dp *dp)
+static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
+ bool enhanced, bool downspread)
{
u32 reg;
- u8 bw_code = dp->mode.bw_code;
- u8 lane_cnt = dp->mode.lane_cnt;
u8 aux_lane_cnt = lane_cnt;
- bool enhanced;
int ret;
zynqmp_dp_write(dp, ZYNQMP_DP_LANE_COUNT_SET, lane_cnt);
- enhanced = drm_dp_enhanced_frame_cap(dp->dpcd);
if (enhanced) {
zynqmp_dp_write(dp, ZYNQMP_DP_ENHANCED_FRAME_EN, 1);
aux_lane_cnt |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
}
- if (dp->dpcd[3] & 0x1) {
+ if (downspread) {
zynqmp_dp_write(dp, ZYNQMP_DP_DOWNSPREAD_CTL, 1);
drm_dp_dpcd_writeb(&dp->aux, DP_DOWNSPREAD_CTRL,
DP_SPREAD_AMP_0_5);
@@ -843,8 +844,24 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
}
zynqmp_dp_write(dp, ZYNQMP_DP_PHY_CLOCK_SELECT, reg);
- ret = zynqmp_dp_phy_ready(dp);
- if (ret < 0)
+ return zynqmp_dp_phy_ready(dp);
+}
+
+
+/**
+ * zynqmp_dp_train - Train the link
+ * @dp: DisplayPort IP core structure
+ *
+ * Return: 0 if all trains are done successfully, or corresponding error code.
+ */
+static int zynqmp_dp_train(struct zynqmp_dp *dp)
+{
+ int ret;
+
+ ret = zynqmp_dp_setup(dp, dp->mode.bw_code, dp->mode.lane_cnt,
+ drm_dp_enhanced_frame_cap(dp->dpcd),
+ dp->dpcd[3] & 0x1);
+ if (ret)
return ret;
zynqmp_dp_write(dp, ZYNQMP_DP_SCRAMBLING_DISABLE, 1);
--
2.35.1.1320.gc452695387.dirty
Add some locking, since none is provided by the drm subsystem. This will
prevent the IRQ/workers/bridge API calls from stepping on each other's
toes.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++---------
1 file changed, 42 insertions(+), 17 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 8635b5673386..d2dee58e7bf2 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -279,6 +279,7 @@ struct zynqmp_dp_config {
* @dpsub: Display subsystem
* @iomem: device I/O memory for register access
* @reset: reset controller
+ * @lock: Mutex protecting this struct and register access (but not AUX)
* @irq: irq
* @bridge: DRM bridge for the DP encoder
* @next_bridge: The downstream bridge
@@ -299,6 +300,7 @@ struct zynqmp_dp {
struct zynqmp_dpsub *dpsub;
void __iomem *iomem;
struct reset_control *reset;
+ struct mutex lock;
int irq;
struct drm_bridge bridge;
@@ -308,7 +310,7 @@ struct zynqmp_dp {
struct drm_dp_aux aux;
struct phy *phy[ZYNQMP_DP_MAX_LANES];
u8 num_lanes;
- struct delayed_work hpd_work;
+ struct delayed_work hpd_work, hpd_irq_work;
enum drm_connector_status status;
bool enabled;
@@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
}
/* Check with link rate and lane count */
+ mutex_lock(&dp->lock);
rate = zynqmp_dp_max_rate(dp->link_config.max_rate,
dp->link_config.max_lanes, dp->config.bpp);
+ mutex_unlock(&dp->lock);
if (mode->clock > rate) {
dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n",
mode->name);
@@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
pm_runtime_get_sync(dp->dev);
+ mutex_lock(&dp->lock);
zynqmp_dp_disp_enable(dp, old_bridge_state);
/*
@@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
ZYNQMP_DP_SOFTWARE_RESET_ALL);
zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
+ mutex_unlock(&dp->lock);
}
static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
@@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
{
struct zynqmp_dp *dp = bridge_to_dp(bridge);
+ mutex_lock(&dp->lock);
dp->enabled = false;
cancel_delayed_work(&dp->hpd_work);
zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0);
@@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0);
zynqmp_dp_disp_disable(dp, old_bridge_state);
+ mutex_unlock(&dp->lock);
pm_runtime_put_sync(dp->dev);
}
@@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
u32 state, i;
int ret;
+ mutex_lock(&dp->lock);
+
/*
* This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to
* get the HPD signal with some monitors.
@@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
dp->num_lanes);
dp->status = connector_status_connected;
+ mutex_unlock(&dp->lock);
return connector_status_connected;
}
disconnected:
dp->status = connector_status_disconnected;
+ mutex_unlock(&dp->lock);
return connector_status_disconnected;
}
@@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
drm_bridge_hpd_notify(&dp->bridge, status);
}
+static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
+{
+ struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp,
+ hpd_irq_work.work);
+ u8 status[DP_LINK_STATUS_SIZE + 2];
+ int err;
+
+ mutex_lock(&dp->lock);
+ err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
+ DP_LINK_STATUS_SIZE + 2);
+ if (err < 0) {
+ dev_dbg_ratelimited(dp->dev,
+ "could not read sink status: %d\n", err);
+ } else {
+ if (status[4] & DP_LINK_STATUS_UPDATED ||
+ !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
+ !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
+ zynqmp_dp_train_loop(dp);
+ }
+ }
+ mutex_unlock(&dp->lock);
+}
+
static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
{
struct zynqmp_dp *dp = (struct zynqmp_dp *)data;
@@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
if (status & ZYNQMP_DP_INT_HPD_EVENT)
schedule_delayed_work(&dp->hpd_work, 0);
- if (status & ZYNQMP_DP_INT_HPD_IRQ) {
- int ret;
- u8 status[DP_LINK_STATUS_SIZE + 2];
+ if (status & ZYNQMP_DP_INT_HPD_IRQ)
+ schedule_delayed_work(&dp->hpd_irq_work, 0);
- ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
- DP_LINK_STATUS_SIZE + 2);
- if (ret < 0)
- goto handled;
-
- if (status[4] & DP_LINK_STATUS_UPDATED ||
- !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
- !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
- zynqmp_dp_train_loop(dp);
- }
- }
-
-handled:
return IRQ_HANDLED;
}
@@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
dp->dev = &pdev->dev;
dp->dpsub = dpsub;
dp->status = connector_status_disconnected;
+ mutex_init(&dp->lock);
INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func);
+ INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func);
/* Acquire all resources (IOMEM, IRQ and PHYs). */
res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp");
@@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL);
disable_irq(dp->irq);
+ cancel_delayed_work_sync(&dp->hpd_irq_work);
cancel_delayed_work_sync(&dp->hpd_work);
zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0);
@@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
zynqmp_dp_phy_exit(dp);
zynqmp_dp_reset(dp, true);
+ mutex_destroy(&dp->lock);
}
--
2.35.1.1320.gc452695387.dirty
When testing, it's convenient to be able to ignore DPCD errors if there
is test equipment which can't emulate a DPRX connected to the output.
Add some (currently-unused) options to ignore these errors and just
reconfigure our internal registers as we usually would.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 37 ++++++++++++++++++++------------
1 file changed, 23 insertions(+), 14 deletions(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 24043847dab4..040f7b88ee51 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -628,6 +628,7 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
* zynqmp_dp_update_vs_emph - Update the training values
* @dp: DisplayPort IP core structure
* @train_set: A set of training values
+ * @ignore_dpcd: Ignore DPCD errors
*
* Update the training values based on the request from sink. The mapped values
* are predefined, and values(vs, pe, pc) are from the device manual.
@@ -635,15 +636,19 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
* Return: 0 if vs and emph are updated successfully, or the error code returned
* by drm_dp_dpcd_write().
*/
-static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set)
+static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set,
+ bool ignore_dpcd)
{
unsigned int i;
int ret;
ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set,
dp->mode.lane_cnt);
- if (ret < 0)
- return ret;
+ if (ret < 0) {
+ if (!ignore_dpcd)
+ return ret;
+ dev_warn(dp->dev, "failed to update vs/emph\n");
+ }
for (i = 0; i < dp->mode.lane_cnt; i++) {
u32 reg = ZYNQMP_DP_SUB_TX_PHY_PRECURSOR_LANE_0 + i * 4;
@@ -692,7 +697,7 @@ static int zynqmp_dp_link_train_cr(struct zynqmp_dp *dp)
* So, This loop should exit before 512 iterations
*/
for (max_tries = 0; max_tries < 512; max_tries++) {
- ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
+ ret = zynqmp_dp_update_vs_emph(dp, dp->train_set, false);
if (ret)
return ret;
@@ -757,7 +762,7 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
return ret;
for (tries = 0; tries < DP_MAX_TRAINING_TRIES; tries++) {
- ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
+ ret = zynqmp_dp_update_vs_emph(dp, dp->train_set, false);
if (ret)
return ret;
@@ -785,11 +790,12 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
* @lane_cnt: The number of lanes to use
* @enhanced: Use enhanced framing
* @downspread: Enable spread-spectrum clocking
+ * @ignore_dpcd: Ignore DPCD errors; useful for testing
*
* Return: 0 on success, or -errno on failure
*/
static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
- bool enhanced, bool downspread)
+ bool enhanced, bool downspread, bool ignore_dpcd)
{
u32 reg;
u8 aux_lane_cnt = lane_cnt;
@@ -812,21 +818,24 @@ static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, aux_lane_cnt);
if (ret < 0) {
- dev_err(dp->dev, "failed to set lane count\n");
- return ret;
+ dev_warn(dp->dev, "failed to set lane count\n");
+ if (!ignore_dpcd)
+ return ret;
}
ret = drm_dp_dpcd_writeb(&dp->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
DP_SET_ANSI_8B10B);
if (ret < 0) {
- dev_err(dp->dev, "failed to set ANSI 8B/10B encoding\n");
- return ret;
+ dev_warn(dp->dev, "failed to set ANSI 8B/10B encoding\n");
+ if (!ignore_dpcd)
+ return ret;
}
ret = drm_dp_dpcd_writeb(&dp->aux, DP_LINK_BW_SET, bw_code);
if (ret < 0) {
- dev_err(dp->dev, "failed to set DP bandwidth\n");
- return ret;
+ dev_warn(dp->dev, "failed to set DP bandwidth\n");
+ if (!ignore_dpcd)
+ return ret;
}
zynqmp_dp_write(dp, ZYNQMP_DP_LINK_BW_SET, bw_code);
@@ -860,7 +869,7 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
ret = zynqmp_dp_setup(dp, dp->mode.bw_code, dp->mode.lane_cnt,
drm_dp_enhanced_frame_cap(dp->dpcd),
- dp->dpcd[3] & 0x1);
+ dp->dpcd[3] & 0x1, false);
if (ret)
return ret;
@@ -877,7 +886,7 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
DP_TRAINING_PATTERN_DISABLE);
if (ret < 0) {
- dev_err(dp->dev, "failed to disable training pattern\n");
+ dev_warn(dp->dev, "failed to disable training pattern\n");
return ret;
}
zynqmp_dp_write(dp, ZYNQMP_DP_TRAINING_PATTERN_SET,
--
2.35.1.1320.gc452695387.dirty
Add a debugfs interface for exercising the various test modes supported
by the DisplayPort controller. This allows performing compliance
testing, or performing signal integrity measurements on a failing link.
At the moment, we do not support sink-driven link quality testing,
although such support would be fairly easy to add.
Signed-off-by: Sean Anderson <[email protected]>
---
drivers/gpu/drm/xlnx/zynqmp_dp.c | 591 ++++++++++++++++++++++++++++++-
1 file changed, 590 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
index 040f7b88ee51..57032186e1ca 100644
--- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
+++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
@@ -18,7 +18,9 @@
#include <drm/drm_modes.h>
#include <drm/drm_of.h>
+#include <linux/bitfield.h>
#include <linux/clk.h>
+#include <linux/debugfs.h>
#include <linux/delay.h>
#include <linux/device.h>
#include <linux/io.h>
@@ -50,6 +52,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
#define ZYNQMP_DP_LANE_COUNT_SET 0x4
#define ZYNQMP_DP_ENHANCED_FRAME_EN 0x8
#define ZYNQMP_DP_TRAINING_PATTERN_SET 0xc
+#define ZYNQMP_DP_LINK_QUAL_PATTERN_SET 0x10
#define ZYNQMP_DP_SCRAMBLING_DISABLE 0x14
#define ZYNQMP_DP_DOWNSPREAD_CTL 0x18
#define ZYNQMP_DP_SOFTWARE_RESET 0x1c
@@ -63,6 +66,9 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
ZYNQMP_DP_SOFTWARE_RESET_STREAM3 | \
ZYNQMP_DP_SOFTWARE_RESET_STREAM4 | \
ZYNQMP_DP_SOFTWARE_RESET_AUX)
+#define ZYNQMP_DP_COMP_PATTERN_80BIT_1 0x20
+#define ZYNQMP_DP_COMP_PATTERN_80BIT_2 0x24
+#define ZYNQMP_DP_COMP_PATTERN_80BIT_3 0x28
/* Core enable registers */
#define ZYNQMP_DP_TRANSMITTER_ENABLE 0x80
@@ -206,6 +212,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
#define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_2 BIT(2)
#define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_3 BIT(3)
#define ZYNQMP_DP_TX_PHY_POWER_DOWN_ALL 0xf
+#define ZYNQMP_DP_TRANSMIT_PRBS7 0x230
#define ZYNQMP_DP_PHY_PRECURSOR_LANE_0 0x23c
#define ZYNQMP_DP_PHY_PRECURSOR_LANE_1 0x240
#define ZYNQMP_DP_PHY_PRECURSOR_LANE_2 0x244
@@ -273,6 +280,69 @@ struct zynqmp_dp_config {
u8 bpp;
};
+/**
+ * enum test_pattern - Test patterns for test testing
+ * TEST_VIDEO: Use regular video input
+ * TEST_SYMBOL_ERROR: Symbol error measurement pattern
+ * TEST_PRBS7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial
+ * TEST_80BIT_CUSTOM: A custom 80-bit pattern
+ * TEST_CP2520: HBR2 compliance eye pattern
+ * TEST_TPS1: Link training symbol pattern TPS1 (/D10.2/)
+ * TEST_TPS2: Link training symbol pattern TPS2
+ * TEST_TPS3: Link training symbol pattern TPS3 (for HBR2)
+ */
+enum test_pattern {
+ TEST_VIDEO,
+ TEST_TPS1,
+ TEST_TPS2,
+ TEST_TPS3,
+ TEST_SYMBOL_ERROR,
+ TEST_PRBS7,
+ TEST_80BIT_CUSTOM,
+ TEST_CP2520,
+};
+
+static const char *const test_pattern_str[] = {
+ [TEST_VIDEO] = "video",
+ [TEST_TPS1] = "tps1",
+ [TEST_TPS2] = "tps2",
+ [TEST_TPS3] = "tps3",
+ [TEST_SYMBOL_ERROR] = "symbol-error",
+ [TEST_PRBS7] = "prbs7",
+ [TEST_80BIT_CUSTOM] = "80bit-custom",
+ [TEST_CP2520] = "cp2520",
+};
+
+/**
+ * struct zynqmp_dp_test - Configuration for test mode
+ * @pattern: The test pattern
+ * @enhanced: Use enhanced framing
+ * @downspread: Use SSC
+ * @active: Whether test mode is active
+ * @custom: Custom pattern for %TEST_80BIT_CUSTOM
+ * @train_set: Voltage/preemphasis settings
+ * @bw_code: Bandwidth code for the link
+ * @link_cnt: Number of lanes
+ */
+struct zynqmp_dp_test {
+ enum test_pattern pattern;
+ bool enhanced, downspread, active;
+ u8 custom[10];
+ u8 train_set[ZYNQMP_DP_MAX_LANES];
+ u8 bw_code;
+ u8 link_cnt;
+};
+
+/**
+ * struct zynqmp_dp_train_set_priv - Private data for train_set debugfs files
+ * @dp: DisplayPort IP core structure
+ * @lane: The lane for this file
+ */
+struct zynqmp_dp_train_set_priv {
+ struct zynqmp_dp *dp;
+ int lane;
+};
+
/**
* struct zynqmp_dp - Xilinx DisplayPort core
* @dev: device structure
@@ -283,6 +353,7 @@ struct zynqmp_dp_config {
* @irq: irq
* @bridge: DRM bridge for the DP encoder
* @next_bridge: The downstream bridge
+ * @test: Configuration for test mode
* @config: IP core configuration from DTS
* @aux: aux channel
* @phy: PHY handles for DP lanes
@@ -294,6 +365,7 @@ struct zynqmp_dp_config {
* @link_config: common link configuration between IP core and sink device
* @mode: current mode between IP core and sink device
* @train_set: set of training data
+ * @debugfs_train_set: Debugfs private data for @train_set
*/
struct zynqmp_dp {
struct device *dev;
@@ -306,6 +378,7 @@ struct zynqmp_dp {
struct drm_bridge bridge;
struct drm_bridge *next_bridge;
+ struct zynqmp_dp_test test;
struct zynqmp_dp_config config;
struct drm_dp_aux aux;
struct phy *phy[ZYNQMP_DP_MAX_LANES];
@@ -318,6 +391,7 @@ struct zynqmp_dp {
struct zynqmp_dp_link_config link_config;
struct zynqmp_dp_mode mode;
u8 train_set[ZYNQMP_DP_MAX_LANES];
+ struct zynqmp_dp_train_set_priv debugfs_train_set[ZYNQMP_DP_MAX_LANES];
};
static inline struct zynqmp_dp *bridge_to_dp(struct drm_bridge *bridge)
@@ -1599,6 +1673,510 @@ static struct edid *zynqmp_dp_bridge_get_edid(struct drm_bridge *bridge,
return drm_get_edid(connector, &dp->aux.ddc);
}
+/* -----------------------------------------------------------------------------
+ * debugfs
+ */
+
+/**
+ * zynqmp_dp_set_test_pattern() - Configure the link for a test pattern
+ * @dp: DisplayPort IP core structure
+ */
+static void zynqmp_dp_set_test_pattern(struct zynqmp_dp *dp,
+ enum test_pattern pattern,
+ u8 *const custom)
+{
+ bool scramble = false;
+ u32 train_pattern = 0;
+ u32 link_pattern = 0;
+ u8 dpcd_train = 0;
+ u8 dpcd_link = 0;
+ int err;
+
+ switch (pattern) {
+ case TEST_TPS1:
+ train_pattern = 1;
+ break;
+ case TEST_TPS2:
+ train_pattern = 2;
+ break;
+ case TEST_TPS3:
+ train_pattern = 3;
+ break;
+ case TEST_SYMBOL_ERROR:
+ scramble = true;
+ link_pattern = DP_PHY_TEST_PATTERN_ERROR_COUNT;
+ break;
+ case TEST_PRBS7:
+ /* We use a dedicated register to enable PRBS7 */
+ dpcd_link = DP_LINK_QUAL_PATTERN_ERROR_RATE;
+ break;
+ case TEST_80BIT_CUSTOM: {
+ const u8 *p = custom;
+
+ link_pattern = DP_LINK_QUAL_PATTERN_80BIT_CUSTOM;
+
+ zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_1,
+ (p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]);
+ zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_2,
+ (p[7] << 24) | (p[6] << 16) | (p[5] << 8) | p[4]);
+ zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_3,
+ (p[9] << 8) | p[8]);
+ break;
+ }
+ case TEST_CP2520:
+ link_pattern = DP_LINK_QUAL_PATTERN_CP2520_PAT_1;
+ break;
+ default:
+ WARN_ON_ONCE(1);
+ fallthrough;
+ case TEST_VIDEO:
+ scramble = true;
+ }
+
+ zynqmp_dp_write(dp, ZYNQMP_DP_SCRAMBLING_DISABLE, !scramble);
+ zynqmp_dp_write(dp, ZYNQMP_DP_TRAINING_PATTERN_SET, train_pattern);
+ zynqmp_dp_write(dp, ZYNQMP_DP_LINK_QUAL_PATTERN_SET, link_pattern);
+ zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMIT_PRBS7, pattern == TEST_PRBS7);
+
+ dpcd_link = dpcd_link ?: link_pattern;
+ dpcd_train = train_pattern;
+ if (!scramble)
+ dpcd_train |= DP_LINK_SCRAMBLING_DISABLE;
+
+ if (dp->dpcd[DP_DPCD_REV] < 0x12) {
+ if (pattern == TEST_CP2520)
+ dev_warn(dp->dev,
+ "can't set sink link quality pattern to CP2520 for DPCD < r1.2; error counters will be invalid\n");
+ else
+ dpcd_train |= FIELD_PREP(DP_LINK_QUAL_PATTERN_11_MASK,
+ dpcd_link);
+ } else {
+ u8 dpcd_link_lane[ZYNQMP_DP_MAX_LANES];
+
+ memset(dpcd_link_lane, dpcd_link, ZYNQMP_DP_MAX_LANES);
+ err = drm_dp_dpcd_write(&dp->aux, DP_LINK_QUAL_LANE0_SET,
+ dpcd_link_lane, ZYNQMP_DP_MAX_LANES);
+ if (err < 0)
+ dev_err(dp->dev, "failed to set quality pattern\n");
+ }
+
+ err = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, dpcd_train);
+ if (err < 0)
+ dev_err(dp->dev, "failed to set training pattern\n");
+}
+
+static int zynqmp_dp_test_setup(struct zynqmp_dp *dp)
+{
+ return zynqmp_dp_setup(dp, dp->test.bw_code, dp->test.link_cnt,
+ dp->test.enhanced, dp->test.downspread, true);
+}
+
+static ssize_t zynqmp_dp_pattern_read(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct zynqmp_dp *dp = file->private_data;
+ char buf[16];
+ ssize_t ret;
+
+ ret = debugfs_file_get(dentry);
+ if (unlikely(ret))
+ return ret;
+
+ mutex_lock(&dp->lock);
+ ret = snprintf(buf, sizeof(buf), "%s\n",
+ test_pattern_str[dp->test.pattern]);
+ mutex_unlock(&dp->lock);
+
+ debugfs_file_put(dentry);
+ return simple_read_from_buffer(user_buf, count, ppos, buf, ret);
+}
+
+static ssize_t zynqmp_dp_pattern_write(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+
+ struct dentry *dentry = file->f_path.dentry;
+ struct zynqmp_dp *dp = file->private_data;
+ char buf[16];
+ ssize_t ret;
+ int pattern;
+
+ ret = debugfs_file_get(dentry);
+ if (unlikely(ret))
+ return ret;
+
+ ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf,
+ count);
+ if (ret < 0)
+ goto out;
+ buf[ret] = '\0';
+
+ pattern = sysfs_match_string(test_pattern_str, buf);
+ if (pattern < 0) {
+ ret = -EINVAL;
+ goto out;
+ }
+
+ mutex_lock(&dp->lock);
+ dp->test.pattern = pattern;
+ if (dp->test.active)
+ zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
+ dp->test.custom);
+ mutex_unlock(&dp->lock);
+
+out:
+ debugfs_file_put(dentry);
+ return ret;
+}
+
+static const struct file_operations fops_zynqmp_dp_pattern = {
+ .read = zynqmp_dp_pattern_read,
+ .write = zynqmp_dp_pattern_write,
+ .open = simple_open,
+ .llseek = noop_llseek,
+};
+
+static int zynqmp_dp_enhanced_get(void *data, u64 *val)
+{
+ struct zynqmp_dp *dp = data;
+
+ mutex_lock(&dp->lock);
+ *val = dp->test.enhanced;
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_enhanced_set(void *data, u64 val)
+{
+ struct zynqmp_dp *dp = data;
+ int ret = 0;
+
+ mutex_lock(&dp->lock);
+ dp->test.enhanced = val;
+ if (dp->test.active)
+ ret = zynqmp_dp_test_setup(dp);
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_enhanced, zynqmp_dp_enhanced_get,
+ zynqmp_dp_enhanced_set, "%llu\n");
+
+static int zynqmp_dp_downspread_get(void *data, u64 *val)
+{
+ struct zynqmp_dp *dp = data;
+
+ mutex_lock(&dp->lock);
+ *val = dp->test.downspread;
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_downspread_set(void *data, u64 val)
+{
+ struct zynqmp_dp *dp = data;
+ int ret = 0;
+
+ mutex_lock(&dp->lock);
+ dp->test.downspread = val;
+ if (dp->test.active)
+ ret = zynqmp_dp_test_setup(dp);
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_downspread, zynqmp_dp_downspread_get,
+ zynqmp_dp_downspread_set, "%llu\n");
+
+static int zynqmp_dp_active_get(void *data, u64 *val)
+{
+ struct zynqmp_dp *dp = data;
+
+ mutex_lock(&dp->lock);
+ *val = dp->test.active;
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_active_set(void *data, u64 val)
+{
+ struct zynqmp_dp *dp = data;
+ int ret = 0;
+
+ mutex_lock(&dp->lock);
+ if (val) {
+ if (val < 2) {
+ ret = zynqmp_dp_test_setup(dp);
+ if (ret)
+ goto out;
+ }
+
+ zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
+ dp->test.custom);
+ zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
+ dp->test.active = true;
+ } else {
+ dp->test.active = false;
+ zynqmp_dp_set_test_pattern(dp, TEST_VIDEO, NULL);
+ zynqmp_dp_train_loop(dp);
+ }
+out:
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_active, zynqmp_dp_active_get,
+ zynqmp_dp_active_set, "%llu\n");
+
+static ssize_t zynqmp_dp_custom_read(struct file *file, char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+ struct dentry *dentry = file->f_path.dentry;
+ struct zynqmp_dp *dp = file->private_data;
+ ssize_t ret;
+
+ ret = debugfs_file_get(dentry);
+ if (unlikely(ret))
+ return ret;
+
+ mutex_lock(&dp->lock);
+ ret = simple_read_from_buffer(user_buf, count, ppos, &dp->test.custom,
+ sizeof(dp->test.custom));
+ mutex_unlock(&dp->lock);
+
+ debugfs_file_put(dentry);
+ return ret;
+}
+
+static ssize_t zynqmp_dp_custom_write(struct file *file,
+ const char __user *user_buf,
+ size_t count, loff_t *ppos)
+{
+
+ struct dentry *dentry = file->f_path.dentry;
+ struct zynqmp_dp *dp = file->private_data;
+ ssize_t ret;
+ char buf[sizeof(dp->test.custom)];
+
+ ret = debugfs_file_get(dentry);
+ if (unlikely(ret))
+ return ret;
+
+ ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
+ if (ret < 0)
+ goto out;
+
+ mutex_lock(&dp->lock);
+ memcpy(dp->test.custom, buf, ret);
+ if (dp->test.active)
+ zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
+ dp->test.custom);
+ mutex_unlock(&dp->lock);
+
+out:
+ debugfs_file_put(dentry);
+ return ret;
+}
+
+static const struct file_operations fops_zynqmp_dp_custom = {
+ .read = zynqmp_dp_custom_read,
+ .write = zynqmp_dp_custom_write,
+ .open = simple_open,
+ .llseek = noop_llseek,
+};
+
+static int zynqmp_dp_swing_get(void *data, u64 *val)
+{
+ struct zynqmp_dp_train_set_priv *priv = data;
+ struct zynqmp_dp *dp = priv->dp;
+
+ mutex_lock(&dp->lock);
+ *val = dp->test.train_set[priv->lane] & DP_TRAIN_VOLTAGE_SWING_MASK;
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_swing_set(void *data, u64 val)
+{
+ struct zynqmp_dp_train_set_priv *priv = data;
+ struct zynqmp_dp *dp = priv->dp;
+ u8 *train_set = &dp->test.train_set[priv->lane];
+ int ret = 0;
+
+ if (val > 3)
+ return -EINVAL;
+
+ mutex_lock(&dp->lock);
+ *train_set &= ~(DP_TRAIN_MAX_SWING_REACHED |
+ DP_TRAIN_VOLTAGE_SWING_MASK);
+ *train_set |= val;
+ if (val == 3)
+ *train_set |= DP_TRAIN_MAX_SWING_REACHED;
+
+ if (dp->test.active)
+ zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_swing, zynqmp_dp_swing_get,
+ zynqmp_dp_swing_set, "%llu\n");
+
+static int zynqmp_dp_preemphasis_get(void *data, u64 *val)
+{
+ struct zynqmp_dp_train_set_priv *priv = data;
+ struct zynqmp_dp *dp = priv->dp;
+
+ mutex_lock(&dp->lock);
+ *val = FIELD_GET(DP_TRAIN_PRE_EMPHASIS_MASK,
+ dp->test.train_set[priv->lane]);
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_preemphasis_set(void *data, u64 val)
+{
+ struct zynqmp_dp_train_set_priv *priv = data;
+ struct zynqmp_dp *dp = priv->dp;
+ u8 *train_set = &dp->test.train_set[priv->lane];
+ int ret = 0;
+
+ if (val > 2)
+ return -EINVAL;
+
+ mutex_lock(&dp->lock);
+ *train_set &= ~(DP_TRAIN_MAX_PRE_EMPHASIS_REACHED |
+ DP_TRAIN_PRE_EMPHASIS_MASK);
+ *train_set |= val;
+ if (val == 2)
+ *train_set |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
+
+ if (dp->test.active)
+ zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_preemphasis, zynqmp_dp_preemphasis_get,
+ zynqmp_dp_preemphasis_set, "%llu\n");
+
+static int zynqmp_dp_lanes_get(void *data, u64 *val)
+{
+ struct zynqmp_dp *dp = data;
+
+ mutex_lock(&dp->lock);
+ *val = dp->test.link_cnt;
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_lanes_set(void *data, u64 val)
+{
+ struct zynqmp_dp *dp = data;
+ int ret = 0;
+
+ if (val > ZYNQMP_DP_MAX_LANES)
+ return -EINVAL;
+
+ mutex_lock(&dp->lock);
+ if (val > dp->num_lanes) {
+ ret = -EINVAL;
+ } else {
+ dp->test.link_cnt = val;
+ if (dp->test.active)
+ ret = zynqmp_dp_test_setup(dp);
+ }
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_lanes, zynqmp_dp_lanes_get,
+ zynqmp_dp_lanes_set, "%llu\n");
+
+static int zynqmp_dp_rate_get(void *data, u64 *val)
+{
+ struct zynqmp_dp *dp = data;
+
+ mutex_lock(&dp->lock);
+ *val = drm_dp_bw_code_to_link_rate(dp->test.bw_code) * 10000;
+ mutex_unlock(&dp->lock);
+ return 0;
+}
+
+static int zynqmp_dp_rate_set(void *data, u64 val)
+{
+ struct zynqmp_dp *dp = data;
+ u8 bw_code = drm_dp_link_rate_to_bw_code(val / 10000);
+ int link_rate = drm_dp_bw_code_to_link_rate(bw_code);
+ int ret = 0;
+
+ if (val / 10000 != link_rate)
+ return -EINVAL;
+
+ if (bw_code != DP_LINK_BW_1_62 && bw_code != DP_LINK_BW_2_7 &&
+ bw_code != DP_LINK_BW_5_4)
+ return -EINVAL;
+
+ mutex_lock(&dp->lock);
+ dp->test.bw_code = bw_code;
+ if (dp->test.active)
+ ret = zynqmp_dp_test_setup(dp);
+ mutex_unlock(&dp->lock);
+
+ return ret;
+}
+
+DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
+ zynqmp_dp_rate_set, "%llu\n");
+
+static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
+ struct dentry *root)
+{
+ struct zynqmp_dp *dp = bridge_to_dp(bridge);
+ struct dentry *test;
+ int i;
+
+ dp->test.bw_code = DP_LINK_BW_5_4;
+ dp->test.link_cnt = dp->num_lanes;
+
+ test = debugfs_create_dir("test", root);
+#define CREATE_FILE(name) \
+ debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
+ CREATE_FILE(pattern);
+ CREATE_FILE(enhanced);
+ CREATE_FILE(downspread);
+ CREATE_FILE(active);
+ CREATE_FILE(custom);
+ CREATE_FILE(rate);
+ CREATE_FILE(lanes);
+
+ for (i = 0; i < dp->num_lanes; i++) {
+ static const char fmt[] = "lane%d_preemphasis";
+ char name[sizeof(fmt)];
+
+ dp->debugfs_train_set[i].dp = dp;
+ dp->debugfs_train_set[i].lane = i;
+
+ sprintf(name, fmt, i);
+ debugfs_create_file(name, 0600, test,
+ &dp->debugfs_train_set[i],
+ &fops_zynqmp_dp_preemphasis);
+
+ sprintf(name, "lane%d_swing", i);
+ debugfs_create_file(name, 0600, test,
+ &dp->debugfs_train_set[i],
+ &fops_zynqmp_dp_swing);
+ }
+}
+
static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
.attach = zynqmp_dp_bridge_attach,
.detach = zynqmp_dp_bridge_detach,
@@ -1611,6 +2189,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
.atomic_check = zynqmp_dp_bridge_atomic_check,
.detect = zynqmp_dp_bridge_detect,
.get_edid = zynqmp_dp_bridge_get_edid,
+ .debugfs_init = zynqmp_dp_bridge_debugfs_init,
};
/* -----------------------------------------------------------------------------
@@ -1645,6 +2224,9 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
hpd_work.work);
enum drm_connector_status status;
+ if (dp->test.active)
+ return;
+
status = zynqmp_dp_bridge_detect(&dp->bridge);
drm_bridge_hpd_notify(&dp->bridge, status);
}
@@ -1666,7 +2248,14 @@ static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
if (status[4] & DP_LINK_STATUS_UPDATED ||
!drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
!drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
- zynqmp_dp_train_loop(dp);
+ if (dp->test.active) {
+ dev_dbg(dp->dev, "Ignoring HPD IRQ in test mode\n");
+ } else {
+ dev_dbg(dp->dev,
+ "Retraining due to HPD IRQ (status is [%*ph])\n",
+ (int)sizeof(status), status);
+ zynqmp_dp_train_loop(dp);
+ }
}
}
mutex_unlock(&dp->lock);
--
2.35.1.1320.gc452695387.dirty
Hi Sean,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.8]
[also build test WARNING on linus/master next-20240315]
[cannot apply to drm-misc/drm-misc-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
base: v6.8
patch link: https://lore.kernel.org/r/20240315230916.1759060-4-sean.anderson%40linux.dev
patch subject: [PATCH 3/6] drm: zynqmp_dp: Add locking
config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
compiler: alpha-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
>> drivers/gpu/drm/xlnx/zynqmp_dp.c:321: warning: Function parameter or struct member 'hpd_irq_work' not described in 'zynqmp_dp'
vim +321 drivers/gpu/drm/xlnx/zynqmp_dp.c
d76271d22694e8 Hyun Kwon 2018-07-07 275
d76271d22694e8 Hyun Kwon 2018-07-07 276 /**
d76271d22694e8 Hyun Kwon 2018-07-07 277 * struct zynqmp_dp - Xilinx DisplayPort core
d76271d22694e8 Hyun Kwon 2018-07-07 278 * @dev: device structure
d76271d22694e8 Hyun Kwon 2018-07-07 279 * @dpsub: Display subsystem
d76271d22694e8 Hyun Kwon 2018-07-07 280 * @iomem: device I/O memory for register access
d76271d22694e8 Hyun Kwon 2018-07-07 281 * @reset: reset controller
8ce380e6568015 Sean Anderson 2024-03-15 282 * @lock: Mutex protecting this struct and register access (but not AUX)
d76271d22694e8 Hyun Kwon 2018-07-07 283 * @irq: irq
47e801bd0749f0 Laurent Pinchart 2021-08-04 284 * @bridge: DRM bridge for the DP encoder
bd68b9b3cb2e0d Laurent Pinchart 2021-08-04 285 * @next_bridge: The downstream bridge
d76271d22694e8 Hyun Kwon 2018-07-07 286 * @config: IP core configuration from DTS
d76271d22694e8 Hyun Kwon 2018-07-07 287 * @aux: aux channel
d76271d22694e8 Hyun Kwon 2018-07-07 288 * @phy: PHY handles for DP lanes
d76271d22694e8 Hyun Kwon 2018-07-07 289 * @num_lanes: number of enabled phy lanes
d76271d22694e8 Hyun Kwon 2018-07-07 290 * @hpd_work: hot plug detection worker
d76271d22694e8 Hyun Kwon 2018-07-07 291 * @status: connection status
d76271d22694e8 Hyun Kwon 2018-07-07 292 * @enabled: flag to indicate if the device is enabled
d76271d22694e8 Hyun Kwon 2018-07-07 293 * @dpcd: DP configuration data from currently connected sink device
d76271d22694e8 Hyun Kwon 2018-07-07 294 * @link_config: common link configuration between IP core and sink device
d76271d22694e8 Hyun Kwon 2018-07-07 295 * @mode: current mode between IP core and sink device
d76271d22694e8 Hyun Kwon 2018-07-07 296 * @train_set: set of training data
d76271d22694e8 Hyun Kwon 2018-07-07 297 */
d76271d22694e8 Hyun Kwon 2018-07-07 298 struct zynqmp_dp {
d76271d22694e8 Hyun Kwon 2018-07-07 299 struct device *dev;
d76271d22694e8 Hyun Kwon 2018-07-07 300 struct zynqmp_dpsub *dpsub;
d76271d22694e8 Hyun Kwon 2018-07-07 301 void __iomem *iomem;
d76271d22694e8 Hyun Kwon 2018-07-07 302 struct reset_control *reset;
8ce380e6568015 Sean Anderson 2024-03-15 303 struct mutex lock;
d76271d22694e8 Hyun Kwon 2018-07-07 304 int irq;
d76271d22694e8 Hyun Kwon 2018-07-07 305
47e801bd0749f0 Laurent Pinchart 2021-08-04 306 struct drm_bridge bridge;
bd68b9b3cb2e0d Laurent Pinchart 2021-08-04 307 struct drm_bridge *next_bridge;
47e801bd0749f0 Laurent Pinchart 2021-08-04 308
d76271d22694e8 Hyun Kwon 2018-07-07 309 struct zynqmp_dp_config config;
d76271d22694e8 Hyun Kwon 2018-07-07 310 struct drm_dp_aux aux;
d76271d22694e8 Hyun Kwon 2018-07-07 311 struct phy *phy[ZYNQMP_DP_MAX_LANES];
d76271d22694e8 Hyun Kwon 2018-07-07 312 u8 num_lanes;
8ce380e6568015 Sean Anderson 2024-03-15 313 struct delayed_work hpd_work, hpd_irq_work;
d76271d22694e8 Hyun Kwon 2018-07-07 314 enum drm_connector_status status;
d76271d22694e8 Hyun Kwon 2018-07-07 315 bool enabled;
d76271d22694e8 Hyun Kwon 2018-07-07 316
d76271d22694e8 Hyun Kwon 2018-07-07 317 u8 dpcd[DP_RECEIVER_CAP_SIZE];
d76271d22694e8 Hyun Kwon 2018-07-07 318 struct zynqmp_dp_link_config link_config;
d76271d22694e8 Hyun Kwon 2018-07-07 319 struct zynqmp_dp_mode mode;
d76271d22694e8 Hyun Kwon 2018-07-07 320 u8 train_set[ZYNQMP_DP_MAX_LANES];
d76271d22694e8 Hyun Kwon 2018-07-07 @321 };
d76271d22694e8 Hyun Kwon 2018-07-07 322
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Sean,
kernel test robot noticed the following build warnings:
[auto build test WARNING on v6.8]
[cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
base: v6.8
patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
compiler: microblaze-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All warnings (new ones prefixed by >>):
drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init':
>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
2168 | sprintf(name, fmt, i);
| ^~~
drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19
2168 | sprintf(name, fmt, i);
| ^~~~~~~~~~~~~~~~~~~~~
vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c
2136
2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
2138 zynqmp_dp_rate_set, "%llu\n");
2139
2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
2141 struct dentry *root)
2142 {
2143 struct zynqmp_dp *dp = bridge_to_dp(bridge);
2144 struct dentry *test;
2145 int i;
2146
2147 dp->test.bw_code = DP_LINK_BW_5_4;
2148 dp->test.link_cnt = dp->num_lanes;
2149
2150 test = debugfs_create_dir("test", root);
2151 #define CREATE_FILE(name) \
2152 debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
2153 CREATE_FILE(pattern);
2154 CREATE_FILE(enhanced);
2155 CREATE_FILE(downspread);
2156 CREATE_FILE(active);
2157 CREATE_FILE(custom);
2158 CREATE_FILE(rate);
2159 CREATE_FILE(lanes);
2160
2161 for (i = 0; i < dp->num_lanes; i++) {
2162 static const char fmt[] = "lane%d_preemphasis";
2163 char name[sizeof(fmt)];
2164
2165 dp->debugfs_train_set[i].dp = dp;
2166 dp->debugfs_train_set[i].lane = i;
2167
> 2168 sprintf(name, fmt, i);
2169 debugfs_create_file(name, 0600, test,
2170 &dp->debugfs_train_set[i],
2171 &fops_zynqmp_dp_preemphasis);
2172
2173 sprintf(name, "lane%d_swing", i);
2174 debugfs_create_file(name, 0600, test,
2175 &dp->debugfs_train_set[i],
2176 &fops_zynqmp_dp_swing);
2177 }
2178 }
2179
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
Hi Sean,
kernel test robot noticed the following build errors:
[auto build test ERROR on v6.8]
[cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
base: v6.8
patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
compiler: m68k-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
m68k-linux-ld: drivers/gpu/drm/xlnx/zynqmp_dp.o: in function `zynqmp_dp_rate_set':
>> zynqmp_dp.c:(.text+0x1a7a): undefined reference to `__udivdi3'
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Sat, 16 Mar 2024 at 01:09, Sean Anderson <[email protected]> wrote:
>
> Add a debugfs interface for exercising the various test modes supported
> by the DisplayPort controller. This allows performing compliance
> testing, or performing signal integrity measurements on a failing link.
> At the moment, we do not support sink-driven link quality testing,
> although such support would be fairly easy to add.
Could you please point out how this is used for compliance testing? We
have been using the msm_dp_compliance tool [1].
I think it would be nice to rework our drivers towards a common
debugfs interface used for DP connectors, maybe defining generic
internal interface/helpers like Maxime is implementing for HDMI
connectors.
[1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c?ref_type=heads
>
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 591 ++++++++++++++++++++++++++++++-
> 1 file changed, 590 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 040f7b88ee51..57032186e1ca 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -18,7 +18,9 @@
> #include <drm/drm_modes.h>
> #include <drm/drm_of.h>
>
> +#include <linux/bitfield.h>
> #include <linux/clk.h>
> +#include <linux/debugfs.h>
> #include <linux/delay.h>
> #include <linux/device.h>
> #include <linux/io.h>
> @@ -50,6 +52,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
> #define ZYNQMP_DP_LANE_COUNT_SET 0x4
> #define ZYNQMP_DP_ENHANCED_FRAME_EN 0x8
> #define ZYNQMP_DP_TRAINING_PATTERN_SET 0xc
> +#define ZYNQMP_DP_LINK_QUAL_PATTERN_SET 0x10
> #define ZYNQMP_DP_SCRAMBLING_DISABLE 0x14
> #define ZYNQMP_DP_DOWNSPREAD_CTL 0x18
> #define ZYNQMP_DP_SOFTWARE_RESET 0x1c
> @@ -63,6 +66,9 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
> ZYNQMP_DP_SOFTWARE_RESET_STREAM3 | \
> ZYNQMP_DP_SOFTWARE_RESET_STREAM4 | \
> ZYNQMP_DP_SOFTWARE_RESET_AUX)
> +#define ZYNQMP_DP_COMP_PATTERN_80BIT_1 0x20
> +#define ZYNQMP_DP_COMP_PATTERN_80BIT_2 0x24
> +#define ZYNQMP_DP_COMP_PATTERN_80BIT_3 0x28
>
> /* Core enable registers */
> #define ZYNQMP_DP_TRANSMITTER_ENABLE 0x80
> @@ -206,6 +212,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
> #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_2 BIT(2)
> #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_3 BIT(3)
> #define ZYNQMP_DP_TX_PHY_POWER_DOWN_ALL 0xf
> +#define ZYNQMP_DP_TRANSMIT_PRBS7 0x230
> #define ZYNQMP_DP_PHY_PRECURSOR_LANE_0 0x23c
> #define ZYNQMP_DP_PHY_PRECURSOR_LANE_1 0x240
> #define ZYNQMP_DP_PHY_PRECURSOR_LANE_2 0x244
> @@ -273,6 +280,69 @@ struct zynqmp_dp_config {
> u8 bpp;
> };
>
> +/**
> + * enum test_pattern - Test patterns for test testing
> + * TEST_VIDEO: Use regular video input
> + * TEST_SYMBOL_ERROR: Symbol error measurement pattern
> + * TEST_PRBS7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial
> + * TEST_80BIT_CUSTOM: A custom 80-bit pattern
> + * TEST_CP2520: HBR2 compliance eye pattern
> + * TEST_TPS1: Link training symbol pattern TPS1 (/D10.2/)
> + * TEST_TPS2: Link training symbol pattern TPS2
> + * TEST_TPS3: Link training symbol pattern TPS3 (for HBR2)
> + */
> +enum test_pattern {
> + TEST_VIDEO,
> + TEST_TPS1,
> + TEST_TPS2,
> + TEST_TPS3,
> + TEST_SYMBOL_ERROR,
> + TEST_PRBS7,
> + TEST_80BIT_CUSTOM,
> + TEST_CP2520,
> +};
> +
> +static const char *const test_pattern_str[] = {
> + [TEST_VIDEO] = "video",
> + [TEST_TPS1] = "tps1",
> + [TEST_TPS2] = "tps2",
> + [TEST_TPS3] = "tps3",
> + [TEST_SYMBOL_ERROR] = "symbol-error",
> + [TEST_PRBS7] = "prbs7",
> + [TEST_80BIT_CUSTOM] = "80bit-custom",
> + [TEST_CP2520] = "cp2520",
> +};
> +
> +/**
> + * struct zynqmp_dp_test - Configuration for test mode
> + * @pattern: The test pattern
> + * @enhanced: Use enhanced framing
> + * @downspread: Use SSC
> + * @active: Whether test mode is active
> + * @custom: Custom pattern for %TEST_80BIT_CUSTOM
> + * @train_set: Voltage/preemphasis settings
> + * @bw_code: Bandwidth code for the link
> + * @link_cnt: Number of lanes
> + */
> +struct zynqmp_dp_test {
> + enum test_pattern pattern;
> + bool enhanced, downspread, active;
> + u8 custom[10];
> + u8 train_set[ZYNQMP_DP_MAX_LANES];
> + u8 bw_code;
> + u8 link_cnt;
> +};
> +
> +/**
> + * struct zynqmp_dp_train_set_priv - Private data for train_set debugfs files
> + * @dp: DisplayPort IP core structure
> + * @lane: The lane for this file
> + */
> +struct zynqmp_dp_train_set_priv {
> + struct zynqmp_dp *dp;
> + int lane;
> +};
> +
> /**
> * struct zynqmp_dp - Xilinx DisplayPort core
> * @dev: device structure
> @@ -283,6 +353,7 @@ struct zynqmp_dp_config {
> * @irq: irq
> * @bridge: DRM bridge for the DP encoder
> * @next_bridge: The downstream bridge
> + * @test: Configuration for test mode
> * @config: IP core configuration from DTS
> * @aux: aux channel
> * @phy: PHY handles for DP lanes
> @@ -294,6 +365,7 @@ struct zynqmp_dp_config {
> * @link_config: common link configuration between IP core and sink device
> * @mode: current mode between IP core and sink device
> * @train_set: set of training data
> + * @debugfs_train_set: Debugfs private data for @train_set
> */
> struct zynqmp_dp {
> struct device *dev;
> @@ -306,6 +378,7 @@ struct zynqmp_dp {
> struct drm_bridge bridge;
> struct drm_bridge *next_bridge;
>
> + struct zynqmp_dp_test test;
> struct zynqmp_dp_config config;
> struct drm_dp_aux aux;
> struct phy *phy[ZYNQMP_DP_MAX_LANES];
> @@ -318,6 +391,7 @@ struct zynqmp_dp {
> struct zynqmp_dp_link_config link_config;
> struct zynqmp_dp_mode mode;
> u8 train_set[ZYNQMP_DP_MAX_LANES];
> + struct zynqmp_dp_train_set_priv debugfs_train_set[ZYNQMP_DP_MAX_LANES];
> };
>
> static inline struct zynqmp_dp *bridge_to_dp(struct drm_bridge *bridge)
> @@ -1599,6 +1673,510 @@ static struct edid *zynqmp_dp_bridge_get_edid(struct drm_bridge *bridge,
> return drm_get_edid(connector, &dp->aux.ddc);
> }
>
> +/* -----------------------------------------------------------------------------
> + * debugfs
> + */
> +
> +/**
> + * zynqmp_dp_set_test_pattern() - Configure the link for a test pattern
> + * @dp: DisplayPort IP core structure
> + */
> +static void zynqmp_dp_set_test_pattern(struct zynqmp_dp *dp,
> + enum test_pattern pattern,
> + u8 *const custom)
> +{
> + bool scramble = false;
> + u32 train_pattern = 0;
> + u32 link_pattern = 0;
> + u8 dpcd_train = 0;
> + u8 dpcd_link = 0;
> + int err;
> +
> + switch (pattern) {
> + case TEST_TPS1:
> + train_pattern = 1;
> + break;
> + case TEST_TPS2:
> + train_pattern = 2;
> + break;
> + case TEST_TPS3:
> + train_pattern = 3;
> + break;
> + case TEST_SYMBOL_ERROR:
> + scramble = true;
> + link_pattern = DP_PHY_TEST_PATTERN_ERROR_COUNT;
> + break;
> + case TEST_PRBS7:
> + /* We use a dedicated register to enable PRBS7 */
> + dpcd_link = DP_LINK_QUAL_PATTERN_ERROR_RATE;
> + break;
> + case TEST_80BIT_CUSTOM: {
> + const u8 *p = custom;
> +
> + link_pattern = DP_LINK_QUAL_PATTERN_80BIT_CUSTOM;
> +
> + zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_1,
> + (p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]);
> + zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_2,
> + (p[7] << 24) | (p[6] << 16) | (p[5] << 8) | p[4]);
> + zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_3,
> + (p[9] << 8) | p[8]);
> + break;
> + }
> + case TEST_CP2520:
> + link_pattern = DP_LINK_QUAL_PATTERN_CP2520_PAT_1;
> + break;
> + default:
> + WARN_ON_ONCE(1);
> + fallthrough;
> + case TEST_VIDEO:
> + scramble = true;
> + }
> +
> + zynqmp_dp_write(dp, ZYNQMP_DP_SCRAMBLING_DISABLE, !scramble);
> + zynqmp_dp_write(dp, ZYNQMP_DP_TRAINING_PATTERN_SET, train_pattern);
> + zynqmp_dp_write(dp, ZYNQMP_DP_LINK_QUAL_PATTERN_SET, link_pattern);
> + zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMIT_PRBS7, pattern == TEST_PRBS7);
> +
> + dpcd_link = dpcd_link ?: link_pattern;
> + dpcd_train = train_pattern;
> + if (!scramble)
> + dpcd_train |= DP_LINK_SCRAMBLING_DISABLE;
> +
> + if (dp->dpcd[DP_DPCD_REV] < 0x12) {
> + if (pattern == TEST_CP2520)
> + dev_warn(dp->dev,
> + "can't set sink link quality pattern to CP2520 for DPCD < r1.2; error counters will be invalid\n");
> + else
> + dpcd_train |= FIELD_PREP(DP_LINK_QUAL_PATTERN_11_MASK,
> + dpcd_link);
> + } else {
> + u8 dpcd_link_lane[ZYNQMP_DP_MAX_LANES];
> +
> + memset(dpcd_link_lane, dpcd_link, ZYNQMP_DP_MAX_LANES);
> + err = drm_dp_dpcd_write(&dp->aux, DP_LINK_QUAL_LANE0_SET,
> + dpcd_link_lane, ZYNQMP_DP_MAX_LANES);
> + if (err < 0)
> + dev_err(dp->dev, "failed to set quality pattern\n");
> + }
> +
> + err = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, dpcd_train);
> + if (err < 0)
> + dev_err(dp->dev, "failed to set training pattern\n");
> +}
> +
> +static int zynqmp_dp_test_setup(struct zynqmp_dp *dp)
> +{
> + return zynqmp_dp_setup(dp, dp->test.bw_code, dp->test.link_cnt,
> + dp->test.enhanced, dp->test.downspread, true);
> +}
> +
> +static ssize_t zynqmp_dp_pattern_read(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct dentry *dentry = file->f_path.dentry;
> + struct zynqmp_dp *dp = file->private_data;
> + char buf[16];
> + ssize_t ret;
> +
> + ret = debugfs_file_get(dentry);
> + if (unlikely(ret))
> + return ret;
> +
> + mutex_lock(&dp->lock);
> + ret = snprintf(buf, sizeof(buf), "%s\n",
> + test_pattern_str[dp->test.pattern]);
> + mutex_unlock(&dp->lock);
> +
> + debugfs_file_put(dentry);
> + return simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> +}
> +
> +static ssize_t zynqmp_dp_pattern_write(struct file *file,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> +
> + struct dentry *dentry = file->f_path.dentry;
> + struct zynqmp_dp *dp = file->private_data;
> + char buf[16];
> + ssize_t ret;
> + int pattern;
> +
> + ret = debugfs_file_get(dentry);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf,
> + count);
> + if (ret < 0)
> + goto out;
> + buf[ret] = '\0';
> +
> + pattern = sysfs_match_string(test_pattern_str, buf);
> + if (pattern < 0) {
> + ret = -EINVAL;
> + goto out;
> + }
> +
> + mutex_lock(&dp->lock);
> + dp->test.pattern = pattern;
> + if (dp->test.active)
> + zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
> + dp->test.custom);
> + mutex_unlock(&dp->lock);
> +
> +out:
> + debugfs_file_put(dentry);
> + return ret;
> +}
> +
> +static const struct file_operations fops_zynqmp_dp_pattern = {
> + .read = zynqmp_dp_pattern_read,
> + .write = zynqmp_dp_pattern_write,
> + .open = simple_open,
> + .llseek = noop_llseek,
> +};
> +
> +static int zynqmp_dp_enhanced_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp *dp = data;
> +
> + mutex_lock(&dp->lock);
> + *val = dp->test.enhanced;
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_enhanced_set(void *data, u64 val)
> +{
> + struct zynqmp_dp *dp = data;
> + int ret = 0;
> +
> + mutex_lock(&dp->lock);
> + dp->test.enhanced = val;
> + if (dp->test.active)
> + ret = zynqmp_dp_test_setup(dp);
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_enhanced, zynqmp_dp_enhanced_get,
> + zynqmp_dp_enhanced_set, "%llu\n");
> +
> +static int zynqmp_dp_downspread_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp *dp = data;
> +
> + mutex_lock(&dp->lock);
> + *val = dp->test.downspread;
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_downspread_set(void *data, u64 val)
> +{
> + struct zynqmp_dp *dp = data;
> + int ret = 0;
> +
> + mutex_lock(&dp->lock);
> + dp->test.downspread = val;
> + if (dp->test.active)
> + ret = zynqmp_dp_test_setup(dp);
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_downspread, zynqmp_dp_downspread_get,
> + zynqmp_dp_downspread_set, "%llu\n");
> +
> +static int zynqmp_dp_active_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp *dp = data;
> +
> + mutex_lock(&dp->lock);
> + *val = dp->test.active;
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_active_set(void *data, u64 val)
> +{
> + struct zynqmp_dp *dp = data;
> + int ret = 0;
> +
> + mutex_lock(&dp->lock);
> + if (val) {
> + if (val < 2) {
> + ret = zynqmp_dp_test_setup(dp);
> + if (ret)
> + goto out;
> + }
> +
> + zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
> + dp->test.custom);
> + zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
> + dp->test.active = true;
> + } else {
> + dp->test.active = false;
> + zynqmp_dp_set_test_pattern(dp, TEST_VIDEO, NULL);
> + zynqmp_dp_train_loop(dp);
> + }
> +out:
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_active, zynqmp_dp_active_get,
> + zynqmp_dp_active_set, "%llu\n");
> +
> +static ssize_t zynqmp_dp_custom_read(struct file *file, char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> + struct dentry *dentry = file->f_path.dentry;
> + struct zynqmp_dp *dp = file->private_data;
> + ssize_t ret;
> +
> + ret = debugfs_file_get(dentry);
> + if (unlikely(ret))
> + return ret;
> +
> + mutex_lock(&dp->lock);
> + ret = simple_read_from_buffer(user_buf, count, ppos, &dp->test.custom,
> + sizeof(dp->test.custom));
> + mutex_unlock(&dp->lock);
> +
> + debugfs_file_put(dentry);
> + return ret;
> +}
> +
> +static ssize_t zynqmp_dp_custom_write(struct file *file,
> + const char __user *user_buf,
> + size_t count, loff_t *ppos)
> +{
> +
> + struct dentry *dentry = file->f_path.dentry;
> + struct zynqmp_dp *dp = file->private_data;
> + ssize_t ret;
> + char buf[sizeof(dp->test.custom)];
> +
> + ret = debugfs_file_get(dentry);
> + if (unlikely(ret))
> + return ret;
> +
> + ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> + if (ret < 0)
> + goto out;
> +
> + mutex_lock(&dp->lock);
> + memcpy(dp->test.custom, buf, ret);
> + if (dp->test.active)
> + zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
> + dp->test.custom);
> + mutex_unlock(&dp->lock);
> +
> +out:
> + debugfs_file_put(dentry);
> + return ret;
> +}
> +
> +static const struct file_operations fops_zynqmp_dp_custom = {
> + .read = zynqmp_dp_custom_read,
> + .write = zynqmp_dp_custom_write,
> + .open = simple_open,
> + .llseek = noop_llseek,
> +};
> +
> +static int zynqmp_dp_swing_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp_train_set_priv *priv = data;
> + struct zynqmp_dp *dp = priv->dp;
> +
> + mutex_lock(&dp->lock);
> + *val = dp->test.train_set[priv->lane] & DP_TRAIN_VOLTAGE_SWING_MASK;
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_swing_set(void *data, u64 val)
> +{
> + struct zynqmp_dp_train_set_priv *priv = data;
> + struct zynqmp_dp *dp = priv->dp;
> + u8 *train_set = &dp->test.train_set[priv->lane];
> + int ret = 0;
> +
> + if (val > 3)
> + return -EINVAL;
> +
> + mutex_lock(&dp->lock);
> + *train_set &= ~(DP_TRAIN_MAX_SWING_REACHED |
> + DP_TRAIN_VOLTAGE_SWING_MASK);
> + *train_set |= val;
> + if (val == 3)
> + *train_set |= DP_TRAIN_MAX_SWING_REACHED;
> +
> + if (dp->test.active)
> + zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_swing, zynqmp_dp_swing_get,
> + zynqmp_dp_swing_set, "%llu\n");
> +
> +static int zynqmp_dp_preemphasis_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp_train_set_priv *priv = data;
> + struct zynqmp_dp *dp = priv->dp;
> +
> + mutex_lock(&dp->lock);
> + *val = FIELD_GET(DP_TRAIN_PRE_EMPHASIS_MASK,
> + dp->test.train_set[priv->lane]);
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_preemphasis_set(void *data, u64 val)
> +{
> + struct zynqmp_dp_train_set_priv *priv = data;
> + struct zynqmp_dp *dp = priv->dp;
> + u8 *train_set = &dp->test.train_set[priv->lane];
> + int ret = 0;
> +
> + if (val > 2)
> + return -EINVAL;
> +
> + mutex_lock(&dp->lock);
> + *train_set &= ~(DP_TRAIN_MAX_PRE_EMPHASIS_REACHED |
> + DP_TRAIN_PRE_EMPHASIS_MASK);
> + *train_set |= val;
> + if (val == 2)
> + *train_set |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
> +
> + if (dp->test.active)
> + zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_preemphasis, zynqmp_dp_preemphasis_get,
> + zynqmp_dp_preemphasis_set, "%llu\n");
> +
> +static int zynqmp_dp_lanes_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp *dp = data;
> +
> + mutex_lock(&dp->lock);
> + *val = dp->test.link_cnt;
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_lanes_set(void *data, u64 val)
> +{
> + struct zynqmp_dp *dp = data;
> + int ret = 0;
> +
> + if (val > ZYNQMP_DP_MAX_LANES)
> + return -EINVAL;
> +
> + mutex_lock(&dp->lock);
> + if (val > dp->num_lanes) {
> + ret = -EINVAL;
> + } else {
> + dp->test.link_cnt = val;
> + if (dp->test.active)
> + ret = zynqmp_dp_test_setup(dp);
> + }
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_lanes, zynqmp_dp_lanes_get,
> + zynqmp_dp_lanes_set, "%llu\n");
> +
> +static int zynqmp_dp_rate_get(void *data, u64 *val)
> +{
> + struct zynqmp_dp *dp = data;
> +
> + mutex_lock(&dp->lock);
> + *val = drm_dp_bw_code_to_link_rate(dp->test.bw_code) * 10000;
> + mutex_unlock(&dp->lock);
> + return 0;
> +}
> +
> +static int zynqmp_dp_rate_set(void *data, u64 val)
> +{
> + struct zynqmp_dp *dp = data;
> + u8 bw_code = drm_dp_link_rate_to_bw_code(val / 10000);
> + int link_rate = drm_dp_bw_code_to_link_rate(bw_code);
> + int ret = 0;
> +
> + if (val / 10000 != link_rate)
> + return -EINVAL;
> +
> + if (bw_code != DP_LINK_BW_1_62 && bw_code != DP_LINK_BW_2_7 &&
> + bw_code != DP_LINK_BW_5_4)
> + return -EINVAL;
> +
> + mutex_lock(&dp->lock);
> + dp->test.bw_code = bw_code;
> + if (dp->test.active)
> + ret = zynqmp_dp_test_setup(dp);
> + mutex_unlock(&dp->lock);
> +
> + return ret;
> +}
> +
> +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
> + zynqmp_dp_rate_set, "%llu\n");
> +
> +static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
> + struct dentry *root)
> +{
> + struct zynqmp_dp *dp = bridge_to_dp(bridge);
> + struct dentry *test;
> + int i;
> +
> + dp->test.bw_code = DP_LINK_BW_5_4;
> + dp->test.link_cnt = dp->num_lanes;
> +
> + test = debugfs_create_dir("test", root);
> +#define CREATE_FILE(name) \
> + debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
> + CREATE_FILE(pattern);
> + CREATE_FILE(enhanced);
> + CREATE_FILE(downspread);
> + CREATE_FILE(active);
> + CREATE_FILE(custom);
> + CREATE_FILE(rate);
> + CREATE_FILE(lanes);
> +
> + for (i = 0; i < dp->num_lanes; i++) {
> + static const char fmt[] = "lane%d_preemphasis";
> + char name[sizeof(fmt)];
> +
> + dp->debugfs_train_set[i].dp = dp;
> + dp->debugfs_train_set[i].lane = i;
> +
> + sprintf(name, fmt, i);
> + debugfs_create_file(name, 0600, test,
> + &dp->debugfs_train_set[i],
> + &fops_zynqmp_dp_preemphasis);
> +
> + sprintf(name, "lane%d_swing", i);
> + debugfs_create_file(name, 0600, test,
> + &dp->debugfs_train_set[i],
> + &fops_zynqmp_dp_swing);
> + }
> +}
> +
> static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .attach = zynqmp_dp_bridge_attach,
> .detach = zynqmp_dp_bridge_detach,
> @@ -1611,6 +2189,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> .atomic_check = zynqmp_dp_bridge_atomic_check,
> .detect = zynqmp_dp_bridge_detect,
> .get_edid = zynqmp_dp_bridge_get_edid,
> + .debugfs_init = zynqmp_dp_bridge_debugfs_init,
> };
>
> /* -----------------------------------------------------------------------------
> @@ -1645,6 +2224,9 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
> hpd_work.work);
> enum drm_connector_status status;
>
> + if (dp->test.active)
> + return;
> +
> status = zynqmp_dp_bridge_detect(&dp->bridge);
> drm_bridge_hpd_notify(&dp->bridge, status);
> }
> @@ -1666,7 +2248,14 @@ static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
> if (status[4] & DP_LINK_STATUS_UPDATED ||
> !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
> !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
> - zynqmp_dp_train_loop(dp);
> + if (dp->test.active) {
> + dev_dbg(dp->dev, "Ignoring HPD IRQ in test mode\n");
> + } else {
> + dev_dbg(dp->dev,
> + "Retraining due to HPD IRQ (status is [%*ph])\n",
> + (int)sizeof(status), status);
> + zynqmp_dp_train_loop(dp);
> + }
> }
> }
> mutex_unlock(&dp->lock);
> --
> 2.35.1.1320.gc452695387.dirty
>
--
With best wishes
Dmitry
On 3/16/24 06:14, kernel test robot wrote:
> Hi Sean,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on v6.8]
> [cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
> base: v6.8
> patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
> patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
> config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
> compiler: microblaze-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init':
>>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
> 2168 | sprintf(name, fmt, i);
> | ^~~
> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19
> 2168 | sprintf(name, fmt, i);
> | ^~~~~~~~~~~~~~~~~~~~~
Not a bug, as i will be at most 4, which uses 1 digit.
--Sean
> vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c
>
> 2136
> 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
> 2138 zynqmp_dp_rate_set, "%llu\n");
> 2139
> 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
> 2141 struct dentry *root)
> 2142 {
> 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge);
> 2144 struct dentry *test;
> 2145 int i;
> 2146
> 2147 dp->test.bw_code = DP_LINK_BW_5_4;
> 2148 dp->test.link_cnt = dp->num_lanes;
> 2149
> 2150 test = debugfs_create_dir("test", root);
> 2151 #define CREATE_FILE(name) \
> 2152 debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
> 2153 CREATE_FILE(pattern);
> 2154 CREATE_FILE(enhanced);
> 2155 CREATE_FILE(downspread);
> 2156 CREATE_FILE(active);
> 2157 CREATE_FILE(custom);
> 2158 CREATE_FILE(rate);
> 2159 CREATE_FILE(lanes);
> 2160
> 2161 for (i = 0; i < dp->num_lanes; i++) {
> 2162 static const char fmt[] = "lane%d_preemphasis";
> 2163 char name[sizeof(fmt)];
> 2164
> 2165 dp->debugfs_train_set[i].dp = dp;
> 2166 dp->debugfs_train_set[i].lane = i;
> 2167
>> 2168 sprintf(name, fmt, i);
> 2169 debugfs_create_file(name, 0600, test,
> 2170 &dp->debugfs_train_set[i],
> 2171 &fops_zynqmp_dp_preemphasis);
> 2172
> 2173 sprintf(name, "lane%d_swing", i);
> 2174 debugfs_create_file(name, 0600, test,
> 2175 &dp->debugfs_train_set[i],
> 2176 &fops_zynqmp_dp_swing);
> 2177 }
> 2178 }
> 2179
>
On 3/16/24 06:55, kernel test robot wrote:
> Hi Sean,
>
> kernel test robot noticed the following build errors:
>
> [auto build test ERROR on v6.8]
> [cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
> base: v6.8
> patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
> patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
> config: m68k-allyesconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
> compiler: m68k-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All errors (new ones prefixed by >>):
>
> m68k-linux-ld: drivers/gpu/drm/xlnx/zynqmp_dp.o: in function `zynqmp_dp_rate_set':
>>> zynqmp_dp.c:(.text+0x1a7a): undefined reference to `__udivdi3'
>
Will fix.
--Sean
On 3/16/24 05:52, kernel test robot wrote:
> Hi Sean,
>
> kernel test robot noticed the following build warnings:
>
> [auto build test WARNING on v6.8]
> [also build test WARNING on linus/master next-20240315]
> [cannot apply to drm-misc/drm-misc-next]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch#_base_tree_information]
>
> url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
> base: v6.8
> patch link: https://lore.kernel.org/r/20240315230916.1759060-4-sean.anderson%40linux.dev
> patch subject: [PATCH 3/6] drm: zynqmp_dp: Add locking
> config: alpha-allyesconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
> compiler: alpha-linux-gcc (GCC) 13.2.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
> | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>
> All warnings (new ones prefixed by >>):
>
>>> drivers/gpu/drm/xlnx/zynqmp_dp.c:321: warning: Function parameter or struct member 'hpd_irq_work' not described in 'zynqmp_dp'
Will document.
--Sean
>
> vim +321 drivers/gpu/drm/xlnx/zynqmp_dp.c
>
> d76271d22694e8 Hyun Kwon 2018-07-07 275
> d76271d22694e8 Hyun Kwon 2018-07-07 276 /**
> d76271d22694e8 Hyun Kwon 2018-07-07 277 * struct zynqmp_dp - Xilinx DisplayPort core
> d76271d22694e8 Hyun Kwon 2018-07-07 278 * @dev: device structure
> d76271d22694e8 Hyun Kwon 2018-07-07 279 * @dpsub: Display subsystem
> d76271d22694e8 Hyun Kwon 2018-07-07 280 * @iomem: device I/O memory for register access
> d76271d22694e8 Hyun Kwon 2018-07-07 281 * @reset: reset controller
> 8ce380e6568015 Sean Anderson 2024-03-15 282 * @lock: Mutex protecting this struct and register access (but not AUX)
> d76271d22694e8 Hyun Kwon 2018-07-07 283 * @irq: irq
> 47e801bd0749f0 Laurent Pinchart 2021-08-04 284 * @bridge: DRM bridge for the DP encoder
> bd68b9b3cb2e0d Laurent Pinchart 2021-08-04 285 * @next_bridge: The downstream bridge
> d76271d22694e8 Hyun Kwon 2018-07-07 286 * @config: IP core configuration from DTS
> d76271d22694e8 Hyun Kwon 2018-07-07 287 * @aux: aux channel
> d76271d22694e8 Hyun Kwon 2018-07-07 288 * @phy: PHY handles for DP lanes
> d76271d22694e8 Hyun Kwon 2018-07-07 289 * @num_lanes: number of enabled phy lanes
> d76271d22694e8 Hyun Kwon 2018-07-07 290 * @hpd_work: hot plug detection worker
> d76271d22694e8 Hyun Kwon 2018-07-07 291 * @status: connection status
> d76271d22694e8 Hyun Kwon 2018-07-07 292 * @enabled: flag to indicate if the device is enabled
> d76271d22694e8 Hyun Kwon 2018-07-07 293 * @dpcd: DP configuration data from currently connected sink device
> d76271d22694e8 Hyun Kwon 2018-07-07 294 * @link_config: common link configuration between IP core and sink device
> d76271d22694e8 Hyun Kwon 2018-07-07 295 * @mode: current mode between IP core and sink device
> d76271d22694e8 Hyun Kwon 2018-07-07 296 * @train_set: set of training data
> d76271d22694e8 Hyun Kwon 2018-07-07 297 */
> d76271d22694e8 Hyun Kwon 2018-07-07 298 struct zynqmp_dp {
> d76271d22694e8 Hyun Kwon 2018-07-07 299 struct device *dev;
> d76271d22694e8 Hyun Kwon 2018-07-07 300 struct zynqmp_dpsub *dpsub;
> d76271d22694e8 Hyun Kwon 2018-07-07 301 void __iomem *iomem;
> d76271d22694e8 Hyun Kwon 2018-07-07 302 struct reset_control *reset;
> 8ce380e6568015 Sean Anderson 2024-03-15 303 struct mutex lock;
> d76271d22694e8 Hyun Kwon 2018-07-07 304 int irq;
> d76271d22694e8 Hyun Kwon 2018-07-07 305
> 47e801bd0749f0 Laurent Pinchart 2021-08-04 306 struct drm_bridge bridge;
> bd68b9b3cb2e0d Laurent Pinchart 2021-08-04 307 struct drm_bridge *next_bridge;
> 47e801bd0749f0 Laurent Pinchart 2021-08-04 308
> d76271d22694e8 Hyun Kwon 2018-07-07 309 struct zynqmp_dp_config config;
> d76271d22694e8 Hyun Kwon 2018-07-07 310 struct drm_dp_aux aux;
> d76271d22694e8 Hyun Kwon 2018-07-07 311 struct phy *phy[ZYNQMP_DP_MAX_LANES];
> d76271d22694e8 Hyun Kwon 2018-07-07 312 u8 num_lanes;
> 8ce380e6568015 Sean Anderson 2024-03-15 313 struct delayed_work hpd_work, hpd_irq_work;
> d76271d22694e8 Hyun Kwon 2018-07-07 314 enum drm_connector_status status;
> d76271d22694e8 Hyun Kwon 2018-07-07 315 bool enabled;
> d76271d22694e8 Hyun Kwon 2018-07-07 316
> d76271d22694e8 Hyun Kwon 2018-07-07 317 u8 dpcd[DP_RECEIVER_CAP_SIZE];
> d76271d22694e8 Hyun Kwon 2018-07-07 318 struct zynqmp_dp_link_config link_config;
> d76271d22694e8 Hyun Kwon 2018-07-07 319 struct zynqmp_dp_mode mode;
> d76271d22694e8 Hyun Kwon 2018-07-07 320 u8 train_set[ZYNQMP_DP_MAX_LANES];
> d76271d22694e8 Hyun Kwon 2018-07-07 @321 };
> d76271d22694e8 Hyun Kwon 2018-07-07 322
>
On 3/16/24 13:56, Dmitry Baryshkov wrote:
> On Sat, 16 Mar 2024 at 01:09, Sean Anderson <[email protected]> wrote:
>>
>> Add a debugfs interface for exercising the various test modes supported
>> by the DisplayPort controller. This allows performing compliance
>> testing, or performing signal integrity measurements on a failing link.
>> At the moment, we do not support sink-driven link quality testing,
>> although such support would be fairly easy to add.
>
> Could you please point out how this is used for compliance testing? We
> have been using the msm_dp_compliance tool [1].
Here's some quick documentation I wrote up. This probably could be put
under Documentation for v2.
The following files in /sys/kernel/debug/dri/1/DP-1/test/ control the
DisplayPort test modes:
active:
Writing a 1 to this file will activate test mode, and writing a 0 will
deactivate test mode. Writing a 1 or 0 when the test mode is already
active/inactive will reactivate/re-deactivate test mode. When test mode
is inactive, changes made to other files will have no effect. When
test mode is active, changes made to other files will apply instantly.
Additionally, hotplug events (as removing the cable or if the monitor
requests link retraining) are ignored.
custom:
Custom test pattern value
downspread:
Enable/disable clock downspreading (spread-spectrum clocking) by
writing 1/0
enhanced:
Enable/disable enhanced framing
lane0_preemphasis:
Preemphasis from 0 (lowest) to 2 (most) for lane 0
lane0_swing:
Voltage swing from 0 (lowest) to 3 (most) for lane 0
lane1_preemphasis:
Preemphasis from 0 (lowest) to 2 (most) for lane 1
lane1_swing:
Voltage swing from 0 (lowest) to 3 (most) for lane 1
lanes:
Number of lanes to use (1 or 2)
pattern:
Test pattern. May be one of:
- video: Use regular video input
- symbol-error: Symbol error measurement pattern
- prbs7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial
- 80bit-custom: A custom 80-bit pattern
- cp2520: HBR2 compliance eye pattern
- tps1: Link training symbol pattern TPS1 (/D10.2/)
- tps2: Link training symbol pattern TPS2
- tps3: Link training symbol pattern TPS3 (for HBR2)
rate:
Rate in hertz. One of
- 5400000000: HBR2
- 2700000000: HBR
- 1620000000: RBR
You can dump the displayport test settings with the following command:
for prop in /sys/kernel/debug/dri/1/DP-1/test/*; do
printf '%-20s ' ${prop##*/}
if [ ${prop##*/} = custom ]; then
hexdump -C $prop | head -1
else
cat $prop
fi
done
The output could look something like
active 1
custom 00000000 00 00 00 00 00 00 00 00 00 00 |..........|
downspread 0
enhanced 1
lane0_preemphasis 0
lane0_swing 3
lane1_preemphasis 0
lane1_swing 3
lanes 2
pattern prbs7
rate 1620000000
The recommended test procedure is to connect the board to a monitor,
configure test mode, activate test mode, and then disconnect the cable
and connect it to your test equipment of choice. For example, one
sequence of commands could be:
echo 1 > /sys/kernel/debug/dri/1/DP-1/test/enhanced
echo tps1 > /sys/kernel/debug/dri/1/DP-1/test/pattern
echo 1620000000 > /sys/kernel/debug/dri/1/DP-1/test/rate
echo 1 > /sys/kernel/debug/dri/1/DP-1/test/active
at which point the cable could be disconnected from the monitor. When
the cable is disconnected there will be several errors while changing
the settings. This is expected.
> I think it would be nice to rework our drivers towards a common
> debugfs interface used for DP connectors, maybe defining generic
> internal interface/helpers like Maxime is implementing for HDMI
> connectors.
>
> [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c?ref_type=heads
I was definitely inspired by the msm, intel, and amd approaches.
However, those debugfs implementations seem to be oriented towards
DisplayPort text fixtures which emulate DPRXs. In particular, both the
intel and msm debugfs interfaces provide no method for configuring test
parameters in userspace. As test fixtures supporting DPCD can run into
the thousands of dollars, I think it is more economical to support
userspace-driven testing. I was particularly inspired by the AMD
approach:
/* Usage: set DP physical test pattern using debugfs with normal DP
* panel. Then plug out DP panel and connect a scope to measure
* For normal video mode and test pattern generated from CRCT,
* they are visibile to user. So do not disable HPD.
* Video Mode is also set to clear the test pattern, so enable HPD
* because it might have been disabled after a test pattern was set.
* AUX depends on HPD * sequence dependent, do not move!
*/
But I chose to always disable HPD events and ignore DPCD
errors in test mode. I think this is pretty convenient, since you can
run the same commands regardless of whether you have a monitor attached.
Although the initial setup does need a monitor (which is likely since
not everything gets set up by activating test mode; definitely fixable
but I didn't need it).
--Sean
Hi Sean,
Thank you for the patch.
On Fri, Mar 15, 2024 at 07:09:11PM -0400, Sean Anderson wrote:
> Enable this message for verbose debugging only as it is otherwise
> printed after every AUX message, quickly filling the log buffer.
>
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index a0606fab0e22..98a32e6a0459 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -1006,7 +1006,7 @@ zynqmp_dp_aux_transfer(struct drm_dp_aux *aux, struct drm_dp_aux_msg *msg)
> msg->buffer, msg->size,
> &msg->reply);
> if (!ret) {
> - dev_dbg(dp->dev, "aux %d retries\n", i);
> + dev_vdbg(dp->dev, "aux %d retries\n", i);
I didn't know about dev_vdbg(). I suppose this makes sense,
Reviewed-by: Laurent Pinchart <[email protected]>
> return msg->size;
> }
>
--
Regards,
Laurent Pinchart
Hi Sean,
Thank you for the patch.
On Fri, Mar 15, 2024 at 07:09:12PM -0400, Sean Anderson wrote:
> The feedback we get from the DPRX is per-lane. Make changes using this
> information, instead of picking the maximum values from all lanes. This
> results in more-consistent training on marginal links.
>
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 23 ++++++++---------------
> 1 file changed, 8 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 98a32e6a0459..8635b5673386 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -605,28 +605,21 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
> u8 link_status[DP_LINK_STATUS_SIZE])
> {
> u8 *train_set = dp->train_set;
> - u8 voltage = 0, preemphasis = 0;
> u8 i;
>
> for (i = 0; i < dp->mode.lane_cnt; i++) {
> - u8 v = drm_dp_get_adjust_request_voltage(link_status, i);
> - u8 p = drm_dp_get_adjust_request_pre_emphasis(link_status, i);
> + u8 voltage = drm_dp_get_adjust_request_voltage(link_status, i);
> + u8 preemphasis =
> + drm_dp_get_adjust_request_pre_emphasis(link_status, i);
>
> - if (v > voltage)
> - voltage = v;
> + if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3)
> + voltage |= DP_TRAIN_MAX_SWING_REACHED;
>
> - if (p > preemphasis)
> - preemphasis = p;
> - }
> + if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2)
> + preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
>
> - if (voltage >= DP_TRAIN_VOLTAGE_SWING_LEVEL_3)
> - voltage |= DP_TRAIN_MAX_SWING_REACHED;
> -
> - if (preemphasis >= DP_TRAIN_PRE_EMPH_LEVEL_2)
> - preemphasis |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
> -
> - for (i = 0; i < dp->mode.lane_cnt; i++)
> train_set[i] = voltage | preemphasis;
> + }
I don't have enough DP knowledge to review this :-(
> }
>
> /**
--
Regards,
Laurent Pinchart
Hi Sean,
Thank you for the patch.
On Fri, Mar 15, 2024 at 07:09:13PM -0400, Sean Anderson wrote:
> Add some locking, since none is provided by the drm subsystem. This will
That's not quite right, the DRM core doesn't call bridge operations
concurrently. We may need locking to protect against race conditions
between bridge operations and interrupts though.
> prevent the IRQ/workers/bridge API calls from stepping on each other's
> toes.
>
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++---------
> 1 file changed, 42 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 8635b5673386..d2dee58e7bf2 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -279,6 +279,7 @@ struct zynqmp_dp_config {
> * @dpsub: Display subsystem
> * @iomem: device I/O memory for register access
> * @reset: reset controller
> + * @lock: Mutex protecting this struct and register access (but not AUX)
This patch does two things at once, it defers link training from the IRQ
handler to a work queue, and covers everything with a big lock. The
scope is too large. Please restrict the lock scope and document the
individual fields that need to be protected, and explain the locking
design in the commit message (or comments in the code).
> * @irq: irq
> * @bridge: DRM bridge for the DP encoder
> * @next_bridge: The downstream bridge
> @@ -299,6 +300,7 @@ struct zynqmp_dp {
> struct zynqmp_dpsub *dpsub;
> void __iomem *iomem;
> struct reset_control *reset;
> + struct mutex lock;
> int irq;
>
> struct drm_bridge bridge;
> @@ -308,7 +310,7 @@ struct zynqmp_dp {
> struct drm_dp_aux aux;
> struct phy *phy[ZYNQMP_DP_MAX_LANES];
> u8 num_lanes;
> - struct delayed_work hpd_work;
> + struct delayed_work hpd_work, hpd_irq_work;
One variable per line please.
> enum drm_connector_status status;
> bool enabled;
>
> @@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
> }
>
> /* Check with link rate and lane count */
> + mutex_lock(&dp->lock);
> rate = zynqmp_dp_max_rate(dp->link_config.max_rate,
> dp->link_config.max_lanes, dp->config.bpp);
> + mutex_unlock(&dp->lock);
> if (mode->clock > rate) {
> dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n",
> mode->name);
> @@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>
> pm_runtime_get_sync(dp->dev);
>
> + mutex_lock(&dp->lock);
> zynqmp_dp_disp_enable(dp, old_bridge_state);
>
> /*
> @@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
> ZYNQMP_DP_SOFTWARE_RESET_ALL);
> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
> + mutex_unlock(&dp->lock);
> }
>
> static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> @@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> {
> struct zynqmp_dp *dp = bridge_to_dp(bridge);
>
> + mutex_lock(&dp->lock);
> dp->enabled = false;
> cancel_delayed_work(&dp->hpd_work);
> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0);
> @@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0);
>
> zynqmp_dp_disp_disable(dp, old_bridge_state);
> + mutex_unlock(&dp->lock);
>
> pm_runtime_put_sync(dp->dev);
> }
> @@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
> u32 state, i;
> int ret;
>
> + mutex_lock(&dp->lock);
> +
> /*
> * This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to
> * get the HPD signal with some monitors.
> @@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
> dp->num_lanes);
>
> dp->status = connector_status_connected;
> + mutex_unlock(&dp->lock);
> return connector_status_connected;
> }
>
> disconnected:
> dp->status = connector_status_disconnected;
> + mutex_unlock(&dp->lock);
> return connector_status_disconnected;
> }
>
> @@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
> drm_bridge_hpd_notify(&dp->bridge, status);
> }
>
> +static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
> +{
> + struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp,
> + hpd_irq_work.work);
> + u8 status[DP_LINK_STATUS_SIZE + 2];
> + int err;
> +
> + mutex_lock(&dp->lock);
> + err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
> + DP_LINK_STATUS_SIZE + 2);
> + if (err < 0) {
> + dev_dbg_ratelimited(dp->dev,
> + "could not read sink status: %d\n", err);
> + } else {
> + if (status[4] & DP_LINK_STATUS_UPDATED ||
> + !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
> + !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
> + zynqmp_dp_train_loop(dp);
> + }
> + }
> + mutex_unlock(&dp->lock);
> +}
> +
> static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
> {
> struct zynqmp_dp *dp = (struct zynqmp_dp *)data;
> @@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
> if (status & ZYNQMP_DP_INT_HPD_EVENT)
> schedule_delayed_work(&dp->hpd_work, 0);
>
> - if (status & ZYNQMP_DP_INT_HPD_IRQ) {
> - int ret;
> - u8 status[DP_LINK_STATUS_SIZE + 2];
> + if (status & ZYNQMP_DP_INT_HPD_IRQ)
> + schedule_delayed_work(&dp->hpd_irq_work, 0);
>
> - ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
> - DP_LINK_STATUS_SIZE + 2);
> - if (ret < 0)
> - goto handled;
> -
> - if (status[4] & DP_LINK_STATUS_UPDATED ||
> - !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
> - !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
> - zynqmp_dp_train_loop(dp);
> - }
> - }
> -
> -handled:
> return IRQ_HANDLED;
> }
>
> @@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
> dp->dev = &pdev->dev;
> dp->dpsub = dpsub;
> dp->status = connector_status_disconnected;
> + mutex_init(&dp->lock);
>
> INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func);
> + INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func);
>
> /* Acquire all resources (IOMEM, IRQ and PHYs). */
> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp");
> @@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
> zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL);
> disable_irq(dp->irq);
>
> + cancel_delayed_work_sync(&dp->hpd_irq_work);
> cancel_delayed_work_sync(&dp->hpd_work);
>
> zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0);
> @@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>
> zynqmp_dp_phy_exit(dp);
> zynqmp_dp_reset(dp, true);
> + mutex_destroy(&dp->lock);
> }
--
Regards,
Laurent Pinchart
On 3/18/24 13:16, Laurent Pinchart wrote:
> Hi Sean,
>
> Thank you for the patch.
>
> On Fri, Mar 15, 2024 at 07:09:13PM -0400, Sean Anderson wrote:
>> Add some locking, since none is provided by the drm subsystem. This will
>
> That's not quite right, the DRM core doesn't call bridge operations
> concurrently.
I figured something like this was going on.
> We may need locking to protect against race conditions
> between bridge operations and interrupts though.
And of course this will only get worse once we let userspace get involved.
>> prevent the IRQ/workers/bridge API calls from stepping on each other's
>> toes.
>>
>> Signed-off-by: Sean Anderson <[email protected]>
>> ---
>>
>> drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++---------
>> 1 file changed, 42 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> index 8635b5673386..d2dee58e7bf2 100644
>> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> @@ -279,6 +279,7 @@ struct zynqmp_dp_config {
>> * @dpsub: Display subsystem
>> * @iomem: device I/O memory for register access
>> * @reset: reset controller
>> + * @lock: Mutex protecting this struct and register access (but not AUX)
>
> This patch does two things at once, it defers link training from the IRQ
> handler to a work queue, and covers everything with a big lock. The
> scope is too large.
OK, I can split this.
> Please restrict the lock scope and document the
> individual fields that need to be protected, and explain the locking
> design in the commit message (or comments in the code).
As said, this lock protects
- Non-atomic registers configuring the link. That is, everything but the IRQ
registers (since these are accessed in an atomic fashion), and the DP AUX
registers (since these don't affect the link).
- Link configuration. This is effectively everything in zynqmp_dp which isn't
read-only after probe time. So from next_bridge onward.
It's designed to protect configuration changes so we don't have to do anything
tricky. Configuration should never be in the hot path, so I'm not worried about
performance.
>> * @irq: irq
>> * @bridge: DRM bridge for the DP encoder
>> * @next_bridge: The downstream bridge
>> @@ -299,6 +300,7 @@ struct zynqmp_dp {
>> struct zynqmp_dpsub *dpsub;
>> void __iomem *iomem;
>> struct reset_control *reset;
>> + struct mutex lock;
>> int irq;
>>
>> struct drm_bridge bridge;
>> @@ -308,7 +310,7 @@ struct zynqmp_dp {
>> struct drm_dp_aux aux;
>> struct phy *phy[ZYNQMP_DP_MAX_LANES];
>> u8 num_lanes;
>> - struct delayed_work hpd_work;
>> + struct delayed_work hpd_work, hpd_irq_work;
>
> One variable per line please.
OK
>> enum drm_connector_status status;
>> bool enabled;
>>
>> @@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
>> }
>>
>> /* Check with link rate and lane count */
>> + mutex_lock(&dp->lock);
>> rate = zynqmp_dp_max_rate(dp->link_config.max_rate,
>> dp->link_config.max_lanes, dp->config.bpp);
>> + mutex_unlock(&dp->lock);
>> if (mode->clock > rate) {
>> dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n",
>> mode->name);
>> @@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>>
>> pm_runtime_get_sync(dp->dev);
>>
>> + mutex_lock(&dp->lock);
>> zynqmp_dp_disp_enable(dp, old_bridge_state);
>>
>> /*
>> @@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>> zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
>> ZYNQMP_DP_SOFTWARE_RESET_ALL);
>> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
>> + mutex_unlock(&dp->lock);
>> }
>>
>> static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
>> @@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
>> {
>> struct zynqmp_dp *dp = bridge_to_dp(bridge);
>>
>> + mutex_lock(&dp->lock);
>> dp->enabled = false;
>> cancel_delayed_work(&dp->hpd_work);
>> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0);
>> @@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
>> zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0);
>>
>> zynqmp_dp_disp_disable(dp, old_bridge_state);
>> + mutex_unlock(&dp->lock);
>>
>> pm_runtime_put_sync(dp->dev);
>> }
>> @@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
>> u32 state, i;
>> int ret;
>>
>> + mutex_lock(&dp->lock);
>> +
>> /*
>> * This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to
>> * get the HPD signal with some monitors.
>> @@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
>> dp->num_lanes);
>>
>> dp->status = connector_status_connected;
>> + mutex_unlock(&dp->lock);
>> return connector_status_connected;
>> }
>>
>> disconnected:
>> dp->status = connector_status_disconnected;
>> + mutex_unlock(&dp->lock);
>> return connector_status_disconnected;
>> }
>>
>> @@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
>> drm_bridge_hpd_notify(&dp->bridge, status);
>> }
>>
>> +static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
>> +{
>> + struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp,
>> + hpd_irq_work.work);
>> + u8 status[DP_LINK_STATUS_SIZE + 2];
>> + int err;
>> +
>> + mutex_lock(&dp->lock);
>> + err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
>> + DP_LINK_STATUS_SIZE + 2);
>> + if (err < 0) {
>> + dev_dbg_ratelimited(dp->dev,
>> + "could not read sink status: %d\n", err);
>> + } else {
>> + if (status[4] & DP_LINK_STATUS_UPDATED ||
>> + !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
>> + !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
>> + zynqmp_dp_train_loop(dp);
>> + }
>> + }
>> + mutex_unlock(&dp->lock);
>> +}
>> +
>> static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
>> {
>> struct zynqmp_dp *dp = (struct zynqmp_dp *)data;
>> @@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
>> if (status & ZYNQMP_DP_INT_HPD_EVENT)
>> schedule_delayed_work(&dp->hpd_work, 0);
>>
>> - if (status & ZYNQMP_DP_INT_HPD_IRQ) {
>> - int ret;
>> - u8 status[DP_LINK_STATUS_SIZE + 2];
>> + if (status & ZYNQMP_DP_INT_HPD_IRQ)
>> + schedule_delayed_work(&dp->hpd_irq_work, 0);
>>
>> - ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
>> - DP_LINK_STATUS_SIZE + 2);
>> - if (ret < 0)
>> - goto handled;
>> -
>> - if (status[4] & DP_LINK_STATUS_UPDATED ||
>> - !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
>> - !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
>> - zynqmp_dp_train_loop(dp);
>> - }
>> - }
>> -
>> -handled:
>> return IRQ_HANDLED;
>> }
>>
>> @@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
>> dp->dev = &pdev->dev;
>> dp->dpsub = dpsub;
>> dp->status = connector_status_disconnected;
>> + mutex_init(&dp->lock);
>>
>> INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func);
>> + INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func);
>>
>> /* Acquire all resources (IOMEM, IRQ and PHYs). */
>> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp");
>> @@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>> zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL);
>> disable_irq(dp->irq);
>>
>> + cancel_delayed_work_sync(&dp->hpd_irq_work);
>> cancel_delayed_work_sync(&dp->hpd_work);
>>
>> zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0);
>> @@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>>
>> zynqmp_dp_phy_exit(dp);
>> zynqmp_dp_reset(dp, true);
>> + mutex_destroy(&dp->lock);
>> }
>
Hi Sean,
Thank you for the patch.
On Fri, Mar 15, 2024 at 07:09:15PM -0400, Sean Anderson wrote:
> When testing, it's convenient to be able to ignore DPCD errors if there
> is test equipment which can't emulate a DPRX connected to the output.
> Add some (currently-unused) options to ignore these errors and just
> reconfigure our internal registers as we usually would.
This seems to be a problem that is not limited to the ZynqMP DP.
Wouldn't it be better to solve it in the DRM DP DPCD helpers instead ?
You could expose a parameter on the AUX bus in debugfs to ignore errors,
and cause the drm_dp_dpcd_write*() functions to return 0.
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 37 ++++++++++++++++++++------------
> 1 file changed, 23 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index 24043847dab4..040f7b88ee51 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -628,6 +628,7 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
> * zynqmp_dp_update_vs_emph - Update the training values
> * @dp: DisplayPort IP core structure
> * @train_set: A set of training values
> + * @ignore_dpcd: Ignore DPCD errors
> *
> * Update the training values based on the request from sink. The mapped values
> * are predefined, and values(vs, pe, pc) are from the device manual.
> @@ -635,15 +636,19 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
> * Return: 0 if vs and emph are updated successfully, or the error code returned
> * by drm_dp_dpcd_write().
> */
> -static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set)
> +static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set,
> + bool ignore_dpcd)
> {
> unsigned int i;
> int ret;
>
> ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set,
> dp->mode.lane_cnt);
> - if (ret < 0)
> - return ret;
> + if (ret < 0) {
> + if (!ignore_dpcd)
> + return ret;
> + dev_warn(dp->dev, "failed to update vs/emph\n");
> + }
>
> for (i = 0; i < dp->mode.lane_cnt; i++) {
> u32 reg = ZYNQMP_DP_SUB_TX_PHY_PRECURSOR_LANE_0 + i * 4;
> @@ -692,7 +697,7 @@ static int zynqmp_dp_link_train_cr(struct zynqmp_dp *dp)
> * So, This loop should exit before 512 iterations
> */
> for (max_tries = 0; max_tries < 512; max_tries++) {
> - ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
> + ret = zynqmp_dp_update_vs_emph(dp, dp->train_set, false);
> if (ret)
> return ret;
>
> @@ -757,7 +762,7 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
> return ret;
>
> for (tries = 0; tries < DP_MAX_TRAINING_TRIES; tries++) {
> - ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
> + ret = zynqmp_dp_update_vs_emph(dp, dp->train_set, false);
> if (ret)
> return ret;
>
> @@ -785,11 +790,12 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
> * @lane_cnt: The number of lanes to use
> * @enhanced: Use enhanced framing
> * @downspread: Enable spread-spectrum clocking
> + * @ignore_dpcd: Ignore DPCD errors; useful for testing
> *
> * Return: 0 on success, or -errno on failure
> */
> static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
> - bool enhanced, bool downspread)
> + bool enhanced, bool downspread, bool ignore_dpcd)
> {
> u32 reg;
> u8 aux_lane_cnt = lane_cnt;
> @@ -812,21 +818,24 @@ static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
>
> ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, aux_lane_cnt);
> if (ret < 0) {
> - dev_err(dp->dev, "failed to set lane count\n");
> - return ret;
> + dev_warn(dp->dev, "failed to set lane count\n");
> + if (!ignore_dpcd)
> + return ret;
> }
>
> ret = drm_dp_dpcd_writeb(&dp->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
> DP_SET_ANSI_8B10B);
> if (ret < 0) {
> - dev_err(dp->dev, "failed to set ANSI 8B/10B encoding\n");
> - return ret;
> + dev_warn(dp->dev, "failed to set ANSI 8B/10B encoding\n");
> + if (!ignore_dpcd)
> + return ret;
> }
>
> ret = drm_dp_dpcd_writeb(&dp->aux, DP_LINK_BW_SET, bw_code);
> if (ret < 0) {
> - dev_err(dp->dev, "failed to set DP bandwidth\n");
> - return ret;
> + dev_warn(dp->dev, "failed to set DP bandwidth\n");
> + if (!ignore_dpcd)
> + return ret;
> }
>
> zynqmp_dp_write(dp, ZYNQMP_DP_LINK_BW_SET, bw_code);
> @@ -860,7 +869,7 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
>
> ret = zynqmp_dp_setup(dp, dp->mode.bw_code, dp->mode.lane_cnt,
> drm_dp_enhanced_frame_cap(dp->dpcd),
> - dp->dpcd[3] & 0x1);
> + dp->dpcd[3] & 0x1, false);
> if (ret)
> return ret;
>
> @@ -877,7 +886,7 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
> ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
> DP_TRAINING_PATTERN_DISABLE);
> if (ret < 0) {
> - dev_err(dp->dev, "failed to disable training pattern\n");
> + dev_warn(dp->dev, "failed to disable training pattern\n");
> return ret;
> }
> zynqmp_dp_write(dp, ZYNQMP_DP_TRAINING_PATTERN_SET,
--
Regards,
Laurent Pinchart
On Mon, Mar 18, 2024 at 11:06:40AM -0400, Sean Anderson wrote:
> On 3/16/24 06:14, kernel test robot wrote:
> > Hi Sean,
> >
> > kernel test robot noticed the following build warnings:
> >
> > [auto build test WARNING on v6.8]
> > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> > And when submitting patch, we suggest to use '--base' as documented in
> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >
> > url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
> > base: v6.8
> > patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
> > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
> > config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
> > compiler: microblaze-linux-gcc (GCC) 13.2.0
> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
> > | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >
> > All warnings (new ones prefixed by >>):
> >
> > drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init':
> >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
> > 2168 | sprintf(name, fmt, i);
> > | ^~~
> > drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19
> > 2168 | sprintf(name, fmt, i);
> > | ^~~~~~~~~~~~~~~~~~~~~
>
> Not a bug, as i will be at most 4, which uses 1 digit.
The compiler can't know that. Please fix this, there's a zero warning
policy.
> > vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c
> >
> > 2136
> > 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
> > 2138 zynqmp_dp_rate_set, "%llu\n");
> > 2139
> > 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
> > 2141 struct dentry *root)
> > 2142 {
> > 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge);
> > 2144 struct dentry *test;
> > 2145 int i;
> > 2146
> > 2147 dp->test.bw_code = DP_LINK_BW_5_4;
> > 2148 dp->test.link_cnt = dp->num_lanes;
> > 2149
> > 2150 test = debugfs_create_dir("test", root);
> > 2151 #define CREATE_FILE(name) \
> > 2152 debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
> > 2153 CREATE_FILE(pattern);
> > 2154 CREATE_FILE(enhanced);
> > 2155 CREATE_FILE(downspread);
> > 2156 CREATE_FILE(active);
> > 2157 CREATE_FILE(custom);
> > 2158 CREATE_FILE(rate);
> > 2159 CREATE_FILE(lanes);
> > 2160
> > 2161 for (i = 0; i < dp->num_lanes; i++) {
> > 2162 static const char fmt[] = "lane%d_preemphasis";
> > 2163 char name[sizeof(fmt)];
> > 2164
> > 2165 dp->debugfs_train_set[i].dp = dp;
> > 2166 dp->debugfs_train_set[i].lane = i;
> > 2167
> >> 2168 sprintf(name, fmt, i);
> > 2169 debugfs_create_file(name, 0600, test,
> > 2170 &dp->debugfs_train_set[i],
> > 2171 &fops_zynqmp_dp_preemphasis);
> > 2172
> > 2173 sprintf(name, "lane%d_swing", i);
> > 2174 debugfs_create_file(name, 0600, test,
> > 2175 &dp->debugfs_train_set[i],
> > 2176 &fops_zynqmp_dp_swing);
> > 2177 }
> > 2178 }
> > 2179
--
Regards,
Laurent Pinchart
On Sat, Mar 16, 2024 at 07:56:52PM +0200, Dmitry Baryshkov wrote:
> On Sat, 16 Mar 2024 at 01:09, Sean Anderson <[email protected]> wrote:
> >
> > Add a debugfs interface for exercising the various test modes supported
> > by the DisplayPort controller. This allows performing compliance
> > testing, or performing signal integrity measurements on a failing link.
> > At the moment, we do not support sink-driven link quality testing,
> > although such support would be fairly easy to add.
>
> Could you please point out how this is used for compliance testing? We
> have been using the msm_dp_compliance tool [1].
>
> I think it would be nice to rework our drivers towards a common
> debugfs interface used for DP connectors, maybe defining generic
> internal interface/helpers like Maxime is implementing for HDMI
> connectors.
This would be really nice :-)
> [1] https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tools/msm_dp_compliance.c?ref_type=heads
>
> >
> > Signed-off-by: Sean Anderson <[email protected]>
> > ---
> >
> > drivers/gpu/drm/xlnx/zynqmp_dp.c | 591 ++++++++++++++++++++++++++++++-
> > 1 file changed, 590 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > index 040f7b88ee51..57032186e1ca 100644
> > --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> > @@ -18,7 +18,9 @@
> > #include <drm/drm_modes.h>
> > #include <drm/drm_of.h>
> >
> > +#include <linux/bitfield.h>
> > #include <linux/clk.h>
> > +#include <linux/debugfs.h>
> > #include <linux/delay.h>
> > #include <linux/device.h>
> > #include <linux/io.h>
> > @@ -50,6 +52,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
> > #define ZYNQMP_DP_LANE_COUNT_SET 0x4
> > #define ZYNQMP_DP_ENHANCED_FRAME_EN 0x8
> > #define ZYNQMP_DP_TRAINING_PATTERN_SET 0xc
> > +#define ZYNQMP_DP_LINK_QUAL_PATTERN_SET 0x10
> > #define ZYNQMP_DP_SCRAMBLING_DISABLE 0x14
> > #define ZYNQMP_DP_DOWNSPREAD_CTL 0x18
> > #define ZYNQMP_DP_SOFTWARE_RESET 0x1c
> > @@ -63,6 +66,9 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
> > ZYNQMP_DP_SOFTWARE_RESET_STREAM3 | \
> > ZYNQMP_DP_SOFTWARE_RESET_STREAM4 | \
> > ZYNQMP_DP_SOFTWARE_RESET_AUX)
> > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_1 0x20
> > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_2 0x24
> > +#define ZYNQMP_DP_COMP_PATTERN_80BIT_3 0x28
> >
> > /* Core enable registers */
> > #define ZYNQMP_DP_TRANSMITTER_ENABLE 0x80
> > @@ -206,6 +212,7 @@ MODULE_PARM_DESC(power_on_delay_ms, "DP power on delay in msec (default: 4)");
> > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_2 BIT(2)
> > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_LANE_3 BIT(3)
> > #define ZYNQMP_DP_TX_PHY_POWER_DOWN_ALL 0xf
> > +#define ZYNQMP_DP_TRANSMIT_PRBS7 0x230
> > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_0 0x23c
> > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_1 0x240
> > #define ZYNQMP_DP_PHY_PRECURSOR_LANE_2 0x244
> > @@ -273,6 +280,69 @@ struct zynqmp_dp_config {
> > u8 bpp;
> > };
> >
> > +/**
> > + * enum test_pattern - Test patterns for test testing
> > + * TEST_VIDEO: Use regular video input
> > + * TEST_SYMBOL_ERROR: Symbol error measurement pattern
> > + * TEST_PRBS7: Output of the PRBS7 (x^7 + x^6 + 1) polynomial
> > + * TEST_80BIT_CUSTOM: A custom 80-bit pattern
> > + * TEST_CP2520: HBR2 compliance eye pattern
> > + * TEST_TPS1: Link training symbol pattern TPS1 (/D10.2/)
> > + * TEST_TPS2: Link training symbol pattern TPS2
> > + * TEST_TPS3: Link training symbol pattern TPS3 (for HBR2)
> > + */
> > +enum test_pattern {
> > + TEST_VIDEO,
> > + TEST_TPS1,
> > + TEST_TPS2,
> > + TEST_TPS3,
> > + TEST_SYMBOL_ERROR,
> > + TEST_PRBS7,
> > + TEST_80BIT_CUSTOM,
> > + TEST_CP2520,
> > +};
> > +
> > +static const char *const test_pattern_str[] = {
> > + [TEST_VIDEO] = "video",
> > + [TEST_TPS1] = "tps1",
> > + [TEST_TPS2] = "tps2",
> > + [TEST_TPS3] = "tps3",
> > + [TEST_SYMBOL_ERROR] = "symbol-error",
> > + [TEST_PRBS7] = "prbs7",
> > + [TEST_80BIT_CUSTOM] = "80bit-custom",
> > + [TEST_CP2520] = "cp2520",
> > +};
> > +
> > +/**
> > + * struct zynqmp_dp_test - Configuration for test mode
> > + * @pattern: The test pattern
> > + * @enhanced: Use enhanced framing
> > + * @downspread: Use SSC
> > + * @active: Whether test mode is active
> > + * @custom: Custom pattern for %TEST_80BIT_CUSTOM
> > + * @train_set: Voltage/preemphasis settings
> > + * @bw_code: Bandwidth code for the link
> > + * @link_cnt: Number of lanes
> > + */
> > +struct zynqmp_dp_test {
> > + enum test_pattern pattern;
> > + bool enhanced, downspread, active;
> > + u8 custom[10];
> > + u8 train_set[ZYNQMP_DP_MAX_LANES];
> > + u8 bw_code;
> > + u8 link_cnt;
> > +};
> > +
> > +/**
> > + * struct zynqmp_dp_train_set_priv - Private data for train_set debugfs files
> > + * @dp: DisplayPort IP core structure
> > + * @lane: The lane for this file
> > + */
> > +struct zynqmp_dp_train_set_priv {
> > + struct zynqmp_dp *dp;
> > + int lane;
> > +};
> > +
> > /**
> > * struct zynqmp_dp - Xilinx DisplayPort core
> > * @dev: device structure
> > @@ -283,6 +353,7 @@ struct zynqmp_dp_config {
> > * @irq: irq
> > * @bridge: DRM bridge for the DP encoder
> > * @next_bridge: The downstream bridge
> > + * @test: Configuration for test mode
> > * @config: IP core configuration from DTS
> > * @aux: aux channel
> > * @phy: PHY handles for DP lanes
> > @@ -294,6 +365,7 @@ struct zynqmp_dp_config {
> > * @link_config: common link configuration between IP core and sink device
> > * @mode: current mode between IP core and sink device
> > * @train_set: set of training data
> > + * @debugfs_train_set: Debugfs private data for @train_set
> > */
> > struct zynqmp_dp {
> > struct device *dev;
> > @@ -306,6 +378,7 @@ struct zynqmp_dp {
> > struct drm_bridge bridge;
> > struct drm_bridge *next_bridge;
> >
> > + struct zynqmp_dp_test test;
> > struct zynqmp_dp_config config;
> > struct drm_dp_aux aux;
> > struct phy *phy[ZYNQMP_DP_MAX_LANES];
> > @@ -318,6 +391,7 @@ struct zynqmp_dp {
> > struct zynqmp_dp_link_config link_config;
> > struct zynqmp_dp_mode mode;
> > u8 train_set[ZYNQMP_DP_MAX_LANES];
> > + struct zynqmp_dp_train_set_priv debugfs_train_set[ZYNQMP_DP_MAX_LANES];
> > };
> >
> > static inline struct zynqmp_dp *bridge_to_dp(struct drm_bridge *bridge)
> > @@ -1599,6 +1673,510 @@ static struct edid *zynqmp_dp_bridge_get_edid(struct drm_bridge *bridge,
> > return drm_get_edid(connector, &dp->aux.ddc);
> > }
> >
> > +/* -----------------------------------------------------------------------------
> > + * debugfs
> > + */
> > +
> > +/**
> > + * zynqmp_dp_set_test_pattern() - Configure the link for a test pattern
> > + * @dp: DisplayPort IP core structure
> > + */
> > +static void zynqmp_dp_set_test_pattern(struct zynqmp_dp *dp,
> > + enum test_pattern pattern,
> > + u8 *const custom)
> > +{
> > + bool scramble = false;
> > + u32 train_pattern = 0;
> > + u32 link_pattern = 0;
> > + u8 dpcd_train = 0;
> > + u8 dpcd_link = 0;
> > + int err;
> > +
> > + switch (pattern) {
> > + case TEST_TPS1:
> > + train_pattern = 1;
> > + break;
> > + case TEST_TPS2:
> > + train_pattern = 2;
> > + break;
> > + case TEST_TPS3:
> > + train_pattern = 3;
> > + break;
> > + case TEST_SYMBOL_ERROR:
> > + scramble = true;
> > + link_pattern = DP_PHY_TEST_PATTERN_ERROR_COUNT;
> > + break;
> > + case TEST_PRBS7:
> > + /* We use a dedicated register to enable PRBS7 */
> > + dpcd_link = DP_LINK_QUAL_PATTERN_ERROR_RATE;
> > + break;
> > + case TEST_80BIT_CUSTOM: {
> > + const u8 *p = custom;
> > +
> > + link_pattern = DP_LINK_QUAL_PATTERN_80BIT_CUSTOM;
> > +
> > + zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_1,
> > + (p[3] << 24) | (p[2] << 16) | (p[1] << 8) | p[0]);
> > + zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_2,
> > + (p[7] << 24) | (p[6] << 16) | (p[5] << 8) | p[4]);
> > + zynqmp_dp_write(dp, ZYNQMP_DP_COMP_PATTERN_80BIT_3,
> > + (p[9] << 8) | p[8]);
> > + break;
> > + }
> > + case TEST_CP2520:
> > + link_pattern = DP_LINK_QUAL_PATTERN_CP2520_PAT_1;
> > + break;
> > + default:
> > + WARN_ON_ONCE(1);
> > + fallthrough;
> > + case TEST_VIDEO:
> > + scramble = true;
> > + }
> > +
> > + zynqmp_dp_write(dp, ZYNQMP_DP_SCRAMBLING_DISABLE, !scramble);
> > + zynqmp_dp_write(dp, ZYNQMP_DP_TRAINING_PATTERN_SET, train_pattern);
> > + zynqmp_dp_write(dp, ZYNQMP_DP_LINK_QUAL_PATTERN_SET, link_pattern);
> > + zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMIT_PRBS7, pattern == TEST_PRBS7);
> > +
> > + dpcd_link = dpcd_link ?: link_pattern;
> > + dpcd_train = train_pattern;
> > + if (!scramble)
> > + dpcd_train |= DP_LINK_SCRAMBLING_DISABLE;
> > +
> > + if (dp->dpcd[DP_DPCD_REV] < 0x12) {
> > + if (pattern == TEST_CP2520)
> > + dev_warn(dp->dev,
> > + "can't set sink link quality pattern to CP2520 for DPCD < r1.2; error counters will be invalid\n");
> > + else
> > + dpcd_train |= FIELD_PREP(DP_LINK_QUAL_PATTERN_11_MASK,
> > + dpcd_link);
> > + } else {
> > + u8 dpcd_link_lane[ZYNQMP_DP_MAX_LANES];
> > +
> > + memset(dpcd_link_lane, dpcd_link, ZYNQMP_DP_MAX_LANES);
> > + err = drm_dp_dpcd_write(&dp->aux, DP_LINK_QUAL_LANE0_SET,
> > + dpcd_link_lane, ZYNQMP_DP_MAX_LANES);
> > + if (err < 0)
> > + dev_err(dp->dev, "failed to set quality pattern\n");
> > + }
> > +
> > + err = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET, dpcd_train);
> > + if (err < 0)
> > + dev_err(dp->dev, "failed to set training pattern\n");
> > +}
> > +
> > +static int zynqmp_dp_test_setup(struct zynqmp_dp *dp)
> > +{
> > + return zynqmp_dp_setup(dp, dp->test.bw_code, dp->test.link_cnt,
> > + dp->test.enhanced, dp->test.downspread, true);
> > +}
> > +
> > +static ssize_t zynqmp_dp_pattern_read(struct file *file, char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct dentry *dentry = file->f_path.dentry;
> > + struct zynqmp_dp *dp = file->private_data;
> > + char buf[16];
> > + ssize_t ret;
> > +
> > + ret = debugfs_file_get(dentry);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + mutex_lock(&dp->lock);
> > + ret = snprintf(buf, sizeof(buf), "%s\n",
> > + test_pattern_str[dp->test.pattern]);
> > + mutex_unlock(&dp->lock);
> > +
> > + debugfs_file_put(dentry);
> > + return simple_read_from_buffer(user_buf, count, ppos, buf, ret);
> > +}
> > +
> > +static ssize_t zynqmp_dp_pattern_write(struct file *file,
> > + const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > +
> > + struct dentry *dentry = file->f_path.dentry;
> > + struct zynqmp_dp *dp = file->private_data;
> > + char buf[16];
> > + ssize_t ret;
> > + int pattern;
> > +
> > + ret = debugfs_file_get(dentry);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + ret = simple_write_to_buffer(buf, sizeof(buf) - 1, ppos, user_buf,
> > + count);
> > + if (ret < 0)
> > + goto out;
> > + buf[ret] = '\0';
> > +
> > + pattern = sysfs_match_string(test_pattern_str, buf);
> > + if (pattern < 0) {
> > + ret = -EINVAL;
> > + goto out;
> > + }
> > +
> > + mutex_lock(&dp->lock);
> > + dp->test.pattern = pattern;
> > + if (dp->test.active)
> > + zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
> > + dp->test.custom);
> > + mutex_unlock(&dp->lock);
> > +
> > +out:
> > + debugfs_file_put(dentry);
> > + return ret;
> > +}
> > +
> > +static const struct file_operations fops_zynqmp_dp_pattern = {
> > + .read = zynqmp_dp_pattern_read,
> > + .write = zynqmp_dp_pattern_write,
> > + .open = simple_open,
> > + .llseek = noop_llseek,
> > +};
> > +
> > +static int zynqmp_dp_enhanced_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = dp->test.enhanced;
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_enhanced_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > + int ret = 0;
> > +
> > + mutex_lock(&dp->lock);
> > + dp->test.enhanced = val;
> > + if (dp->test.active)
> > + ret = zynqmp_dp_test_setup(dp);
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_enhanced, zynqmp_dp_enhanced_get,
> > + zynqmp_dp_enhanced_set, "%llu\n");
> > +
> > +static int zynqmp_dp_downspread_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = dp->test.downspread;
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_downspread_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > + int ret = 0;
> > +
> > + mutex_lock(&dp->lock);
> > + dp->test.downspread = val;
> > + if (dp->test.active)
> > + ret = zynqmp_dp_test_setup(dp);
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_downspread, zynqmp_dp_downspread_get,
> > + zynqmp_dp_downspread_set, "%llu\n");
> > +
> > +static int zynqmp_dp_active_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = dp->test.active;
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_active_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > + int ret = 0;
> > +
> > + mutex_lock(&dp->lock);
> > + if (val) {
> > + if (val < 2) {
> > + ret = zynqmp_dp_test_setup(dp);
> > + if (ret)
> > + goto out;
> > + }
> > +
> > + zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
> > + dp->test.custom);
> > + zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
> > + dp->test.active = true;
> > + } else {
> > + dp->test.active = false;
> > + zynqmp_dp_set_test_pattern(dp, TEST_VIDEO, NULL);
> > + zynqmp_dp_train_loop(dp);
> > + }
> > +out:
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_active, zynqmp_dp_active_get,
> > + zynqmp_dp_active_set, "%llu\n");
> > +
> > +static ssize_t zynqmp_dp_custom_read(struct file *file, char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > + struct dentry *dentry = file->f_path.dentry;
> > + struct zynqmp_dp *dp = file->private_data;
> > + ssize_t ret;
> > +
> > + ret = debugfs_file_get(dentry);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + mutex_lock(&dp->lock);
> > + ret = simple_read_from_buffer(user_buf, count, ppos, &dp->test.custom,
> > + sizeof(dp->test.custom));
> > + mutex_unlock(&dp->lock);
> > +
> > + debugfs_file_put(dentry);
> > + return ret;
> > +}
> > +
> > +static ssize_t zynqmp_dp_custom_write(struct file *file,
> > + const char __user *user_buf,
> > + size_t count, loff_t *ppos)
> > +{
> > +
> > + struct dentry *dentry = file->f_path.dentry;
> > + struct zynqmp_dp *dp = file->private_data;
> > + ssize_t ret;
> > + char buf[sizeof(dp->test.custom)];
> > +
> > + ret = debugfs_file_get(dentry);
> > + if (unlikely(ret))
> > + return ret;
> > +
> > + ret = simple_write_to_buffer(buf, sizeof(buf), ppos, user_buf, count);
> > + if (ret < 0)
> > + goto out;
> > +
> > + mutex_lock(&dp->lock);
> > + memcpy(dp->test.custom, buf, ret);
> > + if (dp->test.active)
> > + zynqmp_dp_set_test_pattern(dp, dp->test.pattern,
> > + dp->test.custom);
> > + mutex_unlock(&dp->lock);
> > +
> > +out:
> > + debugfs_file_put(dentry);
> > + return ret;
> > +}
> > +
> > +static const struct file_operations fops_zynqmp_dp_custom = {
> > + .read = zynqmp_dp_custom_read,
> > + .write = zynqmp_dp_custom_write,
> > + .open = simple_open,
> > + .llseek = noop_llseek,
> > +};
> > +
> > +static int zynqmp_dp_swing_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp_train_set_priv *priv = data;
> > + struct zynqmp_dp *dp = priv->dp;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = dp->test.train_set[priv->lane] & DP_TRAIN_VOLTAGE_SWING_MASK;
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_swing_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp_train_set_priv *priv = data;
> > + struct zynqmp_dp *dp = priv->dp;
> > + u8 *train_set = &dp->test.train_set[priv->lane];
> > + int ret = 0;
> > +
> > + if (val > 3)
> > + return -EINVAL;
> > +
> > + mutex_lock(&dp->lock);
> > + *train_set &= ~(DP_TRAIN_MAX_SWING_REACHED |
> > + DP_TRAIN_VOLTAGE_SWING_MASK);
> > + *train_set |= val;
> > + if (val == 3)
> > + *train_set |= DP_TRAIN_MAX_SWING_REACHED;
> > +
> > + if (dp->test.active)
> > + zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_swing, zynqmp_dp_swing_get,
> > + zynqmp_dp_swing_set, "%llu\n");
> > +
> > +static int zynqmp_dp_preemphasis_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp_train_set_priv *priv = data;
> > + struct zynqmp_dp *dp = priv->dp;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = FIELD_GET(DP_TRAIN_PRE_EMPHASIS_MASK,
> > + dp->test.train_set[priv->lane]);
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_preemphasis_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp_train_set_priv *priv = data;
> > + struct zynqmp_dp *dp = priv->dp;
> > + u8 *train_set = &dp->test.train_set[priv->lane];
> > + int ret = 0;
> > +
> > + if (val > 2)
> > + return -EINVAL;
> > +
> > + mutex_lock(&dp->lock);
> > + *train_set &= ~(DP_TRAIN_MAX_PRE_EMPHASIS_REACHED |
> > + DP_TRAIN_PRE_EMPHASIS_MASK);
> > + *train_set |= val;
> > + if (val == 2)
> > + *train_set |= DP_TRAIN_MAX_PRE_EMPHASIS_REACHED;
> > +
> > + if (dp->test.active)
> > + zynqmp_dp_update_vs_emph(dp, dp->test.train_set, true);
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_preemphasis, zynqmp_dp_preemphasis_get,
> > + zynqmp_dp_preemphasis_set, "%llu\n");
> > +
> > +static int zynqmp_dp_lanes_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = dp->test.link_cnt;
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_lanes_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > + int ret = 0;
> > +
> > + if (val > ZYNQMP_DP_MAX_LANES)
> > + return -EINVAL;
> > +
> > + mutex_lock(&dp->lock);
> > + if (val > dp->num_lanes) {
> > + ret = -EINVAL;
> > + } else {
> > + dp->test.link_cnt = val;
> > + if (dp->test.active)
> > + ret = zynqmp_dp_test_setup(dp);
> > + }
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_lanes, zynqmp_dp_lanes_get,
> > + zynqmp_dp_lanes_set, "%llu\n");
> > +
> > +static int zynqmp_dp_rate_get(void *data, u64 *val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > +
> > + mutex_lock(&dp->lock);
> > + *val = drm_dp_bw_code_to_link_rate(dp->test.bw_code) * 10000;
> > + mutex_unlock(&dp->lock);
> > + return 0;
> > +}
> > +
> > +static int zynqmp_dp_rate_set(void *data, u64 val)
> > +{
> > + struct zynqmp_dp *dp = data;
> > + u8 bw_code = drm_dp_link_rate_to_bw_code(val / 10000);
> > + int link_rate = drm_dp_bw_code_to_link_rate(bw_code);
> > + int ret = 0;
> > +
> > + if (val / 10000 != link_rate)
> > + return -EINVAL;
> > +
> > + if (bw_code != DP_LINK_BW_1_62 && bw_code != DP_LINK_BW_2_7 &&
> > + bw_code != DP_LINK_BW_5_4)
> > + return -EINVAL;
> > +
> > + mutex_lock(&dp->lock);
> > + dp->test.bw_code = bw_code;
> > + if (dp->test.active)
> > + ret = zynqmp_dp_test_setup(dp);
> > + mutex_unlock(&dp->lock);
> > +
> > + return ret;
> > +}
> > +
> > +DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
> > + zynqmp_dp_rate_set, "%llu\n");
> > +
> > +static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
> > + struct dentry *root)
> > +{
> > + struct zynqmp_dp *dp = bridge_to_dp(bridge);
> > + struct dentry *test;
> > + int i;
> > +
> > + dp->test.bw_code = DP_LINK_BW_5_4;
> > + dp->test.link_cnt = dp->num_lanes;
> > +
> > + test = debugfs_create_dir("test", root);
> > +#define CREATE_FILE(name) \
> > + debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
> > + CREATE_FILE(pattern);
> > + CREATE_FILE(enhanced);
> > + CREATE_FILE(downspread);
> > + CREATE_FILE(active);
> > + CREATE_FILE(custom);
> > + CREATE_FILE(rate);
> > + CREATE_FILE(lanes);
> > +
> > + for (i = 0; i < dp->num_lanes; i++) {
> > + static const char fmt[] = "lane%d_preemphasis";
> > + char name[sizeof(fmt)];
> > +
> > + dp->debugfs_train_set[i].dp = dp;
> > + dp->debugfs_train_set[i].lane = i;
> > +
> > + sprintf(name, fmt, i);
> > + debugfs_create_file(name, 0600, test,
> > + &dp->debugfs_train_set[i],
> > + &fops_zynqmp_dp_preemphasis);
> > +
> > + sprintf(name, "lane%d_swing", i);
> > + debugfs_create_file(name, 0600, test,
> > + &dp->debugfs_train_set[i],
> > + &fops_zynqmp_dp_swing);
> > + }
> > +}
> > +
> > static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> > .attach = zynqmp_dp_bridge_attach,
> > .detach = zynqmp_dp_bridge_detach,
> > @@ -1611,6 +2189,7 @@ static const struct drm_bridge_funcs zynqmp_dp_bridge_funcs = {
> > .atomic_check = zynqmp_dp_bridge_atomic_check,
> > .detect = zynqmp_dp_bridge_detect,
> > .get_edid = zynqmp_dp_bridge_get_edid,
> > + .debugfs_init = zynqmp_dp_bridge_debugfs_init,
> > };
> >
> > /* -----------------------------------------------------------------------------
> > @@ -1645,6 +2224,9 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
> > hpd_work.work);
> > enum drm_connector_status status;
> >
> > + if (dp->test.active)
> > + return;
> > +
> > status = zynqmp_dp_bridge_detect(&dp->bridge);
> > drm_bridge_hpd_notify(&dp->bridge, status);
> > }
> > @@ -1666,7 +2248,14 @@ static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
> > if (status[4] & DP_LINK_STATUS_UPDATED ||
> > !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
> > !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
> > - zynqmp_dp_train_loop(dp);
> > + if (dp->test.active) {
> > + dev_dbg(dp->dev, "Ignoring HPD IRQ in test mode\n");
> > + } else {
> > + dev_dbg(dp->dev,
> > + "Retraining due to HPD IRQ (status is [%*ph])\n",
> > + (int)sizeof(status), status);
> > + zynqmp_dp_train_loop(dp);
> > + }
> > }
> > }
> > mutex_unlock(&dp->lock);
--
Regards,
Laurent Pinchart
Hi Sean,
Thank you for the patch.
On Fri, Mar 15, 2024 at 07:09:14PM -0400, Sean Anderson wrote:
> In preparation for supporting compliance testing, split off several
> helper functions. No functional change intended.
>
> Signed-off-by: Sean Anderson <[email protected]>
> ---
>
> drivers/gpu/drm/xlnx/zynqmp_dp.c | 49 +++++++++++++++++++++-----------
> 1 file changed, 33 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> index d2dee58e7bf2..24043847dab4 100644
> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> @@ -627,6 +627,7 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
> /**
> * zynqmp_dp_update_vs_emph - Update the training values
> * @dp: DisplayPort IP core structure
> + * @train_set: A set of training values
> *
> * Update the training values based on the request from sink. The mapped values
> * are predefined, and values(vs, pe, pc) are from the device manual.
> @@ -634,12 +635,12 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
> * Return: 0 if vs and emph are updated successfully, or the error code returned
> * by drm_dp_dpcd_write().
> */
> -static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp)
> +static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set)
> {
> unsigned int i;
> int ret;
>
> - ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, dp->train_set,
> + ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set,
> dp->mode.lane_cnt);
> if (ret < 0)
> return ret;
> @@ -647,7 +648,7 @@ static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp)
> for (i = 0; i < dp->mode.lane_cnt; i++) {
> u32 reg = ZYNQMP_DP_SUB_TX_PHY_PRECURSOR_LANE_0 + i * 4;
> union phy_configure_opts opts = { 0 };
> - u8 train = dp->train_set[i];
> + u8 train = train_set[i];
>
> opts.dp.voltage[0] = (train & DP_TRAIN_VOLTAGE_SWING_MASK)
> >> DP_TRAIN_VOLTAGE_SWING_SHIFT;
> @@ -691,7 +692,7 @@ static int zynqmp_dp_link_train_cr(struct zynqmp_dp *dp)
> * So, This loop should exit before 512 iterations
> */
> for (max_tries = 0; max_tries < 512; max_tries++) {
> - ret = zynqmp_dp_update_vs_emph(dp);
> + ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
> if (ret)
> return ret;
>
> @@ -756,7 +757,7 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
> return ret;
>
> for (tries = 0; tries < DP_MAX_TRAINING_TRIES; tries++) {
> - ret = zynqmp_dp_update_vs_emph(dp);
> + ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
> if (ret)
> return ret;
>
> @@ -779,28 +780,28 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
> }
>
> /**
> - * zynqmp_dp_train - Train the link
> - * @dp: DisplayPort IP core structure
> + * zynqmp_dp_setup() - Set up major link parameters
> + * @bw_code: The link bandwidth as a multiple of 270 MHz
> + * @lane_cnt: The number of lanes to use
> + * @enhanced: Use enhanced framing
> + * @downspread: Enable spread-spectrum clocking
> *
> - * Return: 0 if all trains are done successfully, or corresponding error code.
> + * Return: 0 on success, or -errno on failure
> */
> -static int zynqmp_dp_train(struct zynqmp_dp *dp)
> +static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
> + bool enhanced, bool downspread)
> {
> u32 reg;
> - u8 bw_code = dp->mode.bw_code;
> - u8 lane_cnt = dp->mode.lane_cnt;
> u8 aux_lane_cnt = lane_cnt;
> - bool enhanced;
> int ret;
>
> zynqmp_dp_write(dp, ZYNQMP_DP_LANE_COUNT_SET, lane_cnt);
> - enhanced = drm_dp_enhanced_frame_cap(dp->dpcd);
> if (enhanced) {
> zynqmp_dp_write(dp, ZYNQMP_DP_ENHANCED_FRAME_EN, 1);
> aux_lane_cnt |= DP_LANE_COUNT_ENHANCED_FRAME_EN;
> }
>
> - if (dp->dpcd[3] & 0x1) {
> + if (downspread) {
> zynqmp_dp_write(dp, ZYNQMP_DP_DOWNSPREAD_CTL, 1);
> drm_dp_dpcd_writeb(&dp->aux, DP_DOWNSPREAD_CTRL,
> DP_SPREAD_AMP_0_5);
> @@ -843,8 +844,24 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
> }
>
> zynqmp_dp_write(dp, ZYNQMP_DP_PHY_CLOCK_SELECT, reg);
> - ret = zynqmp_dp_phy_ready(dp);
> - if (ret < 0)
> + return zynqmp_dp_phy_ready(dp);
> +}
> +
> +
> +/**
> + * zynqmp_dp_train - Train the link
> + * @dp: DisplayPort IP core structure
> + *
> + * Return: 0 if all trains are done successfully, or corresponding error code.
> + */
> +static int zynqmp_dp_train(struct zynqmp_dp *dp)
> +{
> + int ret;
> +
> + ret = zynqmp_dp_setup(dp, dp->mode.bw_code, dp->mode.lane_cnt,
> + drm_dp_enhanced_frame_cap(dp->dpcd),
> + dp->dpcd[3] & 0x1);
This patch looks OK. Assuming you make correct use of the new functions
in the next patches,
Reviewed-by: Laurent Pinchart <[email protected]>
On a side note, I think the above could be written
ret = zynqmp_dp_setup(dp, dp->mode.bw_code, dp->mode.lane_cnt,
drm_dp_enhanced_frame_cap(dp->dpcd),
dp->dpcd[DP_MAX_DOWNSPREAD] & DP_MAX_DOWNSPREAD_0_5);
> + if (ret)
> return ret;
>
> zynqmp_dp_write(dp, ZYNQMP_DP_SCRAMBLING_DISABLE, 1);
--
Regards,
Laurent Pinchart
Hi Sean,
On Mon, Mar 18, 2024 at 01:29:12PM -0400, Sean Anderson wrote:
> On 3/18/24 13:16, Laurent Pinchart wrote:
> > On Fri, Mar 15, 2024 at 07:09:13PM -0400, Sean Anderson wrote:
> >> Add some locking, since none is provided by the drm subsystem. This will
> >
> > That's not quite right, the DRM core doesn't call bridge operations
> > concurrently.
>
> I figured something like this was going on.
>
> > We may need locking to protect against race conditions
> > between bridge operations and interrupts though.
>
> And of course this will only get worse once we let userspace get involved.
>
> >> prevent the IRQ/workers/bridge API calls from stepping on each other's
> >> toes.
> >>
> >> Signed-off-by: Sean Anderson <[email protected]>
> >> ---
> >>
> >> drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++---------
> >> 1 file changed, 42 insertions(+), 17 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> >> index 8635b5673386..d2dee58e7bf2 100644
> >> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
> >> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
> >> @@ -279,6 +279,7 @@ struct zynqmp_dp_config {
> >> * @dpsub: Display subsystem
> >> * @iomem: device I/O memory for register access
> >> * @reset: reset controller
> >> + * @lock: Mutex protecting this struct and register access (but not AUX)
> >
> > This patch does two things at once, it defers link training from the IRQ
> > handler to a work queue, and covers everything with a big lock. The
> > scope is too large.
>
> OK, I can split this.
>
> > Please restrict the lock scope and document the
> > individual fields that need to be protected, and explain the locking
> > design in the commit message (or comments in the code).
>
> As said, this lock protects
>
> - Non-atomic registers configuring the link. That is, everything but the IRQ
> registers (since these are accessed in an atomic fashion), and the DP AUX
> registers (since these don't affect the link).
> - Link configuration. This is effectively everything in zynqmp_dp which isn't
> read-only after probe time. So from next_bridge onward.
>
> It's designed to protect configuration changes so we don't have to do anything
> tricky. Configuration should never be in the hot path, so I'm not worried about
> performance.
If userspace can control all this directly through debugfs, can you
guarantee that locks will be enough ? The driver doesn't expect direct
userspace access. I have a feeling this is really quite hacky.
> >> * @irq: irq
> >> * @bridge: DRM bridge for the DP encoder
> >> * @next_bridge: The downstream bridge
> >> @@ -299,6 +300,7 @@ struct zynqmp_dp {
> >> struct zynqmp_dpsub *dpsub;
> >> void __iomem *iomem;
> >> struct reset_control *reset;
> >> + struct mutex lock;
> >> int irq;
> >>
> >> struct drm_bridge bridge;
> >> @@ -308,7 +310,7 @@ struct zynqmp_dp {
> >> struct drm_dp_aux aux;
> >> struct phy *phy[ZYNQMP_DP_MAX_LANES];
> >> u8 num_lanes;
> >> - struct delayed_work hpd_work;
> >> + struct delayed_work hpd_work, hpd_irq_work;
> >
> > One variable per line please.
>
> OK
>
> >> enum drm_connector_status status;
> >> bool enabled;
> >>
> >> @@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
> >> }
> >>
> >> /* Check with link rate and lane count */
> >> + mutex_lock(&dp->lock);
> >> rate = zynqmp_dp_max_rate(dp->link_config.max_rate,
> >> dp->link_config.max_lanes, dp->config.bpp);
> >> + mutex_unlock(&dp->lock);
> >> if (mode->clock > rate) {
> >> dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n",
> >> mode->name);
> >> @@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> >>
> >> pm_runtime_get_sync(dp->dev);
> >>
> >> + mutex_lock(&dp->lock);
> >> zynqmp_dp_disp_enable(dp, old_bridge_state);
> >>
> >> /*
> >> @@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> >> zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
> >> ZYNQMP_DP_SOFTWARE_RESET_ALL);
> >> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
> >> + mutex_unlock(&dp->lock);
> >> }
> >>
> >> static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> >> @@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> >> {
> >> struct zynqmp_dp *dp = bridge_to_dp(bridge);
> >>
> >> + mutex_lock(&dp->lock);
> >> dp->enabled = false;
> >> cancel_delayed_work(&dp->hpd_work);
> >> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0);
> >> @@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> >> zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0);
> >>
> >> zynqmp_dp_disp_disable(dp, old_bridge_state);
> >> + mutex_unlock(&dp->lock);
> >>
> >> pm_runtime_put_sync(dp->dev);
> >> }
> >> @@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
> >> u32 state, i;
> >> int ret;
> >>
> >> + mutex_lock(&dp->lock);
> >> +
> >> /*
> >> * This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to
> >> * get the HPD signal with some monitors.
> >> @@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
> >> dp->num_lanes);
> >>
> >> dp->status = connector_status_connected;
> >> + mutex_unlock(&dp->lock);
> >> return connector_status_connected;
> >> }
> >>
> >> disconnected:
> >> dp->status = connector_status_disconnected;
> >> + mutex_unlock(&dp->lock);
> >> return connector_status_disconnected;
> >> }
> >>
> >> @@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
> >> drm_bridge_hpd_notify(&dp->bridge, status);
> >> }
> >>
> >> +static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
> >> +{
> >> + struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp,
> >> + hpd_irq_work.work);
> >> + u8 status[DP_LINK_STATUS_SIZE + 2];
> >> + int err;
> >> +
> >> + mutex_lock(&dp->lock);
> >> + err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
> >> + DP_LINK_STATUS_SIZE + 2);
> >> + if (err < 0) {
> >> + dev_dbg_ratelimited(dp->dev,
> >> + "could not read sink status: %d\n", err);
> >> + } else {
> >> + if (status[4] & DP_LINK_STATUS_UPDATED ||
> >> + !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
> >> + !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
> >> + zynqmp_dp_train_loop(dp);
> >> + }
> >> + }
> >> + mutex_unlock(&dp->lock);
> >> +}
> >> +
> >> static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
> >> {
> >> struct zynqmp_dp *dp = (struct zynqmp_dp *)data;
> >> @@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
> >> if (status & ZYNQMP_DP_INT_HPD_EVENT)
> >> schedule_delayed_work(&dp->hpd_work, 0);
> >>
> >> - if (status & ZYNQMP_DP_INT_HPD_IRQ) {
> >> - int ret;
> >> - u8 status[DP_LINK_STATUS_SIZE + 2];
> >> + if (status & ZYNQMP_DP_INT_HPD_IRQ)
> >> + schedule_delayed_work(&dp->hpd_irq_work, 0);
> >>
> >> - ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
> >> - DP_LINK_STATUS_SIZE + 2);
> >> - if (ret < 0)
> >> - goto handled;
> >> -
> >> - if (status[4] & DP_LINK_STATUS_UPDATED ||
> >> - !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
> >> - !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
> >> - zynqmp_dp_train_loop(dp);
> >> - }
> >> - }
> >> -
> >> -handled:
> >> return IRQ_HANDLED;
> >> }
> >>
> >> @@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
> >> dp->dev = &pdev->dev;
> >> dp->dpsub = dpsub;
> >> dp->status = connector_status_disconnected;
> >> + mutex_init(&dp->lock);
> >>
> >> INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func);
> >> + INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func);
> >>
> >> /* Acquire all resources (IOMEM, IRQ and PHYs). */
> >> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp");
> >> @@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
> >> zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL);
> >> disable_irq(dp->irq);
> >>
> >> + cancel_delayed_work_sync(&dp->hpd_irq_work);
> >> cancel_delayed_work_sync(&dp->hpd_work);
> >>
> >> zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0);
> >> @@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
> >>
> >> zynqmp_dp_phy_exit(dp);
> >> zynqmp_dp_reset(dp, true);
> >> + mutex_destroy(&dp->lock);
> >> }
--
Regards,
Laurent Pinchart
On 3/18/24 13:59, Laurent Pinchart wrote:
> Hi Sean,
>
> On Mon, Mar 18, 2024 at 01:29:12PM -0400, Sean Anderson wrote:
>> On 3/18/24 13:16, Laurent Pinchart wrote:
>> > On Fri, Mar 15, 2024 at 07:09:13PM -0400, Sean Anderson wrote:
>> >> Add some locking, since none is provided by the drm subsystem. This will
>> >
>> > That's not quite right, the DRM core doesn't call bridge operations
>> > concurrently.
>>
>> I figured something like this was going on.
>>
>> > We may need locking to protect against race conditions
>> > between bridge operations and interrupts though.
>>
>> And of course this will only get worse once we let userspace get involved.
>>
>> >> prevent the IRQ/workers/bridge API calls from stepping on each other's
>> >> toes.
>> >>
>> >> Signed-off-by: Sean Anderson <[email protected]>
>> >> ---
>> >>
>> >> drivers/gpu/drm/xlnx/zynqmp_dp.c | 59 +++++++++++++++++++++++---------
>> >> 1 file changed, 42 insertions(+), 17 deletions(-)
>> >>
>> >> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> >> index 8635b5673386..d2dee58e7bf2 100644
>> >> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> >> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> >> @@ -279,6 +279,7 @@ struct zynqmp_dp_config {
>> >> * @dpsub: Display subsystem
>> >> * @iomem: device I/O memory for register access
>> >> * @reset: reset controller
>> >> + * @lock: Mutex protecting this struct and register access (but not AUX)
>> >
>> > This patch does two things at once, it defers link training from the IRQ
>> > handler to a work queue, and covers everything with a big lock. The
>> > scope is too large.
>>
>> OK, I can split this.
>>
>> > Please restrict the lock scope and document the
>> > individual fields that need to be protected, and explain the locking
>> > design in the commit message (or comments in the code).
>>
>> As said, this lock protects
>>
>> - Non-atomic registers configuring the link. That is, everything but the IRQ
>> registers (since these are accessed in an atomic fashion), and the DP AUX
>> registers (since these don't affect the link).
>> - Link configuration. This is effectively everything in zynqmp_dp which isn't
>> read-only after probe time. So from next_bridge onward.
>>
>> It's designed to protect configuration changes so we don't have to do anything
>> tricky. Configuration should never be in the hot path, so I'm not worried about
>> performance.
>
> If userspace can control all this directly through debugfs, can you
> guarantee that locks will be enough ? The driver doesn't expect direct
> userspace access. I have a feeling this is really quite hacky.
Yes, this is fine. The most userspace can do is force a lot of retraining. But we
have timeouts on everything so I'm not really concerned.
--Sean
>> >> * @irq: irq
>> >> * @bridge: DRM bridge for the DP encoder
>> >> * @next_bridge: The downstream bridge
>> >> @@ -299,6 +300,7 @@ struct zynqmp_dp {
>> >> struct zynqmp_dpsub *dpsub;
>> >> void __iomem *iomem;
>> >> struct reset_control *reset;
>> >> + struct mutex lock;
>> >> int irq;
>> >>
>> >> struct drm_bridge bridge;
>> >> @@ -308,7 +310,7 @@ struct zynqmp_dp {
>> >> struct drm_dp_aux aux;
>> >> struct phy *phy[ZYNQMP_DP_MAX_LANES];
>> >> u8 num_lanes;
>> >> - struct delayed_work hpd_work;
>> >> + struct delayed_work hpd_work, hpd_irq_work;
>> >
>> > One variable per line please.
>>
>> OK
>>
>> >> enum drm_connector_status status;
>> >> bool enabled;
>> >>
>> >> @@ -1371,8 +1373,10 @@ zynqmp_dp_bridge_mode_valid(struct drm_bridge *bridge,
>> >> }
>> >>
>> >> /* Check with link rate and lane count */
>> >> + mutex_lock(&dp->lock);
>> >> rate = zynqmp_dp_max_rate(dp->link_config.max_rate,
>> >> dp->link_config.max_lanes, dp->config.bpp);
>> >> + mutex_unlock(&dp->lock);
>> >> if (mode->clock > rate) {
>> >> dev_dbg(dp->dev, "filtered mode %s for high pixel rate\n",
>> >> mode->name);
>> >> @@ -1399,6 +1403,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>> >>
>> >> pm_runtime_get_sync(dp->dev);
>> >>
>> >> + mutex_lock(&dp->lock);
>> >> zynqmp_dp_disp_enable(dp, old_bridge_state);
>> >>
>> >> /*
>> >> @@ -1459,6 +1464,7 @@ static void zynqmp_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>> >> zynqmp_dp_write(dp, ZYNQMP_DP_SOFTWARE_RESET,
>> >> ZYNQMP_DP_SOFTWARE_RESET_ALL);
>> >> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 1);
>> >> + mutex_unlock(&dp->lock);
>> >> }
>> >>
>> >> static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
>> >> @@ -1466,6 +1472,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
>> >> {
>> >> struct zynqmp_dp *dp = bridge_to_dp(bridge);
>> >>
>> >> + mutex_lock(&dp->lock);
>> >> dp->enabled = false;
>> >> cancel_delayed_work(&dp->hpd_work);
>> >> zynqmp_dp_write(dp, ZYNQMP_DP_MAIN_STREAM_ENABLE, 0);
>> >> @@ -1476,6 +1483,7 @@ static void zynqmp_dp_bridge_atomic_disable(struct drm_bridge *bridge,
>> >> zynqmp_dp_write(dp, ZYNQMP_DP_TX_AUDIO_CONTROL, 0);
>> >>
>> >> zynqmp_dp_disp_disable(dp, old_bridge_state);
>> >> + mutex_unlock(&dp->lock);
>> >>
>> >> pm_runtime_put_sync(dp->dev);
>> >> }
>> >> @@ -1518,6 +1526,8 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
>> >> u32 state, i;
>> >> int ret;
>> >>
>> >> + mutex_lock(&dp->lock);
>> >> +
>> >> /*
>> >> * This is from heuristic. It takes some delay (ex, 100 ~ 500 msec) to
>> >> * get the HPD signal with some monitors.
>> >> @@ -1545,11 +1555,13 @@ static enum drm_connector_status zynqmp_dp_bridge_detect(struct drm_bridge *brid
>> >> dp->num_lanes);
>> >>
>> >> dp->status = connector_status_connected;
>> >> + mutex_unlock(&dp->lock);
>> >> return connector_status_connected;
>> >> }
>> >>
>> >> disconnected:
>> >> dp->status = connector_status_disconnected;
>> >> + mutex_unlock(&dp->lock);
>> >> return connector_status_disconnected;
>> >> }
>> >>
>> >> @@ -1611,6 +1623,29 @@ static void zynqmp_dp_hpd_work_func(struct work_struct *work)
>> >> drm_bridge_hpd_notify(&dp->bridge, status);
>> >> }
>> >>
>> >> +static void zynqmp_dp_hpd_irq_work_func(struct work_struct *work)
>> >> +{
>> >> + struct zynqmp_dp *dp = container_of(work, struct zynqmp_dp,
>> >> + hpd_irq_work.work);
>> >> + u8 status[DP_LINK_STATUS_SIZE + 2];
>> >> + int err;
>> >> +
>> >> + mutex_lock(&dp->lock);
>> >> + err = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
>> >> + DP_LINK_STATUS_SIZE + 2);
>> >> + if (err < 0) {
>> >> + dev_dbg_ratelimited(dp->dev,
>> >> + "could not read sink status: %d\n", err);
>> >> + } else {
>> >> + if (status[4] & DP_LINK_STATUS_UPDATED ||
>> >> + !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
>> >> + !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
>> >> + zynqmp_dp_train_loop(dp);
>> >> + }
>> >> + }
>> >> + mutex_unlock(&dp->lock);
>> >> +}
>> >> +
>> >> static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
>> >> {
>> >> struct zynqmp_dp *dp = (struct zynqmp_dp *)data;
>> >> @@ -1635,23 +1670,9 @@ static irqreturn_t zynqmp_dp_irq_handler(int irq, void *data)
>> >> if (status & ZYNQMP_DP_INT_HPD_EVENT)
>> >> schedule_delayed_work(&dp->hpd_work, 0);
>> >>
>> >> - if (status & ZYNQMP_DP_INT_HPD_IRQ) {
>> >> - int ret;
>> >> - u8 status[DP_LINK_STATUS_SIZE + 2];
>> >> + if (status & ZYNQMP_DP_INT_HPD_IRQ)
>> >> + schedule_delayed_work(&dp->hpd_irq_work, 0);
>> >>
>> >> - ret = drm_dp_dpcd_read(&dp->aux, DP_SINK_COUNT, status,
>> >> - DP_LINK_STATUS_SIZE + 2);
>> >> - if (ret < 0)
>> >> - goto handled;
>> >> -
>> >> - if (status[4] & DP_LINK_STATUS_UPDATED ||
>> >> - !drm_dp_clock_recovery_ok(&status[2], dp->mode.lane_cnt) ||
>> >> - !drm_dp_channel_eq_ok(&status[2], dp->mode.lane_cnt)) {
>> >> - zynqmp_dp_train_loop(dp);
>> >> - }
>> >> - }
>> >> -
>> >> -handled:
>> >> return IRQ_HANDLED;
>> >> }
>> >>
>> >> @@ -1674,8 +1695,10 @@ int zynqmp_dp_probe(struct zynqmp_dpsub *dpsub)
>> >> dp->dev = &pdev->dev;
>> >> dp->dpsub = dpsub;
>> >> dp->status = connector_status_disconnected;
>> >> + mutex_init(&dp->lock);
>> >>
>> >> INIT_DELAYED_WORK(&dp->hpd_work, zynqmp_dp_hpd_work_func);
>> >> + INIT_DELAYED_WORK(&dp->hpd_irq_work, zynqmp_dp_hpd_irq_work_func);
>> >>
>> >> /* Acquire all resources (IOMEM, IRQ and PHYs). */
>> >> res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "dp");
>> >> @@ -1775,6 +1798,7 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>> >> zynqmp_dp_write(dp, ZYNQMP_DP_INT_DS, ZYNQMP_DP_INT_ALL);
>> >> disable_irq(dp->irq);
>> >>
>> >> + cancel_delayed_work_sync(&dp->hpd_irq_work);
>> >> cancel_delayed_work_sync(&dp->hpd_work);
>> >>
>> >> zynqmp_dp_write(dp, ZYNQMP_DP_TRANSMITTER_ENABLE, 0);
>> >> @@ -1782,4 +1806,5 @@ void zynqmp_dp_remove(struct zynqmp_dpsub *dpsub)
>> >>
>> >> zynqmp_dp_phy_exit(dp);
>> >> zynqmp_dp_reset(dp, true);
>> >> + mutex_destroy(&dp->lock);
>> >> }
>
On 3/18/24 13:47, Laurent Pinchart wrote:
> Hi Sean,
>
> Thank you for the patch.
>
> On Fri, Mar 15, 2024 at 07:09:15PM -0400, Sean Anderson wrote:
>> When testing, it's convenient to be able to ignore DPCD errors if there
>> is test equipment which can't emulate a DPRX connected to the output.
>> Add some (currently-unused) options to ignore these errors and just
>> reconfigure our internal registers as we usually would.
>
> This seems to be a problem that is not limited to the ZynqMP DP.
> Wouldn't it be better to solve it in the DRM DP DPCD helpers instead ?
> You could expose a parameter on the AUX bus in debugfs to ignore errors,
> and cause the drm_dp_dpcd_write*() functions to return 0.
I think this is probably the easiest thing. I'll add this for v2.
I think something similar might be nice for HPD events (instead of always
ignoring them in test mode). This would make it easier to add DPRX-driven
testing in the future.
--Sean
>> Signed-off-by: Sean Anderson <[email protected]>
>> ---
>>
>> drivers/gpu/drm/xlnx/zynqmp_dp.c | 37 ++++++++++++++++++++------------
>> 1 file changed, 23 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/xlnx/zynqmp_dp.c b/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> index 24043847dab4..040f7b88ee51 100644
>> --- a/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> +++ b/drivers/gpu/drm/xlnx/zynqmp_dp.c
>> @@ -628,6 +628,7 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
>> * zynqmp_dp_update_vs_emph - Update the training values
>> * @dp: DisplayPort IP core structure
>> * @train_set: A set of training values
>> + * @ignore_dpcd: Ignore DPCD errors
>> *
>> * Update the training values based on the request from sink. The mapped values
>> * are predefined, and values(vs, pe, pc) are from the device manual.
>> @@ -635,15 +636,19 @@ static void zynqmp_dp_adjust_train(struct zynqmp_dp *dp,
>> * Return: 0 if vs and emph are updated successfully, or the error code returned
>> * by drm_dp_dpcd_write().
>> */
>> -static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set)
>> +static int zynqmp_dp_update_vs_emph(struct zynqmp_dp *dp, u8 *train_set,
>> + bool ignore_dpcd)
>> {
>> unsigned int i;
>> int ret;
>>
>> ret = drm_dp_dpcd_write(&dp->aux, DP_TRAINING_LANE0_SET, train_set,
>> dp->mode.lane_cnt);
>> - if (ret < 0)
>> - return ret;
>> + if (ret < 0) {
>> + if (!ignore_dpcd)
>> + return ret;
>> + dev_warn(dp->dev, "failed to update vs/emph\n");
>> + }
>>
>> for (i = 0; i < dp->mode.lane_cnt; i++) {
>> u32 reg = ZYNQMP_DP_SUB_TX_PHY_PRECURSOR_LANE_0 + i * 4;
>> @@ -692,7 +697,7 @@ static int zynqmp_dp_link_train_cr(struct zynqmp_dp *dp)
>> * So, This loop should exit before 512 iterations
>> */
>> for (max_tries = 0; max_tries < 512; max_tries++) {
>> - ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
>> + ret = zynqmp_dp_update_vs_emph(dp, dp->train_set, false);
>> if (ret)
>> return ret;
>>
>> @@ -757,7 +762,7 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
>> return ret;
>>
>> for (tries = 0; tries < DP_MAX_TRAINING_TRIES; tries++) {
>> - ret = zynqmp_dp_update_vs_emph(dp, dp->train_set);
>> + ret = zynqmp_dp_update_vs_emph(dp, dp->train_set, false);
>> if (ret)
>> return ret;
>>
>> @@ -785,11 +790,12 @@ static int zynqmp_dp_link_train_ce(struct zynqmp_dp *dp)
>> * @lane_cnt: The number of lanes to use
>> * @enhanced: Use enhanced framing
>> * @downspread: Enable spread-spectrum clocking
>> + * @ignore_dpcd: Ignore DPCD errors; useful for testing
>> *
>> * Return: 0 on success, or -errno on failure
>> */
>> static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
>> - bool enhanced, bool downspread)
>> + bool enhanced, bool downspread, bool ignore_dpcd)
>> {
>> u32 reg;
>> u8 aux_lane_cnt = lane_cnt;
>> @@ -812,21 +818,24 @@ static int zynqmp_dp_setup(struct zynqmp_dp *dp, u8 bw_code, u8 lane_cnt,
>>
>> ret = drm_dp_dpcd_writeb(&dp->aux, DP_LANE_COUNT_SET, aux_lane_cnt);
>> if (ret < 0) {
>> - dev_err(dp->dev, "failed to set lane count\n");
>> - return ret;
>> + dev_warn(dp->dev, "failed to set lane count\n");
>> + if (!ignore_dpcd)
>> + return ret;
>> }
>>
>> ret = drm_dp_dpcd_writeb(&dp->aux, DP_MAIN_LINK_CHANNEL_CODING_SET,
>> DP_SET_ANSI_8B10B);
>> if (ret < 0) {
>> - dev_err(dp->dev, "failed to set ANSI 8B/10B encoding\n");
>> - return ret;
>> + dev_warn(dp->dev, "failed to set ANSI 8B/10B encoding\n");
>> + if (!ignore_dpcd)
>> + return ret;
>> }
>>
>> ret = drm_dp_dpcd_writeb(&dp->aux, DP_LINK_BW_SET, bw_code);
>> if (ret < 0) {
>> - dev_err(dp->dev, "failed to set DP bandwidth\n");
>> - return ret;
>> + dev_warn(dp->dev, "failed to set DP bandwidth\n");
>> + if (!ignore_dpcd)
>> + return ret;
>> }
>>
>> zynqmp_dp_write(dp, ZYNQMP_DP_LINK_BW_SET, bw_code);
>> @@ -860,7 +869,7 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
>>
>> ret = zynqmp_dp_setup(dp, dp->mode.bw_code, dp->mode.lane_cnt,
>> drm_dp_enhanced_frame_cap(dp->dpcd),
>> - dp->dpcd[3] & 0x1);
>> + dp->dpcd[3] & 0x1, false);
>> if (ret)
>> return ret;
>>
>> @@ -877,7 +886,7 @@ static int zynqmp_dp_train(struct zynqmp_dp *dp)
>> ret = drm_dp_dpcd_writeb(&dp->aux, DP_TRAINING_PATTERN_SET,
>> DP_TRAINING_PATTERN_DISABLE);
>> if (ret < 0) {
>> - dev_err(dp->dev, "failed to disable training pattern\n");
>> + dev_warn(dp->dev, "failed to disable training pattern\n");
>> return ret;
>> }
>> zynqmp_dp_write(dp, ZYNQMP_DP_TRAINING_PATTERN_SET,
>
On 3/18/24 13:50, Laurent Pinchart wrote:
> On Mon, Mar 18, 2024 at 11:06:40AM -0400, Sean Anderson wrote:
>> On 3/16/24 06:14, kernel test robot wrote:
>> > Hi Sean,
>> >
>> > kernel test robot noticed the following build warnings:
>> >
>> > [auto build test WARNING on v6.8]
>> > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
>> > [If your patch is applied to the wrong git tree, kindly drop us a note.
>> > And when submitting patch, we suggest to use '--base' as documented in
>> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
>> >
>> > url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
>> > base: v6.8
>> > patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
>> > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
>> > config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
>> > compiler: microblaze-linux-gcc (GCC) 13.2.0
>> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
>> > | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
>> >
>> > All warnings (new ones prefixed by >>):
>> >
>> > drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init':
>> >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
>> > 2168 | sprintf(name, fmt, i);
>> > | ^~~
>> > drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19
>> > 2168 | sprintf(name, fmt, i);
>> > | ^~~~~~~~~~~~~~~~~~~~~
>>
>> Not a bug, as i will be at most 4, which uses 1 digit.
>
> The compiler can't know that. Please fix this, there's a zero warning
> policy.
I cannot reproduce this with GCC 13.2.0. So given that this is not a bug and I can't reproduce
it, I don't see how I can verify any fix.
--Sean
>> > vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c
>> >
>> > 2136
>> > 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
>> > 2138 zynqmp_dp_rate_set, "%llu\n");
>> > 2139
>> > 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
>> > 2141 struct dentry *root)
>> > 2142 {
>> > 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge);
>> > 2144 struct dentry *test;
>> > 2145 int i;
>> > 2146
>> > 2147 dp->test.bw_code = DP_LINK_BW_5_4;
>> > 2148 dp->test.link_cnt = dp->num_lanes;
>> > 2149
>> > 2150 test = debugfs_create_dir("test", root);
>> > 2151 #define CREATE_FILE(name) \
>> > 2152 debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
>> > 2153 CREATE_FILE(pattern);
>> > 2154 CREATE_FILE(enhanced);
>> > 2155 CREATE_FILE(downspread);
>> > 2156 CREATE_FILE(active);
>> > 2157 CREATE_FILE(custom);
>> > 2158 CREATE_FILE(rate);
>> > 2159 CREATE_FILE(lanes);
>> > 2160
>> > 2161 for (i = 0; i < dp->num_lanes; i++) {
>> > 2162 static const char fmt[] = "lane%d_preemphasis";
>> > 2163 char name[sizeof(fmt)];
>> > 2164
>> > 2165 dp->debugfs_train_set[i].dp = dp;
>> > 2166 dp->debugfs_train_set[i].lane = i;
>> > 2167
>> >> 2168 sprintf(name, fmt, i);
>> > 2169 debugfs_create_file(name, 0600, test,
>> > 2170 &dp->debugfs_train_set[i],
>> > 2171 &fops_zynqmp_dp_preemphasis);
>> > 2172
>> > 2173 sprintf(name, "lane%d_swing", i);
>> > 2174 debugfs_create_file(name, 0600, test,
>> > 2175 &dp->debugfs_train_set[i],
>> > 2176 &fops_zynqmp_dp_swing);
>> > 2177 }
>> > 2178 }
>> > 2179
>
Hi Sean,
On Mon, Mar 18, 2024 at 03:05:57PM -0400, Sean Anderson wrote:
> On 3/18/24 13:50, Laurent Pinchart wrote:
> > On Mon, Mar 18, 2024 at 11:06:40AM -0400, Sean Anderson wrote:
> >> On 3/16/24 06:14, kernel test robot wrote:
> >> > Hi Sean,
> >> >
> >> > kernel test robot noticed the following build warnings:
> >> >
> >> > [auto build test WARNING on v6.8]
> >> > [cannot apply to drm-misc/drm-misc-next linus/master next-20240315]
> >> > [If your patch is applied to the wrong git tree, kindly drop us a note.
> >> > And when submitting patch, we suggest to use '--base' as documented in
> >> > https://git-scm.com/docs/git-format-patch#_base_tree_information]
> >> >
> >> > url: https://github.com/intel-lab-lkp/linux/commits/Sean-Anderson/drm-zynqmp_dp-Downgrade-log-level-for-aux-retries-message/20240316-071208
> >> > base: v6.8
> >> > patch link: https://lore.kernel.org/r/20240315230916.1759060-7-sean.anderson%40linux.dev
> >> > patch subject: [PATCH 6/6] drm: zynqmp_dp: Add debugfs interface for compliance testing
> >> > config: microblaze-allmodconfig (https://download.01.org/0day-ci/archive/20240316/[email protected]/config)
> >> > compiler: microblaze-linux-gcc (GCC) 13.2.0
> >> > reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240316/[email protected]/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 <[email protected]>
> >> > | Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
> >> >
> >> > All warnings (new ones prefixed by >>):
> >> >
> >> > drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init':
> >> >>> drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
> >> > 2168 | sprintf(name, fmt, i);
> >> > | ^~~
> >> > drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19
> >> > 2168 | sprintf(name, fmt, i);
> >> > | ^~~~~~~~~~~~~~~~~~~~~
> >>
> >> Not a bug, as i will be at most 4, which uses 1 digit.
> >
> > The compiler can't know that. Please fix this, there's a zero warning
> > policy.
>
> I cannot reproduce this with GCC 13.2.0. So given that this is not a bug and I can't reproduce
> it, I don't see how I can verify any fix.
There is a "reproduce" link in the bot's report. We followed the steps
in that link and found that the warning could be reproduced. Please
notice that this is a "W=1" warning. BTW, we also tested the v2 patch
and the warning has been fixed there. Just for your information.
$ cd linux
$ git checkout v6.8
HEAD is now at e8f897f4afef0 Linux 6.8
$ b4 am https://lore.kernel.org/r/[email protected]
$ git am ./20240315_sean_anderson_drm_zynqmp_dp_misc_patches_and_debugfs_support.mbx
Applying: drm: zynqmp_dp: Downgrade log level for aux retries message
Applying: drm: zynqmp_dp: Adjust training values per-lane
Applying: drm: zynqmp_dp: Add locking
Applying: drm: zynqmp_dp: Split off several helper functions
Applying: drm: zynqmp_dp: Optionally ignore DPCD errors
Applying: drm: zynqmp_dp: Add debugfs interface for compliance testing
$ wget https://download.01.org/0day-ci/archive/20240316/[email protected]/config
$ mkdir build_dir
$ cp config build_dir/.config
$ git clone https://github.com/intel/lkp-tests.git ~/lkp-tests
$ COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-13.2.0 ~/lkp-tests/kbuild/make.cross W=1 O=build_dir ARCH=microblaze olddefconfig
$ COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-13.2.0 ~/lkp-tests/kbuild/make.cross W=1 O=build_dir ARCH=microblaze drivers/gpu/drm/xlnx/zynqmp_dp.o
..
CC [M] drivers/gpu/drm/xlnx/zynqmp_dp.o
./drivers/gpu/drm/xlnx/zynqmp_dp.c: In function 'zynqmp_dp_bridge_debugfs_init':
./drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:31: warning: 'sprintf' may write a terminating nul past the end of the destination [-Wformat-overflow=]
2168 | sprintf(name, fmt, i);
| ^~~
./drivers/gpu/drm/xlnx/zynqmp_dp.c:2168:17: note: 'sprintf' output between 18 and 20 bytes into a destination of size 19
2168 | sprintf(name, fmt, i);
| ^~~~~~~~~~~~~~~~~~~~~
Best Regards,
Yujie
>
> --Sean
>
> >> > vim +/sprintf +2168 drivers/gpu/drm/xlnx/zynqmp_dp.c
> >> >
> >> > 2136
> >> > 2137 DEFINE_DEBUGFS_ATTRIBUTE(fops_zynqmp_dp_rate, zynqmp_dp_rate_get,
> >> > 2138 zynqmp_dp_rate_set, "%llu\n");
> >> > 2139
> >> > 2140 static void zynqmp_dp_bridge_debugfs_init(struct drm_bridge *bridge,
> >> > 2141 struct dentry *root)
> >> > 2142 {
> >> > 2143 struct zynqmp_dp *dp = bridge_to_dp(bridge);
> >> > 2144 struct dentry *test;
> >> > 2145 int i;
> >> > 2146
> >> > 2147 dp->test.bw_code = DP_LINK_BW_5_4;
> >> > 2148 dp->test.link_cnt = dp->num_lanes;
> >> > 2149
> >> > 2150 test = debugfs_create_dir("test", root);
> >> > 2151 #define CREATE_FILE(name) \
> >> > 2152 debugfs_create_file(#name, 0600, test, dp, &fops_zynqmp_dp_##name)
> >> > 2153 CREATE_FILE(pattern);
> >> > 2154 CREATE_FILE(enhanced);
> >> > 2155 CREATE_FILE(downspread);
> >> > 2156 CREATE_FILE(active);
> >> > 2157 CREATE_FILE(custom);
> >> > 2158 CREATE_FILE(rate);
> >> > 2159 CREATE_FILE(lanes);
> >> > 2160
> >> > 2161 for (i = 0; i < dp->num_lanes; i++) {
> >> > 2162 static const char fmt[] = "lane%d_preemphasis";
> >> > 2163 char name[sizeof(fmt)];
> >> > 2164
> >> > 2165 dp->debugfs_train_set[i].dp = dp;
> >> > 2166 dp->debugfs_train_set[i].lane = i;
> >> > 2167
> >> >> 2168 sprintf(name, fmt, i);
> >> > 2169 debugfs_create_file(name, 0600, test,
> >> > 2170 &dp->debugfs_train_set[i],
> >> > 2171 &fops_zynqmp_dp_preemphasis);
> >> > 2172
> >> > 2173 sprintf(name, "lane%d_swing", i);
> >> > 2174 debugfs_create_file(name, 0600, test,
> >> > 2175 &dp->debugfs_train_set[i],
> >> > 2176 &fops_zynqmp_dp_swing);
> >> > 2177 }
> >> > 2178 }
> >> > 2179
> >
>
>