| Message ID | 20240509184010.4065359-1-devarsht@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v7,1/8] media: dt-bindings: Add Imagination E5010 JPEG Encoder | expand |
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?
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.
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?
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
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;
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(-)