All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Alexandru-Cosmin Gheorghe <Alexandru-Cosmin.Gheorghe@arm.com>
Cc: charles.xu@arm.com, nd@arm.com, matt.szczesiak@arm.com,
	airlied@linux.ie, liviu.dudau@arm.com,
	dri-devel@lists.freedesktop.org, maxime.ripard@bootlin.com,
	david.garbett@arm.com, seanpaul@chromium.org, lisa.wu@arm.com,
	daniel.vetter@ffwll.ch, malidp@foss.arm.com, ayan.halder@arm.com,
	james.qian.wang@arm.com
Subject: Re: [PATCH v5 9/9] drm/selftests: Add tests for drm_format_info* helpers
Date: Tue, 23 Oct 2018 15:56:57 +0200	[thread overview]
Message-ID: <20181023135657.GT324@phenom.ffwll.local> (raw)
In-Reply-To: <20181022103210.GD17598@e114479-lin.cambridge.arm.com>

On Mon, Oct 22, 2018 at 11:32:10AM +0100, Alexandru-Cosmin Gheorghe wrote:
> On Fri, Oct 19, 2018 at 05:29:15PM +0200, Daniel Vetter wrote:
> > On Fri, Oct 19, 2018 at 11:57:52AM +0100, Alexandru Gheorghe wrote:
> > > Add selftests for the following newly added functions:
> > >  - drm_format_info_block_width
> > >  - drm_format_info_block_height
> > >  - drm_format_info_min_pitch
> > > 
> > > Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@arm.com>
> > > ---
> > >  drivers/gpu/drm/selftests/Makefile            |   3 +-
> > >  .../gpu/drm/selftests/drm_modeset_selftests.h |   3 +
> > >  drivers/gpu/drm/selftests/test-drm_format.c   | 290 ++++++++++++++++++
> > >  .../drm/selftests/test-drm_modeset_common.h   |   3 +
> > >  4 files changed, 298 insertions(+), 1 deletion(-)
> > >  create mode 100644 drivers/gpu/drm/selftests/test-drm_format.c
> > > 
> > > diff --git a/drivers/gpu/drm/selftests/Makefile b/drivers/gpu/drm/selftests/Makefile
> > > index 7e6581921da0..07b4f88b422a 100644
> > > --- a/drivers/gpu/drm/selftests/Makefile
> > > +++ b/drivers/gpu/drm/selftests/Makefile
> > > @@ -1,3 +1,4 @@
> > > -test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o
> > > +test-drm_modeset-y := test-drm_modeset_common.o test-drm_plane_helper.o \
> > > +                      test-drm_format.o
> > >  
> > >  obj-$(CONFIG_DRM_DEBUG_SELFTEST) += test-drm_mm.o test-drm_modeset.o
> > > diff --git a/drivers/gpu/drm/selftests/drm_modeset_selftests.h b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> > > index 9771290ed228..4e203ac8c134 100644
> > > --- a/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> > > +++ b/drivers/gpu/drm/selftests/drm_modeset_selftests.h
> > > @@ -7,3 +7,6 @@
> > >   * Tests are executed in order by igt/drm_selftests_helper
> > >   */
> > >  selftest(check_plane_state, igt_check_plane_state)
> > > +selftest(check_drm_format_block_width, igt_check_drm_format_block_width)
> > > +selftest(check_drm_format_block_height, igt_check_drm_format_block_height)
> > > +selftest(check_drm_format_min_pitch, igt_check_drm_format_min_pitch)
> > > diff --git a/drivers/gpu/drm/selftests/test-drm_format.c b/drivers/gpu/drm/selftests/test-drm_format.c
> > > new file mode 100644
> > > index 000000000000..5abfd5e28d7d
> > > --- /dev/null
> > > +++ b/drivers/gpu/drm/selftests/test-drm_format.c
> > > @@ -0,0 +1,290 @@
> > > +// SPDX-License-Identifier: GPL-2.0
> > > +/*
> > > + * Test cases for the drm_format functions
> > > + */
> > > +
> > > +#define pr_fmt(fmt) "drm_format: " fmt
> > > +
> > > +#include <linux/errno.h>
> > > +#include <linux/kernel.h>
> > > +
> > > +#include <drm/drm_fourcc.h>
> > > +
> > > +#include "test-drm_modeset_common.h"
> > > +
> > > +int igt_check_drm_format_block_width(void *ignored)
> > > +{
> > > +	const struct drm_format_info *info = NULL;
> > > +
> > > +	/* Test invalid arguments */
> > > +	FAIL_ON(drm_format_info_block_width(info, 0) != 0);
> > > +	FAIL_ON(drm_format_info_block_width(info, -1) != 0);
> > > +	FAIL_ON(drm_format_info_block_width(info, 1) != 0);
> > > +
> > > +	/* Test 1 plane format */
> > > +	info = drm_format_info(DRM_FORMAT_XRGB4444);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_width(info, 0) != 1);
> > > +	FAIL_ON(drm_format_info_block_width(info, 1) != 0);
> > > +	FAIL_ON(drm_format_info_block_width(info, -1) != 0);
> > > +
> > > +	/* Test 2 planes format */
> > > +	info = drm_format_info(DRM_FORMAT_NV12);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_width(info, 0) != 1);
> > > +	FAIL_ON(drm_format_info_block_width(info, 1) != 1);
> > > +	FAIL_ON(drm_format_info_block_width(info, 2) != 0);
> > > +	FAIL_ON(drm_format_info_block_width(info, -1) != 0);
> > > +
> > > +	/* Test 3 planes format */
> > > +	info = drm_format_info(DRM_FORMAT_YUV422);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_width(info, 0) != 1);
> > > +	FAIL_ON(drm_format_info_block_width(info, 1) != 1);
> > > +	FAIL_ON(drm_format_info_block_width(info, 2) != 1);
> > > +	FAIL_ON(drm_format_info_block_width(info, 3) != 0);
> > > +	FAIL_ON(drm_format_info_block_width(info, -1) != 0);
> > > +
> > > +	/* Test a tiled format */
> > > +	info = drm_format_info(DRM_FORMAT_X0L0);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_width(info, 0) != 2);
> > > +	FAIL_ON(drm_format_info_block_width(info, 1) != 0);
> > > +	FAIL_ON(drm_format_info_block_width(info, -1) != 0);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int igt_check_drm_format_block_height(void *ignored)
> > > +{
> > > +	const struct drm_format_info *info = NULL;
> > > +
> > > +	/* Test invalid arguments */
> > > +	FAIL_ON(drm_format_info_block_height(info, 0) != 0);
> > > +	FAIL_ON(drm_format_info_block_height(info, -1) != 0);
> > > +	FAIL_ON(drm_format_info_block_height(info, 1) != 0);
> > > +
> > > +	/* Test 1 plane format */
> > > +	info = drm_format_info(DRM_FORMAT_XRGB4444);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_height(info, 0) != 1);
> > > +	FAIL_ON(drm_format_info_block_height(info, 1) != 0);
> > > +	FAIL_ON(drm_format_info_block_height(info, -1) != 0);
> > > +
> > > +	/* Test 2 planes format */
> > > +	info = drm_format_info(DRM_FORMAT_NV12);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_height(info, 0) != 1);
> > > +	FAIL_ON(drm_format_info_block_height(info, 1) != 1);
> > > +	FAIL_ON(drm_format_info_block_height(info, 2) != 0);
> > > +	FAIL_ON(drm_format_info_block_height(info, -1) != 0);
> > > +
> > > +	/* Test 3 planes format */
> > > +	info = drm_format_info(DRM_FORMAT_YUV422);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_height(info, 0) != 1);
> > > +	FAIL_ON(drm_format_info_block_height(info, 1) != 1);
> > > +	FAIL_ON(drm_format_info_block_height(info, 2) != 1);
> > > +	FAIL_ON(drm_format_info_block_height(info, 3) != 0);
> > > +	FAIL_ON(drm_format_info_block_height(info, -1) != 0);
> > > +
> > > +	/* Test a tiled format */
> > > +	info = drm_format_info(DRM_FORMAT_X0L0);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_block_height(info, 0) != 2);
> > > +	FAIL_ON(drm_format_info_block_height(info, 1) != 0);
> > > +	FAIL_ON(drm_format_info_block_height(info, -1) != 0);
> > > +
> > > +	return 0;
> > > +}
> > > +
> > > +int igt_check_drm_format_min_pitch(void *ignored)
> > > +{
> > > +	const struct drm_format_info *info = NULL;
> > > +
> > > +	/* Test invalid arguments */
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	/* Test 1 plane 8 bits per pixel format */
> > > +	info = drm_format_info(DRM_FORMAT_RGB332);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 640);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 1024);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 1920);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 4096);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 671);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) !=
> > > +			(uint64_t)(UINT_MAX - 1));
> > > +
> > > +	/* Test 1 plane 16 bits per pixel format */
> > > +	info = drm_format_info(DRM_FORMAT_XRGB4444);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 4);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1280);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 2048);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 3840);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 8192);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 1342);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX * 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) !=
> > > +			(uint64_t)(UINT_MAX - 1) * 2);
> > > +
> > > +	/* Test 1 plane 24 bits per pixel format */
> > > +	info = drm_format_info(DRM_FORMAT_RGB888);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 3);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 6);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1920);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 3072);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 5760);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 12288);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 2013);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX * 3);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX - 1) !=
> > > +			(uint64_t)(UINT_MAX - 1) * 3);
> > > +
> > > +	/* Test 1 plane 32 bits per pixel format */
> > > +	info = drm_format_info(DRM_FORMAT_ABGR8888);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 4);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 8);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 2560);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 4096);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 7680);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 16384);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 2684);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX * 4);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX - 1) !=
> > > +			(uint64_t)(UINT_MAX - 1) * 4);
> > > +
> > > +	/* Test 2 planes format */
> > > +	info = drm_format_info(DRM_FORMAT_NV12);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 1) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 1) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 640);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 320) != 640);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 1024);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 512) != 1024);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 1920);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 960) != 1920);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 4096);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 2048) != 4096);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 671);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 336) != 672);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, UINT_MAX / 2 + 1) !=
> > > +			(uint64_t)UINT_MAX + 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1)) !=
> > > +			(uint64_t)(UINT_MAX - 1));
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, (UINT_MAX - 1) /  2) !=
> > > +			(uint64_t)(UINT_MAX - 1));
> > > +
> > > +	/* Test 3 planes 8 bits per pixel format */
> > > +	info = drm_format_info(DRM_FORMAT_YUV422);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 3, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 1) != 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 1) != 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 2) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 2) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 640);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 320) != 320);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 320) != 320);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 1024);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 512) != 512);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 512) != 512);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 1920);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 960) != 960);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 960) != 960);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 4096);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 2048) != 2048);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 2048) != 2048);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 671);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 336) != 336);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, 336) != 336);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, UINT_MAX / 2 + 1) !=
> > > +			(uint64_t)UINT_MAX / 2 + 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, UINT_MAX / 2 + 1) !=
> > > +			(uint64_t)UINT_MAX / 2 + 1);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, (UINT_MAX - 1) / 2) !=
> > > +			(uint64_t)(UINT_MAX - 1) / 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, (UINT_MAX - 1) / 2) !=
> > > +			(uint64_t)(UINT_MAX - 1) / 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 2, (UINT_MAX - 1) / 2) !=
> > > +			(uint64_t)(UINT_MAX - 1) / 2);
> > > +
> > > +	/* Test tiled format */
> > > +	info = drm_format_info(DRM_FORMAT_X0L2);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 2) != 4);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 640) != 1280);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1024) != 2048);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1920) != 3840);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 4096) != 8192);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 671) != 1342);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) !=
> > > +			(uint64_t)UINT_MAX * 2);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX - 1) !=
> > > +			(uint64_t)(UINT_MAX - 1) * 2);
> > > +
> > > +	/* Test format with cpp/char_per_block 0 */
> > > +	info = drm_format_info(DRM_FORMAT_VUY101010);
> > > +	FAIL_ON(!info);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, -1, 0) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 1, 0) != 0);
> > > +
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, 1) != 0);
> > > +	FAIL_ON(drm_format_info_min_pitch(info, 0, UINT_MAX) != 0);
> > > +
> > > +	return 0;
> > > +}
> > 
> > Comprehensive, but also fairly boring. Anyway, it's a start, so:
> 
> Unit tests usually are :)
> 
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Can I also get your reviewed-by on the patches that adds this functions ?
> 
> > 
> > What I had in mind is maybe not quite so low-level unit testing, but
> > exercising the entire framebuffer_create machinery. The recently
> > drm_client_framebuffer_create essentially gives you that interface for
> > modules (but I guess we could also EXPORT_SYMBOL_FOR_TESTS_ONLY,
> > conditional on drm selftests being enabled, directly on
> > drm_internal_framebuffer_create).
> 
> Yeah, that's exactly what I thought I started looking at it, but I got
> discouraged, by the lack of mocking, so I decided to start small.

There's no formal mocking framework, but initializing a few bare-bones
drm_device structs has become really simple. We've done lots of work in
making all kinds of vtables optional :-)

And addfb here is very minimal, so kinda ideal to get started on this
journey.

"Test coverage not at 100% yet, let's not even bother" is kinda a
fallacy, since we do have all the CI already (at least for intel stuff),
so any test you add _will_ run, and it _will_ catch bugs. Even if it's
really lonely :-)

> > This needs some fairly minimal mocking of a drm_device with a bunch of
> > dummy functions (throw them into test-drm_modeset_common.c if you feel
> > like). Then go through a pile of valid and invalid combinations and make
> > sure your ->fb_create driver callback is only called when when it should
> > be (and not for anything invalid). I think that would yield the much more
> > interesting testing ...
> 
> This combined with EXPORT_SYMBOL_FOR_TESTS_ONLY should be fairly
> simple, but it still feels like a drop in the ocean, I can't even
> test the other functions that patch [1] touches like:
> drm_gem_fb_create_with_funcs and drm_fb_cma_get_gem_addr.
> 
> [1] https://lists.freedesktop.org/archives/dri-devel/2018-October/193671.html

We need to start somewhere imo.

Also note that igt does fairly massive testing of a lot of infrastructure
too, I think best to aim for complementary testing. fbdev not having
perfect test coverage from the go is not a big deal imo, if we can just
start with the core code and core helpers, that's already a huge step
forward.
-Daniel

> 
> > 
> > That will also pave the ground for some rather more serious addfb testing
> > in the future. And eventually even more serious testing on e.g. atomic
> > helpers.
> > 
> > Anyway, if you feel like, would be awesome to follow up a bit in that
> > direction.
> > 
> > Cheers, Daniel
> > 
> > > diff --git a/drivers/gpu/drm/selftests/test-drm_modeset_common.h b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> > > index b0065a2eb067..592a6581b189 100644
> > > --- a/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> > > +++ b/drivers/gpu/drm/selftests/test-drm_modeset_common.h
> > > @@ -14,5 +14,8 @@
> > >  #define FAIL_ON(x) FAIL((x), "%s", "FAIL_ON(" __stringify(x) ")\n")
> > >  
> > >  int igt_check_plane_state(void *ignored);
> > > +int igt_check_drm_format_block_width(void *ignored);
> > > +int igt_check_drm_format_block_height(void *ignored);
> > > +int igt_check_drm_format_min_pitch(void *ignored);
> > >  
> > >  #endif
> > > -- 
> > > 2.18.0
> > > 
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Cheers,
> Alex G

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

      reply	other threads:[~2018-10-23 13:57 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-19 10:57 [PATCH v5 0/9] Add method to describe tile/bit_level_packed formats Alexandru Gheorghe
2018-10-19 10:57 ` [PATCH v5 1/9] drm: fourcc: Convert drm_format_info kerneldoc to in-line member documentation Alexandru Gheorghe
2018-10-19 12:54   ` Maxime Ripard
2018-10-22  9:36     ` Alexandru-Cosmin Gheorghe
2018-10-19 10:57 ` [PATCH v5 2/9] drm/fourcc: Add char_per_block, block_w and block_h in drm_format_info Alexandru Gheorghe
2018-10-19 13:09   ` Brian Starkey
2018-10-22 10:07     ` Alexandru-Cosmin Gheorghe
2018-10-19 10:57 ` [PATCH v5 3/9] drm/fourcc: Add fourcc for Mali linear tiled formats Alexandru Gheorghe
2018-10-19 13:12   ` Brian Starkey
2018-10-19 10:57 ` [PATCH v5 4/9] drm: mali-dp: Enable Mali-DP tiled buffer formats Alexandru Gheorghe
2018-10-19 13:17   ` Brian Starkey
2018-10-22 10:08     ` Alexandru-Cosmin Gheorghe
2018-10-19 10:57 ` [PATCH v5 5/9] drm: Extend framebuffer_check to handle formats with cpp/char_per_block 0 Alexandru Gheorghe
2018-10-19 13:21   ` Brian Starkey
2018-10-22 10:09     ` Alexandru-Cosmin Gheorghe
2018-10-19 10:57 ` [PATCH v5 6/9] drm/fourcc: Add AFBC yuv fourccs for Mali Alexandru Gheorghe
2018-10-19 10:57 ` [PATCH v5 7/9] drm/afbc: Add AFBC modifier usage documentation Alexandru Gheorghe
2018-10-19 10:57 ` [PATCH v5 8/9] drm/selftest: Refactor test-drm_plane_helper Alexandru Gheorghe
2018-10-19 15:14   ` Daniel Vetter
2018-10-22  9:40     ` Alexandru-Cosmin Gheorghe
2018-10-19 10:57 ` [PATCH v5 9/9] drm/selftests: Add tests for drm_format_info* helpers Alexandru Gheorghe
2018-10-19 15:29   ` Daniel Vetter
2018-10-22 10:32     ` Alexandru-Cosmin Gheorghe
2018-10-23 13:56       ` Daniel Vetter [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181023135657.GT324@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=Alexandru-Cosmin.Gheorghe@arm.com \
    --cc=airlied@linux.ie \
    --cc=ayan.halder@arm.com \
    --cc=charles.xu@arm.com \
    --cc=daniel.vetter@ffwll.ch \
    --cc=david.garbett@arm.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=james.qian.wang@arm.com \
    --cc=lisa.wu@arm.com \
    --cc=liviu.dudau@arm.com \
    --cc=malidp@foss.arm.com \
    --cc=matt.szczesiak@arm.com \
    --cc=maxime.ripard@bootlin.com \
    --cc=nd@arm.com \
    --cc=seanpaul@chromium.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.