2021-05-19 18:06:10

by Sam McNally

[permalink] [raw]
Subject: [PATCH v4 1/3] drm/dp_mst: Add self-tests for up requests

Up requests are decoded by drm_dp_sideband_parse_req(), which operates
on a drm_dp_sideband_msg_rx, unlike down requests. Expand the existing
self-test helper sideband_msg_req_encode_decode() to copy the message
contents and length from a drm_dp_sideband_msg_tx to
drm_dp_sideband_msg_rx and use the parse function under test in place of
decode.

Add support for currently-supported up requests to
drm_dp_dump_sideband_msg_req_body(); add support to
drm_dp_encode_sideband_req() to allow encoding for the self-tests.

Add self-tests for CONNECTION_STATUS_NOTIFY and RESOURCE_STATUS_NOTIFY.

Signed-off-by: Sam McNally <[email protected]>
---

Changes in v4:
- New in v4

drivers/gpu/drm/drm_dp_mst_topology.c | 54 ++++++-
.../gpu/drm/drm_dp_mst_topology_internal.h | 4 +
.../drm/selftests/test-drm_dp_mst_helper.c | 147 ++++++++++++++++--
3 files changed, 190 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 54604633e65c..573f39a3dc16 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -442,6 +442,37 @@ drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req,
idx++;
}
break;
+ case DP_CONNECTION_STATUS_NOTIFY: {
+ const struct drm_dp_connection_status_notify *msg;
+
+ msg = &req->u.conn_stat;
+ buf[idx] = (msg->port_number & 0xf) << 4;
+ idx++;
+ memcpy(&raw->msg[idx], msg->guid, 16);
+ idx += 16;
+ raw->msg[idx] = 0;
+ raw->msg[idx] |= msg->legacy_device_plug_status ? BIT(6) : 0;
+ raw->msg[idx] |= msg->displayport_device_plug_status ? BIT(5) : 0;
+ raw->msg[idx] |= msg->message_capability_status ? BIT(4) : 0;
+ raw->msg[idx] |= msg->input_port ? BIT(3) : 0;
+ raw->msg[idx] |= FIELD_PREP(GENMASK(2, 0), msg->peer_device_type);
+ idx++;
+ break;
+ }
+ case DP_RESOURCE_STATUS_NOTIFY: {
+ const struct drm_dp_resource_status_notify *msg;
+
+ msg = &req->u.resource_stat;
+ buf[idx] = (msg->port_number & 0xf) << 4;
+ idx++;
+ memcpy(&raw->msg[idx], msg->guid, 16);
+ idx += 16;
+ buf[idx] = (msg->available_pbn & 0xff00) >> 8;
+ idx++;
+ buf[idx] = (msg->available_pbn & 0xff);
+ idx++;
+ break;
+ }
}
raw->cur_len = idx;
}
@@ -672,6 +703,22 @@ drm_dp_dump_sideband_msg_req_body(const struct drm_dp_sideband_msg_req_body *req
req->u.enc_status.stream_behavior,
req->u.enc_status.valid_stream_behavior);
break;
+ case DP_CONNECTION_STATUS_NOTIFY:
+ P("port=%d guid=%*ph legacy=%d displayport=%d messaging=%d input=%d peer_type=%d",
+ req->u.conn_stat.port_number,
+ (int)ARRAY_SIZE(req->u.conn_stat.guid), req->u.conn_stat.guid,
+ req->u.conn_stat.legacy_device_plug_status,
+ req->u.conn_stat.displayport_device_plug_status,
+ req->u.conn_stat.message_capability_status,
+ req->u.conn_stat.input_port,
+ req->u.conn_stat.peer_device_type);
+ break;
+ case DP_RESOURCE_STATUS_NOTIFY:
+ P("port=%d guid=%*ph pbn=%d",
+ req->u.resource_stat.port_number,
+ (int)ARRAY_SIZE(req->u.resource_stat.guid), req->u.resource_stat.guid,
+ req->u.resource_stat.available_pbn);
+ break;
default:
P("???\n");
break;
@@ -1116,9 +1163,9 @@ static bool drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
return false;
}

-static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
- struct drm_dp_sideband_msg_rx *raw,
- struct drm_dp_sideband_msg_req_body *msg)
+bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_sideband_msg_rx *raw,
+ struct drm_dp_sideband_msg_req_body *msg)
{
memset(msg, 0, sizeof(*msg));
msg->req_type = (raw->msg[0] & 0x7f);
@@ -1134,6 +1181,7 @@ static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
return false;
}
}
+EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_dp_sideband_parse_req);

static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg,
u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology_internal.h b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
index eeda9a61c657..0356a2e0dba1 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
+++ b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
@@ -21,4 +21,8 @@ void
drm_dp_dump_sideband_msg_req_body(const struct drm_dp_sideband_msg_req_body *req,
int indent, struct drm_printer *printer);

+bool
+drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_sideband_msg_rx *raw,
+ struct drm_dp_sideband_msg_req_body *msg);
#endif /* !_DRM_DP_MST_HELPER_INTERNAL_H_ */
diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
index 6b4759ed6bfd..22aaedc63aec 100644
--- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -13,6 +13,10 @@
#include "../drm_dp_mst_topology_internal.h"
#include "test-drm_modeset_common.h"

+static void mock_release(struct device *dev)
+{
+}
+
int igt_dp_mst_calc_pbn_mode(void *ignored)
{
int pbn, i;
@@ -120,27 +124,59 @@ sideband_msg_req_equal(const struct drm_dp_sideband_msg_req_body *in,
static bool
sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
{
- struct drm_dp_sideband_msg_req_body *out;
+ struct drm_dp_sideband_msg_req_body *out = NULL;
struct drm_printer p = drm_err_printer(PREFIX_STR);
- struct drm_dp_sideband_msg_tx *txmsg;
+ struct drm_dp_sideband_msg_tx *txmsg = NULL;
+ struct drm_dp_sideband_msg_rx *rxmsg = NULL;
+ struct drm_dp_mst_topology_mgr *mgr = NULL;
int i, ret;
- bool result = true;
+ bool result = false;

out = kzalloc(sizeof(*out), GFP_KERNEL);
if (!out)
- return false;
+ goto out;

txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
if (!txmsg)
- return false;
+ goto out;

- drm_dp_encode_sideband_req(in, txmsg);
- ret = drm_dp_decode_sideband_req(txmsg, out);
- if (ret < 0) {
- drm_printf(&p, "Failed to decode sideband request: %d\n",
- ret);
- result = false;
+ rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
+ if (!rxmsg)
goto out;
+
+ mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
+ if (!mgr)
+ goto out;
+
+ mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
+ if (!mgr->dev)
+ goto out;
+
+ mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
+ if (!mgr->dev->dev)
+ goto out;
+
+ mgr->dev->dev->release = mock_release;
+ device_initialize(mgr->dev->dev);
+
+ drm_dp_encode_sideband_req(in, txmsg);
+ switch (in->req_type) {
+ case DP_CONNECTION_STATUS_NOTIFY:
+ case DP_RESOURCE_STATUS_NOTIFY:
+ memcpy(&rxmsg->msg, txmsg->msg, ARRAY_SIZE(rxmsg->msg));
+ rxmsg->curlen = txmsg->cur_len;
+ if (!drm_dp_sideband_parse_req(mgr, rxmsg, out)) {
+ drm_printf(&p, "Failed to decode sideband request\n");
+ goto out;
+ }
+ break;
+ default:
+ ret = drm_dp_decode_sideband_req(txmsg, out);
+ if (ret < 0) {
+ drm_printf(&p, "Failed to decode sideband request: %d\n", ret);
+ goto out;
+ }
+ break;
}

if (!sideband_msg_req_equal(in, out)) {
@@ -148,9 +184,9 @@ sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
drm_dp_dump_sideband_msg_req_body(in, 1, &p);
drm_printf(&p, "Got:\n");
drm_dp_dump_sideband_msg_req_body(out, 1, &p);
- result = false;
goto out;
}
+ result = true;

switch (in->req_type) {
case DP_REMOTE_DPCD_WRITE:
@@ -171,6 +207,65 @@ sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
out:
kfree(out);
kfree(txmsg);
+ kfree(rxmsg);
+ if (mgr) {
+ if (mgr->dev) {
+ put_device(mgr->dev->dev);
+ kfree(mgr->dev);
+ }
+ kfree(mgr);
+ }
+ return result;
+}
+
+static bool
+sideband_msg_req_parse(int req_type)
+{
+ struct drm_dp_sideband_msg_req_body *out = NULL;
+ struct drm_printer p = drm_err_printer(PREFIX_STR);
+ struct drm_dp_sideband_msg_rx *rxmsg = NULL;
+ struct drm_dp_mst_topology_mgr *mgr = NULL;
+ bool result = false;
+
+ out = kzalloc(sizeof(*out), GFP_KERNEL);
+ if (!out)
+ goto out;
+
+ rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
+ if (!rxmsg)
+ goto out;
+
+ mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
+ if (!mgr)
+ goto out;
+
+ mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
+ if (!mgr->dev)
+ goto out;
+
+ mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
+ if (!mgr->dev->dev)
+ goto out;
+
+ mgr->dev->dev->release = mock_release;
+ device_initialize(mgr->dev->dev);
+
+ rxmsg->curlen = 1;
+ rxmsg->msg[0] = req_type & 0x7f;
+ if (drm_dp_sideband_parse_req(mgr, rxmsg, out))
+ drm_printf(&p, "Unexpectedly decoded invalid sideband request\n");
+ else
+ result = true;
+out:
+ kfree(out);
+ kfree(rxmsg);
+ if (mgr) {
+ if (mgr->dev) {
+ put_device(mgr->dev->dev);
+ kfree(mgr->dev);
+ }
+ kfree(mgr);
+ }
return result;
}

@@ -268,6 +363,34 @@ int igt_dp_mst_sideband_msg_req_decode(void *unused)
in.u.enc_status.valid_stream_behavior = 1;
DO_TEST();

+ in.req_type = DP_CONNECTION_STATUS_NOTIFY;
+ in.u.conn_stat.port_number = 0xf;
+ get_random_bytes(in.u.conn_stat.guid, sizeof(in.u.conn_stat.guid));
+ in.u.conn_stat.legacy_device_plug_status = 1;
+ in.u.conn_stat.displayport_device_plug_status = 0;
+ in.u.conn_stat.message_capability_status = 0;
+ in.u.conn_stat.input_port = 0;
+ in.u.conn_stat.peer_device_type = 7;
+ DO_TEST();
+ in.u.conn_stat.displayport_device_plug_status = 1;
+ DO_TEST();
+ in.u.conn_stat.message_capability_status = 1;
+ DO_TEST();
+ in.u.conn_stat.input_port = 1;
+ DO_TEST();
+
+ in.req_type = DP_RESOURCE_STATUS_NOTIFY;
+ in.u.resource_stat.port_number = 0xf;
+ get_random_bytes(in.u.resource_stat.guid, sizeof(in.u.resource_stat.guid));
+ in.u.resource_stat.available_pbn = 0xcdef;
+ DO_TEST();
+
+#undef DO_TEST
+#define DO_TEST(req_type) FAIL_ON(!sideband_msg_req_parse(req_type))
+ DO_TEST(DP_CONNECTION_STATUS_NOTIFY);
+ DO_TEST(DP_RESOURCE_STATUS_NOTIFY);
+
+ DO_TEST(DP_REMOTE_I2C_WRITE);
#undef DO_TEST
return 0;
}
--
2.31.1.751.gd2f1c929bd-goog



2021-05-19 18:07:41

by Sam McNally

[permalink] [raw]
Subject: [PATCH v4 2/3] drm/dp_mst: Add support for sink event notify messages

Sink event notify messages are used for MST CEC IRQs. Add parsing
support for sink event notify messages in preparation for handling MST
CEC IRQs.

Signed-off-by: Sam McNally <[email protected]>
---

Changes in v4:
- Changed logging to use drm_dbg_kms()
- Added self-test

drivers/gpu/drm/drm_dp_mst_topology.c | 57 ++++++++++++++++++-
.../drm/selftests/test-drm_dp_mst_helper.c | 8 +++
include/drm/drm_dp_mst_helper.h | 14 +++++
3 files changed, 78 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 573f39a3dc16..29aad3b6b31a 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -473,6 +473,20 @@ drm_dp_encode_sideband_req(const struct drm_dp_sideband_msg_req_body *req,
idx++;
break;
}
+ case DP_SINK_EVENT_NOTIFY: {
+ const struct drm_dp_sink_event_notify *msg;
+
+ msg = &req->u.sink_event;
+ buf[idx] = (msg->port_number & 0xf) << 4;
+ idx++;
+ memcpy(&raw->msg[idx], msg->guid, 16);
+ idx += 16;
+ buf[idx] = (msg->event_id & 0xff00) >> 8;
+ idx++;
+ buf[idx] = (msg->event_id & 0xff);
+ idx++;
+ break;
+ }
}
raw->cur_len = idx;
}
@@ -719,6 +733,12 @@ drm_dp_dump_sideband_msg_req_body(const struct drm_dp_sideband_msg_req_body *req
(int)ARRAY_SIZE(req->u.resource_stat.guid), req->u.resource_stat.guid,
req->u.resource_stat.available_pbn);
break;
+ case DP_SINK_EVENT_NOTIFY:
+ P("port=%d guid=%*ph event=%d",
+ req->u.sink_event.port_number,
+ (int)ARRAY_SIZE(req->u.sink_event.guid), req->u.sink_event.guid,
+ req->u.sink_event.event_id);
+ break;
default:
P("???\n");
break;
@@ -1163,6 +1183,30 @@ static bool drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
return false;
}

+static bool drm_dp_sideband_parse_sink_event_notify(const struct drm_dp_mst_topology_mgr *mgr,
+ struct drm_dp_sideband_msg_rx *raw,
+ struct drm_dp_sideband_msg_req_body *msg)
+{
+ int idx = 1;
+
+ msg->u.sink_event.port_number = (raw->msg[idx] & 0xf0) >> 4;
+ idx++;
+ if (idx > raw->curlen)
+ goto fail_len;
+
+ memcpy(msg->u.sink_event.guid, &raw->msg[idx], 16);
+ idx += 16;
+ if (idx > raw->curlen)
+ goto fail_len;
+
+ msg->u.sink_event.event_id = (raw->msg[idx] << 8) | (raw->msg[idx + 1]);
+ idx++;
+ return true;
+fail_len:
+ drm_dbg_kms(mgr->dev, "sink event notify parse length fail %d %d\n", idx, raw->curlen);
+ return false;
+}
+
bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
struct drm_dp_sideband_msg_rx *raw,
struct drm_dp_sideband_msg_req_body *msg)
@@ -1175,6 +1219,8 @@ bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
return drm_dp_sideband_parse_connection_status_notify(mgr, raw, msg);
case DP_RESOURCE_STATUS_NOTIFY:
return drm_dp_sideband_parse_resource_status_notify(mgr, raw, msg);
+ case DP_SINK_EVENT_NOTIFY:
+ return drm_dp_sideband_parse_sink_event_notify(mgr, raw, msg);
default:
drm_err(mgr->dev, "Got unknown request 0x%02x (%s)\n",
msg->req_type, drm_dp_mst_req_type_str(msg->req_type));
@@ -4106,6 +4152,8 @@ drm_dp_mst_process_up_req(struct drm_dp_mst_topology_mgr *mgr,
guid = msg->u.conn_stat.guid;
else if (msg->req_type == DP_RESOURCE_STATUS_NOTIFY)
guid = msg->u.resource_stat.guid;
+ else if (msg->req_type == DP_SINK_EVENT_NOTIFY)
+ guid = msg->u.sink_event.guid;

if (guid)
mstb = drm_dp_get_mst_branch_device_by_guid(mgr, guid);
@@ -4177,7 +4225,8 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
drm_dp_sideband_parse_req(mgr, &mgr->up_req_recv, &up_req->msg);

if (up_req->msg.req_type != DP_CONNECTION_STATUS_NOTIFY &&
- up_req->msg.req_type != DP_RESOURCE_STATUS_NOTIFY) {
+ up_req->msg.req_type != DP_RESOURCE_STATUS_NOTIFY &&
+ up_req->msg.req_type != DP_SINK_EVENT_NOTIFY) {
drm_dbg_kms(mgr->dev, "Received unknown up req type, ignoring: %x\n",
up_req->msg.req_type);
kfree(up_req);
@@ -4205,6 +4254,12 @@ static int drm_dp_mst_handle_up_req(struct drm_dp_mst_topology_mgr *mgr)
drm_dbg_kms(mgr->dev, "Got RSN: pn: %d avail_pbn %d\n",
res_stat->port_number,
res_stat->available_pbn);
+ } else if (up_req->msg.req_type == DP_SINK_EVENT_NOTIFY) {
+ const struct drm_dp_sink_event_notify *sink_event =
+ &up_req->msg.u.sink_event;
+
+ drm_dbg_kms(mgr->dev, "Got SEN: pn: %d event_id %d\n",
+ sink_event->port_number, sink_event->event_id);
}

up_req->hdr = mgr->up_req_recv.initial_hdr;
diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
index 22aaedc63aec..84b1411460d7 100644
--- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -163,6 +163,7 @@ sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
switch (in->req_type) {
case DP_CONNECTION_STATUS_NOTIFY:
case DP_RESOURCE_STATUS_NOTIFY:
+ case DP_SINK_EVENT_NOTIFY:
memcpy(&rxmsg->msg, txmsg->msg, ARRAY_SIZE(rxmsg->msg));
rxmsg->curlen = txmsg->cur_len;
if (!drm_dp_sideband_parse_req(mgr, rxmsg, out)) {
@@ -385,10 +386,17 @@ int igt_dp_mst_sideband_msg_req_decode(void *unused)
in.u.resource_stat.available_pbn = 0xcdef;
DO_TEST();

+ in.req_type = DP_SINK_EVENT_NOTIFY;
+ in.u.sink_event.port_number = 0xf;
+ get_random_bytes(in.u.sink_event.guid, sizeof(in.u.sink_event.guid));
+ in.u.sink_event.event_id = 0xcdef;
+ DO_TEST();
+
#undef DO_TEST
#define DO_TEST(req_type) FAIL_ON(!sideband_msg_req_parse(req_type))
DO_TEST(DP_CONNECTION_STATUS_NOTIFY);
DO_TEST(DP_RESOURCE_STATUS_NOTIFY);
+ DO_TEST(DP_SINK_EVENT_NOTIFY);

DO_TEST(DP_REMOTE_I2C_WRITE);
#undef DO_TEST
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index c87a829b6498..96c87f761129 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -439,6 +439,19 @@ struct drm_dp_resource_status_notify {
u16 available_pbn;
};

+#define DP_SINK_EVENT_PANEL_REPLAY_ACTIVE_FRAME_CRC_ERROR BIT(0)
+#define DP_SINK_EVENT_PANEL_REPLAY_RFB_STORAGE_ERROR BIT(1)
+#define DP_SINK_EVENT_DSC_RC_BUFFER_UNDER_RUN BIT(2)
+#define DP_SINK_EVENT_DSC_RC_BUFFER_OVERFLOW BIT(3)
+#define DP_SINK_EVENT_DSC_CHUNK_LENGTH_ERROR BIT(4)
+#define DP_SINK_EVENT_CEC_IRQ_EVENT BIT(5)
+
+struct drm_dp_sink_event_notify {
+ u8 port_number;
+ u8 guid[16];
+ u16 event_id;
+};
+
struct drm_dp_query_payload_ack_reply {
u8 port_number;
u16 allocated_pbn;
@@ -450,6 +463,7 @@ struct drm_dp_sideband_msg_req_body {
struct drm_dp_connection_status_notify conn_stat;
struct drm_dp_port_number_req port_num;
struct drm_dp_resource_status_notify resource_stat;
+ struct drm_dp_sink_event_notify sink_event;

struct drm_dp_query_payload query_payload;
struct drm_dp_allocate_payload allocate_payload;
--
2.31.1.751.gd2f1c929bd-goog


2021-05-19 18:08:50

by Sam McNally

[permalink] [raw]
Subject: [PATCH v4 3/3] drm_dp_cec: add MST support

With DP v2.0 errata E5, CEC tunneling can be supported through an MST
topology.

When tunneling CEC through an MST port, CEC IRQs are delivered via a
sink event notify message; when a sink event notify message is received,
trigger CEC IRQ handling - ESI1 is not used for remote CEC IRQs so its
value is not checked.

Register and unregister for all MST connectors, ensuring their
drm_dp_aux_cec struct won't be accessed uninitialized.

Reviewed-by: Hans Verkuil <[email protected]>
Signed-off-by: Sam McNally <[email protected]>
---

Changes in v4:
- Removed use of work queues
- Updated checks of aux.transfer to accept aux.is_remote

Changes in v3:
- Fixed whitespace in drm_dp_cec_mst_irq_work()
- Moved drm_dp_cec_mst_set_edid_work() with the other set_edid functions

Changes in v2:
- Used aux->is_remote instead of aux->cec.is_mst, removing the need for
the previous patch in the series
- Added a defensive check for null edid in the deferred set_edid work,
in case the edid is no longer valid at that point

drivers/gpu/drm/drm_dp_cec.c | 20 ++++++++++++++++----
drivers/gpu/drm/drm_dp_mst_topology.c | 24 ++++++++++++++++++++++++
2 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
index 3ab2609f9ec7..1abd3f4654dc 100644
--- a/drivers/gpu/drm/drm_dp_cec.c
+++ b/drivers/gpu/drm/drm_dp_cec.c
@@ -14,6 +14,7 @@
#include <drm/drm_connector.h>
#include <drm/drm_device.h>
#include <drm/drm_dp_helper.h>
+#include <drm/drm_dp_mst_helper.h>

/*
* Unfortunately it turns out that we have a chicken-and-egg situation
@@ -245,13 +246,22 @@ void drm_dp_cec_irq(struct drm_dp_aux *aux)
int ret;

/* No transfer function was set, so not a DP connector */
- if (!aux->transfer)
+ if (!aux->transfer && !aux->is_remote)
return;

mutex_lock(&aux->cec.lock);
if (!aux->cec.adap)
goto unlock;

+ if (aux->is_remote) {
+ /*
+ * For remote connectors, CEC IRQ is triggered by an explicit
+ * message so ESI1 is not involved.
+ */
+ drm_dp_cec_handle_irq(aux);
+ goto unlock;
+ }
+
ret = drm_dp_dpcd_readb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
&cec_irq);
if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
@@ -307,7 +317,7 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
u8 cap;

/* No transfer function was set, so not a DP connector */
- if (!aux->transfer)
+ if (!aux->transfer && !aux->is_remote)
return;

#ifndef CONFIG_MEDIA_CEC_RC
@@ -375,6 +385,7 @@ void drm_dp_cec_set_edid(struct drm_dp_aux *aux, const struct edid *edid)
unlock:
mutex_unlock(&aux->cec.lock);
}
+
EXPORT_SYMBOL(drm_dp_cec_set_edid);

/*
@@ -383,7 +394,7 @@ EXPORT_SYMBOL(drm_dp_cec_set_edid);
void drm_dp_cec_unset_edid(struct drm_dp_aux *aux)
{
/* No transfer function was set, so not a DP connector */
- if (!aux->transfer)
+ if (!aux->transfer && !aux->is_remote)
return;

cancel_delayed_work_sync(&aux->cec.unregister_work);
@@ -393,6 +404,7 @@ void drm_dp_cec_unset_edid(struct drm_dp_aux *aux)
goto unlock;

cec_phys_addr_invalidate(aux->cec.adap);
+
/*
* We're done if we want to keep the CEC device
* (drm_dp_cec_unregister_delay is >= NEVER_UNREG_DELAY) or if the
@@ -428,7 +440,7 @@ void drm_dp_cec_register_connector(struct drm_dp_aux *aux,
struct drm_connector *connector)
{
WARN_ON(aux->cec.adap);
- if (WARN_ON(!aux->transfer))
+ if (WARN_ON(!aux->transfer && !aux->is_remote))
return;
aux->cec.connector = connector;
INIT_DELAYED_WORK(&aux->cec.unregister_work,
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index 29aad3b6b31a..5612caf9fb49 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -2359,6 +2359,8 @@ static void build_mst_prop_path(const struct drm_dp_mst_branch *mstb,
int drm_dp_mst_connector_late_register(struct drm_connector *connector,
struct drm_dp_mst_port *port)
{
+ drm_dp_cec_register_connector(&port->aux, connector);
+
drm_dbg_kms(port->mgr->dev, "registering %s remote bus for %s\n",
port->aux.name, connector->kdev->kobj.name);

@@ -2382,6 +2384,8 @@ void drm_dp_mst_connector_early_unregister(struct drm_connector *connector,
drm_dbg_kms(port->mgr->dev, "unregistering %s remote bus for %s\n",
port->aux.name, connector->kdev->kobj.name);
drm_dp_aux_unregister_devnode(&port->aux);
+
+ drm_dp_cec_unregister_connector(&port->aux);
}
EXPORT_SYMBOL(drm_dp_mst_connector_early_unregister);

@@ -2682,6 +2686,21 @@ drm_dp_mst_handle_conn_stat(struct drm_dp_mst_branch *mstb,
queue_work(system_long_wq, &mstb->mgr->work);
}

+static void
+drm_dp_mst_handle_sink_event(struct drm_dp_mst_branch *mstb,
+ struct drm_dp_sink_event_notify *sink_event)
+{
+ struct drm_dp_mst_port *port;
+
+ if (sink_event->event_id & DP_SINK_EVENT_CEC_IRQ_EVENT) {
+ port = drm_dp_get_port(mstb, sink_event->port_number);
+ if (port) {
+ drm_dp_cec_irq(&port->aux);
+ drm_dp_mst_topology_put_port(port);
+ }
+ }
+}
+
static struct drm_dp_mst_branch *drm_dp_get_mst_branch_device(struct drm_dp_mst_topology_mgr *mgr,
u8 lct, u8 *rad)
{
@@ -4170,6 +4189,8 @@ drm_dp_mst_process_up_req(struct drm_dp_mst_topology_mgr *mgr,
if (msg->req_type == DP_CONNECTION_STATUS_NOTIFY) {
drm_dp_mst_handle_conn_stat(mstb, &msg->u.conn_stat);
hotplug = true;
+ } else if (msg->req_type == DP_SINK_EVENT_NOTIFY) {
+ drm_dp_mst_handle_sink_event(mstb, &msg->u.sink_event);
}

drm_dp_mst_topology_put_mstb(mstb);
@@ -4362,6 +4383,8 @@ drm_dp_mst_detect_port(struct drm_connector *connector,
break;
}
out:
+ if (ret != connector_status_connected)
+ drm_dp_cec_unset_edid(&port->aux);
drm_dp_mst_topology_put_port(port);
return ret;
}
@@ -4392,6 +4415,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_
edid = drm_get_edid(connector, &port->aux.ddc);
}
port->has_audio = drm_detect_monitor_audio(edid);
+ drm_dp_cec_set_edid(&port->aux, edid);
drm_dp_mst_topology_put_port(port);
return edid;
}
--
2.31.1.751.gd2f1c929bd-goog


2021-05-19 18:42:21

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] drm/dp_mst: Add self-tests for up requests

Looks like these tests might still need to be fixed up a bit:

[ 34.785042] (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]] connection status reply parse length fail 2 1
[ 34.785082] (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]] resource status reply parse length fail 2 1
[ 34.785114] (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]] sink event notify parse length fail 2 1
[ 34.785146] (null): [drm] *ERROR* Got unknown request 0x23 (REMOTE_I2C_WRITE)


On Tue, 2021-05-18 at 22:35 +1000, Sam McNally wrote:
Up requests are decoded by drm_dp_sideband_parse_req(), which operates
on a drm_dp_sideband_msg_rx, unlike down requests. Expand the existing
self-test helper sideband_msg_req_encode_decode() to copy the message
contents and length from a drm_dp_sideband_msg_tx to
drm_dp_sideband_msg_rx and use the parse function under test in place of
decode.

Add support for currently-supported up requests to
drm_dp_dump_sideband_msg_req_body(); add support to
drm_dp_encode_sideband_req() to allow encoding for the self-tests.

Add self-tests for CONNECTION_STATUS_NOTIFY and RESOURCE_STATUS_NOTIFY.

Signed-off-by: Sam McNally <[email protected]>
---

Changes in v4:
- New in v4

 drivers/gpu/drm/drm_dp_mst_topology.c         |  54 ++++++-
 .../gpu/drm/drm_dp_mst_topology_internal.h    |   4 +
 .../drm/selftests/test-drm_dp_mst_helper.c    | 147 ++++++++++++++++--
 3 files changed, 190 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
b/drivers/gpu/drm/drm_dp_mst_topology.c
index 54604633e65c..573f39a3dc16 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -442,6 +442,37 @@ drm_dp_encode_sideband_req(const struct
drm_dp_sideband_msg_req_body *req,
                idx++;
                }
                break;
+       case DP_CONNECTION_STATUS_NOTIFY: {
+               const struct drm_dp_connection_status_notify *msg;
+
+               msg = &req->u.conn_stat;
+               buf[idx] = (msg->port_number & 0xf) << 4;
+               idx++;
+               memcpy(&raw->msg[idx], msg->guid, 16);
+               idx += 16;
+               raw->msg[idx] = 0;
+               raw->msg[idx] |= msg->legacy_device_plug_status ? BIT(6) : 0;
+               raw->msg[idx] |= msg->displayport_device_plug_status ? BIT(5) :
0;
+               raw->msg[idx] |= msg->message_capability_status ? BIT(4) : 0;
+               raw->msg[idx] |= msg->input_port ? BIT(3) : 0;
+               raw->msg[idx] |= FIELD_PREP(GENMASK(2, 0), msg-
>peer_device_type);
+               idx++;
+               break;
+       }
+       case DP_RESOURCE_STATUS_NOTIFY: {
+               const struct drm_dp_resource_status_notify *msg;
+
+               msg = &req->u.resource_stat;
+               buf[idx] = (msg->port_number & 0xf) << 4;
+               idx++;
+               memcpy(&raw->msg[idx], msg->guid, 16);
+               idx += 16;
+               buf[idx] = (msg->available_pbn & 0xff00) >> 8;
+               idx++;
+               buf[idx] = (msg->available_pbn & 0xff);
+               idx++;
+               break;
+       }
        }
        raw->cur_len = idx;
 }
@@ -672,6 +703,22 @@ drm_dp_dump_sideband_msg_req_body(const struct
drm_dp_sideband_msg_req_body *req
                  req->u.enc_status.stream_behavior,
                  req->u.enc_status.valid_stream_behavior);
                break;
+       case DP_CONNECTION_STATUS_NOTIFY:
+               P("port=%d guid=%*ph legacy=%d displayport=%d messaging=%d
input=%d peer_type=%d",
+                 req->u.conn_stat.port_number,
+                 (int)ARRAY_SIZE(req->u.conn_stat.guid), req->u.conn_stat.guid,
+                 req->u.conn_stat.legacy_device_plug_status,
+                 req->u.conn_stat.displayport_device_plug_status,
+                 req->u.conn_stat.message_capability_status,
+                 req->u.conn_stat.input_port,
+                 req->u.conn_stat.peer_device_type);
+               break;
+       case DP_RESOURCE_STATUS_NOTIFY:
+               P("port=%d guid=%*ph pbn=%d",
+                 req->u.resource_stat.port_number,
+                 (int)ARRAY_SIZE(req->u.resource_stat.guid), req-
>u.resource_stat.guid,
+                 req->u.resource_stat.available_pbn);
+               break;
        default:
                P("???\n");
                break;
@@ -1116,9 +1163,9 @@ static bool
drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
        return false;
 }
 
-static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr
*mgr,
-                                     struct drm_dp_sideband_msg_rx *raw,
-                                     struct drm_dp_sideband_msg_req_body *msg)
+bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
+                              struct drm_dp_sideband_msg_rx *raw,
+                              struct drm_dp_sideband_msg_req_body *msg)
 {
        memset(msg, 0, sizeof(*msg));
        msg->req_type = (raw->msg[0] & 0x7f);
@@ -1134,6 +1181,7 @@ static bool drm_dp_sideband_parse_req(const struct
drm_dp_mst_topology_mgr *mgr,
                return false;
        }
 }
+EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_dp_sideband_parse_req);
 
 static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg,
                             u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
diff --git a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
index eeda9a61c657..0356a2e0dba1 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
+++ b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
@@ -21,4 +21,8 @@ void
 drm_dp_dump_sideband_msg_req_body(const struct drm_dp_sideband_msg_req_body
*req,
                                  int indent, struct drm_printer *printer);
 
+bool
+drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
+                         struct drm_dp_sideband_msg_rx *raw,
+                         struct drm_dp_sideband_msg_req_body *msg);
 #endif /* !_DRM_DP_MST_HELPER_INTERNAL_H_ */
diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
index 6b4759ed6bfd..22aaedc63aec 100644
--- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -13,6 +13,10 @@
 #include "../drm_dp_mst_topology_internal.h"
 #include "test-drm_modeset_common.h"
 
+static void mock_release(struct device *dev)
+{
+}
+
 int igt_dp_mst_calc_pbn_mode(void *ignored)
 {
        int pbn, i;
@@ -120,27 +124,59 @@ sideband_msg_req_equal(const struct
drm_dp_sideband_msg_req_body *in,
 static bool
 sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
 {
-       struct drm_dp_sideband_msg_req_body *out;
+       struct drm_dp_sideband_msg_req_body *out = NULL;
        struct drm_printer p = drm_err_printer(PREFIX_STR);
-       struct drm_dp_sideband_msg_tx *txmsg;
+       struct drm_dp_sideband_msg_tx *txmsg = NULL;
+       struct drm_dp_sideband_msg_rx *rxmsg = NULL;
+       struct drm_dp_mst_topology_mgr *mgr = NULL;
        int i, ret;
-       bool result = true;
+       bool result = false;
 
        out = kzalloc(sizeof(*out), GFP_KERNEL);
        if (!out)
-               return false;
+               goto out;
 
        txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
        if (!txmsg)
-               return false;
+               goto out;
 
-       drm_dp_encode_sideband_req(in, txmsg);
-       ret = drm_dp_decode_sideband_req(txmsg, out);
-       if (ret < 0) {
-               drm_printf(&p, "Failed to decode sideband request: %d\n",
-                          ret);
-               result = false;
+       rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
+       if (!rxmsg)
                goto out;
+
+       mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
+       if (!mgr)
+               goto out;
+
+       mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
+       if (!mgr->dev)
+               goto out;
+
+       mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
+       if (!mgr->dev->dev)
+               goto out;
+
+       mgr->dev->dev->release = mock_release;
+       device_initialize(mgr->dev->dev);
+
+       drm_dp_encode_sideband_req(in, txmsg);
+       switch (in->req_type) {
+       case DP_CONNECTION_STATUS_NOTIFY:
+       case DP_RESOURCE_STATUS_NOTIFY:
+               memcpy(&rxmsg->msg, txmsg->msg, ARRAY_SIZE(rxmsg->msg));
+               rxmsg->curlen = txmsg->cur_len;
+               if (!drm_dp_sideband_parse_req(mgr, rxmsg, out)) {
+                       drm_printf(&p, "Failed to decode sideband request\n");
+                       goto out;
+               }
+               break;
+       default:
+               ret = drm_dp_decode_sideband_req(txmsg, out);
+               if (ret < 0) {
+                       drm_printf(&p, "Failed to decode sideband request:
%d\n", ret);
+                       goto out;
+               }
+               break;
        }
 
        if (!sideband_msg_req_equal(in, out)) {
@@ -148,9 +184,9 @@ sideband_msg_req_encode_decode(struct
drm_dp_sideband_msg_req_body *in)
                drm_dp_dump_sideband_msg_req_body(in, 1, &p);
                drm_printf(&p, "Got:\n");
                drm_dp_dump_sideband_msg_req_body(out, 1, &p);
-               result = false;
                goto out;
        }
+       result = true;
 
        switch (in->req_type) {
        case DP_REMOTE_DPCD_WRITE:
@@ -171,6 +207,65 @@ sideband_msg_req_encode_decode(struct
drm_dp_sideband_msg_req_body *in)
 out:
        kfree(out);
        kfree(txmsg);
+       kfree(rxmsg);
+       if (mgr) {
+               if (mgr->dev) {
+                       put_device(mgr->dev->dev);
+                       kfree(mgr->dev);
+               }
+               kfree(mgr);
+       }
+       return result;
+}
+
+static bool
+sideband_msg_req_parse(int req_type)
+{
+       struct drm_dp_sideband_msg_req_body *out = NULL;
+       struct drm_printer p = drm_err_printer(PREFIX_STR);
+       struct drm_dp_sideband_msg_rx *rxmsg = NULL;
+       struct drm_dp_mst_topology_mgr *mgr = NULL;
+       bool result = false;
+
+       out = kzalloc(sizeof(*out), GFP_KERNEL);
+       if (!out)
+               goto out;
+
+       rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
+       if (!rxmsg)
+               goto out;
+
+       mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
+       if (!mgr)
+               goto out;
+
+       mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
+       if (!mgr->dev)
+               goto out;
+
+       mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
+       if (!mgr->dev->dev)
+               goto out;
+
+       mgr->dev->dev->release = mock_release;
+       device_initialize(mgr->dev->dev);
+
+       rxmsg->curlen = 1;
+       rxmsg->msg[0] = req_type & 0x7f;
+       if (drm_dp_sideband_parse_req(mgr, rxmsg, out))
+               drm_printf(&p, "Unexpectedly decoded invalid sideband
request\n");
+       else
+               result = true;
+out:
+       kfree(out);
+       kfree(rxmsg);
+       if (mgr) {
+               if (mgr->dev) {
+                       put_device(mgr->dev->dev);
+                       kfree(mgr->dev);
+               }
+               kfree(mgr);
+       }
        return result;
 }
 
@@ -268,6 +363,34 @@ int igt_dp_mst_sideband_msg_req_decode(void *unused)
        in.u.enc_status.valid_stream_behavior = 1;
        DO_TEST();
 
+       in.req_type = DP_CONNECTION_STATUS_NOTIFY;
+       in.u.conn_stat.port_number = 0xf;
+       get_random_bytes(in.u.conn_stat.guid, sizeof(in.u.conn_stat.guid));
+       in.u.conn_stat.legacy_device_plug_status = 1;
+       in.u.conn_stat.displayport_device_plug_status = 0;
+       in.u.conn_stat.message_capability_status = 0;
+       in.u.conn_stat.input_port = 0;
+       in.u.conn_stat.peer_device_type = 7;
+       DO_TEST();
+       in.u.conn_stat.displayport_device_plug_status = 1;
+       DO_TEST();
+       in.u.conn_stat.message_capability_status = 1;
+       DO_TEST();
+       in.u.conn_stat.input_port = 1;
+       DO_TEST();
+
+       in.req_type = DP_RESOURCE_STATUS_NOTIFY;
+       in.u.resource_stat.port_number = 0xf;
+       get_random_bytes(in.u.resource_stat.guid,
sizeof(in.u.resource_stat.guid));
+       in.u.resource_stat.available_pbn = 0xcdef;
+       DO_TEST();
+
+#undef DO_TEST
+#define DO_TEST(req_type) FAIL_ON(!sideband_msg_req_parse(req_type))
+       DO_TEST(DP_CONNECTION_STATUS_NOTIFY);
+       DO_TEST(DP_RESOURCE_STATUS_NOTIFY);
+
+       DO_TEST(DP_REMOTE_I2C_WRITE);
 #undef DO_TEST
        return 0;
 }

--
Sincerely,
Lyude Paul (she/her)
Software Engineer at Red Hat

Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!


2021-05-19 19:11:47

by Sam McNally

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] drm/dp_mst: Add self-tests for up requests

On Wed, 19 May 2021 at 08:01, Lyude Paul <[email protected]> wrote:
>
> Looks like these tests might still need to be fixed up a bit:
>
> [ 34.785042] (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]] connection status reply parse length fail 2 1
> [ 34.785082] (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]] resource status reply parse length fail 2 1
> [ 34.785114] (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]] sink event notify parse length fail 2 1
> [ 34.785146] (null): [drm] *ERROR* Got unknown request 0x23 (REMOTE_I2C_WRITE)
>

Those are expected parse failures - testing that parse rejects
messages that are too short or are unsupported. I'll set the mock
device name to make this clearer in the next version, producing
logging like:
[ 25.163682] [drm_dp_mst_helper] expected parse failure:
[drm:drm_dp_sideband_parse_req] connection status reply parse length
fail 2 1
[ 25.163706] [drm_dp_mst_helper] expected parse failure:
[drm:drm_dp_sideband_parse_req] resource status reply parse length
fail 2 1
[ 25.163719] [drm_dp_mst_helper] expected parse failure:
[drm:drm_dp_sideband_parse_req] sink event notify parse length fail 2
1
[ 25.163730] [drm_dp_mst_helper] expected parse failure: [drm]
*ERROR* Got unknown request 0x23 (REMOTE_I2C_WRITE)

>
> On Tue, 2021-05-18 at 22:35 +1000, Sam McNally wrote:
> Up requests are decoded by drm_dp_sideband_parse_req(), which operates
> on a drm_dp_sideband_msg_rx, unlike down requests. Expand the existing
> self-test helper sideband_msg_req_encode_decode() to copy the message
> contents and length from a drm_dp_sideband_msg_tx to
> drm_dp_sideband_msg_rx and use the parse function under test in place of
> decode.
>
> Add support for currently-supported up requests to
> drm_dp_dump_sideband_msg_req_body(); add support to
> drm_dp_encode_sideband_req() to allow encoding for the self-tests.
>
> Add self-tests for CONNECTION_STATUS_NOTIFY and RESOURCE_STATUS_NOTIFY.
>
> Signed-off-by: Sam McNally <[email protected]>
> ---
>
> Changes in v4:
> - New in v4
>
> drivers/gpu/drm/drm_dp_mst_topology.c | 54 ++++++-
> .../gpu/drm/drm_dp_mst_topology_internal.h | 4 +
> .../drm/selftests/test-drm_dp_mst_helper.c | 147 ++++++++++++++++--
> 3 files changed, 190 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index 54604633e65c..573f39a3dc16 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -442,6 +442,37 @@ drm_dp_encode_sideband_req(const struct
> drm_dp_sideband_msg_req_body *req,
> idx++;
> }
> break;
> + case DP_CONNECTION_STATUS_NOTIFY: {
> + const struct drm_dp_connection_status_notify *msg;
> +
> + msg = &req->u.conn_stat;
> + buf[idx] = (msg->port_number & 0xf) << 4;
> + idx++;
> + memcpy(&raw->msg[idx], msg->guid, 16);
> + idx += 16;
> + raw->msg[idx] = 0;
> + raw->msg[idx] |= msg->legacy_device_plug_status ? BIT(6) : 0;
> + raw->msg[idx] |= msg->displayport_device_plug_status ? BIT(5) :
> 0;
> + raw->msg[idx] |= msg->message_capability_status ? BIT(4) : 0;
> + raw->msg[idx] |= msg->input_port ? BIT(3) : 0;
> + raw->msg[idx] |= FIELD_PREP(GENMASK(2, 0), msg-
> >peer_device_type);
> + idx++;
> + break;
> + }
> + case DP_RESOURCE_STATUS_NOTIFY: {
> + const struct drm_dp_resource_status_notify *msg;
> +
> + msg = &req->u.resource_stat;
> + buf[idx] = (msg->port_number & 0xf) << 4;
> + idx++;
> + memcpy(&raw->msg[idx], msg->guid, 16);
> + idx += 16;
> + buf[idx] = (msg->available_pbn & 0xff00) >> 8;
> + idx++;
> + buf[idx] = (msg->available_pbn & 0xff);
> + idx++;
> + break;
> + }
> }
> raw->cur_len = idx;
> }
> @@ -672,6 +703,22 @@ drm_dp_dump_sideband_msg_req_body(const struct
> drm_dp_sideband_msg_req_body *req
> req->u.enc_status.stream_behavior,
> req->u.enc_status.valid_stream_behavior);
> break;
> + case DP_CONNECTION_STATUS_NOTIFY:
> + P("port=%d guid=%*ph legacy=%d displayport=%d messaging=%d
> input=%d peer_type=%d",
> + req->u.conn_stat.port_number,
> + (int)ARRAY_SIZE(req->u.conn_stat.guid), req->u.conn_stat.guid,
> + req->u.conn_stat.legacy_device_plug_status,
> + req->u.conn_stat.displayport_device_plug_status,
> + req->u.conn_stat.message_capability_status,
> + req->u.conn_stat.input_port,
> + req->u.conn_stat.peer_device_type);
> + break;
> + case DP_RESOURCE_STATUS_NOTIFY:
> + P("port=%d guid=%*ph pbn=%d",
> + req->u.resource_stat.port_number,
> + (int)ARRAY_SIZE(req->u.resource_stat.guid), req-
> >u.resource_stat.guid,
> + req->u.resource_stat.available_pbn);
> + break;
> default:
> P("???\n");
> break;
> @@ -1116,9 +1163,9 @@ static bool
> drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
> return false;
> }
>
> -static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr
> *mgr,
> - struct drm_dp_sideband_msg_rx *raw,
> - struct drm_dp_sideband_msg_req_body *msg)
> +bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
> + struct drm_dp_sideband_msg_rx *raw,
> + struct drm_dp_sideband_msg_req_body *msg)
> {
> memset(msg, 0, sizeof(*msg));
> msg->req_type = (raw->msg[0] & 0x7f);
> @@ -1134,6 +1181,7 @@ static bool drm_dp_sideband_parse_req(const struct
> drm_dp_mst_topology_mgr *mgr,
> return false;
> }
> }
> +EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_dp_sideband_parse_req);
>
> static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg,
> u8 port_num, u32 offset, u8 num_bytes, u8 *bytes)
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> index eeda9a61c657..0356a2e0dba1 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> +++ b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> @@ -21,4 +21,8 @@ void
> drm_dp_dump_sideband_msg_req_body(const struct drm_dp_sideband_msg_req_body
> *req,
> int indent, struct drm_printer *printer);
>
> +bool
> +drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
> + struct drm_dp_sideband_msg_rx *raw,
> + struct drm_dp_sideband_msg_req_body *msg);
> #endif /* !_DRM_DP_MST_HELPER_INTERNAL_H_ */
> diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> index 6b4759ed6bfd..22aaedc63aec 100644
> --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> @@ -13,6 +13,10 @@
> #include "../drm_dp_mst_topology_internal.h"
> #include "test-drm_modeset_common.h"
>
> +static void mock_release(struct device *dev)
> +{
> +}
> +
> int igt_dp_mst_calc_pbn_mode(void *ignored)
> {
> int pbn, i;
> @@ -120,27 +124,59 @@ sideband_msg_req_equal(const struct
> drm_dp_sideband_msg_req_body *in,
> static bool
> sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
> {
> - struct drm_dp_sideband_msg_req_body *out;
> + struct drm_dp_sideband_msg_req_body *out = NULL;
> struct drm_printer p = drm_err_printer(PREFIX_STR);
> - struct drm_dp_sideband_msg_tx *txmsg;
> + struct drm_dp_sideband_msg_tx *txmsg = NULL;
> + struct drm_dp_sideband_msg_rx *rxmsg = NULL;
> + struct drm_dp_mst_topology_mgr *mgr = NULL;
> int i, ret;
> - bool result = true;
> + bool result = false;
>
> out = kzalloc(sizeof(*out), GFP_KERNEL);
> if (!out)
> - return false;
> + goto out;
>
> txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> if (!txmsg)
> - return false;
> + goto out;
>
> - drm_dp_encode_sideband_req(in, txmsg);
> - ret = drm_dp_decode_sideband_req(txmsg, out);
> - if (ret < 0) {
> - drm_printf(&p, "Failed to decode sideband request: %d\n",
> - ret);
> - result = false;
> + rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
> + if (!rxmsg)
> goto out;
> +
> + mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
> + if (!mgr)
> + goto out;
> +
> + mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
> + if (!mgr->dev)
> + goto out;
> +
> + mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
> + if (!mgr->dev->dev)
> + goto out;
> +
> + mgr->dev->dev->release = mock_release;
> + device_initialize(mgr->dev->dev);
> +
> + drm_dp_encode_sideband_req(in, txmsg);
> + switch (in->req_type) {
> + case DP_CONNECTION_STATUS_NOTIFY:
> + case DP_RESOURCE_STATUS_NOTIFY:
> + memcpy(&rxmsg->msg, txmsg->msg, ARRAY_SIZE(rxmsg->msg));
> + rxmsg->curlen = txmsg->cur_len;
> + if (!drm_dp_sideband_parse_req(mgr, rxmsg, out)) {
> + drm_printf(&p, "Failed to decode sideband request\n");
> + goto out;
> + }
> + break;
> + default:
> + ret = drm_dp_decode_sideband_req(txmsg, out);
> + if (ret < 0) {
> + drm_printf(&p, "Failed to decode sideband request:
> %d\n", ret);
> + goto out;
> + }
> + break;
> }
>
> if (!sideband_msg_req_equal(in, out)) {
> @@ -148,9 +184,9 @@ sideband_msg_req_encode_decode(struct
> drm_dp_sideband_msg_req_body *in)
> drm_dp_dump_sideband_msg_req_body(in, 1, &p);
> drm_printf(&p, "Got:\n");
> drm_dp_dump_sideband_msg_req_body(out, 1, &p);
> - result = false;
> goto out;
> }
> + result = true;
>
> switch (in->req_type) {
> case DP_REMOTE_DPCD_WRITE:
> @@ -171,6 +207,65 @@ sideband_msg_req_encode_decode(struct
> drm_dp_sideband_msg_req_body *in)
> out:
> kfree(out);
> kfree(txmsg);
> + kfree(rxmsg);
> + if (mgr) {
> + if (mgr->dev) {
> + put_device(mgr->dev->dev);
> + kfree(mgr->dev);
> + }
> + kfree(mgr);
> + }
> + return result;
> +}
> +
> +static bool
> +sideband_msg_req_parse(int req_type)
> +{
> + struct drm_dp_sideband_msg_req_body *out = NULL;
> + struct drm_printer p = drm_err_printer(PREFIX_STR);
> + struct drm_dp_sideband_msg_rx *rxmsg = NULL;
> + struct drm_dp_mst_topology_mgr *mgr = NULL;
> + bool result = false;
> +
> + out = kzalloc(sizeof(*out), GFP_KERNEL);
> + if (!out)
> + goto out;
> +
> + rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
> + if (!rxmsg)
> + goto out;
> +
> + mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
> + if (!mgr)
> + goto out;
> +
> + mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
> + if (!mgr->dev)
> + goto out;
> +
> + mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
> + if (!mgr->dev->dev)
> + goto out;
> +
> + mgr->dev->dev->release = mock_release;
> + device_initialize(mgr->dev->dev);
> +
> + rxmsg->curlen = 1;
> + rxmsg->msg[0] = req_type & 0x7f;
> + if (drm_dp_sideband_parse_req(mgr, rxmsg, out))
> + drm_printf(&p, "Unexpectedly decoded invalid sideband
> request\n");
> + else
> + result = true;
> +out:
> + kfree(out);
> + kfree(rxmsg);
> + if (mgr) {
> + if (mgr->dev) {
> + put_device(mgr->dev->dev);
> + kfree(mgr->dev);
> + }
> + kfree(mgr);
> + }
> return result;
> }
>
> @@ -268,6 +363,34 @@ int igt_dp_mst_sideband_msg_req_decode(void *unused)
> in.u.enc_status.valid_stream_behavior = 1;
> DO_TEST();
>
> + in.req_type = DP_CONNECTION_STATUS_NOTIFY;
> + in.u.conn_stat.port_number = 0xf;
> + get_random_bytes(in.u.conn_stat.guid, sizeof(in.u.conn_stat.guid));
> + in.u.conn_stat.legacy_device_plug_status = 1;
> + in.u.conn_stat.displayport_device_plug_status = 0;
> + in.u.conn_stat.message_capability_status = 0;
> + in.u.conn_stat.input_port = 0;
> + in.u.conn_stat.peer_device_type = 7;
> + DO_TEST();
> + in.u.conn_stat.displayport_device_plug_status = 1;
> + DO_TEST();
> + in.u.conn_stat.message_capability_status = 1;
> + DO_TEST();
> + in.u.conn_stat.input_port = 1;
> + DO_TEST();
> +
> + in.req_type = DP_RESOURCE_STATUS_NOTIFY;
> + in.u.resource_stat.port_number = 0xf;
> + get_random_bytes(in.u.resource_stat.guid,
> sizeof(in.u.resource_stat.guid));
> + in.u.resource_stat.available_pbn = 0xcdef;
> + DO_TEST();
> +
> +#undef DO_TEST
> +#define DO_TEST(req_type) FAIL_ON(!sideband_msg_req_parse(req_type))
> + DO_TEST(DP_CONNECTION_STATUS_NOTIFY);
> + DO_TEST(DP_RESOURCE_STATUS_NOTIFY);
> +
> + DO_TEST(DP_REMOTE_I2C_WRITE);
> #undef DO_TEST
> return 0;
> }
>
> --
> Sincerely,
> Lyude Paul (she/her)
> Software Engineer at Red Hat
>
> Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
> asked me a question, are waiting for a review/merge on a patch, etc. and I
> haven't responded in a while, please feel free to send me another email to check
> on my status. I don't bite!
>

2021-05-19 20:17:21

by Lyude Paul

[permalink] [raw]
Subject: Re: [PATCH v4 1/3] drm/dp_mst: Add self-tests for up requests

On Wed, 2021-05-19 at 13:51 +1000, Sam McNally wrote:
> On Wed, 19 May 2021 at 08:01, Lyude Paul <[email protected]> wrote:
> >
> > Looks like these tests might still need to be fixed up a bit:
> >
> > [   34.785042]  (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]]
> > connection status reply parse length fail 2 1
> > [   34.785082]  (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]]
> > resource status reply parse length fail 2 1
> > [   34.785114]  (null): [drm:drm_dp_sideband_parse_req [drm_kms_helper]]
> > sink event notify parse length fail 2 1
> > [   34.785146]  (null): [drm] *ERROR* Got unknown request 0x23
> > (REMOTE_I2C_WRITE)
> >
>
> Those are expected parse failures - testing that parse rejects
> messages that are too short or are unsupported. I'll set the mock
> device name to make this clearer in the next version, producing
> logging like:
> [   25.163682]  [drm_dp_mst_helper] expected parse failure:
> [drm:drm_dp_sideband_parse_req] connection status reply parse length
> fail 2 1
> [   25.163706]  [drm_dp_mst_helper] expected parse failure:
> [drm:drm_dp_sideband_parse_req] resource status reply parse length
> fail 2 1
> [   25.163719]  [drm_dp_mst_helper] expected parse failure:
> [drm:drm_dp_sideband_parse_req] sink event notify parse length fail 2
> 1
> [   25.163730]  [drm_dp_mst_helper] expected parse failure: [drm]
> *ERROR* Got unknown request 0x23 (REMOTE_I2C_WRITE)
>

sgtm

> >
> > On Tue, 2021-05-18 at 22:35 +1000, Sam McNally wrote:
> > Up requests are decoded by drm_dp_sideband_parse_req(), which operates
> > on a drm_dp_sideband_msg_rx, unlike down requests. Expand the existing
> > self-test helper sideband_msg_req_encode_decode() to copy the message
> > contents and length from a drm_dp_sideband_msg_tx to
> > drm_dp_sideband_msg_rx and use the parse function under test in place of
> > decode.
> >
> > Add support for currently-supported up requests to
> > drm_dp_dump_sideband_msg_req_body(); add support to
> > drm_dp_encode_sideband_req() to allow encoding for the self-tests.
> >
> > Add self-tests for CONNECTION_STATUS_NOTIFY and RESOURCE_STATUS_NOTIFY.
> >
> > Signed-off-by: Sam McNally <[email protected]>
> > ---
> >
> > Changes in v4:
> > - New in v4
> >
> >  drivers/gpu/drm/drm_dp_mst_topology.c         |  54 ++++++-
> >  .../gpu/drm/drm_dp_mst_topology_internal.h    |   4 +
> >  .../drm/selftests/test-drm_dp_mst_helper.c    | 147 ++++++++++++++++--
> >  3 files changed, 190 insertions(+), 15 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> > b/drivers/gpu/drm/drm_dp_mst_topology.c
> > index 54604633e65c..573f39a3dc16 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> > @@ -442,6 +442,37 @@ drm_dp_encode_sideband_req(const struct
> > drm_dp_sideband_msg_req_body *req,
> >                 idx++;
> >                 }
> >                 break;
> > +       case DP_CONNECTION_STATUS_NOTIFY: {
> > +               const struct drm_dp_connection_status_notify *msg;
> > +
> > +               msg = &req->u.conn_stat;
> > +               buf[idx] = (msg->port_number & 0xf) << 4;
> > +               idx++;
> > +               memcpy(&raw->msg[idx], msg->guid, 16);
> > +               idx += 16;
> > +               raw->msg[idx] = 0;
> > +               raw->msg[idx] |= msg->legacy_device_plug_status ? BIT(6) :
> > 0;
> > +               raw->msg[idx] |= msg->displayport_device_plug_status ?
> > BIT(5) :
> > 0;
> > +               raw->msg[idx] |= msg->message_capability_status ? BIT(4) :
> > 0;
> > +               raw->msg[idx] |= msg->input_port ? BIT(3) : 0;
> > +               raw->msg[idx] |= FIELD_PREP(GENMASK(2, 0), msg-
> > > peer_device_type);
> > +               idx++;
> > +               break;
> > +       }
> > +       case DP_RESOURCE_STATUS_NOTIFY: {
> > +               const struct drm_dp_resource_status_notify *msg;
> > +
> > +               msg = &req->u.resource_stat;
> > +               buf[idx] = (msg->port_number & 0xf) << 4;
> > +               idx++;
> > +               memcpy(&raw->msg[idx], msg->guid, 16);
> > +               idx += 16;
> > +               buf[idx] = (msg->available_pbn & 0xff00) >> 8;
> > +               idx++;
> > +               buf[idx] = (msg->available_pbn & 0xff);
> > +               idx++;
> > +               break;
> > +       }
> >         }
> >         raw->cur_len = idx;
> >  }
> > @@ -672,6 +703,22 @@ drm_dp_dump_sideband_msg_req_body(const struct
> > drm_dp_sideband_msg_req_body *req
> >                   req->u.enc_status.stream_behavior,
> >                   req->u.enc_status.valid_stream_behavior);
> >                 break;
> > +       case DP_CONNECTION_STATUS_NOTIFY:
> > +               P("port=%d guid=%*ph legacy=%d displayport=%d messaging=%d
> > input=%d peer_type=%d",
> > +                 req->u.conn_stat.port_number,
> > +                 (int)ARRAY_SIZE(req->u.conn_stat.guid), req-
> > >u.conn_stat.guid,
> > +                 req->u.conn_stat.legacy_device_plug_status,
> > +                 req->u.conn_stat.displayport_device_plug_status,
> > +                 req->u.conn_stat.message_capability_status,
> > +                 req->u.conn_stat.input_port,
> > +                 req->u.conn_stat.peer_device_type);
> > +               break;
> > +       case DP_RESOURCE_STATUS_NOTIFY:
> > +               P("port=%d guid=%*ph pbn=%d",
> > +                 req->u.resource_stat.port_number,
> > +                 (int)ARRAY_SIZE(req->u.resource_stat.guid), req-
> > > u.resource_stat.guid,
> > +                 req->u.resource_stat.available_pbn);
> > +               break;
> >         default:
> >                 P("???\n");
> >                 break;
> > @@ -1116,9 +1163,9 @@ static bool
> > drm_dp_sideband_parse_resource_status_notify(const struct drm_dp_mst
> >         return false;
> >  }
> >
> > -static bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr
> > *mgr,
> > -                                     struct drm_dp_sideband_msg_rx *raw,
> > -                                     struct drm_dp_sideband_msg_req_body
> > *msg)
> > +bool drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
> > +                              struct drm_dp_sideband_msg_rx *raw,
> > +                              struct drm_dp_sideband_msg_req_body *msg)
> >  {
> >         memset(msg, 0, sizeof(*msg));
> >         msg->req_type = (raw->msg[0] & 0x7f);
> > @@ -1134,6 +1181,7 @@ static bool drm_dp_sideband_parse_req(const struct
> > drm_dp_mst_topology_mgr *mgr,
> >                 return false;
> >         }
> >  }
> > +EXPORT_SYMBOL_FOR_TESTS_ONLY(drm_dp_sideband_parse_req);
> >
> >  static void build_dpcd_write(struct drm_dp_sideband_msg_tx *msg,
> >                              u8 port_num, u32 offset, u8 num_bytes, u8
> > *bytes)
> > diff --git a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> > b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> > index eeda9a61c657..0356a2e0dba1 100644
> > --- a/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> > +++ b/drivers/gpu/drm/drm_dp_mst_topology_internal.h
> > @@ -21,4 +21,8 @@ void
> >  drm_dp_dump_sideband_msg_req_body(const struct drm_dp_sideband_msg_req_body
> > *req,
> >                                   int indent, struct drm_printer *printer);
> >
> > +bool
> > +drm_dp_sideband_parse_req(const struct drm_dp_mst_topology_mgr *mgr,
> > +                         struct drm_dp_sideband_msg_rx *raw,
> > +                         struct drm_dp_sideband_msg_req_body *msg);
> >  #endif /* !_DRM_DP_MST_HELPER_INTERNAL_H_ */
> > diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> > b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> > index 6b4759ed6bfd..22aaedc63aec 100644
> > --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> > +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> > @@ -13,6 +13,10 @@
> >  #include "../drm_dp_mst_topology_internal.h"
> >  #include "test-drm_modeset_common.h"
> >
> > +static void mock_release(struct device *dev)
> > +{
> > +}
> > +
> >  int igt_dp_mst_calc_pbn_mode(void *ignored)
> >  {
> >         int pbn, i;
> > @@ -120,27 +124,59 @@ sideband_msg_req_equal(const struct
> > drm_dp_sideband_msg_req_body *in,
> >  static bool
> >  sideband_msg_req_encode_decode(struct drm_dp_sideband_msg_req_body *in)
> >  {
> > -       struct drm_dp_sideband_msg_req_body *out;
> > +       struct drm_dp_sideband_msg_req_body *out = NULL;
> >         struct drm_printer p = drm_err_printer(PREFIX_STR);
> > -       struct drm_dp_sideband_msg_tx *txmsg;
> > +       struct drm_dp_sideband_msg_tx *txmsg = NULL;
> > +       struct drm_dp_sideband_msg_rx *rxmsg = NULL;
> > +       struct drm_dp_mst_topology_mgr *mgr = NULL;
> >         int i, ret;
> > -       bool result = true;
> > +       bool result = false;
> >
> >         out = kzalloc(sizeof(*out), GFP_KERNEL);
> >         if (!out)
> > -               return false;
> > +               goto out;
> >
> >         txmsg = kzalloc(sizeof(*txmsg), GFP_KERNEL);
> >         if (!txmsg)
> > -               return false;
> > +               goto out;
> >
> > -       drm_dp_encode_sideband_req(in, txmsg);
> > -       ret = drm_dp_decode_sideband_req(txmsg, out);
> > -       if (ret < 0) {
> > -               drm_printf(&p, "Failed to decode sideband request: %d\n",
> > -                          ret);
> > -               result = false;
> > +       rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
> > +       if (!rxmsg)
> >                 goto out;
> > +
> > +       mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
> > +       if (!mgr)
> > +               goto out;
> > +
> > +       mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
> > +       if (!mgr->dev)
> > +               goto out;
> > +
> > +       mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
> > +       if (!mgr->dev->dev)
> > +               goto out;
> > +
> > +       mgr->dev->dev->release = mock_release;
> > +       device_initialize(mgr->dev->dev);
> > +
> > +       drm_dp_encode_sideband_req(in, txmsg);
> > +       switch (in->req_type) {
> > +       case DP_CONNECTION_STATUS_NOTIFY:
> > +       case DP_RESOURCE_STATUS_NOTIFY:
> > +               memcpy(&rxmsg->msg, txmsg->msg, ARRAY_SIZE(rxmsg->msg));
> > +               rxmsg->curlen = txmsg->cur_len;
> > +               if (!drm_dp_sideband_parse_req(mgr, rxmsg, out)) {
> > +                       drm_printf(&p, "Failed to decode sideband
> > request\n");
> > +                       goto out;
> > +               }
> > +               break;
> > +       default:
> > +               ret = drm_dp_decode_sideband_req(txmsg, out);
> > +               if (ret < 0) {
> > +                       drm_printf(&p, "Failed to decode sideband request:
> > %d\n", ret);
> > +                       goto out;
> > +               }
> > +               break;
> >         }
> >
> >         if (!sideband_msg_req_equal(in, out)) {
> > @@ -148,9 +184,9 @@ sideband_msg_req_encode_decode(struct
> > drm_dp_sideband_msg_req_body *in)
> >                 drm_dp_dump_sideband_msg_req_body(in, 1, &p);
> >                 drm_printf(&p, "Got:\n");
> >                 drm_dp_dump_sideband_msg_req_body(out, 1, &p);
> > -               result = false;
> >                 goto out;
> >         }
> > +       result = true;
> >
> >         switch (in->req_type) {
> >         case DP_REMOTE_DPCD_WRITE:
> > @@ -171,6 +207,65 @@ sideband_msg_req_encode_decode(struct
> > drm_dp_sideband_msg_req_body *in)
> >  out:
> >         kfree(out);
> >         kfree(txmsg);
> > +       kfree(rxmsg);
> > +       if (mgr) {
> > +               if (mgr->dev) {
> > +                       put_device(mgr->dev->dev);
> > +                       kfree(mgr->dev);
> > +               }
> > +               kfree(mgr);
> > +       }
> > +       return result;
> > +}
> > +
> > +static bool
> > +sideband_msg_req_parse(int req_type)
> > +{
> > +       struct drm_dp_sideband_msg_req_body *out = NULL;
> > +       struct drm_printer p = drm_err_printer(PREFIX_STR);
> > +       struct drm_dp_sideband_msg_rx *rxmsg = NULL;
> > +       struct drm_dp_mst_topology_mgr *mgr = NULL;
> > +       bool result = false;
> > +
> > +       out = kzalloc(sizeof(*out), GFP_KERNEL);
> > +       if (!out)
> > +               goto out;
> > +
> > +       rxmsg = kzalloc(sizeof(*rxmsg), GFP_KERNEL);
> > +       if (!rxmsg)
> > +               goto out;
> > +
> > +       mgr = kzalloc(sizeof(*mgr), GFP_KERNEL);
> > +       if (!mgr)
> > +               goto out;
> > +
> > +       mgr->dev = kzalloc(sizeof(*mgr->dev), GFP_KERNEL);
> > +       if (!mgr->dev)
> > +               goto out;
> > +
> > +       mgr->dev->dev = kzalloc(sizeof(*mgr->dev->dev), GFP_KERNEL);
> > +       if (!mgr->dev->dev)
> > +               goto out;
> > +
> > +       mgr->dev->dev->release = mock_release;
> > +       device_initialize(mgr->dev->dev);
> > +
> > +       rxmsg->curlen = 1;
> > +       rxmsg->msg[0] = req_type & 0x7f;
> > +       if (drm_dp_sideband_parse_req(mgr, rxmsg, out))
> > +               drm_printf(&p, "Unexpectedly decoded invalid sideband
> > request\n");
> > +       else
> > +               result = true;
> > +out:
> > +       kfree(out);
> > +       kfree(rxmsg);
> > +       if (mgr) {
> > +               if (mgr->dev) {
> > +                       put_device(mgr->dev->dev);
> > +                       kfree(mgr->dev);
> > +               }
> > +               kfree(mgr);
> > +       }
> >         return result;
> >  }
> >
> > @@ -268,6 +363,34 @@ int igt_dp_mst_sideband_msg_req_decode(void *unused)
> >         in.u.enc_status.valid_stream_behavior = 1;
> >         DO_TEST();
> >
> > +       in.req_type = DP_CONNECTION_STATUS_NOTIFY;
> > +       in.u.conn_stat.port_number = 0xf;
> > +       get_random_bytes(in.u.conn_stat.guid, sizeof(in.u.conn_stat.guid));
> > +       in.u.conn_stat.legacy_device_plug_status = 1;
> > +       in.u.conn_stat.displayport_device_plug_status = 0;
> > +       in.u.conn_stat.message_capability_status = 0;
> > +       in.u.conn_stat.input_port = 0;
> > +       in.u.conn_stat.peer_device_type = 7;
> > +       DO_TEST();
> > +       in.u.conn_stat.displayport_device_plug_status = 1;
> > +       DO_TEST();
> > +       in.u.conn_stat.message_capability_status = 1;
> > +       DO_TEST();
> > +       in.u.conn_stat.input_port = 1;
> > +       DO_TEST();
> > +
> > +       in.req_type = DP_RESOURCE_STATUS_NOTIFY;
> > +       in.u.resource_stat.port_number = 0xf;
> > +       get_random_bytes(in.u.resource_stat.guid,
> > sizeof(in.u.resource_stat.guid));
> > +       in.u.resource_stat.available_pbn = 0xcdef;
> > +       DO_TEST();
> > +
> > +#undef DO_TEST
> > +#define DO_TEST(req_type) FAIL_ON(!sideband_msg_req_parse(req_type))
> > +       DO_TEST(DP_CONNECTION_STATUS_NOTIFY);
> > +       DO_TEST(DP_RESOURCE_STATUS_NOTIFY);
> > +
> > +       DO_TEST(DP_REMOTE_I2C_WRITE);
> >  #undef DO_TEST
> >         return 0;
> >  }
> >
> > --
> > Sincerely,
> >    Lyude Paul (she/her)
> >    Software Engineer at Red Hat
> >
> > Note: I deal with a lot of emails and have a lot of bugs on my plate. If
> > you've
> > asked me a question, are waiting for a review/merge on a patch, etc. and I
> > haven't responded in a while, please feel free to send me another email to
> > check
> > on my status. I don't bite!
> >
>

--
Sincerely,
Lyude Paul (she/her)
Software Engineer at Red Hat

Note: I deal with a lot of emails and have a lot of bugs on my plate. If you've
asked me a question, are waiting for a review/merge on a patch, etc. and I
haven't responded in a while, please feel free to send me another email to check
on my status. I don't bite!