On Fri, May 16, 2025 at 06:48:39PM +0200, Luca Ceresoli wrote: > Add KUnit tests for the newly introduced devm_drm_bridge_alloc(). > > Signed-off-by: Luca Ceresoli <[email protected]> > > --- > > Changed in v8: > - rebase on new patch converting drm_bridge_test.c to > devm_drm_bridge_alloc() > - add check that bridge is removed (thanks to the .destroy callback) > - add a check with get/put > > Changed in v7: > - rebase on current drm-misc-next, which now has a drm_bridge_test.c file > - cleanup commit message > > Changed in v6: > - update to new devm_drm_bridge_alloc() API > - remove drm_test_drm_bridge_put test, not straightforward to write with > the new API and the current notification mechanism > - do not allocate a drm_device: a bridge is allocated without one > - rename some identifiers for easier code reading > > This patch was added in v5. > --- > drivers/gpu/drm/tests/drm_bridge_test.c | 84 > +++++++++++++++++++++++++++++++++ > 1 file changed, 84 insertions(+) > > diff --git a/drivers/gpu/drm/tests/drm_bridge_test.c > b/drivers/gpu/drm/tests/drm_bridge_test.c > index > f3a625c536f610dc8560b56531056df7c613f564..32db3a82fe6d14a3e9d6536bcf4b19f1bc65969a > 100644 > --- a/drivers/gpu/drm/tests/drm_bridge_test.c > +++ b/drivers/gpu/drm/tests/drm_bridge_test.c > @@ -8,6 +8,7 @@ > #include <drm/drm_bridge_helper.h> > #include <drm/drm_kunit_helpers.h> > > +#include <kunit/device.h> > #include <kunit/test.h> > > /* > @@ -21,6 +22,7 @@ struct dummy_drm_bridge { > unsigned int enable_count; > unsigned int disable_count; > struct drm_bridge bridge; > + void *data; > }; > > struct drm_bridge_init_priv { > @@ -422,11 +424,93 @@ static struct kunit_suite > drm_bridge_helper_reset_crtc_test_suite = { > .test_cases = drm_bridge_helper_reset_crtc_tests, > }; > > +struct drm_bridge_alloc_test_ctx {
drm_bridge_alloc_priv
> + struct device *dev;
> + struct dummy_drm_bridge *dummy_br;
> + bool destroyed;
This can be in drm_bridge_priv
> +};
> +
> +static void dummy_drm_bridge_destroy(struct drm_bridge *bridge)
> +{
> + struct dummy_drm_bridge *dummy_br = bridge_to_dummy_bridge(bridge);
> + struct drm_bridge_alloc_test_ctx *ctx = (struct
> drm_bridge_alloc_test_ctx *)dummy_br->data;
> +
> + ctx->destroyed = true;
> +}
> +
> +static const struct drm_bridge_funcs drm_bridge_dummy_funcs = {
> + .destroy = dummy_drm_bridge_destroy,
> +};
And same here, you don't need to create yet another function set, just
add it to the existing ones.
> +static int drm_test_bridge_alloc_init(struct kunit *test)
> +{
> + struct drm_bridge_alloc_test_ctx *ctx;
> +
> + ctx = kunit_kzalloc(test, sizeof(*ctx), GFP_KERNEL);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx);
> +
> + ctx->dev = kunit_device_register(test, "drm-bridge-dev");
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dev);
> +
> + test->priv = ctx;
> +
> + ctx->dummy_br = devm_drm_bridge_alloc(ctx->dev, struct
> dummy_drm_bridge, bridge,
> + &drm_bridge_dummy_funcs);
> + KUNIT_ASSERT_NOT_ERR_OR_NULL(test, ctx->dummy_br);
> +
> + ctx->dummy_br->data = ctx;
> +
> + KUNIT_ASSERT_FALSE(test, ctx->destroyed);
> +
> + return 0;
> +}
> +
> +static void drm_test_drm_bridge_alloc_basic(struct kunit *test)
You need a comment explaining what this test is about
> +{
> + struct drm_bridge_alloc_test_ctx *ctx = test->priv;
> +
> + KUNIT_ASSERT_FALSE(test, ctx->destroyed);
> +
> + kunit_device_unregister(test, ctx->dev);
> + KUNIT_ASSERT_TRUE(test, ctx->destroyed);
EXPECT
> +}
> +
> +static void drm_test_drm_bridge_alloc_get_put(struct kunit *test)
Comment here
> +{
> + struct drm_bridge_alloc_test_ctx *ctx = test->priv;
> +
> + KUNIT_ASSERT_FALSE(test, ctx->destroyed);
> +
> + drm_bridge_get(&ctx->dummy_br->bridge);
> + KUNIT_ASSERT_FALSE(test, ctx->destroyed);
EXPECT
> + kunit_device_unregister(test, ctx->dev);
> + KUNIT_ASSERT_FALSE(test, ctx->destroyed);
Ditto
> + drm_bridge_put(&ctx->dummy_br->bridge);
> + KUNIT_ASSERT_TRUE(test, ctx->destroyed);
Ditto
> +}
> +
> +static struct kunit_case drm_bridge_alloc_tests[] = {
> + KUNIT_CASE(drm_test_drm_bridge_alloc_basic),
> + KUNIT_CASE(drm_test_drm_bridge_alloc_get_put),
> + { }
> +};
> +
> +static struct kunit_suite drm_bridge_alloc_test_suite = {
> + .name = "drm_bridge_alloc",
> + .init = drm_test_bridge_alloc_init,
> + .test_cases = drm_bridge_alloc_tests,
> +};
> +
> kunit_test_suites(
> &drm_bridge_get_current_state_test_suite,
> &drm_bridge_helper_reset_crtc_test_suite,
> + &drm_bridge_alloc_test_suite,
> );
>
> MODULE_AUTHOR("Maxime Ripard <[email protected]>");
> +MODULE_AUTHOR("Luca Ceresoli <[email protected]>");
> +
> MODULE_DESCRIPTION("Kunit test for drm_bridge functions");
> MODULE_LICENSE("GPL");
Looks good otherwise, thanks
Maxime
signature.asc
Description: PGP signature
