[PATCH v2 0/4] Add documentation and KUnit tests for functions on drm_framebuffer.c

[PATCH v2 0/4] Add documentation and KUnit tests for functions on drm_framebuffer.c

[Carlos Eduardo Gallo Filho]

This series adds documentation and unit tests for
drm_framebuffer_check_src_coords() and drm_internal_framebuffer_create()
functions on drm_framebuffer.c, including new parameters to the, until
then, only existent test.

Many thanks,
Carlos

---

v2:
    - Replaced strcpy to strscpy (Maxime)
    - Added comments (Maxime)
    - Ordered KUNIT_CASE_PARAM alphabetically (Maxime)
    - Replaced some parameter cases description with more verbose ones

---

Carlos Eduardo Gallo Filho (4):
  drm: Add kernel-doc for drm_framebuffer_check_src_coords()
  drm/tests: Add test for drm_framebuffer_check_src_coords()
  drm/tests: Add parameters to the drm_test_framebuffer_create test
  drm/tests: Add test case for drm_internal_framebuffer_create()

 drivers/gpu/drm/drm_framebuffer.c            |  16 +-
 drivers/gpu/drm/tests/drm_framebuffer_test.c | 178 ++++++++++++++++++-
 2 files changed, 192 insertions(+), 2 deletions(-)

-- 
2.41.0

[PATCH v2 1/4] drm: Add kernel-doc for drm_framebuffer_check_src_coords()

[Carlos Eduardo Gallo Filho]

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 drivers/gpu/drm/drm_framebuffer.c | 16 +++++++++++++++-
 1 file changed, 15 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
index aff3746dedfb..49df3ca3b3ee 100644
--- a/drivers/gpu/drm/drm_framebuffer.c
+++ b/drivers/gpu/drm/drm_framebuffer.c
@@ -73,6 +73,21 @@
  * drm_framebuffer.
  */
 
+/**
+ * drm_framebuffer_check_src_coords - check if the source with given
+ * coordinates and sizes is inside the framebuffer
+ * @src_x: source x coordinate
+ * @src_y: source y coordinate
+ * @src_w: source width
+ * @src_h: source height
+ * @fb: pointer to the framebuffer to check
+ *
+ * This function checks if an object with the given set of coordinates and
+ * sizes fits inside the framebuffer by looking at its size.
+ *
+ * Returns:
+ * Zero on success, negative errno on failure.
+ */
 int drm_framebuffer_check_src_coords(uint32_t src_x, uint32_t src_y,
 				     uint32_t src_w, uint32_t src_h,
 				     const struct drm_framebuffer *fb)
@@ -82,7 +97,6 @@ int drm_framebuffer_check_src_coords(uint32_t src_x, uint32_t src_y,
 	fb_width = fb->width << 16;
 	fb_height = fb->height << 16;
 
-	/* Make sure source coordinates are inside the fb. */
 	if (src_w > fb_width ||
 	    src_x > fb_width - src_w ||
 	    src_h > fb_height ||
-- 
2.41.0

[PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[Carlos Eduardo Gallo Filho]

Add a parametrized test for the drm_framebuffer_check_src_coords function.

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 drivers/gpu/drm/tests/drm_framebuffer_test.c | 126 +++++++++++++++++++
 1 file changed, 126 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index f759d9f3b76e..ee92120cd8e9 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -9,6 +9,7 @@
 
 #include <drm/drm_device.h>
 #include <drm/drm_mode.h>
+#include <drm/drm_framebuffer.h>
 #include <drm/drm_fourcc.h>
 #include <drm/drm_print.h>
 
@@ -366,7 +367,132 @@ static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, c
 KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
 		  drm_framebuffer_test_to_desc);
 
+/* Parameters for testing drm_framebuffer_check_src_coords function */
+struct check_src_coords_case {
+	const char *name; /* Description of the parameter case */
+	const int expect; /* Expected returned value by the function */
+
+	/* All function args */
+	const uint32_t src_x;
+	const uint32_t src_y;
+	const uint32_t src_w;
+	const uint32_t src_h;
+	const struct drm_framebuffer fb;
+};
+
+static const struct check_src_coords_case check_src_coords_cases[] = {
+	/* Regular case where the source just fit in the framebuffer */
+	{ .name = "source inside framebuffer with normal sizes and coordinates",
+	  .expect = 0,
+	  .src_x = 500 << 16, .src_y = 700 << 16,
+	  .src_w = 100 << 16, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	{ .name = "out of bound by both x and y with normal sizes and coordinates",
+	  .expect = -ENOSPC,
+	  .src_x = (500 << 16) + 1, .src_y = (700 << 16) + 1,
+	  .src_w = 100 << 16, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	/* From here, cases involving only x axis */
+	{ .name = "out of bound by x with normal width and x",
+	  .expect = -ENOSPC,
+	  .src_x = (500 << 16) + 1, .src_y = 700 << 16,
+	  .src_w = 100 << 16, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	{ .name = "out of bound by x due to source width higher than framebuffer width",
+	  .expect = -ENOSPC,
+	  .src_x = 0, .src_y = 700 << 16,
+	  .src_w = (600 << 16) + 1, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	/* Source fullfill framebuffer width just by its width */
+	{ .name = "source inside framebuffer with its width equal framebuffer width and zero x",
+	  .expect = 0,
+	  .src_x = 0, .src_y = 700 << 16,
+	  .src_w = 600 << 16, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	/*
+	 * Source fullfill framebuffer with its width and get out of
+	 * bound by having a non-zero x coordinate
+	 */
+	{ .name = "out of bound by x due to source width equal framebuffer width and non-zero x",
+	  .expect = -ENOSPC,
+	  .src_x = 1, .src_y = 700 << 16,
+	  .src_w = 600 << 16, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	{ .name = "out of bound by x due to x coordinate higher than framebuffer width",
+	  .expect = -ENOSPC,
+	  .src_x = (600 << 16) + 1, .src_y = 700 << 16,
+	  .src_w = 0, .src_h = 100 << 16,
+	  .fb = { .width = 600, .height = 800 }
+	},
+	/*
+	 * From here, the same of previous cases involving x axis but with
+	 * src_x and src_w values swapped with src_y and src_h, so we can cover
+	 * the same cases for the y axis
+	 */
+	{ .name = "out of bound by y with normal height and y",
+	  .expect = -ENOSPC,
+	  .src_x = 700 << 16, .src_y = (500 << 16) + 1,
+	  .src_w = 100 << 16, .src_h = 100 << 16,
+	  .fb = { .width = 800, .height = 600 }
+	},
+	{ .name = "out of bound by y due to source height higher than framebuffer height",
+	  .expect = -ENOSPC,
+	  .src_x = 700 << 16, .src_y = 0,
+	  .src_w = 100 << 16, .src_h = (600 << 16) + 1,
+	  .fb = { .width = 800, .height = 600 }
+	},
+	{ .name = "source inside framebuffer with its height equal framebuffer height and zero y",
+	  .expect = 0,
+	  .src_x = 700 << 16, .src_y = 0,
+	  .src_w = 100 << 16, .src_h = 600 << 16,
+	  .fb = { .width = 800, .height = 600 }
+	},
+	{ .name = "out of bound by y due to source height equal framebuffer height and non-zero y",
+	  .expect = -ENOSPC,
+	  .src_x = 700 << 16, .src_y = 1,
+	  .src_w = 100 << 16, .src_h = 600 << 16,
+	  .fb = { .width = 800, .height = 600 }
+	},
+	{ .name = "out of bound by y due to y coordinate higher than framebuffer height",
+	  .expect = -ENOSPC,
+	  .src_x = 700 << 16, .src_y = (600 << 16) + 1,
+	  .src_w = 100 << 16, .src_h = 0,
+	  .fb = { .width = 800, .height = 600 }
+	},
+};
+
+static void drm_test_framebuffer_check_src_coords(struct kunit *test)
+{
+	const struct check_src_coords_case *params = test->param_value;
+	int r;
+
+	/*
+	 * Just compare the expected value with the one returned by the
+	 * function called with args from parameter
+	 */
+	r = drm_framebuffer_check_src_coords(params->src_x, params->src_y,
+					     params->src_w, params->src_h,
+					     &params->fb);
+	KUNIT_EXPECT_EQ(test, r, params->expect);
+}
+
+static void check_src_coords_test_to_desc(const struct check_src_coords_case *t,
+					  char *desc)
+{
+	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
+}
+
+KUNIT_ARRAY_PARAM(check_src_coords, check_src_coords_cases,
+		  check_src_coords_test_to_desc);
+
 static struct kunit_case drm_framebuffer_tests[] = {
+	KUNIT_CASE_PARAM(drm_test_framebuffer_check_src_coords, check_src_coords_gen_params),
 	KUNIT_CASE_PARAM(drm_test_framebuffer_create, drm_framebuffer_create_gen_params),
 	{ }
 };
-- 
2.41.0

[PATCH v2 3/4] drm/tests: Add parameters to the drm_test_framebuffer_create test

[Carlos Eduardo Gallo Filho]

Extend the existing test case to cover:
1. Invalid flag atribute in the struct drm_mode_fb_cmd2.
2. Pixel format which requires non-linear modifier with
DRM_FORMAT_MOD_LINEAR set.
3. Non-zero buffer offset for an unused plane

Also replace strcpy to strscpy on test_to_desc for improved security
and reliability.

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 drivers/gpu/drm/tests/drm_framebuffer_test.c | 23 +++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index ee92120cd8e9..cbab12503f5f 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -20,6 +20,8 @@
 #define MIN_HEIGHT 4
 #define MAX_HEIGHT 4096
 
+#define DRM_MODE_FB_INVALID BIT(2)
+
 struct drm_framebuffer_test {
 	int buffer_created;
 	struct drm_mode_fb_cmd2 cmd;
@@ -84,6 +86,18 @@ static const struct drm_framebuffer_test drm_framebuffer_create_cases[] = {
 		 .pitches = { 4 * MAX_WIDTH, 0, 0 },
 	}
 },
+{ .buffer_created = 0, .name = "ABGR8888 Non-zero buffer offset for unused plane",
+	.cmd = { .width = MAX_WIDTH, .height = MAX_HEIGHT, .pixel_format = DRM_FORMAT_ABGR8888,
+		 .handles = { 1, 0, 0 }, .offsets = { UINT_MAX / 2, UINT_MAX / 2, 0 },
+		 .pitches = { 4 * MAX_WIDTH, 0, 0 }, .flags = DRM_MODE_FB_MODIFIERS,
+	}
+},
+{ .buffer_created = 0, .name = "ABGR8888 Invalid flag",
+	.cmd = { .width = MAX_WIDTH, .height = MAX_HEIGHT, .pixel_format = DRM_FORMAT_ABGR8888,
+		 .handles = { 1, 0, 0 }, .offsets = { UINT_MAX / 2, 0, 0 },
+		 .pitches = { 4 * MAX_WIDTH, 0, 0 }, .flags = DRM_MODE_FB_INVALID,
+	}
+},
 { .buffer_created = 1, .name = "ABGR8888 Set DRM_MODE_FB_MODIFIERS without modifiers",
 	.cmd = { .width = MAX_WIDTH, .height = MAX_HEIGHT, .pixel_format = DRM_FORMAT_ABGR8888,
 		 .handles = { 1, 0, 0 }, .offsets = { UINT_MAX / 2, 0, 0 },
@@ -263,6 +277,13 @@ static const struct drm_framebuffer_test drm_framebuffer_create_cases[] = {
 		 .pitches = { MAX_WIDTH, DIV_ROUND_UP(MAX_WIDTH, 2), DIV_ROUND_UP(MAX_WIDTH, 2) },
 	}
 },
+{ .buffer_created = 0, .name = "YUV420_10BIT Invalid modifier(DRM_FORMAT_MOD_LINEAR)",
+	.cmd = { .width = MAX_WIDTH, .height = MAX_HEIGHT, .pixel_format = DRM_FORMAT_YUV420_10BIT,
+		 .handles = { 1, 0, 0 }, .flags = DRM_MODE_FB_MODIFIERS,
+		 .modifier = { DRM_FORMAT_MOD_LINEAR, 0, 0 },
+		 .pitches = { MAX_WIDTH, 0, 0 },
+	}
+},
 { .buffer_created = 1, .name = "X0L2 Normal sizes",
 	.cmd = { .width = 600, .height = 600, .pixel_format = DRM_FORMAT_X0L2,
 		 .handles = { 1, 0, 0 }, .pitches = { 1200, 0, 0 }
@@ -361,7 +382,7 @@ static void drm_test_framebuffer_create(struct kunit *test)
 
 static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, char *desc)
 {
-	strcpy(desc, t->name);
+	strscpy(desc, t->name, KUNIT_PARAM_DESC_SIZE);
 }
 
 KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
-- 
2.41.0

[PATCH v2 4/4] drm/tests: Add test case for drm_internal_framebuffer_create()

[Carlos Eduardo Gallo Filho]

Introduce a test to cover the creation of framebuffer with
modifier on a device that don't support it.

Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
---
 drivers/gpu/drm/tests/drm_framebuffer_test.c | 29 ++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
index cbab12503f5f..0f1744b5d2c1 100644
--- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
+++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
@@ -388,6 +388,34 @@ static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, c
 KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
 		  drm_framebuffer_test_to_desc);
 
+/*
+ * This test is very similar to drm_test_framebuffer_create, except that it
+ * set mock->mode_config.fb_modifiers_not_supported member to 1, covering
+ * the case of trying to create a framebuffer with modifiers without the
+ * device really supporting it.
+ */
+static void drm_test_framebuffer_modifiers_not_supported(struct kunit *test)
+{
+	/* A valid cmd with modifier */
+	struct drm_mode_fb_cmd2 cmd = {
+		.width = MAX_WIDTH, .height = MAX_HEIGHT,
+		.pixel_format = DRM_FORMAT_ABGR8888, .handles = { 1, 0, 0 },
+		.offsets = { UINT_MAX / 2, 0, 0 }, .pitches = { 4 * MAX_WIDTH, 0, 0 },
+		.flags = DRM_MODE_FB_MODIFIERS,
+	};
+	struct drm_device *mock = test->priv;
+	int buffer_created = 0;
+
+	mock->dev_private = &buffer_created;
+	mock->mode_config.fb_modifiers_not_supported = 1;
+
+	drm_internal_framebuffer_create(mock, &cmd, NULL);
+	KUNIT_EXPECT_EQ(test, 0, buffer_created);
+
+	/* Restore original value */
+	mock->mode_config.fb_modifiers_not_supported = 0;
+}
+
 /* Parameters for testing drm_framebuffer_check_src_coords function */
 struct check_src_coords_case {
 	const char *name; /* Description of the parameter case */
@@ -513,6 +541,7 @@ KUNIT_ARRAY_PARAM(check_src_coords, check_src_coords_cases,
 		  check_src_coords_test_to_desc);
 
 static struct kunit_case drm_framebuffer_tests[] = {
+	KUNIT_CASE(drm_test_framebuffer_modifiers_not_supported),
 	KUNIT_CASE_PARAM(drm_test_framebuffer_check_src_coords, check_src_coords_gen_params),
 	KUNIT_CASE_PARAM(drm_test_framebuffer_create, drm_framebuffer_create_gen_params),
 	{ }
-- 
2.41.0

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[kernel test robot]

Hi Carlos,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.5-rc2 next-20230718]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Carlos-Eduardo-Gallo-Filho/drm-Add-kernel-doc-for-drm_framebuffer_check_src_coords/20230719-022204
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230718181726.3799-3-gcarlos%40disroot.org
patch subject: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()
config: m68k-randconfig-r035-20230718 (https://download.01.org/0day-ci/archive/20230719/202307191411.kDvvppjm-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230719/202307191411.kDvvppjm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307191411.kDvvppjm-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "drm_framebuffer_check_src_coords" [drivers/gpu/drm/tests/drm_framebuffer_test.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[kernel test robot]

Hi Carlos,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.5-rc2 next-20230718]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Carlos-Eduardo-Gallo-Filho/drm-Add-kernel-doc-for-drm_framebuffer_check_src_coords/20230719-022204
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230718181726.3799-3-gcarlos%40disroot.org
patch subject: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()
config: m68k-randconfig-r035-20230718 (https://download.01.org/0day-ci/archive/20230719/202307191411.kDvvppjm-lkp@intel.com/config)
compiler: m68k-linux-gcc (GCC) 12.3.0
reproduce: (https://download.01.org/0day-ci/archive/20230719/202307191411.kDvvppjm-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307191411.kDvvppjm-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "drm_framebuffer_check_src_coords" [drivers/gpu/drm/tests/drm_framebuffer_test.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 1/4] drm: Add kernel-doc for drm_framebuffer_check_src_coords()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 291 bytes --]

On Tue, Jul 18, 2023 at 03:17:23PM -0300, Carlos Eduardo Gallo Filho wrote:
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>

As a general rule of thumb, a commit log is always needed :)

Looks good otherwise, once fixed
Acked-by: Maxime Ripard <mripard@kernel.org>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

On Wed, Jul 19, 2023 at 02:50:16PM +0800, kernel test robot wrote:
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> ERROR: modpost: "drm_framebuffer_check_src_coords" [drivers/gpu/drm/tests/drm_framebuffer_test.ko] undefined!

Yeah, drm_framebuffer_check_src_coords isn't exported to modules. This
wasn't an issue before because it was only used by the core KMS which is
always compiled in the same module, but your new tests are in another
module :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 494 bytes --]

On Wed, Jul 19, 2023 at 02:50:16PM +0800, kernel test robot wrote:
> All errors (new ones prefixed by >>, old ones prefixed by <<):
> 
> >> ERROR: modpost: "drm_framebuffer_check_src_coords" [drivers/gpu/drm/tests/drm_framebuffer_test.ko] undefined!

Yeah, drm_framebuffer_check_src_coords isn't exported to modules. This
wasn't an issue before because it was only used by the core KMS which is
always compiled in the same module, but your new tests are in another
module :)

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1914 bytes --]

Hi,

On Tue, Jul 18, 2023 at 03:17:24PM -0300, Carlos Eduardo Gallo Filho wrote:
> Add a parametrized test for the drm_framebuffer_check_src_coords function.
> 
> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
> ---
>  drivers/gpu/drm/tests/drm_framebuffer_test.c | 126 +++++++++++++++++++
>  1 file changed, 126 insertions(+)
> 
> diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
> index f759d9f3b76e..ee92120cd8e9 100644
> --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
> +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
> @@ -9,6 +9,7 @@
>  
>  #include <drm/drm_device.h>
>  #include <drm/drm_mode.h>
> +#include <drm/drm_framebuffer.h>
>  #include <drm/drm_fourcc.h>
>  #include <drm/drm_print.h>
>  
> @@ -366,7 +367,132 @@ static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, c
>  KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
>  		  drm_framebuffer_test_to_desc);
>  
> +/* Parameters for testing drm_framebuffer_check_src_coords function */
> +struct check_src_coords_case {
> +	const char *name; /* Description of the parameter case */
> +	const int expect; /* Expected returned value by the function */
> +
> +	/* All function args */
> +	const uint32_t src_x;
> +	const uint32_t src_y;
> +	const uint32_t src_w;
> +	const uint32_t src_h;
> +	const struct drm_framebuffer fb;
> +};
> +
> +static const struct check_src_coords_case check_src_coords_cases[] = {
> +	/* Regular case where the source just fit in the framebuffer */
> +	{ .name = "source inside framebuffer with normal sizes and coordinates",
> +	  .expect = 0,
> +	  .src_x = 500 << 16, .src_y = 700 << 16,
> +	  .src_w = 100 << 16, .src_h = 100 << 16,

I don't think we need to duplicate the << 16 everywhere, this can be
added by the test function.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[kernel test robot]

Hi Carlos,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.5-rc2 next-20230719]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Carlos-Eduardo-Gallo-Filho/drm-Add-kernel-doc-for-drm_framebuffer_check_src_coords/20230719-022204
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230718181726.3799-3-gcarlos%40disroot.org
patch subject: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()
config: hexagon-randconfig-r041-20230718 (https://download.01.org/0day-ci/archive/20230719/202307192136.DPbcKMcd-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230719/202307192136.DPbcKMcd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307192136.DPbcKMcd-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "drm_framebuffer_check_src_coords" [drivers/gpu/drm/tests/drm_framebuffer_test.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[kernel test robot]

Hi Carlos,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on drm/drm-next drm-exynos/exynos-drm-next drm-intel/for-linux-next drm-intel/for-linux-next-fixes drm-tip/drm-tip linus/master v6.5-rc2 next-20230719]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Carlos-Eduardo-Gallo-Filho/drm-Add-kernel-doc-for-drm_framebuffer_check_src_coords/20230719-022204
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230718181726.3799-3-gcarlos%40disroot.org
patch subject: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()
config: hexagon-randconfig-r041-20230718 (https://download.01.org/0day-ci/archive/20230719/202307192136.DPbcKMcd-lkp@intel.com/config)
compiler: clang version 16.0.4 (https://github.com/llvm/llvm-project.git ae42196bc493ffe877a7e3dff8be32035dea4d07)
reproduce: (https://download.01.org/0day-ci/archive/20230719/202307192136.DPbcKMcd-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202307192136.DPbcKMcd-lkp@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

>> ERROR: modpost: "drm_framebuffer_check_src_coords" [drivers/gpu/drm/tests/drm_framebuffer_test.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

Re: [PATCH v2 2/4] drm/tests: Add test for drm_framebuffer_check_src_coords()

[Carlos]

Hi Maxime, thanks for the reviews!

On 7/19/23 04:49, Maxime Ripard wrote:
> Hi,
>
> On Tue, Jul 18, 2023 at 03:17:24PM -0300, Carlos Eduardo Gallo Filho wrote:
>> Add a parametrized test for the drm_framebuffer_check_src_coords function.
>>
>> Signed-off-by: Carlos Eduardo Gallo Filho <gcarlos@disroot.org>
>> ---
>>   drivers/gpu/drm/tests/drm_framebuffer_test.c | 126 +++++++++++++++++++
>>   1 file changed, 126 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/tests/drm_framebuffer_test.c b/drivers/gpu/drm/tests/drm_framebuffer_test.c
>> index f759d9f3b76e..ee92120cd8e9 100644
>> --- a/drivers/gpu/drm/tests/drm_framebuffer_test.c
>> +++ b/drivers/gpu/drm/tests/drm_framebuffer_test.c
>> @@ -9,6 +9,7 @@
>>   
>>   #include <drm/drm_device.h>
>>   #include <drm/drm_mode.h>
>> +#include <drm/drm_framebuffer.h>
>>   #include <drm/drm_fourcc.h>
>>   #include <drm/drm_print.h>
>>   
>> @@ -366,7 +367,132 @@ static void drm_framebuffer_test_to_desc(const struct drm_framebuffer_test *t, c
>>   KUNIT_ARRAY_PARAM(drm_framebuffer_create, drm_framebuffer_create_cases,
>>   		  drm_framebuffer_test_to_desc);
>>   
>> +/* Parameters for testing drm_framebuffer_check_src_coords function */
>> +struct check_src_coords_case {
>> +	const char *name; /* Description of the parameter case */
>> +	const int expect; /* Expected returned value by the function */
>> +
>> +	/* All function args */
>> +	const uint32_t src_x;
>> +	const uint32_t src_y;
>> +	const uint32_t src_w;
>> +	const uint32_t src_h;
>> +	const struct drm_framebuffer fb;
>> +};
>> +
>> +static const struct check_src_coords_case check_src_coords_cases[] = {
>> +	/* Regular case where the source just fit in the framebuffer */
>> +	{ .name = "source inside framebuffer with normal sizes and coordinates",
>> +	  .expect = 0,
>> +	  .src_x = 500 << 16, .src_y = 700 << 16,
>> +	  .src_w = 100 << 16, .src_h = 100 << 16,
> I don't think we need to duplicate the << 16 everywhere, this can be
> added by the test function.
>
> Maxime
I thought about it, but there's some cases where we have some values 
composed
by both a shifted part and a regular one, like ".src_x = (600 << 16) + 
1". If we
left to shift everything on the test function we won't be able to have 
this kind
of values, which would compromise the test. Or if we just put off the 
regular part,
we will deal with a test that won't cover the out-of-bound cases at the 
subpixel
level.

Of course this could be implemented by adding some new members to the 
case struct,
being each src_{x,y,w,h} composed by two, where one is always shifted, 
though I
guess it would be way worse than having the shifts everywhere on the 
cases array.

I'll be happy to know about if you have some suggestion of how it can 
implemented
without throwing away the non-shifted part!

Thanks,
Carlos