[PATCH 39/82] drm/amd/display: Test dm_dp_mst_get_modes without a remote EDID

<[email protected]>
Newsgroups org.freedesktop.lists.amd-gfx
Message-ID <[email protected]>
From: Alex Hung <[email protected]>

[WHAT]
Add KUnit tests for dm_dp_mst_get_modes() covering the path taken when
the remote EDID cannot be read: the default remote sink is registered and
MST_REMOTE_EDID is cleared, DC refusing the sink leaves the connector
without one, and an already attached sink is kept instead of asking DC
for a second one.

[HOW]
A shared fixture builds an MST downstream connector on a real DRM device
with an empty topology manager, so drm_dp_mst_edid_read() takes its "port
is gone" path without needing a branch device. dc_link_add_remote_sink()
and dc_link_remove_remote_sink() are thin wrappers over link_service
callbacks, so fakes for those two are enough to run the connector paths
without a DC core.

Assisted-by: Copilot:Claude-Opus-5
Reviewed-by: Bhawanpreet Lakha <[email protected]>
Signed-off-by: Alex Hung <[email protected]>
Signed-off-by: Ivan Lipski <[email protected]>
---
 .../display/amdgpu_dm/amdgpu_dm_mst_types.c   |   3 +-
 .../display/amdgpu_dm/amdgpu_dm_mst_types.h   |   1 +
 .../tests/amdgpu_dm_mst_types_test.c          | 216 ++++++++++++++++++
 3 files changed, 219 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
index 045a7f88b7546..79e8952edf8c0 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.c
@@ -386,7 +386,7 @@ STATIC_IFN_KUNIT bool retrieve_branch_specific_data(struct amdgpu_dm_connector *
 }
 EXPORT_IF_KUNIT(retrieve_branch_specific_data);
 
-static int dm_dp_mst_get_modes(struct drm_connector *connector)
+STATIC_IFN_KUNIT int dm_dp_mst_get_modes(struct drm_connector *connector)
 {
 	struct amdgpu_dm_connector *aconnector = to_amdgpu_dm_connector(connector);
 	int ret = 0;
@@ -518,6 +518,7 @@ static int dm_dp_mst_get_modes(struct drm_connector *connector)
 
 	return ret;
 }
+EXPORT_IF_KUNIT(dm_dp_mst_get_modes);
 
 STATIC_IFN_KUNIT struct drm_encoder *
 dm_mst_atomic_best_encoder(struct drm_connector *connector,
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h
index 2ae9d4b88888f..406dba95c4aec 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_mst_types.h
@@ -118,6 +118,7 @@ int dm_dp_mst_atomic_check(struct drm_connector *connector,
 				   struct drm_atomic_commit *state);
 int dm_dp_mst_detect(struct drm_connector *connector,
 			     struct drm_modeset_acquire_ctx *ctx, bool force);
+int dm_dp_mst_get_modes(struct drm_connector *connector);
 #endif
 
 #endif
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_mst_types_test.c b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_mst_types_test.c
index 6a5191f9baf61..ee4e1f3963736 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_mst_types_test.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/tests/amdgpu_dm_mst_types_test.c
@@ -8,11 +8,13 @@
 #include <kunit/test.h>
 
 #include <drm/drm_drv.h>
+#include <drm/drm_edid.h>
 #include <drm/drm_fixed.h>
 #include <drm/drm_kunit_helpers.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_mode_config.h>
+#include <drm/drm_modeset_lock.h>
 #include <drm/display/drm_dp.h>
 #include <drm/display/drm_dp_helper.h>
 #include <drm/display/drm_dp_mst_helper.h>
@@ -1343,6 +1345,216 @@ static void dm_mst_test_detect_unregistered(struct kunit *test)
 			(int)connector_status_disconnected);
 }
 
+/*
+ * Fake DC remote-sink backend. dc_link_add_remote_sink() and
+ * dc_link_remove_remote_sink() are thin wrappers over these link_service
+ * callbacks, so overriding them is enough to run the MST connector paths
+ * without a DC core.
+ */
+static struct dc_sink *dm_mst_test_next_remote_sink;
+static struct dc_sink *dm_mst_test_removed_sink;
+static unsigned int dm_mst_test_add_remote_sink_calls;
+static unsigned int dm_mst_test_remove_remote_sink_calls;
+
+/* Signature copied verbatim from struct link_service::add_remote_sink. */
+static struct dc_sink *dm_mst_test_add_remote_sink(
+		struct dc_link *link,
+		const uint8_t *edid,
+		unsigned int len,
+		struct dc_sink_init_data *init_data)
+{
+	struct dc_sink *sink = dm_mst_test_next_remote_sink;
+
+	dm_mst_test_add_remote_sink_calls++;
+
+	if (sink) {
+		sink->link = link;
+		sink->sink_signal = init_data->sink_signal;
+		link->sink_count++;
+	}
+
+	return sink;
+}
+
+/* Signature copied verbatim from struct link_service::remove_remote_sink. */
+static void dm_mst_test_remove_remote_sink(struct dc_link *link, struct dc_sink *sink)
+{
+	dm_mst_test_remove_remote_sink_calls++;
+	dm_mst_test_removed_sink = sink;
+
+	if (link->sink_count)
+		link->sink_count--;
+}
+
+/*
+ * dc_sink_release() frees the sink with kfree(), so these must not come from
+ * the KUnit managed allocator.
+ */
+static struct dc_sink *dm_mst_test_alloc_sink(struct kunit *test)
+{
+	struct dc_sink *sink = kzalloc_obj(*sink);
+
+	KUNIT_ASSERT_NOT_NULL(test, sink);
+	kref_init(&sink->refcount);
+
+	return sink;
+}
+
+struct dm_mst_test_child {
+	struct amdgpu_device *adev;
+	struct amdgpu_dm_connector *aconnector;
+	struct amdgpu_dm_connector *root;
+	struct drm_dp_mst_port *port;
+	struct dc_link *link;
+};
+
+/*
+ * Build an MST downstream connector hanging off a root connector. The topology
+ * manager is left empty (no mst_primary) so the DRM MST helpers take their
+ * "port is gone" paths instead of requiring a real branch device.
+ */
+static void dm_mst_test_init_child(struct kunit *test, struct dm_mst_test_child *child)
+{
+	struct link_service *link_srv;
+	struct amdgpu_device *adev;
+	struct dc *dc;
+	int ret;
+
+	adev = dm_kunit_alloc_adev(test);
+	ret = drmm_mode_config_init(&adev->ddev);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	dc = kunit_kzalloc(test, sizeof(*dc), GFP_KERNEL);
+	link_srv = kunit_kzalloc(test, sizeof(*link_srv), GFP_KERNEL);
+	child->port = kunit_kzalloc(test, sizeof(*child->port), GFP_KERNEL);
+	KUNIT_ASSERT_NOT_NULL(test, dc);
+	KUNIT_ASSERT_NOT_NULL(test, link_srv);
+	KUNIT_ASSERT_NOT_NULL(test, child->port);
+
+	link_srv->add_remote_sink = dm_mst_test_add_remote_sink;
+	link_srv->remove_remote_sink = dm_mst_test_remove_remote_sink;
+	dc->link_srv = link_srv;
+
+	child->adev = adev;
+	child->link = dm_kunit_alloc_link(test);
+	child->link->dc = dc;
+
+	child->root = dm_kunit_alloc_connector(test, adev, child->link);
+	child->root->mst_mgr.dev = &adev->ddev;
+	mutex_init(&child->root->mst_mgr.lock);
+	drm_modeset_lock_init(&child->root->mst_mgr.base.lock);
+
+	child->port->mgr = &child->root->mst_mgr;
+	child->port->aux.name = "dm_mst_test_port_aux";
+	child->port->aux.transfer = dm_mst_test_aux_transfer;
+	drm_dp_aux_init(&child->port->aux);
+	drm_dp_dpcd_set_probe(&child->port->aux, false);
+
+	child->aconnector = dm_kunit_alloc_connector(test, adev, child->link);
+	child->aconnector->mst_root = child->root;
+	child->aconnector->mst_output_port = child->port;
+
+	ret = drm_connector_init(&adev->ddev, &child->aconnector->base,
+				 &dm_mst_test_connector_funcs,
+				 DRM_MODE_CONNECTOR_DisplayPort);
+	KUNIT_ASSERT_EQ(test, ret, 0);
+	/* The MST (un)register helpers log through the connector's kernel device. */
+	child->aconnector->base.kdev = adev->ddev.dev;
+
+	memset(dm_mst_test_dpcd, 0, sizeof(dm_mst_test_dpcd));
+	dm_mst_test_aux_transfer_override = 0;
+	dm_mst_test_aux_write_override = 0;
+	dm_mst_test_next_remote_sink = NULL;
+	dm_mst_test_removed_sink = NULL;
+	dm_mst_test_add_remote_sink_calls = 0;
+	dm_mst_test_remove_remote_sink_calls = 0;
+}
+
+static void dm_mst_test_fini_child(struct dm_mst_test_child *child)
+{
+	drm_edid_free(child->aconnector->drm_edid);
+	drm_edid_free(child->port->cached_edid);
+	drm_connector_cleanup(&child->aconnector->base);
+	drm_modeset_lock_fini(&child->root->mst_mgr.base.lock);
+	mutex_destroy(&child->root->mst_mgr.lock);
+}
+
+/* Tests for dm_dp_mst_get_modes */
+
+/**
+ * dm_mst_test_get_modes_no_edid_adds_default_sink - Test default remote sink
+ * @test: KUnit test context
+ *
+ * When the remote EDID cannot be read, dm_dp_mst_get_modes() must clear the
+ * MST_REMOTE_EDID status and register a default remote sink for the connector.
+ */
+static void dm_mst_test_get_modes_no_edid_adds_default_sink(struct kunit *test)
+{
+	struct dm_mst_test_child child;
+	struct dc_sink *sink;
+
+	dm_mst_test_init_child(test, &child);
+	sink = dm_mst_test_alloc_sink(test);
+	dm_mst_test_next_remote_sink = sink;
+	child.aconnector->mst_status = MST_REMOTE_EDID;
+
+	KUNIT_EXPECT_EQ(test, dm_dp_mst_get_modes(&child.aconnector->base), 0);
+
+	KUNIT_EXPECT_EQ(test, dm_mst_test_add_remote_sink_calls, 1U);
+	KUNIT_EXPECT_PTR_EQ(test, child.aconnector->dc_sink, sink);
+	KUNIT_EXPECT_PTR_EQ(test, sink->priv, (void *)child.aconnector);
+	KUNIT_EXPECT_EQ(test, child.aconnector->mst_status & MST_REMOTE_EDID, 0);
+
+	dc_sink_release(sink);
+	dm_mst_test_fini_child(&child);
+}
+
+/**
+ * dm_mst_test_get_modes_no_edid_sink_alloc_fails - Test remote sink failure
+ * @test: KUnit test context
+ *
+ * If DC cannot add the default remote sink, dm_dp_mst_get_modes() must bail out
+ * with zero modes and leave the connector without a sink.
+ */
+static void dm_mst_test_get_modes_no_edid_sink_alloc_fails(struct kunit *test)
+{
+	struct dm_mst_test_child child;
+
+	dm_mst_test_init_child(test, &child);
+
+	KUNIT_EXPECT_EQ(test, dm_dp_mst_get_modes(&child.aconnector->base), 0);
+
+	KUNIT_EXPECT_EQ(test, dm_mst_test_add_remote_sink_calls, 1U);
+	KUNIT_EXPECT_NULL(test, child.aconnector->dc_sink);
+
+	dm_mst_test_fini_child(&child);
+}
+
+/**
+ * dm_mst_test_get_modes_no_edid_keeps_existing_sink - Test cached sink reuse
+ * @test: KUnit test context
+ *
+ * With no readable EDID but an existing remote sink, dm_dp_mst_get_modes() must
+ * keep that sink and must not ask DC for another one.
+ */
+static void dm_mst_test_get_modes_no_edid_keeps_existing_sink(struct kunit *test)
+{
+	struct dm_mst_test_child child;
+	struct dc_sink *sink;
+
+	dm_mst_test_init_child(test, &child);
+	sink = dm_mst_test_alloc_sink(test);
+	child.aconnector->dc_sink = sink;
+
+	KUNIT_EXPECT_EQ(test, dm_dp_mst_get_modes(&child.aconnector->base), 0);
+
+	KUNIT_EXPECT_EQ(test, dm_mst_test_add_remote_sink_calls, 0U);
+	KUNIT_EXPECT_PTR_EQ(test, child.aconnector->dc_sink, sink);
+
+	dc_sink_release(sink);
+	dm_mst_test_fini_child(&child);
+}
+
 /*
  * Sideband connector with a live topology manager and the DOWN_REP ready bit
  * armed, so dm_handle_mst_sideband_msg_ready_event() reaches its ack path.
@@ -1491,6 +1703,10 @@ static struct kunit_case dm_mst_types_test_cases[] = {
 	KUNIT_CASE(dm_mst_test_atomic_check_no_old_crtc),
 	/* dm_dp_mst_detect tests */
 	KUNIT_CASE(dm_mst_test_detect_unregistered),
+	/* dm_dp_mst_get_modes tests */
+	KUNIT_CASE(dm_mst_test_get_modes_no_edid_adds_default_sink),
+	KUNIT_CASE(dm_mst_test_get_modes_no_edid_sink_alloc_fails),
+	KUNIT_CASE(dm_mst_test_get_modes_no_edid_keeps_existing_sink),
 	/* CONFIG_DRM_AMD_DC_FP disabled public paths */
 #if !defined(CONFIG_DRM_AMD_DC_FP)
 	KUNIT_CASE(dm_mst_test_fp_guarded_public_stubs),
-- 
2.43.0
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.