[PATCH v7 8/8] gpu: ipu-v3: Use generic macro for rounding to nearest multiple

[PATCH v7 8/8] gpu: ipu-v3: Use generic macro for rounding to nearest multiple

[Devarsh Thakkar]

Use generic macro round_closest_up for rounding to nearest multiple instead
of using local function.

Signed-off-by: Devarsh Thakkar <devarsht@ti.com>
---
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

Re: [PATCH v7 8/8] gpu: ipu-v3: Use generic macro for rounding to nearest multiple

[Andy Shevchenko]

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

Re: [PATCH v7 8/8] gpu: ipu-v3: Use generic macro for rounding to nearest multiple

[Laurent Pinchart]

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

Re: [PATCH v7 8/8] gpu: ipu-v3: Use generic macro for rounding to nearest multiple

[Andy Shevchenko]

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

Re: [PATCH v7 8/8] gpu: ipu-v3: Use generic macro for rounding to nearest multiple

[Devarsh Thakkar]

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