[PATCH] drm/tests: bridge: check the bridge chain callback order

[PATCH] drm/tests: bridge: check the bridge chain callback order

[Oarora Etimis]

drm_atomic_bridge_chain_pre_enable() and
drm_atomic_bridge_chain_post_disable() document the order in which a
chain's bridges get their callback called, but nothing tests it. That
order has been wrong before: commit e18aeeda0b69 had to fix both
directions at once.

Add a KUnit test for the example those comments give, and for the chain
where no bridge sets @pre_enable_prev_first.

Signed-off-by: Oarora Etimis <oaroraetimis@gmail.com>
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 112 ++++++++++++++++++++++++++++++++
 1 file changed, 112 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index 15583ade8494..e910f22ee581 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -20,6 +20,9 @@
 
 #include "drm_kunit_edid.h"
 
+/* Chain length of the callback order test, and size of its call log. */
+#define DRM_TEST_BRIDGE_CHAIN_LEN 5
+
 /*
  * Mimick the typical "private" struct defined by a bridge driver, which
  * embeds a bridge plus other fields.
@@ -31,6 +34,8 @@ struct drm_bridge_priv {
 	unsigned int enable_count;
 	unsigned int disable_count;
 	struct drm_bridge bridge;
+	/* Position in the chain, as an index into test_bridges */
+	unsigned int chain_index;
 	void *data;
 };
 
@@ -59,6 +64,11 @@ struct drm_bridge_chain_priv {
 	 *                of which the first @num_bridges entries are valid.
 	 */
 	struct drm_bridge_priv **test_bridges;
+
+	/** @call_log: @test_bridges indices of the bridges whose function was called, in order */
+	unsigned int call_log[DRM_TEST_BRIDGE_CHAIN_LEN];
+	/** @call_log_len: number of calls logged, may exceed the size of @call_log */
+	unsigned int call_log_len;
 };
 
 static struct drm_bridge_priv *bridge_to_priv(struct drm_bridge *bridge)
@@ -99,6 +109,27 @@ static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
 	.atomic_create_state	= drm_atomic_helper_bridge_create_state,
 };
 
+/* @data points to the drm_bridge_chain_priv for bridges using these funcs */
+static void drm_test_bridge_chain_atomic_log(struct drm_bridge *bridge,
+					     struct drm_atomic_commit *state)
+{
+	struct drm_bridge_priv *bridge_priv = bridge_to_priv(bridge);
+	struct drm_bridge_chain_priv *priv = bridge_priv->data;
+
+	if (priv->call_log_len < ARRAY_SIZE(priv->call_log))
+		priv->call_log[priv->call_log_len] = bridge_priv->chain_index;
+
+	priv->call_log_len++;
+}
+
+static const struct drm_bridge_funcs drm_test_bridge_chain_funcs = {
+	.atomic_pre_enable	= drm_test_bridge_chain_atomic_log,
+	.atomic_post_disable	= drm_test_bridge_chain_atomic_log,
+	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
+	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
+	.atomic_create_state	= drm_atomic_helper_bridge_create_state,
+};
+
 static int dummy_clear_infoframe(struct drm_bridge *bridge)
 {
 	return 0;
@@ -459,6 +490,7 @@ drm_test_bridge_chain_init(struct kunit *test, unsigned int num_bridges,
 		return ERR_PTR(-ENOMEM);
 
 	priv->num_bridges = num_bridges;
+	priv->call_log_len = 0;
 
 	for (i = 0; i < num_bridges; i++) {
 		priv->test_bridges[i] = devm_drm_bridge_alloc(dev, struct drm_bridge_priv,
@@ -467,6 +499,7 @@ drm_test_bridge_chain_init(struct kunit *test, unsigned int num_bridges,
 			return ERR_CAST(priv->test_bridges[i]);
 
 		priv->test_bridges[i]->data = priv;
+		priv->test_bridges[i]->chain_index = i;
 	}
 
 	priv->plane = drm_kunit_helper_create_primary_plane(test, drm, NULL, NULL,
@@ -1357,6 +1390,84 @@ static void drm_test_bridge_rgb_yuv_no_path(struct kunit *test)
 	drm_modeset_acquire_fini(&ctx);
 }
 
+/* B, D and E ask for prev first: the example the documentation gives. */
+static const bool bde_prev_first[DRM_TEST_BRIDGE_CHAIN_LEN] = { false, true, false, true, true };
+static const unsigned int bde_pre_enable[DRM_TEST_BRIDGE_CHAIN_LEN] = { 2, 3, 4, 0, 1 };
+static const unsigned int bde_post_disable[DRM_TEST_BRIDGE_CHAIN_LEN] = { 1, 0, 4, 3, 2 };
+
+/* No bridge does: both walks are plain reversals. */
+static const bool none_prev_first[DRM_TEST_BRIDGE_CHAIN_LEN] = { false };
+static const unsigned int none_pre_enable[DRM_TEST_BRIDGE_CHAIN_LEN] = { 4, 3, 2, 1, 0 };
+static const unsigned int none_post_disable[DRM_TEST_BRIDGE_CHAIN_LEN] = { 0, 1, 2, 3, 4 };
+
+/* Order documented by drm_atomic_bridge_chain_pre_enable()/_post_disable(). */
+static void drm_test_bridge_chain_order(struct kunit *test,
+					const bool *prev_first,
+					const unsigned int *expected_pre_enable,
+					const unsigned int *expected_post_disable)
+{
+	static const struct drm_bridge_funcs *funcs[DRM_TEST_BRIDGE_CHAIN_LEN] = {
+		&drm_test_bridge_chain_funcs,
+		&drm_test_bridge_chain_funcs,
+		&drm_test_bridge_chain_funcs,
+		&drm_test_bridge_chain_funcs,
+		&drm_test_bridge_chain_funcs,
+	};
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_chain_priv *priv;
+	struct drm_atomic_commit *state;
+	unsigned int i;
+
+	priv = drm_test_bridge_chain_init(test, ARRAY_SIZE(funcs), funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	for (i = 0; i < ARRAY_SIZE(funcs); i++)
+		priv->test_bridges[i]->bridge.pre_enable_prev_first = prev_first[i];
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+	/* The chain helpers skip the callbacks when @state is NULL. */
+	state = drm_kunit_helper_atomic_state_alloc(test, &priv->drm, &ctx);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
+
+	drm_atomic_bridge_chain_pre_enable(&priv->test_bridges[0]->bridge, state);
+	KUNIT_ASSERT_EQ(test, priv->call_log_len, DRM_TEST_BRIDGE_CHAIN_LEN);
+	KUNIT_EXPECT_MEMEQ(test, priv->call_log, expected_pre_enable,
+			   sizeof(priv->call_log));
+
+	priv->call_log_len = 0;
+	drm_atomic_bridge_chain_post_disable(&priv->test_bridges[0]->bridge, state);
+	KUNIT_ASSERT_EQ(test, priv->call_log_len, DRM_TEST_BRIDGE_CHAIN_LEN);
+	KUNIT_EXPECT_MEMEQ(test, priv->call_log, expected_post_disable,
+			   sizeof(priv->call_log));
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+}
+
+static void drm_test_bridge_chain_callback_order(struct kunit *test)
+{
+	drm_test_bridge_chain_order(test, bde_prev_first, bde_pre_enable,
+				    bde_post_disable);
+}
+
+static void drm_test_bridge_chain_callback_order_no_flags(struct kunit *test)
+{
+	drm_test_bridge_chain_order(test, none_prev_first, none_pre_enable,
+				    none_post_disable);
+}
+
+static struct kunit_case drm_bridge_chain_tests[] = {
+	KUNIT_CASE(drm_test_bridge_chain_callback_order),
+	KUNIT_CASE(drm_test_bridge_chain_callback_order_no_flags),
+	{ }
+};
+
+static struct kunit_suite drm_bridge_chain_test_suite = {
+	.name = "drm_bridge_chain",
+	.test_cases = drm_bridge_chain_tests,
+};
+
 static struct kunit_case drm_bridge_bus_fmt_tests[] = {
 	KUNIT_CASE(drm_test_bridge_rgb_yuv_rgb),
 	KUNIT_CASE(drm_test_bridge_must_convert_to_yuv444),
@@ -1376,6 +1487,7 @@ kunit_test_suites(
 	&drm_bridge_helper_reset_crtc_test_suite,
 	&drm_bridge_alloc_test_suite,
 	&drm_bridge_bus_fmt_test_suite,
+	&drm_bridge_chain_test_suite,
 );
 
 MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");

---
base-commit: c88084027d0e922dd925b96e5a0d184ba04a8f43
change-id: 20261113-bridge-order-e8f4efb0b8fe

Best regards,
--  
Oarora Etimis <oaroraetimis@gmail.com>