| Message ID | 20240226211100.100108-4-harry.wentland@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Color Pipeline API w/ VKMS | expand |
On Mon, 26 Feb 2024 16:10:17 -0500 Harry Wentland <harry.wentland@amd.com> wrote: > A value of 0x80000000 and higher should round up, and > below should round down. VKMS Kunit tests for lerp_u16 > showed that this is not the case. Fix it. > > 1 << (DRM_FIXED_POINT_HALF - 1) = > 1 << 15 = 0x8000 > > This is not 0.5, but 0.00000762939453125. > > Instead of some smart math use a simple if/else to > round up or down. This helps people like me to understand > what the function does. > > Signed-off-by: Harry Wentland <harry.wentland@amd.com> > --- > include/drm/drm_fixed.h | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h > index cb842ba80ddd..8ee549f68537 100644 > --- a/include/drm/drm_fixed.h > +++ b/include/drm/drm_fixed.h > @@ -77,6 +77,8 @@ static inline u32 dfixed_div(fixed20_12 A, fixed20_12 B) > #define DRM_FIXED_DIGITS_MASK (~DRM_FIXED_DECIMAL_MASK) > #define DRM_FIXED_EPSILON 1LL > #define DRM_FIXED_ALMOST_ONE (DRM_FIXED_ONE - DRM_FIXED_EPSILON) > +#define DRM_FIXED_FRACTIONAL 0xffffffffll > +#define DRM_FIXED_HALF 0x80000000ll > > /** > * @drm_sm2fixp > @@ -106,11 +108,6 @@ static inline int drm_fixp2int(s64 a) > return ((s64)a) >> DRM_FIXED_POINT; > } > > -static inline int drm_fixp2int_round(s64 a) > -{ > - return drm_fixp2int(a + (1 << (DRM_FIXED_POINT_HALF - 1))); > -} > - > static inline int drm_fixp2int_ceil(s64 a) > { > if (a >= 0) > @@ -119,6 +116,14 @@ static inline int drm_fixp2int_ceil(s64 a) > return drm_fixp2int(a - DRM_FIXED_ALMOST_ONE); > } > > +static inline int drm_fixp2int_round(s64 a) > +{ > + if ((a & DRM_FIXED_FRACTIONAL) < DRM_FIXED_HALF) > + return drm_fixp2int(a); So, if we take -epsilon (which is -1 in raw s64 value, the largest possible value less than zero), this would return -1.0? That does not sound right. However, the "add 0.5 and truncate" trick always works, so I'd recommend sticking that. Thanks, pq > + else > + return drm_fixp2int_ceil(a); > +} > + > static inline unsigned drm_fixp_msbset(s64 a) > { > unsigned shift, sign = (a >> 63) & 1;
diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h index cb842ba80ddd..8ee549f68537 100644 --- a/include/drm/drm_fixed.h +++ b/include/drm/drm_fixed.h @@ -77,6 +77,8 @@ static inline u32 dfixed_div(fixed20_12 A, fixed20_12 B) #define DRM_FIXED_DIGITS_MASK (~DRM_FIXED_DECIMAL_MASK) #define DRM_FIXED_EPSILON 1LL #define DRM_FIXED_ALMOST_ONE (DRM_FIXED_ONE - DRM_FIXED_EPSILON) +#define DRM_FIXED_FRACTIONAL 0xffffffffll +#define DRM_FIXED_HALF 0x80000000ll /** * @drm_sm2fixp @@ -106,11 +108,6 @@ static inline int drm_fixp2int(s64 a) return ((s64)a) >> DRM_FIXED_POINT; } -static inline int drm_fixp2int_round(s64 a) -{ - return drm_fixp2int(a + (1 << (DRM_FIXED_POINT_HALF - 1))); -} - static inline int drm_fixp2int_ceil(s64 a) { if (a >= 0) @@ -119,6 +116,14 @@ static inline int drm_fixp2int_ceil(s64 a) return drm_fixp2int(a - DRM_FIXED_ALMOST_ONE); } +static inline int drm_fixp2int_round(s64 a) +{ + if ((a & DRM_FIXED_FRACTIONAL) < DRM_FIXED_HALF) + return drm_fixp2int(a); + else + return drm_fixp2int_ceil(a); +} + static inline unsigned drm_fixp_msbset(s64 a) { unsigned shift, sign = (a >> 63) & 1;
A value of 0x80000000 and higher should round up, and below should round down. VKMS Kunit tests for lerp_u16 showed that this is not the case. Fix it. 1 << (DRM_FIXED_POINT_HALF - 1) = 1 << 15 = 0x8000 This is not 0.5, but 0.00000762939453125. Instead of some smart math use a simple if/else to round up or down. This helps people like me to understand what the function does. Signed-off-by: Harry Wentland <harry.wentland@amd.com> --- include/drm/drm_fixed.h | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)