Re: [PATCH] drm/vkms: fix 32bit compilation error by replacing macros

2022-09-11 Thread Igor Matheus Andrade Torrente

On 9/10/22 16:10, Melissa Wen wrote:

On 09/09, Igor Matheus Andrade Torrente wrote:

Hi Mellisa,

Thanks for the patch fixing my mistakes.

On 9/9/22 08:41, Melissa Wen wrote:

Replace vkms_formats macros for fixed-point operations with functions
from drm/drm_fixed.h to do the same job and fix 32-bit compilation
errors.

Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
Tested-by: Sudip Mukherjee 
Reported-by: Sudip Mukherjee 
Reported-by: kernel test robot 
Signed-off-by: Melissa Wen 
---
   drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
   1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_formats.c 
b/drivers/gpu/drm/vkms/vkms_formats.c
index 300abb4d1dfe..ddcd3cfeeaac 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -1,27 +1,12 @@
   // SPDX-License-Identifier: GPL-2.0+
-#include 
+#include 
   #include 
+#include 
+#include 
   #include "vkms_formats.h"
-/* The following macros help doing fixed point arithmetic. */
-/*
- * With Fixed-Point scale 15 we have 17 and 15 bits of integer and fractional
- * parts respectively.
- *  |     0.000    |
- * 31  0
- */
-#define SHIFT 15
-
-#define INT_TO_FIXED(a) ((a) << SHIFT)
-#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
-#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
-/* This macro converts a fixed point number to int, and round half up it */
-#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-
   static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, 
int y)
   {
return frame_info->offset + (y * frame_info->pitch)
@@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer 
*stage_buffer,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
   stage_buffer->n_pixels);
-   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
-   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);


I think you need to add `drm_int2fixp` to 31 and 63.


for (size_t x = 0; x < x_limit; x++, src_pixels++) {
u16 rgb_565 = le16_to_cpu(*src_pixels);
-   s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
-   s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
-   s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
+   s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
+   s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
+   s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);


And we are cast implicitly from 64 bits int to 32 bits which is
implementation-defined AFAIK. So, probably we should be using `s64` for all
of these variables.

I tested the patch. And I'm seeing some differences in the intermediate
results. From my testing, these changes solve those differences.


Hi Igor,

Thanks for checking the calc results and all inputs provided.  I just
sent a second version, can you take a look? I replicated your
suggestions for RGB565_to_argb_u16() in argb_u16_to_RGB565() and
double-checked for i386 and arm. Let me know what yexiuou think.


I tested it here and everything seem to be working :).





Another thing that may have an impact on the final output is the lack of
rounding in drm_fixed.h. This can potentially produce the wrong result.


Yeah, I see... I can include a comment about the rounding issue for
further improvements, or do you plan to work on it?


A comment would be good. Maybe add to the VKMS TODO list?

I'm busy with other stuffs, and can't work on this any time soon. But 
It's pretty simple thing to implement.




Thanks,

Melissa


Thanks,
---
Igor Torrente


out_pixels[x].a = (u16)0x;
-   out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r, 
fp_rb_ratio));
-   out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g, 
fp_g_ratio));
-   out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b, 
fp_rb_ratio));
+   out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
+   out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
+   out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
}
   }
@@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info 
*frame_info,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
src_buffer->n_pixels);
-   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
-   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+   s32 fp_g_ratio = 

Re: [PATCH] drm/vkms: fix 32bit compilation error by replacing macros

2022-09-10 Thread Melissa Wen
On 09/09, Igor Matheus Andrade Torrente wrote:
> Hi Mellisa,
> 
> Thanks for the patch fixing my mistakes.
> 
> On 9/9/22 08:41, Melissa Wen wrote:
> > Replace vkms_formats macros for fixed-point operations with functions
> > from drm/drm_fixed.h to do the same job and fix 32-bit compilation
> > errors.
> > 
> > Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
> > Tested-by: Sudip Mukherjee 
> > Reported-by: Sudip Mukherjee 
> > Reported-by: kernel test robot 
> > Signed-off-by: Melissa Wen 
> > ---
> >   drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
> >   1 file changed, 19 insertions(+), 34 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c 
> > b/drivers/gpu/drm/vkms/vkms_formats.c
> > index 300abb4d1dfe..ddcd3cfeeaac 100644
> > --- a/drivers/gpu/drm/vkms/vkms_formats.c
> > +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> > @@ -1,27 +1,12 @@
> >   // SPDX-License-Identifier: GPL-2.0+
> > -#include 
> > +#include 
> >   #include 
> > +#include 
> > +#include 
> >   #include "vkms_formats.h"
> > -/* The following macros help doing fixed point arithmetic. */
> > -/*
> > - * With Fixed-Point scale 15 we have 17 and 15 bits of integer and 
> > fractional
> > - * parts respectively.
> > - *  |     0.000    |
> > - * 31  0
> > - */
> > -#define SHIFT 15
> > -
> > -#define INT_TO_FIXED(a) ((a) << SHIFT)
> > -#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
> > -#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
> > -/* This macro converts a fixed point number to int, and round half up it */
> > -#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
> > -#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), 
> > INT_TO_FIXED(b)))
> > -#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), 
> > INT_TO_FIXED(b)))
> > -
> >   static size_t pixel_offset(const struct vkms_frame_info *frame_info, int 
> > x, int y)
> >   {
> > return frame_info->offset + (y * frame_info->pitch)
> > @@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer 
> > *stage_buffer,
> > int x_limit = min_t(size_t, drm_rect_width(_info->dst),
> >stage_buffer->n_pixels);
> > -   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
> > -   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
> > +   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
> > +   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
> 
> I think you need to add `drm_int2fixp` to 31 and 63.
> 
> > for (size_t x = 0; x < x_limit; x++, src_pixels++) {
> > u16 rgb_565 = le16_to_cpu(*src_pixels);
> > -   s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
> > -   s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
> > -   s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
> > +   s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
> > +   s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
> > +   s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);
> 
> And we are cast implicitly from 64 bits int to 32 bits which is
> implementation-defined AFAIK. So, probably we should be using `s64` for all
> of these variables.
> 
> I tested the patch. And I'm seeing some differences in the intermediate
> results. From my testing, these changes solve those differences.

Hi Igor,

Thanks for checking the calc results and all inputs provided.  I just
sent a second version, can you take a look? I replicated your
suggestions for RGB565_to_argb_u16() in argb_u16_to_RGB565() and
double-checked for i386 and arm. Let me know what you think.

> 
> Another thing that may have an impact on the final output is the lack of
> rounding in drm_fixed.h. This can potentially produce the wrong result.

Yeah, I see... I can include a comment about the rounding issue for
further improvements, or do you plan to work on it?

Thanks,

Melissa
> 
> Thanks,
> ---
> Igor Torrente
> 
> > out_pixels[x].a = (u16)0x;
> > -   out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r, 
> > fp_rb_ratio));
> > -   out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g, 
> > fp_g_ratio));
> > -   out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b, 
> > fp_rb_ratio));
> > +   out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
> > +   out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
> > +   out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
> > }
> >   }
> > @@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info 
> > *frame_info,
> > int x_limit = min_t(size_t, drm_rect_width(_info->dst),
> > src_buffer->n_pixels);
> > -   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
> > -   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
> > +   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
> > +   s32 fp_g_ratio = 

Re: [PATCH] drm/vkms: fix 32bit compilation error by replacing macros

2022-09-09 Thread Igor Matheus Andrade Torrente

Hi Mellisa,

Thanks for the patch fixing my mistakes.

On 9/9/22 08:41, Melissa Wen wrote:

Replace vkms_formats macros for fixed-point operations with functions
from drm/drm_fixed.h to do the same job and fix 32-bit compilation
errors.

Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
Tested-by: Sudip Mukherjee 
Reported-by: Sudip Mukherjee 
Reported-by: kernel test robot 
Signed-off-by: Melissa Wen 
---
  drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
  1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_formats.c 
b/drivers/gpu/drm/vkms/vkms_formats.c
index 300abb4d1dfe..ddcd3cfeeaac 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -1,27 +1,12 @@
  // SPDX-License-Identifier: GPL-2.0+
  
-#include 

+#include 
  #include 
+#include 
+#include 
  
  #include "vkms_formats.h"
  
-/* The following macros help doing fixed point arithmetic. */

-/*
- * With Fixed-Point scale 15 we have 17 and 15 bits of integer and fractional
- * parts respectively.
- *  |     0.000    |
- * 31  0
- */
-#define SHIFT 15
-
-#define INT_TO_FIXED(a) ((a) << SHIFT)
-#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
-#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
-/* This macro converts a fixed point number to int, and round half up it */
-#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-
  static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, 
int y)
  {
return frame_info->offset + (y * frame_info->pitch)
@@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer 
*stage_buffer,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
   stage_buffer->n_pixels);
  
-	s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);

-   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);


I think you need to add `drm_int2fixp` to 31 and 63.

  
  	for (size_t x = 0; x < x_limit; x++, src_pixels++) {

u16 rgb_565 = le16_to_cpu(*src_pixels);
-   s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
-   s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
-   s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
+   s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
+   s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
+   s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);


And we are cast implicitly from 64 bits int to 32 bits which is 
implementation-defined AFAIK. So, probably we should be using `s64` for 
all of these variables.


I tested the patch. And I'm seeing some differences in the intermediate 
results. From my testing, these changes solve those differences.


Another thing that may have an impact on the final output is the lack of 
rounding in drm_fixed.h. This can potentially produce the wrong result.


Thanks,
---
Igor Torrente

  
  		out_pixels[x].a = (u16)0x;

-   out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r, 
fp_rb_ratio));
-   out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g, 
fp_g_ratio));
-   out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b, 
fp_rb_ratio));
+   out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
+   out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
+   out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
}
  }
  
@@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info *frame_info,

int x_limit = min_t(size_t, drm_rect_width(_info->dst),
src_buffer->n_pixels);
  
-	s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);

-   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
  
  	for (size_t x = 0; x < x_limit; x++, dst_pixels++) {

-   s32 fp_r = INT_TO_FIXED(in_pixels[x].r);
-   s32 fp_g = INT_TO_FIXED(in_pixels[x].g);
-   s32 fp_b = INT_TO_FIXED(in_pixels[x].b);
+   s32 fp_r = drm_int2fixp(in_pixels[x].r);
+   s32 fp_g = drm_int2fixp(in_pixels[x].g);
+   s32 fp_b = drm_int2fixp(in_pixels[x].b);
  
-		u16 r = FIXED_TO_INT_ROUND(FIXED_DIV(fp_r, fp_rb_ratio));

-   u16 g = FIXED_TO_INT_ROUND(FIXED_DIV(fp_g, fp_g_ratio));
-   u16 b = FIXED_TO_INT_ROUND(FIXED_DIV(fp_b, fp_rb_ratio));
+   u16 r = drm_fixp2int(drm_fixp_div(fp_r, fp_rb_ratio));
+   u16 g = 

Re: [PATCH] drm/vkms: fix 32bit compilation error by replacing macros

2022-09-09 Thread Alex Deucher
On Fri, Sep 9, 2022 at 7:42 AM Melissa Wen  wrote:
>
> Replace vkms_formats macros for fixed-point operations with functions
> from drm/drm_fixed.h to do the same job and fix 32-bit compilation
> errors.
>
> Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
> Tested-by: Sudip Mukherjee 
> Reported-by: Sudip Mukherjee 
> Reported-by: kernel test robot 
> Signed-off-by: Melissa Wen 

Reviewed-by: Alex Deucher 

> ---
>  drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
>  1 file changed, 19 insertions(+), 34 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c 
> b/drivers/gpu/drm/vkms/vkms_formats.c
> index 300abb4d1dfe..ddcd3cfeeaac 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -1,27 +1,12 @@
>  // SPDX-License-Identifier: GPL-2.0+
>
> -#include 
> +#include 
>  #include 
> +#include 
> +#include 
>
>  #include "vkms_formats.h"
>
> -/* The following macros help doing fixed point arithmetic. */
> -/*
> - * With Fixed-Point scale 15 we have 17 and 15 bits of integer and fractional
> - * parts respectively.
> - *  |     0.000    |
> - * 31  0
> - */
> -#define SHIFT 15
> -
> -#define INT_TO_FIXED(a) ((a) << SHIFT)
> -#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
> -#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
> -/* This macro converts a fixed point number to int, and round half up it */
> -#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
> -#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
> -#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
> -
>  static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, 
> int y)
>  {
> return frame_info->offset + (y * frame_info->pitch)
> @@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer 
> *stage_buffer,
> int x_limit = min_t(size_t, drm_rect_width(_info->dst),
>stage_buffer->n_pixels);
>
> -   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
> -   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
> +   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
> +   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
>
> for (size_t x = 0; x < x_limit; x++, src_pixels++) {
> u16 rgb_565 = le16_to_cpu(*src_pixels);
> -   s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
> -   s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
> -   s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
> +   s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
> +   s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
> +   s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);
>
> out_pixels[x].a = (u16)0x;
> -   out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r, 
> fp_rb_ratio));
> -   out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g, 
> fp_g_ratio));
> -   out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b, 
> fp_rb_ratio));
> +   out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, 
> fp_rb_ratio));
> +   out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, 
> fp_g_ratio));
> +   out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, 
> fp_rb_ratio));
> }
>  }
>
> @@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info 
> *frame_info,
> int x_limit = min_t(size_t, drm_rect_width(_info->dst),
> src_buffer->n_pixels);
>
> -   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
> -   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
> +   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
> +   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
>
> for (size_t x = 0; x < x_limit; x++, dst_pixels++) {
> -   s32 fp_r = INT_TO_FIXED(in_pixels[x].r);
> -   s32 fp_g = INT_TO_FIXED(in_pixels[x].g);
> -   s32 fp_b = INT_TO_FIXED(in_pixels[x].b);
> +   s32 fp_r = drm_int2fixp(in_pixels[x].r);
> +   s32 fp_g = drm_int2fixp(in_pixels[x].g);
> +   s32 fp_b = drm_int2fixp(in_pixels[x].b);
>
> -   u16 r = FIXED_TO_INT_ROUND(FIXED_DIV(fp_r, fp_rb_ratio));
> -   u16 g = FIXED_TO_INT_ROUND(FIXED_DIV(fp_g, fp_g_ratio));
> -   u16 b = FIXED_TO_INT_ROUND(FIXED_DIV(fp_b, fp_rb_ratio));
> +   u16 r = drm_fixp2int(drm_fixp_div(fp_r, fp_rb_ratio));
> +   u16 g = drm_fixp2int(drm_fixp_div(fp_g, fp_g_ratio));
> +   u16 b = drm_fixp2int(drm_fixp_div(fp_b, fp_rb_ratio));
>
> *dst_pixels = cpu_to_le16(r << 11 | g << 5 | b);
> }
> --
> 2.35.1
>


[PATCH] drm/vkms: fix 32bit compilation error by replacing macros

2022-09-09 Thread Melissa Wen
Replace vkms_formats macros for fixed-point operations with functions
from drm/drm_fixed.h to do the same job and fix 32-bit compilation
errors.

Fixes: a19c2ac9858 ("drm: vkms: Add support to the RGB565 format")
Tested-by: Sudip Mukherjee 
Reported-by: Sudip Mukherjee 
Reported-by: kernel test robot 
Signed-off-by: Melissa Wen 
---
 drivers/gpu/drm/vkms/vkms_formats.c | 53 +++--
 1 file changed, 19 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_formats.c 
b/drivers/gpu/drm/vkms/vkms_formats.c
index 300abb4d1dfe..ddcd3cfeeaac 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -1,27 +1,12 @@
 // SPDX-License-Identifier: GPL-2.0+
 
-#include 
+#include 
 #include 
+#include 
+#include 
 
 #include "vkms_formats.h"
 
-/* The following macros help doing fixed point arithmetic. */
-/*
- * With Fixed-Point scale 15 we have 17 and 15 bits of integer and fractional
- * parts respectively.
- *  |     0.000    |
- * 31  0
- */
-#define SHIFT 15
-
-#define INT_TO_FIXED(a) ((a) << SHIFT)
-#define FIXED_MUL(a, b) ((s32)(((s64)(a) * (b)) >> SHIFT))
-#define FIXED_DIV(a, b) ((s32)(((s64)(a) << SHIFT) / (b)))
-/* This macro converts a fixed point number to int, and round half up it */
-#define FIXED_TO_INT_ROUND(a) (((a) + (1 << (SHIFT - 1))) >> SHIFT)
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-#define INT_TO_FIXED_DIV(a, b) (FIXED_DIV(INT_TO_FIXED(a), INT_TO_FIXED(b)))
-
 static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, 
int y)
 {
return frame_info->offset + (y * frame_info->pitch)
@@ -137,19 +122,19 @@ static void RGB565_to_argb_u16(struct line_buffer 
*stage_buffer,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
   stage_buffer->n_pixels);
 
-   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
-   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
 
for (size_t x = 0; x < x_limit; x++, src_pixels++) {
u16 rgb_565 = le16_to_cpu(*src_pixels);
-   s32 fp_r = INT_TO_FIXED((rgb_565 >> 11) & 0x1f);
-   s32 fp_g = INT_TO_FIXED((rgb_565 >> 5) & 0x3f);
-   s32 fp_b = INT_TO_FIXED(rgb_565 & 0x1f);
+   s32 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f);
+   s32 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f);
+   s32 fp_b = drm_int2fixp(rgb_565 & 0x1f);
 
out_pixels[x].a = (u16)0x;
-   out_pixels[x].r = FIXED_TO_INT_ROUND(FIXED_MUL(fp_r, 
fp_rb_ratio));
-   out_pixels[x].g = FIXED_TO_INT_ROUND(FIXED_MUL(fp_g, 
fp_g_ratio));
-   out_pixels[x].b = FIXED_TO_INT_ROUND(FIXED_MUL(fp_b, 
fp_rb_ratio));
+   out_pixels[x].r = drm_fixp2int(drm_fixp_mul(fp_r, fp_rb_ratio));
+   out_pixels[x].g = drm_fixp2int(drm_fixp_mul(fp_g, fp_g_ratio));
+   out_pixels[x].b = drm_fixp2int(drm_fixp_mul(fp_b, fp_rb_ratio));
}
 }
 
@@ -248,17 +233,17 @@ static void argb_u16_to_RGB565(struct vkms_frame_info 
*frame_info,
int x_limit = min_t(size_t, drm_rect_width(_info->dst),
src_buffer->n_pixels);
 
-   s32 fp_rb_ratio = INT_TO_FIXED_DIV(65535, 31);
-   s32 fp_g_ratio = INT_TO_FIXED_DIV(65535, 63);
+   s32 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), 31);
+   s32 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), 63);
 
for (size_t x = 0; x < x_limit; x++, dst_pixels++) {
-   s32 fp_r = INT_TO_FIXED(in_pixels[x].r);
-   s32 fp_g = INT_TO_FIXED(in_pixels[x].g);
-   s32 fp_b = INT_TO_FIXED(in_pixels[x].b);
+   s32 fp_r = drm_int2fixp(in_pixels[x].r);
+   s32 fp_g = drm_int2fixp(in_pixels[x].g);
+   s32 fp_b = drm_int2fixp(in_pixels[x].b);
 
-   u16 r = FIXED_TO_INT_ROUND(FIXED_DIV(fp_r, fp_rb_ratio));
-   u16 g = FIXED_TO_INT_ROUND(FIXED_DIV(fp_g, fp_g_ratio));
-   u16 b = FIXED_TO_INT_ROUND(FIXED_DIV(fp_b, fp_rb_ratio));
+   u16 r = drm_fixp2int(drm_fixp_div(fp_r, fp_rb_ratio));
+   u16 g = drm_fixp2int(drm_fixp_div(fp_g, fp_g_ratio));
+   u16 b = drm_fixp2int(drm_fixp_div(fp_b, fp_rb_ratio));
 
*dst_pixels = cpu_to_le16(r << 11 | g << 5 | b);
}
-- 
2.35.1