| Message ID | 20230718181726.3799-3-gcarlos@disroot.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add documentation and KUnit tests for functions on drm_framebuffer.c | expand |
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!
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
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
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!
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
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, + ¶ms->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), { } };
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(+)