On Tue, Oct 24, 2023 at 04:10:00PM -0300, Carlos Eduardo Gallo Filho wrote:
> Add a single KUnit test case for the drm_framebuffer_free function.
>
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@xxxxxxxxxxx>
> ---
> v2:
> - Reorder kunit cases alphabetically.
> ---
> drivers/gpu/drm/tests/drm_framebuffer_test.c | 49 ++++++++++++++++++++
> 1 file changed, 49 insertions(+)
>
> diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
> index eedd5e920279..dbe412d0dca4 100644
> --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
> +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
> @@ -602,10 +602,59 @@ static void drm_test_framebuffer_init(struct kunit *test)
> drm_framebuffer_cleanup(&fb1);
> }
>
> +static void destroy_free_mock(struct drm_framebuffer *fb)
> +{
> + struct drm_framebuffer_test_priv *priv = container_of(fb->dev, typeof(*priv), dev);
> + int *buffer_freed = priv->private;
> + *buffer_freed = 1;
> +}
Yeah, definitely not a fan of a pointer being used for multiple things
depending on the caller.
Just add another boolean to drm_framebuffer_test_priv, that way it will
be obvious to anyone, and it will be simpler if we ever rework that part
of the tests.
> +static struct drm_framebuffer_funcs framebuffer_funcs_free_mock = {
> + .destroy = destroy_free_mock,
> +};
> +
> +static void drm_test_framebuffer_free(struct kunit *test)
I'm sure you get the idea now :)
> +{
> + struct drm_framebuffer_test_priv *priv = test->priv;
> + struct drm_device *dev = &priv->dev;
> + struct drm_mode_object *obj;
> + struct drm_framebuffer fb = {
> + .dev = dev,
> + .funcs = &framebuffer_funcs_free_mock,
> + };
> + int buffer_freed = 0;
> + int id, ret;
> +
> + priv->private = &buffer_freed;
> +
> + /*
> + * Case where the fb isn't registered. Just test if
> + * drm_framebuffer_free calls fb->funcs->destroy
> + */
> + drm_framebuffer_free(&fb.base.refcount);
> + KUNIT_EXPECT_EQ(test, buffer_freed, 1);
> +
> + buffer_freed = 0;
> +
> + ret = drm_mode_object_add(dev, &fb.base, DRM_MODE_OBJECT_FB);
> + KUNIT_ASSERT_EQ(test, ret, 0);
> + id = fb.base.id;
> +
> + /* Now, test with the fb registered, that must end unregistered */
> + drm_framebuffer_free(&fb.base.refcount);
> + KUNIT_EXPECT_EQ(test, fb.base.id, 0);
> + KUNIT_EXPECT_EQ(test, buffer_freed, 1);
> +
> + /* Test if the old id of the fb was really removed from the idr pool */
> + obj = drm_mode_object_find(dev, NULL, id, DRM_MODE_OBJECT_FB);
> + KUNIT_EXPECT_PTR_EQ(test, obj, NULL);
> +}
And for that as well, these should all be separate tests.
Maxime
Attachment:
signature.asc
Description: PGP signature
