Hi,
On Friday 12 May 2017 05:52 AM, Matt Roper wrote:
On Mon, May 08, 2017 at 05:18:53PM +0530, Mahesh Kumar wrote:
This patch adds few wrapper to perform fixed_point_16_16 operations
mul_u32_fixed_16_16_round_up : Multiplies u32 and fixed_16_16_t
variables & returns u32 result with
rounding-off.
mul_fixed_16_16 : Multiplies two fixed_16_16_t variable & returns
fixed_16_16
div_round_up_fixed_16_16 : Perform division operation on fixed_16_16_t
variables & return u32 result with round-off
fixed_16_16_div_round_up_u64 : devide uint32_t variable by fixed_16_16
variable and round_up the result to uint32_t.
Minor nitpick, but I'd say "rounding-up" rather than "rounding-off" in
your descriptions here.
Will update.
These wrappers will be used by later patches in the series.
The name on this last one (fixed_16_16_div_round_up_u64) doesn't seem to
match what it does. You're using u64 internally, but the actual
operands are a u32 and a fixed point value. Wouldn't it make more sense
to call it div_round_up_u32_fixed_16_16 (that also keeps the dividend
first and the divisor second, which seems more natural to me).
Actually the naming of all the fixed point math functions is a bit
confusing/inconsistent. Some of them are "op_type[_type][_round_up],"
some of them are "op[_round_up]_type," some of them are
"type_op[_round_up]," and some of them are "type_op[_round_up]_type."
Also, none of them specify the return value type, but I guess that's not
too much of a problem since the use of a fixed point structure will
ensure the compiler warns if someone tries to use them assuming the
wrong kind of result.
Maybe we could standardize the names a bit to help avoid confusion? I'd
suggest "op[_round_up]_type1_type2." If both types are the same, we'd
still repeat the type for clarity, but maybe we could just use "fixed16"
or "fix16" to keep the overall names from getting too long. And for
division we'd always keep the dividend as the first type, divisor as
second.
E.g.,
mul_round_up_u32_fixed16()
div_round_up_u32_fixed16()
mul_fixed16_fixed16()
div_round_up_fixed16_fixed16()
will use name "mul_fixed16 / div_round_up_fixed16" (assuming same type
for both the operand if only one type is there in function name), hope
that is fine with you.
And presumably the existing fixed point operations could be renamed in
the same manner.
thanks for suggestion/review, It sounds logical & more consistent, will
fix it,
In this patch will update only these 4 wrappers, will create a new patch
at end of the series to address naming for other wrappers.
thanks,
-Mahesh
Matt
Signed-off-by: Mahesh Kumar <mahesh1.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_drv.h | 43 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5804734d30a3..5ff0091707c0 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -152,6 +152,38 @@ static inline uint_fixed_16_16_t max_fixed_16_16(uint_fixed_16_16_t max1,
return max;
}
+static inline uint32_t div_round_up_fixed_16_16(uint_fixed_16_16_t val,
+ uint_fixed_16_16_t d)
+{
+ return DIV_ROUND_UP(val.val, d.val);
+}
+
+static inline uint32_t mul_u32_fixed_16_16_round_up(uint32_t val,
+ uint_fixed_16_16_t mul)
+{
+ uint64_t intermediate_val;
+ uint32_t result;
+
+ intermediate_val = (uint64_t) val * mul.val;
+ intermediate_val = DIV_ROUND_UP_ULL(intermediate_val, 1 << 16);
+ WARN_ON(intermediate_val >> 32);
+ result = clamp_t(uint32_t, intermediate_val, 0, ~0);
+ return result;
+}
+
+static inline uint_fixed_16_16_t mul_fixed_16_16(uint_fixed_16_16_t val,
+ uint_fixed_16_16_t mul)
+{
+ uint64_t intermediate_val;
+ uint_fixed_16_16_t fp;
+
+ intermediate_val = (uint64_t) val.val * mul.val;
+ intermediate_val = intermediate_val >> 16;
+ WARN_ON(intermediate_val >> 32);
+ fp.val = clamp_t(uint32_t, intermediate_val, 0, ~0);
+ return fp;
+}
+
static inline uint_fixed_16_16_t fixed_16_16_div(uint32_t val, uint32_t d)
{
uint_fixed_16_16_t fp, res;
@@ -174,6 +206,17 @@ static inline uint_fixed_16_16_t fixed_16_16_div_u64(uint32_t val, uint32_t d)
return res;
}
+static inline uint32_t fixed_16_16_div_round_up_u64(uint32_t val,
+ uint_fixed_16_16_t d)
+{
+ uint64_t interm_val;
+
+ interm_val = (uint64_t)val << 16;
+ interm_val = DIV_ROUND_UP_ULL(interm_val, d.val);
+ WARN_ON(interm_val >> 32);
+ return clamp_t(uint32_t, interm_val, 0, ~0);
+}
+
static inline uint_fixed_16_16_t mul_u32_fixed_16_16(uint32_t val,
uint_fixed_16_16_t mul)
{
--
2.11.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
