[Intel-gfx] [PATCH 5/5] drm/i915: Tidy up fixed16_16

[Chris Wilson]

Quoting Michal Wajdeczko (2017-12-22 12:25:56)
> Tidy up fixed16_16 code before promoting it to the include/linux.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/fixed16_16.h | 96 ++++++++++++++++++---------------------
>  1 file changed, 43 insertions(+), 53 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/fixed16_16.h b/drivers/gpu/drm/i915/fixed16_16.h
> index 43fe0037..a2f8736 100644
> --- a/drivers/gpu/drm/i915/fixed16_16.h
> +++ b/drivers/gpu/drm/i915/fixed16_16.h
> @@ -46,21 +46,19 @@
>         fp; \
>  })
>  
> -static inline bool is_fixed16_zero(fixed16_16_t val)
> +static inline bool is_fixed16_zero(fixed16_16_t fp)
>  {
> -       if (val.val == 0)
> -               return true;
> -       return false;
> +       return fp.val == 0;
>  }
>  
> -static inline fixed16_16_t u32_to_fixed16(u32 val)
> +static inline fixed16_16_t u32_to_fixed16(u32 value)
>  {
>         fixed16_16_t fp;
>  
> -       if (WARN_ON(val > U16_MAX))
> -               val = U16_MAX;
> +       if (WARN_ON(value > U16_MAX))
> +               value = U16_MAX;
>  
> -       fp.val = val << 16;
> +       fp.val = value << 16;
>         return fp;
>  }
>  
> @@ -76,99 +74,91 @@ static inline u32 fixed16_to_u32(fixed16_16_t fp)
>  
>  static inline fixed16_16_t min_fixed16(fixed16_16_t min1, fixed16_16_t min2)
>  {
> -       fixed16_16_t min;
> -
> -       min.val = min(min1.val, min2.val);
> -       return min;
> +       if (min1.val <= min2.val)
> +               return min1;
> +       return min2;
>  }
>  
>  static inline fixed16_16_t max_fixed16(fixed16_16_t max1, fixed16_16_t max2)
>  {
> -       fixed16_16_t max;
> -
> -       max.val = max(max1.val, max2.val);
> -       return max;
> +       if (max1.val >= max2.val)
> +               return max1;
> +       return max2;
>  }
>  
> -static inline fixed16_16_t clamp_u64_to_fixed16(u64 val)
> +static inline fixed16_16_t clamp_u64_to_fixed16(u64 value)
>  {
>         fixed16_16_t fp;
>  
> -       if (WARN_ON(val > U32_MAX))
> -               val = U32_MAX;
> +       if (WARN_ON(value > U32_MAX))
> +               value = U32_MAX;
>  
> -       fp.val = (u32) val;
> +       fp.val = value;
>         return fp;
>  }
>  
> -static inline u32 div_round_up_fixed16(fixed16_16_t val, fixed16_16_t d)
> +static inline u32 div_round_up_fixed16(fixed16_16_t fp, fixed16_16_t d)
>  {
> -       return DIV_ROUND_UP(val.val, d.val);
> +       return DIV_ROUND_UP(fp.val, d.val);
>  }
>  
>  static inline u32 mul_round_up_u32_fixed16(u32 val, fixed16_16_t mul)
>  {
> -       u64 intermediate_val;
> +       u64 tmp;
>  
> -       intermediate_val = (u64) val * mul.val;
> -       intermediate_val = DIV_ROUND_UP_ULL(intermediate_val, 1 << 16);

DIV_ROUND_UP_ULL with a known constant power-of-two. We can do better.

> -       if (WARN_ON(intermediate_val > U32_MAX))
> -               intermediate_val = U32_MAX;
> -       return (u32) intermediate_val;
> +       tmp = (u64) val * mul.val;
> +       tmp = DIV_ROUND_UP_ULL(tmp, 1 << 16);
> +       if (WARN_ON(tmp > U32_MAX))
> +               tmp = U32_MAX;
> +       return tmp;
>  }
>  
>  static inline fixed16_16_t mul_fixed16(fixed16_16_t val, fixed16_16_t mul)
>  {
> -       u64 intermediate_val;
> +       u64 tmp;
>  
> -       intermediate_val = (u64) val.val * mul.val;
> -       intermediate_val = intermediate_val >> 16;
> -       return clamp_u64_to_fixed16(intermediate_val);
> +       tmp = (u64) val.val * mul.val;

Use mul_u32_u32(u32, u32) -> u64