Re: [PATCH v4 9/9] drm: vkms: Add support to the RGB565 format

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2864 bytes --]

On Thu, 24 Feb 2022 22:03:42 -0300
Igor Torrente <igormtorrente@gmail.com> wrote:

> Hi Pekka,
> 
> On Thu, Feb 10, 2022 at 6:50 AM Pekka Paalanen <ppaalanen@gmail.com> wrote:
> 
> > On Fri, 21 Jan 2022 18:38:31 -0300
> > Igor Torrente <igormtorrente@gmail.com> wrote:
> >  
> > > Adds this common format to vkms.
> > >
> > > This commit also adds new helper macros to deal with fixed-point
> > > arithmetic.
> > >
> > > It was done to improve the precision of the conversion to ARGB16161616
> > > since the "conversion ratio" is not an integer.
> > >
> > > Signed-off-by: Igor Torrente <igormtorrente@gmail.com>
> > > ---
> > > V3: Adapt the handlers to the new format introduced in patch 7 V3.
> > > ---
> > >  drivers/gpu/drm/vkms/vkms_formats.c   | 74 +++++++++++++++++++++++++++
> > >  drivers/gpu/drm/vkms/vkms_formats.h   |  6 +++
> > >  drivers/gpu/drm/vkms/vkms_plane.c     |  6 ++-
> > >  drivers/gpu/drm/vkms/vkms_writeback.c |  3 +-
> > >  4 files changed, 86 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c  
> > b/drivers/gpu/drm/vkms/vkms_formats.c  
> > > index 661da39d1276..dc612882dd8c 100644
> > > --- a/drivers/gpu/drm/vkms/vkms_formats.c
> > > +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> > > @@ -11,6 +11,8 @@ format_transform_func get_fmt_transform_function(u32  
> > format)  
> > >               return &get_ARGB16161616;
> > >       else if (format == DRM_FORMAT_XRGB16161616)
> > >               return &XRGB16161616_to_ARGB16161616;
> > > +     else if (format == DRM_FORMAT_RGB565)
> > > +             return &RGB565_to_ARGB16161616;
> > >       else
> > >               return &XRGB8888_to_ARGB16161616;
> > >  }
> > > @@ -23,6 +25,8 @@ format_transform_func  
> > get_wb_fmt_transform_function(u32 format)  
> > >               return &convert_to_ARGB16161616;
> > >       else if (format == DRM_FORMAT_XRGB16161616)
> > >               return &convert_to_XRGB16161616;
> > > +     else if (format == DRM_FORMAT_RGB565)
> > > +             return &convert_to_RGB565;
> > >       else
> > >               return &convert_to_XRGB8888;
> > >  }
> > > @@ -33,6 +37,26 @@ static int pixel_offset(struct vkms_frame_info  
> > *frame_info, int x, int y)  
> > >                                 + (x * frame_info->cpp);
> > >  }
> > >
> > > +/*
> > > + * FP stands for _Fixed Point_ and **not** _Float Point_  
> >
> > Is it common in the kernel that FP always means fixed-point?
> >  
> 
> I cannot say for sure, but I don't think so. I put it for people like me
> that goes automatically to Floating-Point because never worked with
> fixed-point before.

Indeed, so do not use "FP" at all as an abbreviation, please. Use a
name or abbreviation that does not need a comment to prevent easy
misunderstandings.


Thanks,
pq

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]