[PATCH v8 1/3] drm/tests: bridge: convert to devm_drm_bridge_alloc() API
Anusha Srivatsa
asrivats at redhat.com
Wed May 28 22:41:28 UTC 2025
On Fri, May 16, 2025 at 12:48 PM Luca Ceresoli <luca.ceresoli at bootlin.com>
wrote:
> Use the new DRM bridge allocation API, which is the only supported now, for
> the kunit tests.
>
> This change is more massive than for the typical DRM bridge driver because
> struct drm_bridge_init_priv currently embeds a struct drm_bridge, which is
> not supported anymore. We new have to use devm_drm_bridge_alloc() to
>
typo ^^^s/new/now.
Thanks,
Anusha
dynamically allocate a "private driver struct", which is a bit awkward here
> because there is no real bridge driver. Thus let's add a "dummy" DRM bridge
> struct to represent it.
>
> As a nice cleanup we can now move the enable_count and disable_count
> members, which are counting bridge-specific events, into the new "private
> driver struct" (and avoid adding new unnecessary indirections).
>
> Also add a trivial bridge_to_dummy_bridge() just like many drivers do.
>
> Signed-off-by: Luca Ceresoli <luca.ceresoli at bootlin.com>
>
> ---
>
> This patch was added in v8.
> ---
> drivers/gpu/drm/tests/drm_bridge_test.c | 95
> +++++++++++++++++++--------------
> 1 file changed, 55 insertions(+), 40 deletions(-)
>
> diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c
> b/drivers/gpu/drm/tests/drm_bridge_test.c
> index
> ff88ec2e911c9cc9a718483f09d4c764f45f991a..f3a625c536f610dc8560b56531056df7c613f564
> 100644
> --- a/drivers/gpu/drm/tests/drm_bridge_test.c
> +++ b/drivers/gpu/drm/tests/drm_bridge_test.c
> @@ -10,31 +10,45 @@
>
> #include <kunit/test.h>
>
> +/*
> + * Mimick the typical struct defined by a bridge driver, which embeds a
> + * bridge plus other fields.
> + *
> + * Having at least one member before @bridge ensures we test non-zero
> + * @bridge offset.
> + */
> +struct dummy_drm_bridge {
> + unsigned int enable_count;
> + unsigned int disable_count;
> + struct drm_bridge bridge;
> +};
> +
> struct drm_bridge_init_priv {
> struct drm_device drm;
> struct drm_plane *plane;
> struct drm_crtc *crtc;
> struct drm_encoder encoder;
> - struct drm_bridge bridge;
> + struct dummy_drm_bridge *test_bridge;
> struct drm_connector *connector;
> - unsigned int enable_count;
> - unsigned int disable_count;
> };
>
> +static struct dummy_drm_bridge *bridge_to_dummy_bridge(struct drm_bridge
> *bridge)
> +{
> + return container_of(bridge, struct dummy_drm_bridge, bridge);
> +}
> +
> 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);
> + struct dummy_drm_bridge *dummy_br = bridge_to_dummy_bridge(bridge);
>
> - priv->enable_count++;
> + dummy_br->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);
> + struct dummy_drm_bridge *dummy_br = bridge_to_dummy_bridge(bridge);
>
> - priv->disable_count++;
> + dummy_br->disable_count++;
> }
>
> static const struct drm_bridge_funcs drm_test_bridge_legacy_funcs = {
> @@ -45,19 +59,17 @@ static const struct drm_bridge_funcs
> drm_test_bridge_legacy_funcs = {
> 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);
> + struct dummy_drm_bridge *dummy_br = bridge_to_dummy_bridge(bridge);
>
> - priv->enable_count++;
> + dummy_br->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);
> + struct dummy_drm_bridge *dummy_br = bridge_to_dummy_bridge(bridge);
>
> - priv->disable_count++;
> + dummy_br->disable_count++;
> }
>
> static const struct drm_bridge_funcs drm_test_bridge_atomic_funcs = {
> @@ -102,6 +114,10 @@ drm_test_bridge_init(struct kunit *test, const struct
> drm_bridge_funcs *funcs)
> if (IS_ERR(priv))
> return ERR_CAST(priv);
>
> + priv->test_bridge = devm_drm_bridge_alloc(dev, struct
> dummy_drm_bridge, bridge, funcs);
> + if (IS_ERR(priv->test_bridge))
> + return ERR_CAST(priv->test_bridge);
> +
> drm = &priv->drm;
> priv->plane = drm_kunit_helper_create_primary_plane(test, drm,
> NULL,
> @@ -125,9 +141,8 @@ drm_test_bridge_init(struct kunit *test, const struct
> drm_bridge_funcs *funcs)
>
> enc->possible_crtcs = drm_crtc_mask(priv->crtc);
>
> - bridge = &priv->bridge;
> + bridge = &priv->test_bridge->bridge;
> bridge->type = DRM_MODE_CONNECTOR_VIRTUAL;
> - bridge->funcs = funcs;
>
> ret = drm_kunit_bridge_add(test, bridge);
> if (ret)
> @@ -173,7 +188,7 @@ static void
> drm_test_drm_bridge_get_current_state_atomic(struct kunit *test)
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, state);
>
> retry_commit:
> - bridge = &priv->bridge;
> + bridge = &priv->test_bridge->bridge;
> bridge_state = drm_atomic_get_bridge_state(state, bridge);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, bridge_state);
>
> @@ -228,7 +243,7 @@ static void
> drm_test_drm_bridge_get_current_state_legacy(struct kunit *test)
> * locking. The function would return NULL in all cases anyway,
> * so we don't really have any concurrency to worry about.
> */
> - bridge = &priv->bridge;
> + bridge = &priv->test_bridge->bridge;
> KUNIT_EXPECT_NULL(test, drm_bridge_get_current_state(bridge));
> }
>
> @@ -253,7 +268,7 @@ static void
> drm_test_drm_bridge_helper_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;
> + struct dummy_drm_bridge *dummy_br;
> int ret;
>
> priv = drm_test_bridge_init(test, &drm_test_bridge_atomic_funcs);
> @@ -279,14 +294,14 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_atomic(struct kunit *test)
> 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);
> + dummy_br = priv->test_bridge;
> + KUNIT_ASSERT_EQ(test, dummy_br->enable_count, 1);
> + KUNIT_ASSERT_EQ(test, dummy_br->disable_count, 0);
>
> drm_modeset_acquire_init(&ctx, 0);
>
> retry_reset:
> - ret = drm_bridge_helper_reset_crtc(bridge, &ctx);
> + ret = drm_bridge_helper_reset_crtc(&dummy_br->bridge, &ctx);
> if (ret == -EDEADLK) {
> drm_modeset_backoff(&ctx);
> goto retry_reset;
> @@ -296,8 +311,8 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_atomic(struct kunit *test)
> 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);
> + KUNIT_EXPECT_EQ(test, dummy_br->enable_count, 2);
> + KUNIT_EXPECT_EQ(test, dummy_br->disable_count, 1);
> }
>
> /*
> @@ -309,7 +324,7 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
> struct drm_modeset_acquire_ctx ctx;
> struct drm_bridge_init_priv *priv;
> struct drm_display_mode *mode;
> - struct drm_bridge *bridge;
> + struct dummy_drm_bridge *dummy_br;
> int ret;
>
> priv = drm_test_bridge_init(test, &drm_test_bridge_atomic_funcs);
> @@ -318,14 +333,14 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
> mode = drm_kunit_display_mode_from_cea_vic(test, &priv->drm, 16);
> KUNIT_ASSERT_NOT_ERR_OR_NULL(test, mode);
>
> - bridge = &priv->bridge;
> - KUNIT_ASSERT_EQ(test, priv->enable_count, 0);
> - KUNIT_ASSERT_EQ(test, priv->disable_count, 0);
> + dummy_br = priv->test_bridge;
> + KUNIT_ASSERT_EQ(test, dummy_br->enable_count, 0);
> + KUNIT_ASSERT_EQ(test, dummy_br->disable_count, 0);
>
> drm_modeset_acquire_init(&ctx, 0);
>
> retry_reset:
> - ret = drm_bridge_helper_reset_crtc(bridge, &ctx);
> + ret = drm_bridge_helper_reset_crtc(&dummy_br->bridge, &ctx);
> if (ret == -EDEADLK) {
> drm_modeset_backoff(&ctx);
> goto retry_reset;
> @@ -335,8 +350,8 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_atomic_disabled(struct kunit *
> drm_modeset_drop_locks(&ctx);
> drm_modeset_acquire_fini(&ctx);
>
> - KUNIT_EXPECT_EQ(test, priv->enable_count, 0);
> - KUNIT_EXPECT_EQ(test, priv->disable_count, 0);
> + KUNIT_EXPECT_EQ(test, dummy_br->enable_count, 0);
> + KUNIT_EXPECT_EQ(test, dummy_br->disable_count, 0);
> }
>
> /*
> @@ -348,7 +363,7 @@ static void
> drm_test_drm_bridge_helper_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;
> + struct dummy_drm_bridge *dummy_br;
> int ret;
>
> priv = drm_test_bridge_init(test, &drm_test_bridge_legacy_funcs);
> @@ -374,14 +389,14 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
> 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);
> + dummy_br = priv->test_bridge;
> + KUNIT_ASSERT_EQ(test, dummy_br->enable_count, 1);
> + KUNIT_ASSERT_EQ(test, dummy_br->disable_count, 0);
>
> drm_modeset_acquire_init(&ctx, 0);
>
> retry_reset:
> - ret = drm_bridge_helper_reset_crtc(bridge, &ctx);
> + ret = drm_bridge_helper_reset_crtc(&dummy_br->bridge, &ctx);
> if (ret == -EDEADLK) {
> drm_modeset_backoff(&ctx);
> goto retry_reset;
> @@ -391,8 +406,8 @@ static void
> drm_test_drm_bridge_helper_reset_crtc_legacy(struct kunit *test)
> 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);
> + KUNIT_EXPECT_EQ(test, dummy_br->enable_count, 2);
> + KUNIT_EXPECT_EQ(test, dummy_br->disable_count, 1);
> }
>
> static struct kunit_case drm_bridge_helper_reset_crtc_tests[] = {
>
> --
> 2.49.0
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20250528/2fbeefc2/attachment-0001.htm>
More information about the dri-devel
mailing list