On Mon, 26 Feb 2024 16:10:17 -0500 Harry Wentland <harry.wentland@xxxxxxx> 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@xxxxxxx> > --- > 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;
Attachment:
pgpj9AeFXeZxn.pgp
Description: OpenPGP digital signature