| Message ID | 20240814-writeback_line_by_line-v2-2-36541c717569@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Reimplement line-per-line pixel conversion for writeback | expand |
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@bootlin.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 >
On 26/10/24 - 12:08, Maíra Canal wrote: > 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@bootlin.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. I take this version, a bit shorter and not less explicit, thanks! Thanks, Louis Chauvet > 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 > >
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. + */ +#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--; \ + } \ +} + /* * 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
As stated in [2], the write_line functions are very similar and force code duplication. This patch add a macro to avoid code repetition. Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> --- drivers/gpu/drm/vkms/vkms_formats.c | 107 ++++++++++-------------------------- 1 file changed, 30 insertions(+), 77 deletions(-)