* [PATCH 0/6] Add rounding macros and enable KUnit tests
@ 2024-07-08 15:59 Devarsh Thakkar
2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar
` (5 more replies)
0 siblings, 6 replies; 13+ messages in thread
From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw)
To: mchehab, hverkuil-cisco, linux-media, linux-kernel,
sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby,
corbet, broonie, rdunlap, linux-doc
Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra,
b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti,
nicolas, davidgow, dlatypov
This adds new rounding macros to round to closest integer along with
enabling KUnit tests for all math.h macros. It also updates kernel-doc to
render math.h related documentation and lastly updates the imagination jpeg
encoder to use the newly introduced rounding macros to find nearest value.
Also to note that math.h is orphaned and so these changes need
to be pulled by subsystem using it as a first as discussed in below
threads:
Link:
https://lore.kernel.org/all/1db64af7-3688-4bcc-a671-9440d8f02d1a@xs4all.nl/
https://lore.kernel.org/all/ZmHDWeuezCEgj20m@smile.fi.intel.com/
Daniel Latypov (1):
lib: Add basic KUnit test for lib/math
Devarsh Thakkar (5):
math.h: Add macros for rounding to closest value
math.h: Use kernel-doc syntax for divison macros
Documentation: core-api: Add math.h macros and functions
lib: math_kunit: Add tests for new macros related to rounding to
nearest value
media: imagination: Round to closest multiple for cropping region
Documentation/core-api/kernel-api.rst | 6 +
.../platform/imagination/e5010-jpeg-enc.c | 8 +-
include/linux/math.h | 86 ++++-
lib/math/Kconfig | 14 +
lib/math/Makefile | 1 +
lib/math/math_kunit.c | 329 ++++++++++++++++++
6 files changed, 434 insertions(+), 10 deletions(-)
create mode 100644 lib/math/math_kunit.c
--
2.39.1
^ permalink raw reply [flat|nested] 13+ messages in thread* [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar @ 2024-07-08 15:59 ` Devarsh Thakkar 2024-07-09 5:59 ` Jiri Slaby ` (2 more replies) 2024-07-08 15:59 ` [PATCH 2/6] math.h: Use kernel-doc syntax for divison macros Devarsh Thakkar ` (4 subsequent siblings) 5 siblings, 3 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw) To: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti, nicolas, davidgow, dlatypov Add below rounding related macros: round_closest_up(x, y) : Rounds x to closest multiple of y where y is a power of 2, with a preference to round up in case two nearest values are possible. round_closest_down(x, y) : Rounds x to closest multiple of y where y is a power of 2, with a preference to round down in case two nearest values are possible. roundclosest(x, y) : Rounds x to closest multiple of y, this macro should generally be used only when y is not multiple of 2 as otherwise round_closest* macros should be used which are much faster. Examples: * round_closest_up(17, 4) = 16 * round_closest_up(15, 4) = 16 * round_closest_up(14, 4) = 16 * round_closest_down(17, 4) = 16 * round_closest_down(15, 4) = 16 * round_closest_down(14, 4) = 12 * roundclosest(21, 5) = 20 * roundclosest(19, 5) = 20 * roundclosest(17, 5) = 15 Signed-off-by: Devarsh Thakkar <devarsht@ti.com> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] which uses similar macro locally and which is updated in further patch in the series to use this generic macro instead along with other drivers having similar requirements. Link: https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] --- include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 63 insertions(+) diff --git a/include/linux/math.h b/include/linux/math.h index dd4152711de7..79e3dfda77fc 100644 --- a/include/linux/math.h +++ b/include/linux/math.h @@ -34,6 +34,52 @@ */ #define round_down(x, y) ((x) & ~__round_mask(x, y)) +/** + * round_closest_up - round closest to be multiple of specified value (which is + * power of 2) with preference to rounding up + * @x: the value to round + * @y: multiple to round closest to (must be a power of 2) + * + * Rounds @x to closest multiple of @y (which must be a power of 2). + * The value can be either rounded up or rounded down depending upon rounded + * value's closeness to the specified value. If there are two closest possible + * values, i.e. the difference between the specified value and it's rounded up + * and rounded down values is same then preference is given to rounded up + * value. + * + * To perform arbitrary rounding to closest value (not multiple of 2), use + * roundclosest(). + * + * Examples: + * * round_closest_up(17, 4) = 16 + * * round_closest_up(15, 4) = 16 + * * round_closest_up(14, 4) = 16 + */ +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) + +/** + * round_closest_down - round closest to be multiple of specified value (which + * is power of 2) with preference to rounding down + * @x: the value to round + * @y: multiple to round closest to (must be a power of 2) + * + * Rounds @x to closest multiple of @y (which must be a power of 2). + * The value can be either rounded up or rounded down depending upon rounded + * value's closeness to the specified value. If there are two closest possible + * values, i.e. the difference between the specified value and it's rounded up + * and rounded down values is same then preference is given to rounded up + * value. + * + * To perform arbitrary rounding to closest value (not multiple of 2), use + * roundclosest(). + * + * Examples: + * * round_closest_down(17, 4) = 16 + * * round_closest_down(15, 4) = 16 + * * round_closest_down(14, 4) = 12 + */ +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) + #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP #define DIV_ROUND_DOWN_ULL(ll, d) \ @@ -77,6 +123,23 @@ } \ ) +/** + * roundclosest - round to nearest multiple + * @x: the value to round + * @y: multiple to round nearest to + * + * Rounds @x to nearest multiple of @y. + * The rounded value can be greater than or less than @x depending + * upon it's nearness to @x. If @y will always be a power of 2, consider + * using the faster round_closest_up() or round_closest_down(). + * + * Examples: + * * roundclosest(21, 5) = 20 + * * roundclosest(19, 5) = 20 + * * roundclosest(17, 5) = 15 + */ +#define roundclosest(x, y) rounddown((x) + (y) / 2, (y)) + /* * Divide positive or negative dividend by positive or negative divisor * and round to closest integer. Result is undefined for negative -- 2.39.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar @ 2024-07-09 5:59 ` Jiri Slaby 2024-07-09 14:30 ` Devarsh Thakkar 2024-07-09 8:49 ` Nikolay Borisov 2024-07-09 17:18 ` Dave Martin 2 siblings, 1 reply; 13+ messages in thread From: Jiri Slaby @ 2024-07-09 5:59 UTC (permalink / raw) To: Devarsh Thakkar, mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, andi.shyti, nicolas, davidgow, dlatypov On 08. 07. 24, 17:59, Devarsh Thakkar wrote: > Add below rounding related macros: > > round_closest_up(x, y) : Rounds x to closest multiple of y where y is a > power of 2, with a preference to round up in case two nearest values are > possible. > > round_closest_down(x, y) : Rounds x to closest multiple of y where y is a > power of 2, with a preference to round down in case two nearest values are > possible. > > roundclosest(x, y) : Rounds x to closest multiple of y, this macro should > generally be used only when y is not multiple of 2 as otherwise > round_closest* macros should be used which are much faster. > > Examples: > * round_closest_up(17, 4) = 16 > * round_closest_up(15, 4) = 16 > * round_closest_up(14, 4) = 16 > * round_closest_down(17, 4) = 16 > * round_closest_down(15, 4) = 16 > * round_closest_down(14, 4) = 12 > * roundclosest(21, 5) = 20 With consistency in mind, why is there no underscore? > * roundclosest(19, 5) = 20 > * roundclosest(17, 5) = 15 > > Signed-off-by: Devarsh Thakkar <devarsht@ti.com> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] > which uses similar macro locally and which is updated in further patch > in the series to use this generic macro instead along with other drivers > having similar requirements. > > Link: https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] > --- > include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 63 insertions(+) > > diff --git a/include/linux/math.h b/include/linux/math.h > index dd4152711de7..79e3dfda77fc 100644 > --- a/include/linux/math.h > +++ b/include/linux/math.h > @@ -34,6 +34,52 @@ > */ > #define round_down(x, y) ((x) & ~__round_mask(x, y)) > > +/** > + * round_closest_up - round closest to be multiple of specified value (which is > + * power of 2) with preference to rounding up > + * @x: the value to round > + * @y: multiple to round closest to (must be a power of 2) > + * > + * Rounds @x to closest multiple of @y (which must be a power of 2). > + * The value can be either rounded up or rounded down depending upon rounded > + * value's closeness to the specified value. If there are two closest possible > + * values, i.e. the difference between the specified value and it's rounded up > + * and rounded down values is same then preference is given to rounded up > + * value. > + * > + * To perform arbitrary rounding to closest value (not multiple of 2), use > + * roundclosest(). > + * > + * Examples: > + * * round_closest_up(17, 4) = 16 > + * * round_closest_up(15, 4) = 16 > + * * round_closest_up(14, 4) = 16 > + */ > +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) > + > +/** > + * round_closest_down - round closest to be multiple of specified value (which > + * is power of 2) with preference to rounding down > + * @x: the value to round > + * @y: multiple to round closest to (must be a power of 2) > + * > + * Rounds @x to closest multiple of @y (which must be a power of 2). > + * The value can be either rounded up or rounded down depending upon rounded > + * value's closeness to the specified value. If there are two closest possible > + * values, i.e. the difference between the specified value and it's rounded up > + * and rounded down values is same then preference is given to rounded up > + * value. Too heavy sentence. Did you mean "its" not "it's"? What about: There can be two closest values. I.e. the difference between the specified value and its rounded up and down values is the same. In that case, the rounded up value is preferred. ? The same for round_closest_up(). > + * > + * To perform arbitrary rounding to closest value (not multiple of 2), use > + * roundclosest(). > + * > + * Examples: > + * * round_closest_down(17, 4) = 16 > + * * round_closest_down(15, 4) = 16 > + * * round_closest_down(14, 4) = 12 > + */ > +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) > + > #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP > > #define DIV_ROUND_DOWN_ULL(ll, d) \ > @@ -77,6 +123,23 @@ > } \ > ) > > +/** > + * roundclosest - round to nearest multiple > + * @x: the value to round > + * @y: multiple to round nearest to > + * > + * Rounds @x to nearest multiple of @y. > + * The rounded value can be greater than or less than @x depending greater or less than > + * upon it's nearness to @x. "its" > If @y will always be a power of 2, consider If @y is always a power... > + * using the faster round_closest_up() or round_closest_down(). > + * > + * Examples: > + * * roundclosest(21, 5) = 20 > + * * roundclosest(19, 5) = 20 > + * * roundclosest(17, 5) = 15 > + */ > +#define roundclosest(x, y) rounddown((x) + (y) / 2, (y)) > + > /* > * Divide positive or negative dividend by positive or negative divisor > * and round to closest integer. Result is undefined for negative -- js ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-09 5:59 ` Jiri Slaby @ 2024-07-09 14:30 ` Devarsh Thakkar 0 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-09 14:30 UTC (permalink / raw) To: Jiri Slaby, mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, andi.shyti, nicolas, davidgow, dlatypov Hi Jiri, Thanks for the review. On 09/07/24 11:29, Jiri Slaby wrote: > On 08. 07. 24, 17:59, Devarsh Thakkar wrote: >> Add below rounding related macros: >> >> round_closest_up(x, y) : Rounds x to closest multiple of y where y is a >> power of 2, with a preference to round up in case two nearest values are >> possible. >> >> round_closest_down(x, y) : Rounds x to closest multiple of y where y is a >> power of 2, with a preference to round down in case two nearest values are >> possible. >> >> roundclosest(x, y) : Rounds x to closest multiple of y, this macro should >> generally be used only when y is not multiple of 2 as otherwise >> round_closest* macros should be used which are much faster. >> >> Examples: >> * round_closest_up(17, 4) = 16 >> * round_closest_up(15, 4) = 16 >> * round_closest_up(14, 4) = 16 >> * round_closest_down(17, 4) = 16 >> * round_closest_down(15, 4) = 16 >> * round_closest_down(14, 4) = 12 >> * roundclosest(21, 5) = 20 > > With consistency in mind, why is there no underscore? > This is as per the convention followed in math.h for existing rounding macros round_up, roundup, round_down, rounddown : for e.g. It use "_" for macros which work on power of 2 for e.g. we have round_down, round_up macros which work on power of 2 and it remove "_" for normal rounding macros for e.g. rounddown and roundup which are normal rounding macros. There was already a discussion around naming convention in previous patch versions here [1] we aligned on this. >> * roundclosest(19, 5) = 20 >> * roundclosest(17, 5) = 15 >> >> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> >> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] >> which uses similar macro locally and which is updated in further patch >> in the series to use this generic macro instead along with other drivers >> having similar requirements. >> >> Link: >> https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] >> --- >> include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> >> diff --git a/include/linux/math.h b/include/linux/math.h >> index dd4152711de7..79e3dfda77fc 100644 >> --- a/include/linux/math.h >> +++ b/include/linux/math.h >> @@ -34,6 +34,52 @@ >> */ >> #define round_down(x, y) ((x) & ~__round_mask(x, y)) >> +/** >> + * round_closest_up - round closest to be multiple of specified value >> (which is >> + * power of 2) with preference to rounding up >> + * @x: the value to round >> + * @y: multiple to round closest to (must be a power of 2) >> + * >> + * Rounds @x to closest multiple of @y (which must be a power of 2). >> + * The value can be either rounded up or rounded down depending upon rounded >> + * value's closeness to the specified value. If there are two closest possible >> + * values, i.e. the difference between the specified value and it's rounded up >> + * and rounded down values is same then preference is given to rounded up >> + * value. >> + * >> + * To perform arbitrary rounding to closest value (not multiple of 2), use >> + * roundclosest(). >> + * >> + * Examples: >> + * * round_closest_up(17, 4) = 16 >> + * * round_closest_up(15, 4) = 16 >> + * * round_closest_up(14, 4) = 16 >> + */ >> +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) >> + >> +/** >> + * round_closest_down - round closest to be multiple of specified value (which >> + * is power of 2) with preference to rounding down >> + * @x: the value to round >> + * @y: multiple to round closest to (must be a power of 2) >> + * >> + * Rounds @x to closest multiple of @y (which must be a power of 2). >> + * The value can be either rounded up or rounded down depending upon rounded >> + * value's closeness to the specified value. If there are two closest possible >> + * values, i.e. the difference between the specified value and it's rounded up >> + * and rounded down values is same then preference is given to rounded up >> + * value. > > Too heavy sentence. Did you mean "its" not "it's"? Yeah "its" is the correct one. > > What about: > There can be two closest values. I.e. the difference between the specified > value and its rounded up and down values is the same. In that case, the > rounded up value is preferred. > ? > Yeah this looks good but I would still prefer to prepend to this the text "The value can be either rounded up or rounded down depending upon rounded value's closeness to the specified value" just to avoid any confusion as it caused a bit of confusions in earlier iterations. > The same for round_closest_up(). > >> + * >> + * To perform arbitrary rounding to closest value (not multiple of 2), use >> + * roundclosest(). >> + * >> + * Examples: >> + * * round_closest_down(17, 4) = 16 >> + * * round_closest_down(15, 4) = 16 >> + * * round_closest_down(14, 4) = 12 >> + */ >> +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) >> + >> #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP >> #define DIV_ROUND_DOWN_ULL(ll, d) \ >> @@ -77,6 +123,23 @@ >> } \ >> ) >> +/** >> + * roundclosest - round to nearest multiple >> + * @x: the value to round >> + * @y: multiple to round nearest to >> + * >> + * Rounds @x to nearest multiple of @y. >> + * The rounded value can be greater than or less than @x depending > > greater or less than > Agreed. >> + * upon it's nearness to @x. > > "its" > Agreed. >> If @y will always be a power of 2, consider > > If @y is always a power... > Agreed. [1]: https://lore.kernel.org/all/Zj42vTpyH71TWeTk@smile.fi.intel.com/ Regards Devarsh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar 2024-07-09 5:59 ` Jiri Slaby @ 2024-07-09 8:49 ` Nikolay Borisov 2024-07-20 10:01 ` Devarsh Thakkar 2024-07-09 17:18 ` Dave Martin 2 siblings, 1 reply; 13+ messages in thread From: Nikolay Borisov @ 2024-07-09 8:49 UTC (permalink / raw) To: Devarsh Thakkar, mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, andi.shyti, nicolas, davidgow, dlatypov On 8.07.24 г. 18:59 ч., Devarsh Thakkar wrote: > Add below rounding related macros: > > round_closest_up(x, y) : Rounds x to closest multiple of y where y is a > power of 2, with a preference to round up in case two nearest values are > possible. > > round_closest_down(x, y) : Rounds x to closest multiple of y where y is a > power of 2, with a preference to round down in case two nearest values are > possible. > > roundclosest(x, y) : Rounds x to closest multiple of y, this macro should > generally be used only when y is not multiple of 2 as otherwise > round_closest* macros should be used which are much faster. > > Examples: > * round_closest_up(17, 4) = 16 > * round_closest_up(15, 4) = 16 > * round_closest_up(14, 4) = 16 > * round_closest_down(17, 4) = 16 > * round_closest_down(15, 4) = 16 > * round_closest_down(14, 4) = 12 > * roundclosest(21, 5) = 20 > * roundclosest(19, 5) = 20 > * roundclosest(17, 5) = 15 > > Signed-off-by: Devarsh Thakkar <devarsht@ti.com> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] > which uses similar macro locally and which is updated in further patch > in the series to use this generic macro instead along with other drivers > having similar requirements. > > Link: https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] > --- > include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 63 insertions(+) > > diff --git a/include/linux/math.h b/include/linux/math.h > index dd4152711de7..79e3dfda77fc 100644 > --- a/include/linux/math.h > +++ b/include/linux/math.h > @@ -34,6 +34,52 @@ > */ > #define round_down(x, y) ((x) & ~__round_mask(x, y)) > > +/** > + * round_closest_up - round closest to be multiple of specified value (which is > + * power of 2) with preference to rounding up > + * @x: the value to round > + * @y: multiple to round closest to (must be a power of 2) > + * > + * Rounds @x to closest multiple of @y (which must be a power of 2). > + * The value can be either rounded up or rounded down depending upon rounded > + * value's closeness to the specified value. If there are two closest possible > + * values, i.e. the difference between the specified value and it's rounded up > + * and rounded down values is same then preference is given to rounded up > + * value. > + * > + * To perform arbitrary rounding to closest value (not multiple of 2), use > + * roundclosest(). > + * > + * Examples: > + * * round_closest_up(17, 4) = 16 > + * * round_closest_up(15, 4) = 16 > + * * round_closest_up(14, 4) = 16 > + */ > +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) > + > +/** > + * round_closest_down - round closest to be multiple of specified value (which > + * is power of 2) with preference to rounding down > + * @x: the value to round > + * @y: multiple to round closest to (must be a power of 2) > + * > + * Rounds @x to closest multiple of @y (which must be a power of 2). > + * The value can be either rounded up or rounded down depending upon rounded > + * value's closeness to the specified value. If there are two closest possible > + * values, i.e. the difference between the specified value and it's rounded up > + * and rounded down values is same then preference is given to rounded up > + * value. > + * > + * To perform arbitrary rounding to closest value (not multiple of 2), use > + * roundclosest(). > + * > + * Examples: > + * * round_closest_down(17, 4) = 16 > + * * round_closest_down(15, 4) = 16 > + * * round_closest_down(14, 4) = 12 > + */ > +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) This is already identical to the existing round_down, no ? <snip> ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-09 8:49 ` Nikolay Borisov @ 2024-07-20 10:01 ` Devarsh Thakkar 0 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-20 10:01 UTC (permalink / raw) To: Nikolay Borisov, mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, andi.shyti, nicolas, davidgow, dlatypov Hi Nikolay, Sorry for the delay. On 09/07/24 14:19, Nikolay Borisov wrote: > > > On 8.07.24 г. 18:59 ч., Devarsh Thakkar wrote: >> Add below rounding related macros: >> >> round_closest_up(x, y) : Rounds x to closest multiple of y where y is a >> power of 2, with a preference to round up in case two nearest values are >> possible. >> >> round_closest_down(x, y) : Rounds x to closest multiple of y where y is a >> power of 2, with a preference to round down in case two nearest values >> are >> possible. >> >> roundclosest(x, y) : Rounds x to closest multiple of y, this macro should >> generally be used only when y is not multiple of 2 as otherwise >> round_closest* macros should be used which are much faster. >> >> Examples: >> * round_closest_up(17, 4) = 16 >> * round_closest_up(15, 4) = 16 >> * round_closest_up(14, 4) = 16 >> * round_closest_down(17, 4) = 16 >> * round_closest_down(15, 4) = 16 >> * round_closest_down(14, 4) = 12 >> * roundclosest(21, 5) = 20 >> * roundclosest(19, 5) = 20 >> * roundclosest(17, 5) = 15 >> >> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> >> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] >> which uses similar macro locally and which is updated in further patch >> in the series to use this generic macro instead along with other drivers >> having similar requirements. >> >> Link: >> https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] >> --- >> include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> >> diff --git a/include/linux/math.h b/include/linux/math.h >> index dd4152711de7..79e3dfda77fc 100644 >> --- a/include/linux/math.h >> +++ b/include/linux/math.h >> @@ -34,6 +34,52 @@ >> */ >> #define round_down(x, y) ((x) & ~__round_mask(x, y)) >> +/** >> + * round_closest_up - round closest to be multiple of specified value >> (which is >> + * power of 2) with preference to rounding up >> + * @x: the value to round >> + * @y: multiple to round closest to (must be a power of 2) >> + * >> + * Rounds @x to closest multiple of @y (which must be a power of 2). >> + * The value can be either rounded up or rounded down depending upon >> rounded >> + * value's closeness to the specified value. If there are two closest >> possible >> + * values, i.e. the difference between the specified value and it's >> rounded up >> + * and rounded down values is same then preference is given to >> rounded up >> + * value. >> + * >> + * To perform arbitrary rounding to closest value (not multiple of >> 2), use >> + * roundclosest(). >> + * >> + * Examples: >> + * * round_closest_up(17, 4) = 16 >> + * * round_closest_up(15, 4) = 16 >> + * * round_closest_up(14, 4) = 16 >> + */ >> +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) >> + >> +/** >> + * round_closest_down - round closest to be multiple of specified >> value (which >> + * is power of 2) with preference to rounding down >> + * @x: the value to round >> + * @y: multiple to round closest to (must be a power of 2) >> + * >> + * Rounds @x to closest multiple of @y (which must be a power of 2). >> + * The value can be either rounded up or rounded down depending upon >> rounded >> + * value's closeness to the specified value. If there are two closest >> possible >> + * values, i.e. the difference between the specified value and it's >> rounded up >> + * and rounded down values is same then preference is given to >> rounded up >> + * value. >> + * >> + * To perform arbitrary rounding to closest value (not multiple of >> 2), use >> + * roundclosest(). >> + * >> + * Examples: >> + * * round_closest_down(17, 4) = 16 >> + * * round_closest_down(15, 4) = 16 >> + * * round_closest_down(14, 4) = 12 >> + */ >> +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) > > This is already identical to the existing round_down, no ? > Nopes both are different as described in the comments, round_down rounds down to next specified power of 2, but round_closest_down rounds to closest multiple of the specified power (which could be higher or lower) and if there are two closest multiples then it gives preference to lower value as shown in below examples : - round_closest_down(15, 4) = 16 - round_down(15,4) = 12 - round_closest_down(14, 4) = 12 - round_closest_up(14, 4) = 16 Regards Devarsh ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar 2024-07-09 5:59 ` Jiri Slaby 2024-07-09 8:49 ` Nikolay Borisov @ 2024-07-09 17:18 ` Dave Martin 2024-07-20 11:36 ` Devarsh Thakkar 2 siblings, 1 reply; 13+ messages in thread From: Dave Martin @ 2024-07-09 17:18 UTC (permalink / raw) To: Devarsh Thakkar Cc: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc, laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, andi.shyti, nicolas, davidgow, dlatypov On Mon, Jul 08, 2024 at 09:29:38PM +0530, Devarsh Thakkar wrote: > Add below rounding related macros: > > round_closest_up(x, y) : Rounds x to closest multiple of y where y is a > power of 2, with a preference to round up in case two nearest values are > possible. > > round_closest_down(x, y) : Rounds x to closest multiple of y where y is a > power of 2, with a preference to round down in case two nearest values are > possible. > > roundclosest(x, y) : Rounds x to closest multiple of y, this macro should > generally be used only when y is not multiple of 2 as otherwise > round_closest* macros should be used which are much faster. > > Examples: > * round_closest_up(17, 4) = 16 > * round_closest_up(15, 4) = 16 > * round_closest_up(14, 4) = 16 > * round_closest_down(17, 4) = 16 > * round_closest_down(15, 4) = 16 > * round_closest_down(14, 4) = 12 > * roundclosest(21, 5) = 20 > * roundclosest(19, 5) = 20 > * roundclosest(17, 5) = 15 > > Signed-off-by: Devarsh Thakkar <devarsht@ti.com> > Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] > which uses similar macro locally and which is updated in further patch > in the series to use this generic macro instead along with other drivers > having similar requirements. > > Link: https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] > --- > include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 63 insertions(+) > > diff --git a/include/linux/math.h b/include/linux/math.h > index dd4152711de7..79e3dfda77fc 100644 > --- a/include/linux/math.h > +++ b/include/linux/math.h > @@ -34,6 +34,52 @@ > */ > #define round_down(x, y) ((x) & ~__round_mask(x, y)) > > +/** > + * round_closest_up - round closest to be multiple of specified value (which is > + * power of 2) with preference to rounding up > + * @x: the value to round > + * @y: multiple to round closest to (must be a power of 2) > + * > + * Rounds @x to closest multiple of @y (which must be a power of 2). > + * The value can be either rounded up or rounded down depending upon rounded > + * value's closeness to the specified value. If there are two closest possible > + * values, i.e. the difference between the specified value and it's rounded up > + * and rounded down values is same then preference is given to rounded up > + * value. > + * > + * To perform arbitrary rounding to closest value (not multiple of 2), use > + * roundclosest(). > + * > + * Examples: > + * * round_closest_up(17, 4) = 16 > + * * round_closest_up(15, 4) = 16 > + * * round_closest_up(14, 4) = 16 > + */ > +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) > + > +/** > + * round_closest_down - round closest to be multiple of specified value (which > + * is power of 2) with preference to rounding down > + * @x: the value to round > + * @y: multiple to round closest to (must be a power of 2) > + * > + * Rounds @x to closest multiple of @y (which must be a power of 2). > + * The value can be either rounded up or rounded down depending upon rounded > + * value's closeness to the specified value. If there are two closest possible > + * values, i.e. the difference between the specified value and it's rounded up > + * and rounded down values is same then preference is given to rounded up > + * value. > + * > + * To perform arbitrary rounding to closest value (not multiple of 2), use > + * roundclosest(). > + * > + * Examples: > + * * round_closest_down(17, 4) = 16 > + * * round_closest_down(15, 4) = 16 > + * * round_closest_down(14, 4) = 12 > + */ > +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) > + Naming aside, is there an actual use case for having both roundclosest() and round_closest_up() today? (i.e., is there any potential caller that would actually care about the rounding direction for borderline cases?) > #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP > > #define DIV_ROUND_DOWN_ULL(ll, d) \ > @@ -77,6 +123,23 @@ > } \ > ) > > +/** > + * roundclosest - round to nearest multiple > + * @x: the value to round > + * @y: multiple to round nearest to > + * > + * Rounds @x to nearest multiple of @y. > + * The rounded value can be greater than or less than @x depending > + * upon it's nearness to @x. If @y will always be a power of 2, consider > + * using the faster round_closest_up() or round_closest_down(). > + * > + * Examples: > + * * roundclosest(21, 5) = 20 > + * * roundclosest(19, 5) = 20 > + * * roundclosest(17, 5) = 15 > + */ > +#define roundclosest(x, y) rounddown((x) + (y) / 2, (y)) Won't this go wrong if (x) + (y) / 2 overflows? This may happen even in some cases where the correctly rounded value would be in range. The existing rounddown() already leaves something to be desired IIUC: if given a negative dividend, it looks like it actually rounds up, at least on some arches. But maybe people don't use it that way very often. Perhaps I'm missing something. [...] Cheers ---Dave ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [PATCH 1/6] math.h: Add macros for rounding to closest value 2024-07-09 17:18 ` Dave Martin @ 2024-07-20 11:36 ` Devarsh Thakkar 0 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-20 11:36 UTC (permalink / raw) To: Dave Martin Cc: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc, laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, andi.shyti, nicolas, davidgow, dlatypov Hi Dave, Thanks for the review. On 09/07/24 22:48, Dave Martin wrote: > On Mon, Jul 08, 2024 at 09:29:38PM +0530, Devarsh Thakkar wrote: >> Add below rounding related macros: >> >> round_closest_up(x, y) : Rounds x to closest multiple of y where y is a >> power of 2, with a preference to round up in case two nearest values are >> possible. >> >> round_closest_down(x, y) : Rounds x to closest multiple of y where y is a >> power of 2, with a preference to round down in case two nearest values are >> possible. >> >> roundclosest(x, y) : Rounds x to closest multiple of y, this macro should >> generally be used only when y is not multiple of 2 as otherwise >> round_closest* macros should be used which are much faster. >> >> Examples: >> * round_closest_up(17, 4) = 16 >> * round_closest_up(15, 4) = 16 >> * round_closest_up(14, 4) = 16 >> * round_closest_down(17, 4) = 16 >> * round_closest_down(15, 4) = 16 >> * round_closest_down(14, 4) = 12 >> * roundclosest(21, 5) = 20 >> * roundclosest(19, 5) = 20 >> * roundclosest(17, 5) = 15 >> >> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> >> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> >> --- >> NOTE: This patch is inspired from the Mentor Graphics IPU driver [1] >> which uses similar macro locally and which is updated in further patch >> in the series to use this generic macro instead along with other drivers >> having similar requirements. >> >> Link: https://elixir.bootlin.com/linux/v6.8.9/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 [1] >> --- >> include/linux/math.h | 63 ++++++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 63 insertions(+) >> >> diff --git a/include/linux/math.h b/include/linux/math.h >> index dd4152711de7..79e3dfda77fc 100644 >> --- a/include/linux/math.h >> +++ b/include/linux/math.h >> @@ -34,6 +34,52 @@ >> */ >> #define round_down(x, y) ((x) & ~__round_mask(x, y)) >> >> +/** >> + * round_closest_up - round closest to be multiple of specified value (which is >> + * power of 2) with preference to rounding up >> + * @x: the value to round >> + * @y: multiple to round closest to (must be a power of 2) >> + * >> + * Rounds @x to closest multiple of @y (which must be a power of 2). >> + * The value can be either rounded up or rounded down depending upon rounded >> + * value's closeness to the specified value. If there are two closest possible >> + * values, i.e. the difference between the specified value and it's rounded up >> + * and rounded down values is same then preference is given to rounded up >> + * value. >> + * >> + * To perform arbitrary rounding to closest value (not multiple of 2), use >> + * roundclosest(). >> + * >> + * Examples: >> + * * round_closest_up(17, 4) = 16 >> + * * round_closest_up(15, 4) = 16 >> + * * round_closest_up(14, 4) = 16 >> + */ >> +#define round_closest_up(x, y) round_down((x) + (y) / 2, (y)) >> + >> +/** >> + * round_closest_down - round closest to be multiple of specified value (which >> + * is power of 2) with preference to rounding down >> + * @x: the value to round >> + * @y: multiple to round closest to (must be a power of 2) >> + * >> + * Rounds @x to closest multiple of @y (which must be a power of 2). >> + * The value can be either rounded up or rounded down depending upon rounded >> + * value's closeness to the specified value. If there are two closest possible >> + * values, i.e. the difference between the specified value and it's rounded up >> + * and rounded down values is same then preference is given to rounded up >> + * value. >> + * >> + * To perform arbitrary rounding to closest value (not multiple of 2), use >> + * roundclosest(). >> + * >> + * Examples: >> + * * round_closest_down(17, 4) = 16 >> + * * round_closest_down(15, 4) = 16 >> + * * round_closest_down(14, 4) = 12 >> + */ >> +#define round_closest_down(x, y) round_up((x) - (y) / 2, (y)) >> + > > Naming aside, is there an actual use case for having both roundclosest() > and round_closest_up() today? > Both the macros are different, roundclosest is for arbitrary rounding (not multiple of 2) where round_closest_up/down are optimized for rounding to values which are powers of 2. So where there is a surety that rounding value would be power of 2, round_closest* macros are recommended. Regarding the use-cases, there are drivers already using such type of macros locally [1] and new drivers such as [2] required it, so we aligned to have generic macros for all rounding to nearest value scenarios (this patch was earlier part of another series with 7 revisions, see the discussions here [2]). > (i.e., is there any potential caller that would actually care about the > rounding direction for borderline cases?) I think a transparent scheme is better where caller should be aware of rounding direction for borderline cases too so that it gets predictable values w.r.t what it requested for rather than leaving it ambiguous. For e.g. in this patchset [3], it suited more to use round_closest_down instead of round_closest_up keeping in mind hw constraints and use-case requirements, but same might not be true for other drivers. Same was aligned in earlier patch submissions too [2]. > >> #define DIV_ROUND_UP __KERNEL_DIV_ROUND_UP >> >> #define DIV_ROUND_DOWN_ULL(ll, d) \ >> @@ -77,6 +123,23 @@ >> } \ >> ) >> >> +/** >> + * roundclosest - round to nearest multiple >> + * @x: the value to round >> + * @y: multiple to round nearest to >> + * >> + * Rounds @x to nearest multiple of @y. >> + * The rounded value can be greater than or less than @x depending >> + * upon it's nearness to @x. If @y will always be a power of 2, consider >> + * using the faster round_closest_up() or round_closest_down(). >> + * >> + * Examples: >> + * * roundclosest(21, 5) = 20 >> + * * roundclosest(19, 5) = 20 >> + * * roundclosest(17, 5) = 15 >> + */ >> +#define roundclosest(x, y) rounddown((x) + (y) / 2, (y)) > > Won't this go wrong if (x) + (y) / 2 overflows? This may happen even in > some cases where the correctly rounded value would be in range. > Yes I think it is possible, it actually depends upon the datatype of x. But anyways, I could make it as below which would yield the same result as arguments are non-multiple of 2: #define roundclosest(x, y) roundup((x) - (y) / 2, (y)) > The existing rounddown() already leaves something to be desired IIUC: if > given a negative dividend, it looks like it actually rounds up, at least > on some arches. But maybe people don't use it that way very often. > Perhaps I'm missing something. I am not sure about above. [1]: https://elixir.bootlin.com/linux/v6.10/source/drivers/gpu/ipu-v3/ipu-image-convert.c#L480 https://elixir.bootlin.com/linux/v6.10/source/drivers/staging/media/ipu3/ipu3-css-params.c#L443 https://lore.kernel.org/all/ZlTt-YWzyRyhmT9n@smile.fi.intel.com/ [2]: https://lore.kernel.org/all/7e3ad816-6a2a-4e02-9b41-03a8562812ad@ti.com/#r https://lore.kernel.org/all/ZkISG6p1tn9Do-xY@smile.fi.intel.com/#r https://lore.kernel.org/all/ZlTt-YWzyRyhmT9n@smile.fi.intel.com/ https://lore.kernel.org/all/ZmHDWeuezCEgj20m@smile.fi.intel.com/ https://lore.kernel.org/all/ZloMFfGKLry6EWNL@smile.fi.intel.com/ [3]: https://lore.kernel.org/all/20240708155943.2314427-7-devarsht@ti.com/ Regards Devarsh ^ permalink raw reply [flat|nested] 13+ messages in thread
* [PATCH 2/6] math.h: Use kernel-doc syntax for divison macros 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar @ 2024-07-08 15:59 ` Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 3/6] Documentation: core-api: Add math.h macros and functions Devarsh Thakkar ` (3 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw) To: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti, nicolas, davidgow, dlatypov Enable rEST documentation for divison macros DIV_ROUND_CLOSEST and DIV_ROUND_CLOSEST_ULL by using kernel-doc markup and syntax for documenting them. Signed-off-by: Devarsh Thakkar <devarsht@ti.com> --- include/linux/math.h | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/include/linux/math.h b/include/linux/math.h index 79e3dfda77fc..2ab9489bba81 100644 --- a/include/linux/math.h +++ b/include/linux/math.h @@ -140,9 +140,14 @@ */ #define roundclosest(x, y) rounddown((x) + (y) / 2, (y)) -/* - * Divide positive or negative dividend by positive or negative divisor - * and round to closest integer. Result is undefined for negative +/** + * DIV_ROUND_CLOSEST - Divide positive or negative dividend by positive or + * negative divisor and round to closest value + * @x: dividend value + * @divisor: divisor value + * + * Divide positive or negative dividend value @x by positive or negative + * @divisor value and round to closest integer. Result is undefined for negative * divisors if the dividend variable type is unsigned and for negative * dividends if the divisor variable type is unsigned. */ @@ -157,9 +162,15 @@ (((__x) - ((__d) / 2)) / (__d)); \ } \ ) -/* - * Same as above but for u64 dividends. divisor must be a 32-bit - * number. + +/** + * DIV_ROUND_CLOSEST_ULL - Divide 64-bit unsigned dividend by 32-bit divisor and + * round to closest value + * @x: unsigned 64-bit dividend + * @divisor: 32-bit divisor + * + * Divide unsigned 64-bit dividend value @x by 32-bit @divisor value + * and round to closest integer. Result is undefined for negative divisors. */ #define DIV_ROUND_CLOSEST_ULL(x, divisor)( \ { \ -- 2.39.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 3/6] Documentation: core-api: Add math.h macros and functions 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 2/6] math.h: Use kernel-doc syntax for divison macros Devarsh Thakkar @ 2024-07-08 15:59 ` Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 4/6] lib: Add basic KUnit test for lib/math Devarsh Thakkar ` (2 subsequent siblings) 5 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw) To: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti, nicolas, davidgow, dlatypov Add documentation for rounding, scaling, absolute value and 32-bit division related macros and functions exported by math.h header file. Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> Reviewed-by: Randy Dunlap <rdunlap@infradead.org> Tested-by: Randy Dunlap <rdunlap@infradead.org> Signed-off-by: Devarsh Thakkar <devarsht@ti.com> --- Documentation/core-api/kernel-api.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Documentation/core-api/kernel-api.rst b/Documentation/core-api/kernel-api.rst index ae92a2571388..7de494e76fa6 100644 --- a/Documentation/core-api/kernel-api.rst +++ b/Documentation/core-api/kernel-api.rst @@ -185,6 +185,12 @@ Division Functions .. kernel-doc:: lib/math/gcd.c :export: +Rounding, absolute value, division and 32-bit scaling functions +--------------------------------------------------------------- + +.. kernel-doc:: include/linux/math.h + :internal: + UUID/GUID --------- -- 2.39.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 4/6] lib: Add basic KUnit test for lib/math 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar ` (2 preceding siblings ...) 2024-07-08 15:59 ` [PATCH 3/6] Documentation: core-api: Add math.h macros and functions Devarsh Thakkar @ 2024-07-08 15:59 ` Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 5/6] lib: math_kunit: Add tests for new macros related to rounding to nearest value Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 6/6] media: imagination: Round to closest multiple for cropping region Devarsh Thakkar 5 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw) To: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti, nicolas, davidgow, dlatypov From: Daniel Latypov <dlatypov@google.com> Add basic test coverage for files that don't require any config options: * part of math.h (what seem to be the most commonly used macros) * gcd.c * lcm.c * int_sqrt.c * reciprocal_div.c (Ignored int_pow.c since it's a simple textbook algorithm.) These tests aren't particularly interesting, but they * provide short and simple examples of parameterized tests * provide a place to add tests for any new files in this dir * are written so adding new test cases to cover edge cases should be easy * looking at code coverage, we hit all the branches in the .c files Signed-off-by: Daniel Latypov <dlatypov@google.com> Reviewed-by: David Gow <davidgow@google.com> Reviewed-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> [devarsht: Rebase to 6.9, remove kernel.h, update Kconfig and change license to GPL] Signed-off-by: Devarsh Thakkar <devarsht@ti.com> --- lib/math/Kconfig | 14 ++ lib/math/Makefile | 1 + lib/math/math_kunit.c | 294 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 309 insertions(+) create mode 100644 lib/math/math_kunit.c diff --git a/lib/math/Kconfig b/lib/math/Kconfig index 0634b428d0cb..f738d8a433ea 100644 --- a/lib/math/Kconfig +++ b/lib/math/Kconfig @@ -15,3 +15,17 @@ config PRIME_NUMBERS config RATIONAL tristate + +config MATH_KUNIT_TEST + tristate "KUnit test for math helper functions" + depends on KUNIT + default KUNIT_ALL_TESTS + + help + This builds unit test for math helper functions as defined in lib/math + and math.h. + + For more information on KUNIT and unit tests in general, please refer + to the KUnit documentation in Documentation/dev-tools/kunit/. + + If unsure, say N. diff --git a/lib/math/Makefile b/lib/math/Makefile index 91fcdb0c9efe..dcf65d10dab2 100644 --- a/lib/math/Makefile +++ b/lib/math/Makefile @@ -7,3 +7,4 @@ obj-$(CONFIG_RATIONAL) += rational.o obj-$(CONFIG_TEST_DIV64) += test_div64.o obj-$(CONFIG_RATIONAL_KUNIT_TEST) += rational-test.o +obj-$(CONFIG_MATH_KUNIT_TEST) += math_kunit.o diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c new file mode 100644 index 000000000000..be27f2afb8e4 --- /dev/null +++ b/lib/math/math_kunit.c @@ -0,0 +1,294 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Simple KUnit suite for math helper funcs that are always enabled. + * + * Copyright (C) 2020, Google LLC. + * Author: Daniel Latypov <dlatypov@google.com> + */ + +#include <kunit/test.h> +#include <linux/gcd.h> +#include <linux/lcm.h> +#include <linux/math.h> +#include <linux/module.h> +#include <linux/reciprocal_div.h> +#include <linux/types.h> + +static void abs_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, abs((char)0), (char)0); + KUNIT_EXPECT_EQ(test, abs((char)42), (char)42); + KUNIT_EXPECT_EQ(test, abs((char)-42), (char)42); + + /* The expression in the macro is actually promoted to an int. */ + KUNIT_EXPECT_EQ(test, abs((short)0), 0); + KUNIT_EXPECT_EQ(test, abs((short)42), 42); + KUNIT_EXPECT_EQ(test, abs((short)-42), 42); + + KUNIT_EXPECT_EQ(test, abs(0), 0); + KUNIT_EXPECT_EQ(test, abs(42), 42); + KUNIT_EXPECT_EQ(test, abs(-42), 42); + + KUNIT_EXPECT_EQ(test, abs(0L), 0L); + KUNIT_EXPECT_EQ(test, abs(42L), 42L); + KUNIT_EXPECT_EQ(test, abs(-42L), 42L); + + KUNIT_EXPECT_EQ(test, abs(0LL), 0LL); + KUNIT_EXPECT_EQ(test, abs(42LL), 42LL); + KUNIT_EXPECT_EQ(test, abs(-42LL), 42LL); + + /* Unsigned types get casted to signed. */ + KUNIT_EXPECT_EQ(test, abs(0ULL), 0LL); + KUNIT_EXPECT_EQ(test, abs(42ULL), 42LL); +} + +static void int_sqrt_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, int_sqrt(0UL), 0UL); + KUNIT_EXPECT_EQ(test, int_sqrt(1UL), 1UL); + KUNIT_EXPECT_EQ(test, int_sqrt(4UL), 2UL); + KUNIT_EXPECT_EQ(test, int_sqrt(5UL), 2UL); + KUNIT_EXPECT_EQ(test, int_sqrt(8UL), 2UL); + KUNIT_EXPECT_EQ(test, int_sqrt(1UL << 30), 1UL << 15); +} + +static void round_up_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, round_up(0, 1), 0); + KUNIT_EXPECT_EQ(test, round_up(1, 2), 2); + KUNIT_EXPECT_EQ(test, round_up(3, 2), 4); + KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 2), 1 << 30); + KUNIT_EXPECT_EQ(test, round_up((1 << 30) - 1, 1 << 29), 1 << 30); +} + +static void round_down_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, round_down(0, 1), 0); + KUNIT_EXPECT_EQ(test, round_down(1, 2), 0); + KUNIT_EXPECT_EQ(test, round_down(3, 2), 2); + KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 2), (1 << 30) - 2); + KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 1 << 29), 1 << 29); +} + +/* These versions can round to numbers that aren't a power of two */ +static void roundup_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, roundup(0, 1), 0); + KUNIT_EXPECT_EQ(test, roundup(1, 2), 2); + KUNIT_EXPECT_EQ(test, roundup(3, 2), 4); + KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 2), 1 << 30); + KUNIT_EXPECT_EQ(test, roundup((1 << 30) - 1, 1 << 29), 1 << 30); + + KUNIT_EXPECT_EQ(test, roundup(3, 2), 4); + KUNIT_EXPECT_EQ(test, roundup(4, 3), 6); +} + +static void rounddown_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, rounddown(0, 1), 0); + KUNIT_EXPECT_EQ(test, rounddown(1, 2), 0); + KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2); + KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 2), (1 << 30) - 2); + KUNIT_EXPECT_EQ(test, rounddown((1 << 30) - 1, 1 << 29), 1 << 29); + + KUNIT_EXPECT_EQ(test, rounddown(3, 2), 2); + KUNIT_EXPECT_EQ(test, rounddown(4, 3), 3); +} + +static void div_round_up_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(0, 1), 0); + KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(20, 10), 2); + KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 10), 3); + KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 20), 2); + KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(21, 99), 1); +} + +static void div_round_closest_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(0, 1), 0); + KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(20, 10), 2); + KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(21, 10), 2); + KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(25, 10), 3); +} + +/* Generic test case for unsigned long inputs. */ +struct test_case { + unsigned long a, b; + unsigned long result; +}; + +static struct test_case gcd_cases[] = { + { + .a = 0, .b = 0, + .result = 0, + }, + { + .a = 0, .b = 1, + .result = 1, + }, + { + .a = 2, .b = 2, + .result = 2, + }, + { + .a = 2, .b = 4, + .result = 2, + }, + { + .a = 3, .b = 5, + .result = 1, + }, + { + .a = 3 * 9, .b = 3 * 5, + .result = 3, + }, + { + .a = 3 * 5 * 7, .b = 3 * 5 * 11, + .result = 15, + }, + { + .a = 1 << 21, + .b = (1 << 21) - 1, + .result = 1, + }, +}; + +KUNIT_ARRAY_PARAM(gcd, gcd_cases, NULL); + +static void gcd_test(struct kunit *test) +{ + const char *message_fmt = "gcd(%lu, %lu)"; + const struct test_case *test_param = test->param_value; + + KUNIT_EXPECT_EQ_MSG(test, test_param->result, + gcd(test_param->a, test_param->b), + message_fmt, test_param->a, + test_param->b); + + if (test_param->a == test_param->b) + return; + + /* gcd(a,b) == gcd(b,a) */ + KUNIT_EXPECT_EQ_MSG(test, test_param->result, + gcd(test_param->b, test_param->a), + message_fmt, test_param->b, + test_param->a); +} + +static struct test_case lcm_cases[] = { + { + .a = 0, .b = 0, + .result = 0, + }, + { + .a = 0, .b = 1, + .result = 0, + }, + { + .a = 1, .b = 2, + .result = 2, + }, + { + .a = 2, .b = 2, + .result = 2, + }, + { + .a = 3 * 5, .b = 3 * 7, + .result = 3 * 5 * 7, + }, +}; + +KUNIT_ARRAY_PARAM(lcm, lcm_cases, NULL); + +static void lcm_test(struct kunit *test) +{ + const char *message_fmt = "lcm(%lu, %lu)"; + const struct test_case *test_param = test->param_value; + + KUNIT_EXPECT_EQ_MSG(test, test_param->result, + lcm(test_param->a, test_param->b), + message_fmt, test_param->a, + test_param->b); + + if (test_param->a == test_param->b) + return; + + /* lcm(a,b) == lcm(b,a) */ + KUNIT_EXPECT_EQ_MSG(test, test_param->result, + lcm(test_param->b, test_param->a), + message_fmt, test_param->b, + test_param->a); +} + +struct u32_test_case { + u32 a, b; + u32 result; +}; + +static struct u32_test_case reciprocal_div_cases[] = { + { + .a = 0, .b = 1, + .result = 0, + }, + { + .a = 42, .b = 20, + .result = 2, + }, + { + .a = 42, .b = 9999, + .result = 0, + }, + { + .a = (1 << 16), .b = (1 << 14), + .result = 1 << 2, + }, +}; + +KUNIT_ARRAY_PARAM(reciprocal_div, reciprocal_div_cases, NULL); + +static void reciprocal_div_test(struct kunit *test) +{ + const struct u32_test_case *test_param = test->param_value; + struct reciprocal_value rv = reciprocal_value(test_param->b); + + KUNIT_EXPECT_EQ_MSG(test, test_param->result, + reciprocal_divide(test_param->a, rv), + "reciprocal_divide(%u, %u)", + test_param->a, test_param->b); +} + +static void reciprocal_scale_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, reciprocal_scale(0u, 100), 0u); + KUNIT_EXPECT_EQ(test, reciprocal_scale(1u, 100), 0u); + KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 4, 1 << 28), 1u); + KUNIT_EXPECT_EQ(test, reciprocal_scale(1u << 16, 1 << 28), 1u << 12); + KUNIT_EXPECT_EQ(test, reciprocal_scale(~0u, 1 << 28), (1u << 28) - 1); +} + +static struct kunit_case math_test_cases[] = { + KUNIT_CASE(abs_test), + KUNIT_CASE(int_sqrt_test), + KUNIT_CASE(round_up_test), + KUNIT_CASE(round_down_test), + KUNIT_CASE(roundup_test), + KUNIT_CASE(rounddown_test), + KUNIT_CASE(div_round_up_test), + KUNIT_CASE(div_round_closest_test), + KUNIT_CASE_PARAM(gcd_test, gcd_gen_params), + KUNIT_CASE_PARAM(lcm_test, lcm_gen_params), + KUNIT_CASE_PARAM(reciprocal_div_test, reciprocal_div_gen_params), + KUNIT_CASE(reciprocal_scale_test), + {} +}; + +static struct kunit_suite math_test_suite = { + .name = "lib-math", + .test_cases = math_test_cases, +}; + +kunit_test_suites(&math_test_suite); + +MODULE_DESCRIPTION("Math helper functions kunit test"); +MODULE_LICENSE("GPL"); -- 2.39.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 5/6] lib: math_kunit: Add tests for new macros related to rounding to nearest value 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar ` (3 preceding siblings ...) 2024-07-08 15:59 ` [PATCH 4/6] lib: Add basic KUnit test for lib/math Devarsh Thakkar @ 2024-07-08 15:59 ` Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 6/6] media: imagination: Round to closest multiple for cropping region Devarsh Thakkar 5 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw) To: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti, nicolas, davidgow, dlatypov Add tests for round_closest_up/down and roundclosest macros which round to nearest multiple of specified argument. These are tested with kunit tool as shared here [1] : Link: https://gist.github.com/devarsht/3f9042825be3da4e133b8f4eda067876 [1] Signed-off-by: Devarsh Thakkar <devarsht@ti.com> Acked-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> --- lib/math/math_kunit.c | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/lib/math/math_kunit.c b/lib/math/math_kunit.c index be27f2afb8e4..05022f010be6 100644 --- a/lib/math/math_kunit.c +++ b/lib/math/math_kunit.c @@ -70,6 +70,26 @@ static void round_down_test(struct kunit *test) KUNIT_EXPECT_EQ(test, round_down((1 << 30) - 1, 1 << 29), 1 << 29); } +static void round_closest_up_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, round_closest_up(17, 4), 16); + KUNIT_EXPECT_EQ(test, round_closest_up(15, 4), 16); + KUNIT_EXPECT_EQ(test, round_closest_up(14, 4), 16); + KUNIT_EXPECT_EQ(test, round_closest_up((1 << 30) - 1, 1 << 30), 1 << 30); + KUNIT_EXPECT_EQ(test, round_closest_up((1 << 30) + 1, 1 << 30), 1 << 30); + KUNIT_EXPECT_EQ(test, round_closest_up((1 << 30) - 1, 2), 1 << 30); +} + +static void round_closest_down_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, round_closest_down(17, 4), 16); + KUNIT_EXPECT_EQ(test, round_closest_down(15, 4), 16); + KUNIT_EXPECT_EQ(test, round_closest_down(14, 4), 12); + KUNIT_EXPECT_EQ(test, round_closest_down((1 << 30) - 1, 1 << 30), 1 << 30); + KUNIT_EXPECT_EQ(test, round_closest_down((1 << 30) + 1, 1 << 30), 1 << 30); + KUNIT_EXPECT_EQ(test, round_closest_down((1 << 30) - 1, 2), (1 << 30) - 2); +} + /* These versions can round to numbers that aren't a power of two */ static void roundup_test(struct kunit *test) { @@ -95,6 +115,18 @@ static void rounddown_test(struct kunit *test) KUNIT_EXPECT_EQ(test, rounddown(4, 3), 3); } +static void roundclosest_test(struct kunit *test) +{ + KUNIT_EXPECT_EQ(test, roundclosest(21, 5), 20); + KUNIT_EXPECT_EQ(test, roundclosest(19, 5), 20); + KUNIT_EXPECT_EQ(test, roundclosest(17, 5), 15); + KUNIT_EXPECT_EQ(test, roundclosest((1 << 30), 3), (1 << 30) - 1); + KUNIT_EXPECT_EQ(test, roundclosest((1 << 30) - 1, 1 << 29), 1 << 30); + + KUNIT_EXPECT_EQ(test, roundclosest(4, 3), 3); + KUNIT_EXPECT_EQ(test, roundclosest(5, 3), 6); +} + static void div_round_up_test(struct kunit *test) { KUNIT_EXPECT_EQ(test, DIV_ROUND_UP(0, 1), 0); @@ -272,8 +304,11 @@ static struct kunit_case math_test_cases[] = { KUNIT_CASE(int_sqrt_test), KUNIT_CASE(round_up_test), KUNIT_CASE(round_down_test), + KUNIT_CASE(round_closest_up_test), + KUNIT_CASE(round_closest_down_test), KUNIT_CASE(roundup_test), KUNIT_CASE(rounddown_test), + KUNIT_CASE(roundclosest_test), KUNIT_CASE(div_round_up_test), KUNIT_CASE(div_round_closest_test), KUNIT_CASE_PARAM(gcd_test, gcd_gen_params), -- 2.39.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
* [PATCH 6/6] media: imagination: Round to closest multiple for cropping region 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar ` (4 preceding siblings ...) 2024-07-08 15:59 ` [PATCH 5/6] lib: math_kunit: Add tests for new macros related to rounding to nearest value Devarsh Thakkar @ 2024-07-08 15:59 ` Devarsh Thakkar 5 siblings, 0 replies; 13+ messages in thread From: Devarsh Thakkar @ 2024-07-08 15:59 UTC (permalink / raw) To: mchehab, hverkuil-cisco, linux-media, linux-kernel, sebastian.fricke, andriy.shevchenko, jani.nikula, jirislaby, corbet, broonie, rdunlap, linux-doc Cc: laurent.pinchart, praneeth, nm, vigneshr, a-bhatia1, j-luthra, b-brnich, detheridge, p-mantena, vijayp, devarsht, andi.shyti, nicolas, davidgow, dlatypov If neither of the flags to round down (V4L2_SEL_FLAG_LE) or round up (V4L2_SEL_FLAG_GE) are specified by the user, then round to nearest multiple of requested value while updating the crop rectangle coordinates. Use the rounding macro which gives preference to rounding down in case two nearest values (high and low) are possible to raise the probability of cropping rectangle falling inside the bound region. This complies with the VIDIOC_G_SELECTION, VIDIOC_S_SELECTION ioctl description as documented in v4l uapi [1] which specifies that driver should choose crop rectangle as close as possible if no flags are passed by user-space, as quoted below : "``0`` - The driver can adjust the rectangle size freely and shall choose a crop/compose rectangle as close as possible to the requested one." Link: https://www.kernel.org/doc/Documentation/userspace-api/media/v4l/vidioc-g-selection.rst [1] Signed-off-by: Devarsh Thakkar <devarsht@ti.com> --- drivers/media/platform/imagination/e5010-jpeg-enc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/platform/imagination/e5010-jpeg-enc.c b/drivers/media/platform/imagination/e5010-jpeg-enc.c index 187f2d8abfbb..6c3687445803 100644 --- a/drivers/media/platform/imagination/e5010-jpeg-enc.c +++ b/drivers/media/platform/imagination/e5010-jpeg-enc.c @@ -514,10 +514,10 @@ static int e5010_s_selection(struct file *file, void *fh, struct v4l2_selection switch (s->flags) { case 0: - s->r.width = round_down(s->r.width, queue->fmt->frmsize.step_width); - s->r.height = round_down(s->r.height, queue->fmt->frmsize.step_height); - s->r.left = round_down(s->r.left, queue->fmt->frmsize.step_width); - s->r.top = round_down(s->r.top, 2); + s->r.width = round_closest_down(s->r.width, queue->fmt->frmsize.step_width); + s->r.height = round_closest_down(s->r.height, queue->fmt->frmsize.step_height); + s->r.left = round_closest_down(s->r.left, queue->fmt->frmsize.step_width); + s->r.top = round_closest_down(s->r.top, 2); if (s->r.left + s->r.width > queue->width) s->r.width = round_down(s->r.width + s->r.left - queue->width, -- 2.39.1 ^ permalink raw reply related [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-07-20 11:37 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-07-08 15:59 [PATCH 0/6] Add rounding macros and enable KUnit tests Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 1/6] math.h: Add macros for rounding to closest value Devarsh Thakkar 2024-07-09 5:59 ` Jiri Slaby 2024-07-09 14:30 ` Devarsh Thakkar 2024-07-09 8:49 ` Nikolay Borisov 2024-07-20 10:01 ` Devarsh Thakkar 2024-07-09 17:18 ` Dave Martin 2024-07-20 11:36 ` Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 2/6] math.h: Use kernel-doc syntax for divison macros Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 3/6] Documentation: core-api: Add math.h macros and functions Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 4/6] lib: Add basic KUnit test for lib/math Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 5/6] lib: math_kunit: Add tests for new macros related to rounding to nearest value Devarsh Thakkar 2024-07-08 15:59 ` [PATCH 6/6] media: imagination: Round to closest multiple for cropping region Devarsh Thakkar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox