Use generic macro round_closest_up for rounding to nearest multiple instead
of using local function.
Signed-off-by: Devarsh Thakkar <[email protected]>
---
V1->V6 (No change, patch introduced in V7)
---
drivers/gpu/ipu-v3/ipu-image-convert.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c b/drivers/gpu/ipu-v3/ipu-image-convert.c
index 841316582ea9..5192a8b5c02c 100644
--- a/drivers/gpu/ipu-v3/ipu-image-convert.c
+++ b/drivers/gpu/ipu-v3/ipu-image-convert.c
@@ -477,8 +477,6 @@ static int calc_image_resize_coefficients(struct ipu_image_convert_ctx *ctx,
return 0;
}
-#define round_closest(x, y) round_down((x) + (y)/2, (y))
-
/*
* Find the best aligned seam position for the given column / row index.
* Rotation and image offsets are out of scope.
@@ -565,7 +563,7 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
* The closest input sample position that we could actually
* start the input tile at, 19.13 fixed point.
*/
- in_pos_aligned = round_closest(in_pos, 8192U * in_align);
+ in_pos_aligned = round_closest_up(in_pos, 8192U * in_align);
/* Convert 19.13 fixed point to integer */
in_pos_rounded = in_pos_aligned / 8192U;
--
2.39.1
On Fri, May 10, 2024 at 12:10:10AM +0530, Devarsh Thakkar wrote:
> Use generic macro round_closest_up for rounding to nearest multiple instead
round_closest_up()
We refer to the functions as func().
> of using local function.
..
> @@ -565,7 +563,7 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
> * The closest input sample position that we could actually
> * start the input tile at, 19.13 fixed point.
> */
> - in_pos_aligned = round_closest(in_pos, 8192U * in_align);
> + in_pos_aligned = round_closest_up(in_pos, 8192U * in_align);
> /* Convert 19.13 fixed point to integer */
> in_pos_rounded = in_pos_aligned / 8192U;
Oh, these seems to be better to use either ALIGN*(), or PFN_*() / PAGE_*()
families of macros. What the semantic of 8192 is?
--
With Best Regards,
Andy Shevchenko
On Fri, May 10, 2024 at 06:03:52PM +0300, Andy Shevchenko wrote:
> On Fri, May 10, 2024 at 12:10:10AM +0530, Devarsh Thakkar wrote:
> > Use generic macro round_closest_up for rounding to nearest multiple instead
>
> round_closest_up()
>
> We refer to the functions as func().
>
> > of using local function.
>
> ...
>
> > @@ -565,7 +563,7 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
> > * The closest input sample position that we could actually
> > * start the input tile at, 19.13 fixed point.
> > */
> > - in_pos_aligned = round_closest(in_pos, 8192U * in_align);
> > + in_pos_aligned = round_closest_up(in_pos, 8192U * in_align);
> > /* Convert 19.13 fixed point to integer */
> > in_pos_rounded = in_pos_aligned / 8192U;
>
> Oh, these seems to be better to use either ALIGN*(), or PFN_*() / PAGE_*()
> families of macros. What the semantic of 8192 is?
The comment mentions 19.13 fixed point, so I assume that's the
fractional part of the integer. It doesn't seem related to pages.
--
Regards,
Laurent Pinchart
On Fri, May 10, 2024 at 06:16:42PM +0300, Laurent Pinchart wrote:
> On Fri, May 10, 2024 at 06:03:52PM +0300, Andy Shevchenko wrote:
> > On Fri, May 10, 2024 at 12:10:10AM +0530, Devarsh Thakkar wrote:
> > > Use generic macro round_closest_up for rounding to nearest multiple instead
> >
> > round_closest_up()
> >
> > We refer to the functions as func().
> >
> > > of using local function.
..
> > > @@ -565,7 +563,7 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
> > > * The closest input sample position that we could actually
> > > * start the input tile at, 19.13 fixed point.
> > > */
> > > - in_pos_aligned = round_closest(in_pos, 8192U * in_align);
> > > + in_pos_aligned = round_closest_up(in_pos, 8192U * in_align);
> > > /* Convert 19.13 fixed point to integer */
> > > in_pos_rounded = in_pos_aligned / 8192U;
> >
> > Oh, these seems to be better to use either ALIGN*(), or PFN_*() / PAGE_*()
> > families of macros. What the semantic of 8192 is?
>
> The comment mentions 19.13 fixed point, so I assume that's the
> fractional part of the integer. It doesn't seem related to pages.
Okay, and align word in all those variable names?
--
With Best Regards,
Andy Shevchenko
Hi Andy,
Thanks for the quick review.
On 10/05/24 20:33, Andy Shevchenko wrote:
> On Fri, May 10, 2024 at 12:10:10AM +0530, Devarsh Thakkar wrote:
>> Use generic macro round_closest_up for rounding to nearest multiple instead
>
> round_closest_up()
>
> We refer to the functions as func().
>
Agreed. Will fix commit msg to use round_closest_up()
>> of using local function.
>
> ...
>
>> @@ -565,7 +563,7 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
>> * The closest input sample position that we could actually
>> * start the input tile at, 19.13 fixed point.
>> */
>> - in_pos_aligned = round_closest(in_pos, 8192U * in_align);
>> + in_pos_aligned = round_closest_up(in_pos, 8192U * in_align);
>> /* Convert 19.13 fixed point to integer */
>> in_pos_rounded = in_pos_aligned / 8192U;
>
> Oh, these seems to be better to use either ALIGN*(), or PFN_*() / PAGE_*()
> families of macros. What the semantic of 8192 is?
>
As Laurent mentioned, it looks like the fractional part of the integer.
But functionality wise, there is no change with the introduction of this
patch. round_closest_up() does exactly the same thing as what the local
function round_closest used to do before this patch.
Regards
Devarsh