[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <2af1f9b4-0bc8-4585-ba13-d3b97e25845f@riseup.net>
Date: Sat, 26 Oct 2024 12:08:44 -0300
From: Maíra Canal <mairacanal@...eup.net>
To: Louis Chauvet <louis.chauvet@...tlin.com>,
Rodrigo Siqueira <rodrigosiqueiramelo@...il.com>,
Melissa Wen <melissa.srw@...il.com>,
Haneen Mohammed <hamohammed.sa@...il.com>, Daniel Vetter <daniel@...ll.ch>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>
Cc: dri-devel@...ts.freedesktop.org, arthurgrillo@...eup.net,
linux-kernel@...r.kernel.org, jeremie.dautheribes@...tlin.com,
miquel.raynal@...tlin.com, thomas.petazzoni@...tlin.com,
seanpaul@...gle.com, nicolejadeyee@...gle.com
Subject: Re: [PATCH v2 2/3] drm/vkms: Add a macro for write_line functions
Hi Louis,
On 14/08/24 05:42, Louis Chauvet wrote:
> As stated in [2], the write_line functions are very similar and force code
Where is [2]?
> duplication. This patch add a macro to avoid code repetition.
>
> Signed-off-by: Louis Chauvet <louis.chauvet@...tlin.com>
> ---
> drivers/gpu/drm/vkms/vkms_formats.c | 107 ++++++++++--------------------------
> 1 file changed, 30 insertions(+), 77 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index d1309f6d307f..a25cdf656d8a 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -654,6 +654,31 @@ static void argb_u16_to_RGB565(u8 *out_pixel, const struct pixel_argb_u16 *in_pi
> *pixel = cpu_to_le16(r << 11 | g << 5 | b);
> }
>
> +/**
> + * WRITE_LINE() - Generic generator for write_line functions
> + *
> + * This generator can only be used for format with only one plane and block_w == block_h == 1
> + *
> + * @function_name: Name to use for the generated function
> + * @conversion_function: Fonction to use for the conversion from argb_u16 to the required format.
s/Fonction/Function
> + */
> +#define WRITE_LINE(function_name, conversion_function) \
> +static void function_name(struct vkms_writeback_job *wb, \
> + struct pixel_argb_u16 *src_pixels, int count, int x_start, \
> + int y_start) \
> +{ \
> + u8 *dst_pixels; \
> + \
> + packed_pixels_addr_1x1(&wb->wb_frame_info, x_start, y_start, 0, &dst_pixels); \
> + \
> + while (count) { \
> + (conversion_function)(dst_pixels, src_pixels); \
> + dst_pixels += wb->wb_frame_info.fb->format->char_per_block[0]; \
> + src_pixels += 1; \
> + count--; \
Just a nit: What do you think about this loop?
for (; count > 0; src_pixels++, count--)
It doesn't really matter what option you pick.
Best Regards,
- Maíra
> + } \
> +}
> +
> /*
> * The following functions are write_line function for each pixel format supported by VKMS.
> *
> @@ -667,85 +692,13 @@ static void argb_u16_to_RGB565(u8 *out_pixel, const struct pixel_argb_u16 *in_pi
> * [1]: https://lore.kernel.org/dri-devel/d258c8dc-78e9-4509-9037-a98f7f33b3a3@riseup.net/
> */
>
> -static void ARGB8888_write_line(struct vkms_writeback_job *wb,
> - struct pixel_argb_u16 *src_pixels, int count, int x_start,
> - int y_start)
> -{
> - u8 *dst_pixels;
> +WRITE_LINE(ARGB8888_write_line, argb_u16_to_ARGB8888)
> +WRITE_LINE(XRGB8888_write_line, argb_u16_to_XRGB8888)
>
> - packed_pixels_addr_1x1(&wb->wb_frame_info, x_start, y_start, 0, &dst_pixels);
> +WRITE_LINE(ARGB16161616_write_line, argb_u16_to_ARGB16161616)
> +WRITE_LINE(XRGB16161616_write_line, argb_u16_to_XRGB16161616)
>
> - while (count) {
> - argb_u16_to_ARGB8888(dst_pixels, src_pixels);
> - dst_pixels += wb->wb_frame_info.fb->format->char_per_block[0];
> - src_pixels += 1;
> - count--;
> - }
> -}
> -
> -static void XRGB8888_write_line(struct vkms_writeback_job *wb,
> - struct pixel_argb_u16 *src_pixels, int count, int x_start,
> - int y_start)
> -{
> - u8 *dst_pixels;
> -
> - packed_pixels_addr_1x1(&wb->wb_frame_info, x_start, y_start, 0, &dst_pixels);
> -
> - while (count) {
> - argb_u16_to_XRGB8888(dst_pixels, src_pixels);
> - dst_pixels += wb->wb_frame_info.fb->format->char_per_block[0];
> - src_pixels += 1;
> - count--;
> - }
> -}
> -
> -static void ARGB16161616_write_line(struct vkms_writeback_job *wb,
> - struct pixel_argb_u16 *src_pixels, int count, int x_start,
> - int y_start)
> -{
> - u8 *dst_pixels;
> -
> - packed_pixels_addr_1x1(&wb->wb_frame_info, x_start, y_start, 0, &dst_pixels);
> -
> - while (count) {
> - argb_u16_to_ARGB16161616(dst_pixels, src_pixels);
> - dst_pixels += wb->wb_frame_info.fb->format->char_per_block[0];
> - src_pixels += 1;
> - count--;
> - }
> -}
> -
> -static void XRGB16161616_write_line(struct vkms_writeback_job *wb,
> - struct pixel_argb_u16 *src_pixels, int count, int x_start,
> - int y_start)
> -{
> - u8 *dst_pixels;
> -
> - packed_pixels_addr_1x1(&wb->wb_frame_info, x_start, y_start, 0, &dst_pixels);
> -
> - while (count) {
> - argb_u16_to_XRGB16161616(dst_pixels, src_pixels);
> - dst_pixels += wb->wb_frame_info.fb->format->char_per_block[0];
> - src_pixels += 1;
> - count--;
> - }
> -}
> -
> -static void RGB565_write_line(struct vkms_writeback_job *wb,
> - struct pixel_argb_u16 *src_pixels, int count, int x_start,
> - int y_start)
> -{
> - u8 *dst_pixels;
> -
> - packed_pixels_addr_1x1(&wb->wb_frame_info, x_start, y_start, 0, &dst_pixels);
> -
> - while (count) {
> - argb_u16_to_RGB565(dst_pixels, src_pixels);
> - dst_pixels += wb->wb_frame_info.fb->format->char_per_block[0];
> - src_pixels += 1;
> - count--;
> - }
> -}
> +WRITE_LINE(RGB565_write_line, argb_u16_to_RGB565)
>
> /**
> * get_pixel_read_function() - Retrieve the correct read_line function for a specific
>
Powered by blists - more mailing lists