lkml.org 
[lkml]   [2021]   [Sep]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/5] drm: Add drm_fixed_16_16 helper
Hi Alyssa,

Thank you for the patch.

On Wed, Sep 01, 2021 at 01:54:27PM -0400, Alyssa Rosenzweig wrote:
> This constructs a fixed 16.16 rational, useful to specify the minimum
> and maximum scaling in drm_atomic_helper_check_plane_state. It is
> open-coded as a macro in multiple drivers, so let's share the helper.
>
> Signed-off-by: Alyssa Rosenzweig <alyssa@rosenzweig.io>
> ---
> include/drm/drm_fixed.h | 5 +++++
> 1 file changed, 5 insertions(+)
>
> diff --git a/include/drm/drm_fixed.h b/include/drm/drm_fixed.h
> index 553210c02ee0..df1f369b4918 100644
> --- a/include/drm/drm_fixed.h
> +++ b/include/drm/drm_fixed.h
> @@ -208,4 +208,9 @@ static inline s64 drm_fixp_exp(s64 x)
> return sum;
> }
>

Missing documentation :-)

> +static inline int drm_fixed_16_16(s32 mult, s32 div)

You should return a s32.

The function name isn't very explicit, and departs from the naming
scheme of other functions in the same file. As fixed-point numbers are
stored in a s64 for the drm_fixp_* helpers, we shouldn't rese the
drm_fixp_ prefix, maybe drm_fixp_s16_16_ would be a good prefix. The
function should probably be named drm_fixp_s16_16 from_fraction() then,
but then the same logic should possibly be replicated to ensure optimal
precision. I wonder if it wouldn't be best to simply use
drm_fixp_from_fraction() and shift the result right by 16 bits.

> +{
> + return (mult << 16) / div;
> +}
> +
> #endif

--
Regards,

Laurent Pinchart

\
 
 \ /
  Last update: 2021-09-01 20:14    [W:0.102 / U:0.704 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site