2023-08-03 14:02:28

by Andy Shevchenko

[permalink] [raw]
Subject: [PATCH v4 1/1] drm/i915: Move abs_diff() to math.h

abs_diff() belongs to math.h. Move it there.
This will allow others to use it.

Reviewed-by: Jiri Slaby <[email protected]> # tty/serial
Acked-by: Jani Nikula <[email protected]>
Acked-by: Greg Kroah-Hartman <[email protected]>
Reviewed-by: Andi Shyti <[email protected]>
Reviewed-by: Philipp Zabel <[email protected]> # gpu/ipu-v3
Signed-off-by: Andy Shevchenko <[email protected]>
---
v4:
- Cc'ed to Andrew (as Jani told he is okay to route it via other tree)
- added tags
drivers/gpu/drm/i915/display/intel_dpll_mgr.c | 1 +
drivers/gpu/drm/i915/display/intel_dpll_mgr.h | 7 -------
drivers/gpu/ipu-v3/ipu-image-convert.c | 15 +++++++--------
drivers/tty/serial/omap-serial.c | 7 +------
drivers/video/fbdev/core/svgalib.c | 7 +------
include/linux/math.h | 7 +++++++
6 files changed, 17 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
index 6b2d8a1e2aa9..290e856fe9e9 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.c
@@ -21,6 +21,7 @@
* DEALINGS IN THE SOFTWARE.
*/

+#include <linux/math.h>
#include <linux/string_helpers.h>

#include "i915_reg.h"
diff --git a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
index ba62eb5d7c51..04e6810954b2 100644
--- a/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
+++ b/drivers/gpu/drm/i915/display/intel_dpll_mgr.h
@@ -29,13 +29,6 @@

#include "intel_wakeref.h"

-/*FIXME: Move this to a more appropriate place. */
-#define abs_diff(a, b) ({ \
- typeof(a) __a = (a); \
- typeof(b) __b = (b); \
- (void) (&__a == &__b); \
- __a > __b ? (__a - __b) : (__b - __a); })
-
enum tc_port;
struct drm_i915_private;
struct intel_atomic_state;
diff --git a/drivers/gpu/ipu-v3/ipu-image-convert.c b/drivers/gpu/ipu-v3/ipu-image-convert.c
index af1612044eef..841316582ea9 100644
--- a/drivers/gpu/ipu-v3/ipu-image-convert.c
+++ b/drivers/gpu/ipu-v3/ipu-image-convert.c
@@ -7,7 +7,10 @@

#include <linux/interrupt.h>
#include <linux/dma-mapping.h>
+#include <linux/math.h>
+
#include <video/imx-ipu-image-convert.h>
+
#include "ipu-prv.h"

/*
@@ -543,7 +546,7 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
unsigned int in_pos;
unsigned int in_pos_aligned;
unsigned int in_pos_rounded;
- unsigned int abs_diff;
+ unsigned int diff;

/*
* Tiles in the right row / bottom column may not be allowed to
@@ -575,15 +578,11 @@ static void find_best_seam(struct ipu_image_convert_ctx *ctx,
(in_edge - in_pos_rounded) % in_burst)
continue;

- if (in_pos < in_pos_aligned)
- abs_diff = in_pos_aligned - in_pos;
- else
- abs_diff = in_pos - in_pos_aligned;
-
- if (abs_diff < min_diff) {
+ diff = abs_diff(in_pos, in_pos_aligned);
+ if (diff < min_diff) {
in_seam = in_pos_rounded;
out_seam = out_pos;
- min_diff = abs_diff;
+ min_diff = diff;
}
}

diff --git a/drivers/tty/serial/omap-serial.c b/drivers/tty/serial/omap-serial.c
index 3dc14dcb01ca..0ead88c5a19a 100644
--- a/drivers/tty/serial/omap-serial.c
+++ b/drivers/tty/serial/omap-serial.c
@@ -222,16 +222,11 @@ static inline int calculate_baud_abs_diff(struct uart_port *port,
unsigned int baud, unsigned int mode)
{
unsigned int n = port->uartclk / (mode * baud);
- int abs_diff;

if (n == 0)
n = 1;

- abs_diff = baud - (port->uartclk / (mode * n));
- if (abs_diff < 0)
- abs_diff = -abs_diff;
-
- return abs_diff;
+ return abs_diff(baud, port->uartclk / (mode * n));
}

/*
diff --git a/drivers/video/fbdev/core/svgalib.c b/drivers/video/fbdev/core/svgalib.c
index 9e01322fabe3..2cba158888ea 100644
--- a/drivers/video/fbdev/core/svgalib.c
+++ b/drivers/video/fbdev/core/svgalib.c
@@ -14,6 +14,7 @@
#include <linux/kernel.h>
#include <linux/string.h>
#include <linux/fb.h>
+#include <linux/math.h>
#include <linux/svga.h>
#include <asm/types.h>
#include <asm/io.h>
@@ -372,12 +373,6 @@ EXPORT_SYMBOL(svga_get_caps);
* F_VCO = (F_BASE * M) / N
* F_OUT = F_VCO / (2^R)
*/
-
-static inline u32 abs_diff(u32 a, u32 b)
-{
- return (a > b) ? (a - b) : (b - a);
-}
-
int svga_compute_pll(const struct svga_pll *pll, u32 f_wanted, u16 *m, u16 *n, u16 *r, int node)
{
u16 am, an, ar;
diff --git a/include/linux/math.h b/include/linux/math.h
index 2d388650c556..336e3e3678e7 100644
--- a/include/linux/math.h
+++ b/include/linux/math.h
@@ -155,6 +155,13 @@ __STRUCT_FRACT(u32)
__builtin_types_compatible_p(typeof(x), unsigned type), \
({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)

+#define abs_diff(a, b) ({ \
+ typeof(a) __a = (a); \
+ typeof(b) __b = (b); \
+ (void)(&__a == &__b); \
+ __a > __b ? (__a - __b) : (__b - __a); \
+})
+
/**
* reciprocal_scale - "scale" a value into range [0, ep_ro)
* @val: value
--
2.40.0.1.gaa8946217a0b



2023-08-03 17:57:00

by Andrew Morton

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] drm/i915: Move abs_diff() to math.h

On Thu, 3 Aug 2023 16:19:18 +0300 Andy Shevchenko <[email protected]> wrote:

> abs_diff() belongs to math.h. Move it there.
> This will allow others to use it.
>
> ...
>
> --- a/include/linux/math.h
> +++ b/include/linux/math.h
> @@ -155,6 +155,13 @@ __STRUCT_FRACT(u32)
> __builtin_types_compatible_p(typeof(x), unsigned type), \
> ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
>
> +#define abs_diff(a, b) ({ \
> + typeof(a) __a = (a); \
> + typeof(b) __b = (b); \
> + (void)(&__a == &__b); \
> + __a > __b ? (__a - __b) : (__b - __a); \
> +})

Can we document it please?

Also, the open-coded type comparison could be replaced with __typecheck()?

And why the heck isn't __typecheck() in typecheck.h, to be included by
minmax.h.

etcetera. Sigh. I'll grab it, but please at least send along some
kerneldoc?



2023-08-04 06:11:34

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 1/1] drm/i915: Move abs_diff() to math.h

On Thu, Aug 03, 2023 at 10:24:46AM -0700, Andrew Morton wrote:
> On Thu, 3 Aug 2023 16:19:18 +0300 Andy Shevchenko <[email protected]> wrote:

...

> > +#define abs_diff(a, b) ({ \
> > + typeof(a) __a = (a); \
> > + typeof(b) __b = (b); \
> > + (void)(&__a == &__b); \
> > + __a > __b ? (__a - __b) : (__b - __a); \
> > +})
>
> Can we document it please?
>
> Also, the open-coded type comparison could be replaced with __typecheck()?
>
> And why the heck isn't __typecheck() in typecheck.h, to be included by
> minmax.h.
>
> etcetera. Sigh. I'll grab it, but please at least send along some
> kerneldoc?

Sure and thank you!

--
With Best Regards,
Andy Shevchenko



2023-08-04 16:18:43

by David Laight

[permalink] [raw]
Subject: RE: [PATCH v4 1/1] drm/i915: Move abs_diff() to math.h

From: Andrew Morton
> Sent: 03 August 2023 18:25
>
> On Thu, 3 Aug 2023 16:19:18 +0300 Andy Shevchenko <[email protected]> wrote:
>
> > abs_diff() belongs to math.h. Move it there.
> > This will allow others to use it.
> >
> > ...
> >
> > --- a/include/linux/math.h
> > +++ b/include/linux/math.h
> > @@ -155,6 +155,13 @@ __STRUCT_FRACT(u32)
> > __builtin_types_compatible_p(typeof(x), unsigned type), \
> > ({ signed type __x = (x); __x < 0 ? -__x : __x; }), other)
> >
> > +#define abs_diff(a, b) ({ \
> > + typeof(a) __a = (a); \
> > + typeof(b) __b = (b); \
> > + (void)(&__a == &__b); \
> > + __a > __b ? (__a - __b) : (__b - __a); \
> > +})
>
> Can we document it please?
>
> Also, the open-coded type comparison could be replaced with __typecheck()?
>
> And why the heck isn't __typecheck() in typecheck.h, to be included by
> minmax.h.

And why would you want to use __typecheck() anyway?
It pretty much isn't the test you are looking for.
If you are trying to explicitly avoid converting negative value
to large positive unsigned ones then you want something like:
is_signed_type(typeof(a)) == is_signed_type(typeof(b))
but it isn't even that simple :-)

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)