[PATCH v2 09/11] drm/tests: Add test for drm_framebuffer_free()

[Maxime Ripard]

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 at disroot.org>
> ---
> 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
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20231025/8cdb08bd/attachment.sig>