diff mbox series

[v4,08/15] drm/tests: bridge: Provide tests for drm_bridge_reset_crtc

Message ID 20250225-bridge-connector-v4-8-7ecb07b09cad@kernel.org (mailing list archive)
State New
Headers show
Series drm/bridge: Various quality of life improvements | expand

Commit Message

Maxime Ripard Feb. 25, 2025, 4:43 p.m. UTC
Let's provide a bunch of kunit tests to make sure
drm_bridge_reset_crtc() works as expected.

Signed-off-by: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/tests/drm_bridge_test.c | 168 +++++++++++++++++++++++++++++++-
 1 file changed, 167 insertions(+), 1 deletion(-)

Comments

Dmitry Baryshkov Feb. 26, 2025, 6:43 a.m. UTC | #1
On Tue, Feb 25, 2025 at 05:43:56PM +0100, Maxime Ripard wrote:
> Let's provide a bunch of kunit tests to make sure
> drm_bridge_reset_crtc() works as expected.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/tests/drm_bridge_test.c | 168 +++++++++++++++++++++++++++++++-
>  1 file changed, 167 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
> index c0a05c459d957c3f9d281957f002f6bd36cce367..4bcc761b3ba70bfa0ccc56b0685e35f86bde63fd 100644
> --- a/drivers/gpu/drm/tests/drm_bridge_test.c
> +++ b/drivers/gpu/drm/tests/drm_bridge_test.c
> @@ -14,17 +14,57 @@ struct drm_bridge_init_priv {
>  	struct drm_plane *plane;
>  	struct drm_crtc *crtc;
>  	struct drm_encoder encoder;
>  	struct drm_bridge bridge;
>  	struct drm_connector *connector;
> +	unsigned int enable_count;
> +	unsigned int disable_count;
>  };
>  
> +static void drm_test_bridge_enable(struct drm_bridge *bridge)
> +{
> +	struct drm_bridge_init_priv *priv =
> +		container_of(bridge, struct drm_bridge_init_priv, bridge);
> +
> +	priv->enable_count++;
> +}
> +
> +static void drm_test_bridge_disable(struct drm_bridge *bridge)
> +{
> +	struct drm_bridge_init_priv *priv =
> +		container_of(bridge, struct drm_bridge_init_priv, bridge);
> +
> +	priv->disable_count++;
> +}
> +
>  static const struct drm_bridge_funcs drm_test_bridge_legacy_funcs = {
> +	.enable			= drm_test_bridge_enable,
> +	.disable		= drm_test_bridge_disable,
>  };
>  
> +static void drm_test_bridge_atomic_enable(struct drm_bridge *bridge,
> +					  struct drm_atomic_state *state)
> +{
> +	struct drm_bridge_init_priv *priv =
> +		container_of(bridge, struct drm_bridge_init_priv, bridge);
> +
> +	priv->enable_count++;
> +}
> +
> +static void drm_test_bridge_atomic_disable(struct drm_bridge *bridge,
> +					   struct drm_atomic_state *state)
> +{
> +	struct drm_bridge_init_priv *priv =
> +		container_of(bridge, struct drm_bridge_init_priv, bridge);
> +
> +	priv->disable_count++;
> +}
> +
>  static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
> +	.atomic_enable		= drm_test_bridge_atomic_enable,
>  	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
> +	.atomic_disable		= drm_test_bridge_atomic_disable,

I think we usually use paired order rather than just ordering those
alphabetically. It makese sense to group .atomic_enable with
.atomic_disable.

Other than that:

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


>  	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
>  	.atomic_reset		= drm_atomic_helper_bridge_reset,
>  };
>  
>  KUNIT_DEFINE_ACTION_WRAPPER(drm_bridge_remove_wrapper,
Dmitry Baryshkov Feb. 26, 2025, 6:46 a.m. UTC | #2
On Tue, Feb 25, 2025 at 05:43:56PM +0100, Maxime Ripard wrote:
> Let's provide a bunch of kunit tests to make sure
> drm_bridge_reset_crtc() works as expected.
> 
> Signed-off-by: Maxime Ripard <mripard@kernel.org>
> ---
>  drivers/gpu/drm/tests/drm_bridge_test.c | 168 +++++++++++++++++++++++++++++++-
>  1 file changed, 167 insertions(+), 1 deletion(-)
> 

BTW: it would be nice to verify reset of the disabled bridge for the
disabled connector.
diff mbox series

Patch

diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c b/drivers/gpu/drm/tests/drm_bridge_test.c
index c0a05c459d957c3f9d281957f002f6bd36cce367..4bcc761b3ba70bfa0ccc56b0685e35f86bde63fd 100644
--- a/drivers/gpu/drm/tests/drm_bridge_test.c
+++ b/drivers/gpu/drm/tests/drm_bridge_test.c
@@ -14,17 +14,57 @@  struct drm_bridge_init_priv {
 	struct drm_plane *plane;
 	struct drm_crtc *crtc;
 	struct drm_encoder encoder;
 	struct drm_bridge bridge;
 	struct drm_connector *connector;
+	unsigned int enable_count;
+	unsigned int disable_count;
 };
 
+static void drm_test_bridge_enable(struct drm_bridge *bridge)
+{
+	struct drm_bridge_init_priv *priv =
+		container_of(bridge, struct drm_bridge_init_priv, bridge);
+
+	priv->enable_count++;
+}
+
+static void drm_test_bridge_disable(struct drm_bridge *bridge)
+{
+	struct drm_bridge_init_priv *priv =
+		container_of(bridge, struct drm_bridge_init_priv, bridge);
+
+	priv->disable_count++;
+}
+
 static const struct drm_bridge_funcs drm_test_bridge_legacy_funcs = {
+	.enable			= drm_test_bridge_enable,
+	.disable		= drm_test_bridge_disable,
 };
 
+static void drm_test_bridge_atomic_enable(struct drm_bridge *bridge,
+					  struct drm_atomic_state *state)
+{
+	struct drm_bridge_init_priv *priv =
+		container_of(bridge, struct drm_bridge_init_priv, bridge);
+
+	priv->enable_count++;
+}
+
+static void drm_test_bridge_atomic_disable(struct drm_bridge *bridge,
+					   struct drm_atomic_state *state)
+{
+	struct drm_bridge_init_priv *priv =
+		container_of(bridge, struct drm_bridge_init_priv, bridge);
+
+	priv->disable_count++;
+}
+
 static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
+	.atomic_enable		= drm_test_bridge_atomic_enable,
 	.atomic_destroy_state	= drm_atomic_helper_bridge_destroy_state,
+	.atomic_disable		= drm_test_bridge_atomic_disable,
 	.atomic_duplicate_state	= drm_atomic_helper_bridge_duplicate_state,
 	.atomic_reset		= drm_atomic_helper_bridge_reset,
 };
 
 KUNIT_DEFINE_ACTION_WRAPPER(drm_bridge_remove_wrapper,
@@ -201,10 +241,136 @@  static struct kunit_case drm_bridge_get_current_state_tests[] = {
 static struct kunit_suite drm_bridge_get_current_state_test_suite = {
 	.name = "drm_test_bridge_get_current_state",
 	.test_cases = drm_bridge_get_current_state_tests,
 };
 
-kunit_test_suite(drm_bridge_get_current_state_test_suite);
+/*
+ * Test that an atomic bridge is properly power-cycled when calling
+ * drm_bridge_reset_crtc().
+ */
+static void drm_test_drm_bridge_reset_crtc_atomic(struct kunit *test)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_init_priv *priv;
+	struct drm_display_mode *mode;
+	struct drm_bridge *bridge;
+	int ret;
+
+	priv = drm_test_bridge_init(test, &drm_test_bridge_atomic_funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry_commit:
+	ret = drm_kunit_helper_enable_crtc_connector(test,
+						     &priv->drm, priv->crtc,
+						     priv->connector,
+						     mode,
+						     &ctx);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	bridge = &priv->bridge;
+	KUNIT_ASSERT_EQ(test, priv->enable_count, 1);
+	KUNIT_ASSERT_EQ(test, priv->disable_count, 0);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry_reset:
+	ret = drm_bridge_reset_crtc(bridge, &ctx);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_reset;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	KUNIT_EXPECT_EQ(test, priv->enable_count, 2);
+	KUNIT_EXPECT_EQ(test, priv->disable_count, 1);
+}
+
+/*
+ * Test that a non-atomic bridge is properly power-cycled when calling
+ * drm_bridge_reset_crtc().
+ */
+static void drm_test_drm_bridge_reset_crtc_legacy(struct kunit *test)
+{
+	struct drm_modeset_acquire_ctx ctx;
+	struct drm_bridge_init_priv *priv;
+	struct drm_display_mode *mode;
+	struct drm_bridge *bridge;
+	int ret;
+
+	priv = drm_test_bridge_init(test, &drm_test_bridge_legacy_funcs);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, priv);
+
+	mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
+	KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry_commit:
+	ret = drm_kunit_helper_enable_crtc_connector(test,
+						     &priv->drm, priv->crtc,
+						     priv->connector,
+						     mode,
+						     &ctx);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_commit;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	bridge = &priv->bridge;
+	KUNIT_ASSERT_EQ(test, priv->enable_count, 1);
+	KUNIT_ASSERT_EQ(test, priv->disable_count, 0);
+
+	drm_modeset_acquire_init(&ctx, 0);
+
+retry_reset:
+	ret = drm_bridge_reset_crtc(bridge, &ctx);
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(&ctx);
+		goto retry_reset;
+	}
+	KUNIT_ASSERT_EQ(test, ret, 0);
+
+	drm_modeset_drop_locks(&ctx);
+	drm_modeset_acquire_fini(&ctx);
+
+	KUNIT_EXPECT_EQ(test, priv->enable_count, 2);
+	KUNIT_EXPECT_EQ(test, priv->disable_count, 1);
+}
+
+static struct kunit_case drm_bridge_reset_crtc_tests[] = {
+	KUNIT_CASE(drm_test_drm_bridge_reset_crtc_atomic),
+	KUNIT_CASE(drm_test_drm_bridge_reset_crtc_legacy),
+	{ }
+};
+
+static struct kunit_suite drm_bridge_reset_crtc_test_suite = {
+	.name = "drm_test_bridge_reset_crtc",
+	.test_cases = drm_bridge_reset_crtc_tests,
+};
+
+kunit_test_suites(
+	&drm_bridge_get_current_state_test_suite,
+	&drm_bridge_reset_crtc_test_suite,
+);
 
 MODULE_AUTHOR("Maxime Ripard <mripard@kernel.org>");
 MODULE_DESCRIPTION("Kunit test for drm_bridge functions");
 MODULE_LICENSE("GPL");