[PATCH 0/6] Add rounding macros and enable KUnit tests

[PATCH 0/6] Add rounding macros and enable KUnit tests

[Devarsh Thakkar]

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

[PATCH 1/6] math.h: Add macros for rounding to closest value

[Devarsh Thakkar]

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

[PATCH 2/6] math.h: Use kernel-doc syntax for divison macros

[Devarsh Thakkar]

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

[PATCH 3/6] Documentation: core-api: Add math.h macros and functions

[Devarsh Thakkar]

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

[PATCH 4/6] lib: Add basic KUnit test for lib/math

[Devarsh Thakkar]

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

[PATCH 5/6] lib: math_kunit: Add tests for new macros related to rounding to nearest value

[Devarsh Thakkar]

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

[PATCH 6/6] media: imagination: Round to closest multiple for cropping region

[Devarsh Thakkar]

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

Re: [PATCH 1/6] math.h: Add macros for rounding to closest value

[Jiri Slaby]

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

Re: [PATCH 1/6] math.h: Add macros for rounding to closest value

[Nikolay Borisov]

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>

Re: [PATCH 1/6] math.h: Add macros for rounding to closest value

[Devarsh Thakkar]

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

Re: [PATCH 1/6] math.h: Add macros for rounding to closest value

[Dave Martin]

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

Re: [PATCH 1/6] math.h: Add macros for rounding to closest value

[Devarsh Thakkar]

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

Re: [PATCH 1/6] math.h: Add macros for rounding to closest value

[Devarsh Thakkar]

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