All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v5 1/3] drm/dp_mst: Add self-tests for up requests
@ 2021-05-25  0:59 ` Sam McNally
  0 siblings, 0 replies; 16+ messages in thread
From: Sam McNally @ 2021-05-25  0:59 UTC (permalink / raw)
  To: LKML
  Cc: Lyude Paul, Hans Verkuil, Sam McNally, Anshuman Gupta,
	Daniel Vetter, David Airlie, Lee Jones, Maarten Lankhorst,
	Maxime Ripard, Sean Paul, Thomas Zimmermann, dri-devel

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 an additional helper for testing clearly-invalid up
messages, verifying that parse rejects them.

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 <sammc@chromium.org>
---

Changes in v5:
- Set mock device name to more clearly attribute error/debug logging to
  the self-test, in particular for cases where failures are expected

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    | 149 ++++++++++++++++--
 3 files changed, 192 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..7bbeb1e5bc97 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,60 @@ 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;
+	mgr->dev->dev->init_name = PREFIX_STR;
+	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 +185,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 +208,66 @@ 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;
+	mgr->dev->dev->init_name = PREFIX_STR " expected parse failure";
+	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 +365,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.818.g46aad6cb9e-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 1/3] drm/dp_mst: Add self-tests for up requests
@ 2021-05-25  0:59 ` Sam McNally
  0 siblings, 0 replies; 16+ messages in thread
From: Sam McNally @ 2021-05-25  0:59 UTC (permalink / raw)
  To: LKML
  Cc: Thomas Zimmermann, David Airlie, Anshuman Gupta, Hans Verkuil,
	Sam McNally, Sean Paul, dri-devel, Lee Jones

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 an additional helper for testing clearly-invalid up
messages, verifying that parse rejects them.

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 <sammc@chromium.org>
---

Changes in v5:
- Set mock device name to more clearly attribute error/debug logging to
  the self-test, in particular for cases where failures are expected

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    | 149 ++++++++++++++++--
 3 files changed, 192 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..7bbeb1e5bc97 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,60 @@ 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;
+	mgr->dev->dev->init_name = PREFIX_STR;
+	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 +185,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 +208,66 @@ 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;
+	mgr->dev->dev->init_name = PREFIX_STR " expected parse failure";
+	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 +365,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.818.g46aad6cb9e-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 2/3] drm/dp_mst: Add support for sink event notify messages
  2021-05-25  0:59 ` Sam McNally
@ 2021-05-25  0:59   ` Sam McNally
  -1 siblings, 0 replies; 16+ messages in thread
From: Sam McNally @ 2021-05-25  0:59 UTC (permalink / raw)
  To: LKML
  Cc: Lyude Paul, Hans Verkuil, Sam McNally, Anshuman Gupta,
	Daniel Vetter, David Airlie, Lee Jones, Maarten Lankhorst,
	Maxime Ripard, Sean Paul, Thomas Zimmermann, dri-devel

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 <sammc@chromium.org>
---

(no changes since v4)

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 7bbeb1e5bc97..d49c10d52d88 100644
--- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -164,6 +164,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)) {
@@ -387,10 +388,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.818.g46aad6cb9e-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 2/3] drm/dp_mst: Add support for sink event notify messages
@ 2021-05-25  0:59   ` Sam McNally
  0 siblings, 0 replies; 16+ messages in thread
From: Sam McNally @ 2021-05-25  0:59 UTC (permalink / raw)
  To: LKML
  Cc: Thomas Zimmermann, David Airlie, Anshuman Gupta, Hans Verkuil,
	Sam McNally, Sean Paul, dri-devel, Lee Jones

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 <sammc@chromium.org>
---

(no changes since v4)

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 7bbeb1e5bc97..d49c10d52d88 100644
--- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -164,6 +164,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)) {
@@ -387,10 +388,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.818.g46aad6cb9e-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 3/3] drm_dp_cec: add MST support
  2021-05-25  0:59 ` Sam McNally
@ 2021-05-25  0:59   ` Sam McNally
  -1 siblings, 0 replies; 16+ messages in thread
From: Sam McNally @ 2021-05-25  0:59 UTC (permalink / raw)
  To: LKML
  Cc: Lyude Paul, Hans Verkuil, Sam McNally, Hans Verkuil,
	Daniel Vetter, David Airlie, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, dri-devel

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 <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sam McNally <sammc@chromium.org>
---

(no changes since v4)

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.818.g46aad6cb9e-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v5 3/3] drm_dp_cec: add MST support
@ 2021-05-25  0:59   ` Sam McNally
  0 siblings, 0 replies; 16+ messages in thread
From: Sam McNally @ 2021-05-25  0:59 UTC (permalink / raw)
  To: LKML
  Cc: Thomas Zimmermann, David Airlie, Hans Verkuil, Sam McNally,
	dri-devel, Hans Verkuil

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 <hverkuil-cisco@xs4all.nl>
Signed-off-by: Sam McNally <sammc@chromium.org>
---

(no changes since v4)

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.818.g46aad6cb9e-goog


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
  2021-05-25  0:59   ` Sam McNally
  (?)
@ 2021-05-26  1:24     ` kernel test robot
  -1 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-05-26  1:24 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: kbuild-all, Lyude Paul, Hans Verkuil, Sam McNally, Daniel Vetter,
	David Airlie, Maarten Lankhorst, Maxime Ripard,
	Thomas Zimmermann, dri-devel, lkp

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

Hi Sam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20210524]
[cannot apply to linux/master drm-intel/for-linux-next linus/master v5.13-rc3]
[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]

url:    https://github.com/0day-ci/linux/commits/Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-b001-20210524 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # apt-get install iwyu # include-what-you-use
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
        git checkout 95cd83b87fc6deb810e6364643d8b6f198629bee
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross C=1 CHECK=iwyu ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


iwyu warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/drm_dp_cec.c:17:1: iwyu: warning: superfluous #include <drm/drm_dp_mst_helper.h>
   drivers/gpu/drm/drm_dp_cec.c:9:1: iwyu: warning: superfluous #include <linux/module.h>
   drivers/gpu/drm/drm_dp_cec.c:10:1: iwyu: warning: superfluous #include <linux/slab.h>

vim +17 drivers/gpu/drm/drm_dp_cec.c

0aa32f8e572e3e Sam Ravnborg          2019-10-07  13  
ae85b0df124f69 Dariusz Marcinkiewicz 2019-08-14  14  #include <drm/drm_connector.h>
0aa32f8e572e3e Sam Ravnborg          2019-10-07  15  #include <drm/drm_device.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  16  #include <drm/drm_dp_helper.h>
95cd83b87fc6de Sam McNally           2021-05-25 @17  #include <drm/drm_dp_mst_helper.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  18  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35232 bytes --]

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
@ 2021-05-26  1:24     ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-05-26  1:24 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Thomas Zimmermann, David Airlie, Hans Verkuil, Sam McNally,
	dri-devel, kbuild-all, lkp

[-- Attachment #1: Type: text/plain, Size: 2532 bytes --]

Hi Sam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20210524]
[cannot apply to linux/master drm-intel/for-linux-next linus/master v5.13-rc3]
[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]

url:    https://github.com/0day-ci/linux/commits/Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-b001-20210524 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # apt-get install iwyu # include-what-you-use
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
        git checkout 95cd83b87fc6deb810e6364643d8b6f198629bee
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross C=1 CHECK=iwyu ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


iwyu warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/drm_dp_cec.c:17:1: iwyu: warning: superfluous #include <drm/drm_dp_mst_helper.h>
   drivers/gpu/drm/drm_dp_cec.c:9:1: iwyu: warning: superfluous #include <linux/module.h>
   drivers/gpu/drm/drm_dp_cec.c:10:1: iwyu: warning: superfluous #include <linux/slab.h>

vim +17 drivers/gpu/drm/drm_dp_cec.c

0aa32f8e572e3e Sam Ravnborg          2019-10-07  13  
ae85b0df124f69 Dariusz Marcinkiewicz 2019-08-14  14  #include <drm/drm_connector.h>
0aa32f8e572e3e Sam Ravnborg          2019-10-07  15  #include <drm/drm_device.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  16  #include <drm/drm_dp_helper.h>
95cd83b87fc6de Sam McNally           2021-05-25 @17  #include <drm/drm_dp_mst_helper.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  18  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 35232 bytes --]

[-- Attachment #3: Type: text/plain, Size: 149 bytes --]

_______________________________________________
kbuild mailing list -- kbuild@lists.01.org
To unsubscribe send an email to kbuild-leave@lists.01.org

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
@ 2021-05-26  1:24     ` kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-05-26  1:24 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2740 bytes --]

Hi Sam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20210524]
[cannot apply to linux/master drm-intel/for-linux-next linus/master v5.13-rc3]
[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]

url:    https://github.com/0day-ci/linux/commits/Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
config: x86_64-randconfig-b001-20210524 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # apt-get install iwyu # include-what-you-use
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
        git checkout 95cd83b87fc6deb810e6364643d8b6f198629bee
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross C=1 CHECK=iwyu ARCH=x86_64

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


iwyu warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/drm_dp_cec.c:17:1: iwyu: warning: superfluous #include <drm/drm_dp_mst_helper.h>
   drivers/gpu/drm/drm_dp_cec.c:9:1: iwyu: warning: superfluous #include <linux/module.h>
   drivers/gpu/drm/drm_dp_cec.c:10:1: iwyu: warning: superfluous #include <linux/slab.h>

vim +17 drivers/gpu/drm/drm_dp_cec.c

0aa32f8e572e3e Sam Ravnborg          2019-10-07  13  
ae85b0df124f69 Dariusz Marcinkiewicz 2019-08-14  14  #include <drm/drm_connector.h>
0aa32f8e572e3e Sam Ravnborg          2019-10-07  15  #include <drm/drm_device.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  16  #include <drm/drm_dp_helper.h>
95cd83b87fc6de Sam McNally           2021-05-25 @17  #include <drm/drm_dp_mst_helper.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  18  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

_______________________________________________
kbuild mailing list -- kbuild(a)lists.01.org
To unsubscribe send an email to kbuild-leave(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35232 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
  2021-05-25  0:59   ` Sam McNally
@ 2021-05-27  7:28     ` Hans Verkuil
  -1 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2021-05-27  7:28 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Lyude Paul, Hans Verkuil, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, dri-devel

Hi Sam,

Just a minor nitpick below:

On 25/05/2021 02:59, Sam McNally wrote:
> 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 <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Sam McNally <sammc@chromium.org>
> ---
> 
> (no changes since v4)
> 
> 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);
>  }
> +

Spurious newline added, just drop this change.

Regards,

	Hans

>  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;
>  }
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
@ 2021-05-27  7:28     ` Hans Verkuil
  0 siblings, 0 replies; 16+ messages in thread
From: Hans Verkuil @ 2021-05-27  7:28 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Thomas Zimmermann, David Airlie, dri-devel, Hans Verkuil

Hi Sam,

Just a minor nitpick below:

On 25/05/2021 02:59, Sam McNally wrote:
> 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 <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Sam McNally <sammc@chromium.org>
> ---
> 
> (no changes since v4)
> 
> 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);
>  }
> +

Spurious newline added, just drop this change.

Regards,

	Hans

>  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;
>  }
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/3] drm/dp_mst: Add self-tests for up requests
  2021-05-25  0:59 ` Sam McNally
@ 2021-05-27 20:08   ` Lyude Paul
  -1 siblings, 0 replies; 16+ messages in thread
From: Lyude Paul @ 2021-05-27 20:08 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Hans Verkuil, Anshuman Gupta, Daniel Vetter, David Airlie,
	Lee Jones, Maarten Lankhorst, Maxime Ripard, Sean Paul,
	Thomas Zimmermann, dri-devel

On Tue, 2021-05-25 at 10:59 +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 an additional helper for testing clearly-invalid up
> messages, verifying that parse rejects them.
> 
> 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 <sammc@chromium.org>
> ---
> 
> Changes in v5:
> - Set mock device name to more clearly attribute error/debug logging to
>   the self-test, in particular for cases where failures are expected
> 
> 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    | 149 ++++++++++++++++--
>  3 files changed, 192 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..7bbeb1e5bc97 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,60 @@ 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;
> +       mgr->dev->dev->init_name = PREFIX_STR;
> +       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 +185,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 +208,66 @@ 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;
> +       mgr->dev->dev->init_name = PREFIX_STR " expected parse failure";
> +       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 +365,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));

Do you think you might be up to moving some of the random functions i915 uses
for selftests out of drivers/gpu/drm/i915/selftests/i915_random.h so that we
could use them here and be able to print out the actual seeds being used for
randomness so that failures can be reproduced reliably?

Either way, patches 1-2 are:

Reviewed-by: Lyude Paul <lyude@redhat.com>

Will have the third reviewed in just a moment, there's some tiny nitpicks w/
it

> +       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;
>  }

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 1/3] drm/dp_mst: Add self-tests for up requests
@ 2021-05-27 20:08   ` Lyude Paul
  0 siblings, 0 replies; 16+ messages in thread
From: Lyude Paul @ 2021-05-27 20:08 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Thomas Zimmermann, David Airlie, Anshuman Gupta, Hans Verkuil,
	Sean Paul, dri-devel, Lee Jones

On Tue, 2021-05-25 at 10:59 +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 an additional helper for testing clearly-invalid up
> messages, verifying that parse rejects them.
> 
> 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 <sammc@chromium.org>
> ---
> 
> Changes in v5:
> - Set mock device name to more clearly attribute error/debug logging to
>   the self-test, in particular for cases where failures are expected
> 
> 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    | 149 ++++++++++++++++--
>  3 files changed, 192 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..7bbeb1e5bc97 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,60 @@ 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;
> +       mgr->dev->dev->init_name = PREFIX_STR;
> +       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 +185,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 +208,66 @@ 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;
> +       mgr->dev->dev->init_name = PREFIX_STR " expected parse failure";
> +       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 +365,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));

Do you think you might be up to moving some of the random functions i915 uses
for selftests out of drivers/gpu/drm/i915/selftests/i915_random.h so that we
could use them here and be able to print out the actual seeds being used for
randomness so that failures can be reproduced reliably?

Either way, patches 1-2 are:

Reviewed-by: Lyude Paul <lyude@redhat.com>

Will have the third reviewed in just a moment, there's some tiny nitpicks w/
it

> +       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;
>  }

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
  2021-05-25  0:59   ` Sam McNally
@ 2021-05-27 20:12     ` Lyude Paul
  -1 siblings, 0 replies; 16+ messages in thread
From: Lyude Paul @ 2021-05-27 20:12 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Hans Verkuil, Hans Verkuil, Daniel Vetter, David Airlie,
	Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, dri-devel

On Tue, 2021-05-25 at 10:59 +1000, Sam McNally wrote:
> 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 <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Sam McNally <sammc@chromium.org>
> ---
> 
> (no changes since v4)
> 
> 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);

probably want to get rid of this whitespace

With that fixed, this is:

Reviewed-by: Lyude Paul <lyude@redhat.com>

>  
>  /*
> @@ -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;
>  }

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
@ 2021-05-27 20:12     ` Lyude Paul
  0 siblings, 0 replies; 16+ messages in thread
From: Lyude Paul @ 2021-05-27 20:12 UTC (permalink / raw)
  To: Sam McNally, LKML
  Cc: Thomas Zimmermann, David Airlie, Hans Verkuil, dri-devel, Hans Verkuil

On Tue, 2021-05-25 at 10:59 +1000, Sam McNally wrote:
> 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 <hverkuil-cisco@xs4all.nl>
> Signed-off-by: Sam McNally <sammc@chromium.org>
> ---
> 
> (no changes since v4)
> 
> 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);

probably want to get rid of this whitespace

With that fixed, this is:

Reviewed-by: Lyude Paul <lyude@redhat.com>

>  
>  /*
> @@ -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;
>  }

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


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v5 3/3] drm_dp_cec: add MST support
@ 2021-05-25  6:47 kernel test robot
  0 siblings, 0 replies; 16+ messages in thread
From: kernel test robot @ 2021-05-25  6:47 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 2602 bytes --]

CC: kbuild-all(a)lists.01.org
CC: clang-built-linux(a)googlegroups.com
In-Reply-To: <20210525105913.v5.3.If7fc06fd679af0665ada9ff0524291c61dd35d24@changeid>
References: <20210525105913.v5.3.If7fc06fd679af0665ada9ff0524291c61dd35d24(a)changeid>
TO: Sam McNally <sammc@chromium.org>
TO: LKML <linux-kernel@vger.kernel.org>
CC: Lyude Paul <lyude@redhat.com>
CC: Hans Verkuil <hverkuil@xs4all.nl>
CC: Sam McNally <sammc@chromium.org>
CC: Daniel Vetter <daniel@ffwll.ch>
CC: David Airlie <airlied@linux.ie>
CC: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
CC: Maxime Ripard <mripard@kernel.org>
CC: Thomas Zimmermann <tzimmermann@suse.de>
CC: dri-devel(a)lists.freedesktop.org

Hi Sam,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-tip/drm-tip]
[also build test WARNING on next-20210524]
[cannot apply to linux/master drm-intel/for-linux-next linus/master v5.13-rc3]
[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]

url:    https://github.com/0day-ci/linux/commits/Sam-McNally/drm-dp_mst-Add-self-tests-for-up-requests/20210525-090214
base:   git://anongit.freedesktop.org/drm/drm-tip drm-tip
:::::: branch date: 6 hours ago
:::::: commit date: 6 hours ago
config: x86_64-randconfig-b001-20210524 (attached as .config)
compiler: clang version 13.0.0 (https://github.com/llvm/llvm-project 99155e913e9bad5f7f8a247f8bb3a3ff3da74af1)

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


iwyu warnings: (new ones prefixed by >>)
>> drivers/gpu/drm/drm_dp_cec.c:17:1: iwyu: warning: superfluous #include <drm/drm_dp_mst_helper.h>
   drivers/gpu/drm/drm_dp_cec.c:9:1: iwyu: warning: superfluous #include <linux/module.h>
   drivers/gpu/drm/drm_dp_cec.c:10:1: iwyu: warning: superfluous #include <linux/slab.h>

vim +17 drivers/gpu/drm/drm_dp_cec.c

0aa32f8e572e3e Sam Ravnborg          2019-10-07  13  
ae85b0df124f69 Dariusz Marcinkiewicz 2019-08-14  14  #include <drm/drm_connector.h>
0aa32f8e572e3e Sam Ravnborg          2019-10-07  15  #include <drm/drm_device.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  16  #include <drm/drm_dp_helper.h>
95cd83b87fc6de Sam McNally           2021-05-25 @17  #include <drm/drm_dp_mst_helper.h>
2c6d1fffa1d9b0 Hans Verkuil          2018-07-11  18  

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all(a)lists.01.org

[-- Attachment #2: config.gz --]
[-- Type: application/gzip, Size: 35232 bytes --]

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-05-27 20:12 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-25  0:59 [PATCH v5 1/3] drm/dp_mst: Add self-tests for up requests Sam McNally
2021-05-25  0:59 ` Sam McNally
2021-05-25  0:59 ` [PATCH v5 2/3] drm/dp_mst: Add support for sink event notify messages Sam McNally
2021-05-25  0:59   ` Sam McNally
2021-05-25  0:59 ` [PATCH v5 3/3] drm_dp_cec: add MST support Sam McNally
2021-05-25  0:59   ` Sam McNally
2021-05-26  1:24   ` kernel test robot
2021-05-26  1:24     ` kernel test robot
2021-05-26  1:24     ` kernel test robot
2021-05-27  7:28   ` Hans Verkuil
2021-05-27  7:28     ` Hans Verkuil
2021-05-27 20:12   ` Lyude Paul
2021-05-27 20:12     ` Lyude Paul
2021-05-27 20:08 ` [PATCH v5 1/3] drm/dp_mst: Add self-tests for up requests Lyude Paul
2021-05-27 20:08   ` Lyude Paul
2021-05-25  6:47 [PATCH v5 3/3] drm_dp_cec: add MST support kernel test robot

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.