| Message ID | 20240516-yuv-v8-4-cf8d6f86430e@bootlin.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vkms: Reimplement line-per-line pixel conversion for plane reading | expand |
On 5/16/24 10:04, Louis Chauvet wrote: > Introduce two typedefs: pixel_read_t and pixel_write_t. It allows the > compiler to check if the passed functions take the correct arguments. > Such typedefs will help ensuring consistency across the code base in > case of update of these prototypes. > > Rename input/output variable in a consistent way between read_line and > write_line. > > A warn has been added in get_pixel_*_function to alert when an unsupported > pixel format is requested. As those formats are checked before > atomic_update callbacks, it should never happen. > > Document for those typedefs. > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com> > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> > --- > drivers/gpu/drm/vkms/vkms_drv.h | 23 ++++++- > drivers/gpu/drm/vkms/vkms_formats.c | 124 +++++++++++++++++++++--------------- > drivers/gpu/drm/vkms/vkms_formats.h | 4 +- > drivers/gpu/drm/vkms/vkms_plane.c | 2 +- > 4 files changed, 95 insertions(+), 58 deletions(-) > > diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h > index 212f4ab6a71f..b1542b83b090 100644 > --- a/drivers/gpu/drm/vkms/vkms_drv.h > +++ b/drivers/gpu/drm/vkms/vkms_drv.h > @@ -53,12 +53,31 @@ struct line_buffer { > struct pixel_argb_u16 *pixels; > }; > > +/** > + * typedef pixel_write_t - These functions are used to read a pixel from a > + * &struct pixel_argb_u16, convert it in a specific format and write it in the @dst_pixels > + * buffer. > + * > + * @out_pixel: destination address to write the pixel > + * @in_pixel: pixel to write > + */ > +typedef void (*pixel_write_t)(u8 *out_pixel, struct pixel_argb_u16 *in_pixel); > + > struct vkms_writeback_job { > struct iosys_map data[DRM_FORMAT_MAX_PLANES]; > struct vkms_frame_info wb_frame_info; > - void (*pixel_write)(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel); > + pixel_write_t pixel_write; > }; > > +/** > + * typedef pixel_read_t - These functions are used to read a pixel in the source frame, > + * convert it to `struct pixel_argb_u16` and write it to @out_pixel. > + * > + * @in_pixel: pointer to the pixel to read > + * @out_pixel: pointer to write the converted pixel > + */ > +typedef void (*pixel_read_t)(u8 *in_pixel, struct pixel_argb_u16 *out_pixel); > + > /** > * struct vkms_plane_state - Driver specific plane state > * @base: base plane state > @@ -69,7 +88,7 @@ struct vkms_writeback_job { > struct vkms_plane_state { > struct drm_shadow_plane_state base; > struct vkms_frame_info *frame_info; > - void (*pixel_read)(u8 *src_buffer, struct pixel_argb_u16 *out_pixel); > + pixel_read_t pixel_read; > }; > > struct vkms_plane { > diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c > index f157c43da4d6..6b3e17374b19 100644 > --- a/drivers/gpu/drm/vkms/vkms_formats.c > +++ b/drivers/gpu/drm/vkms/vkms_formats.c > @@ -75,7 +75,7 @@ static int get_x_position(const struct vkms_frame_info *frame_info, int limit, i > * They are used in the vkms_compose_row() function to handle multiple formats. > */ > > -static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > +static void ARGB8888_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > { > /* > * The 257 is the "conversion ratio". This number is obtained by the > @@ -83,48 +83,48 @@ static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixe > * the best color value in a pixel format with more possibilities. > * A similar idea applies to others RGB color conversions. > */ > - out_pixel->a = (u16)src_pixels[3] * 257; > - out_pixel->r = (u16)src_pixels[2] * 257; > - out_pixel->g = (u16)src_pixels[1] * 257; > - out_pixel->b = (u16)src_pixels[0] * 257; > + out_pixel->a = (u16)in_pixel[3] * 257; > + out_pixel->r = (u16)in_pixel[2] * 257; > + out_pixel->g = (u16)in_pixel[1] * 257; > + out_pixel->b = (u16)in_pixel[0] * 257; > } > > -static void XRGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > +static void XRGB8888_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > { > out_pixel->a = (u16)0xffff; > - out_pixel->r = (u16)src_pixels[2] * 257; > - out_pixel->g = (u16)src_pixels[1] * 257; > - out_pixel->b = (u16)src_pixels[0] * 257; > + out_pixel->r = (u16)in_pixel[2] * 257; > + out_pixel->g = (u16)in_pixel[1] * 257; > + out_pixel->b = (u16)in_pixel[0] * 257; > } > > -static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > +static void ARGB16161616_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > { > - u16 *pixels = (u16 *)src_pixels; > + u16 *pixel = (u16 *)in_pixel; > > - out_pixel->a = le16_to_cpu(pixels[3]); > - out_pixel->r = le16_to_cpu(pixels[2]); > - out_pixel->g = le16_to_cpu(pixels[1]); > - out_pixel->b = le16_to_cpu(pixels[0]); > + out_pixel->a = le16_to_cpu(pixel[3]); > + out_pixel->r = le16_to_cpu(pixel[2]); > + out_pixel->g = le16_to_cpu(pixel[1]); > + out_pixel->b = le16_to_cpu(pixel[0]); > } > > -static void XRGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > +static void XRGB16161616_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > { > - u16 *pixels = (u16 *)src_pixels; > + u16 *pixel = (u16 *)in_pixel; > > out_pixel->a = (u16)0xffff; > - out_pixel->r = le16_to_cpu(pixels[2]); > - out_pixel->g = le16_to_cpu(pixels[1]); > - out_pixel->b = le16_to_cpu(pixels[0]); > + out_pixel->r = le16_to_cpu(pixel[2]); > + out_pixel->g = le16_to_cpu(pixel[1]); > + out_pixel->b = le16_to_cpu(pixel[0]); > } > > -static void RGB565_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) > +static void RGB565_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) > { > - u16 *pixels = (u16 *)src_pixels; > + u16 *pixel = (u16 *)in_pixel; > > s64 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(31)); > s64 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(63)); > > - u16 rgb_565 = le16_to_cpu(*pixels); > + u16 rgb_565 = le16_to_cpu(*pixel); > s64 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f); > s64 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f); > s64 fp_b = drm_int2fixp(rgb_565 & 0x1f); > @@ -168,12 +168,12 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state > > /* > * The following functions take one &struct pixel_argb_u16 and convert it to a specific format. > - * The result is stored in @dst_pixels. > + * The result is stored in @out_pixel. > * > * They are used in vkms_writeback_row() to convert and store a pixel from the src_buffer to > * the writeback buffer. > */ > -static void argb_u16_to_ARGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > +static void argb_u16_to_ARGB8888(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > { > /* > * This sequence below is important because the format's byte order is > @@ -185,43 +185,43 @@ static void argb_u16_to_ARGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel > * | Addr + 2 | = Red channel > * | Addr + 3 | = Alpha channel > */ > - dst_pixels[3] = DIV_ROUND_CLOSEST(in_pixel->a, 257); > - dst_pixels[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); > - dst_pixels[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); > - dst_pixels[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); > + out_pixel[3] = DIV_ROUND_CLOSEST(in_pixel->a, 257); > + out_pixel[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); > + out_pixel[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); > + out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); > } > > -static void argb_u16_to_XRGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > +static void argb_u16_to_XRGB8888(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > { > - dst_pixels[3] = 0xff; > - dst_pixels[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); > - dst_pixels[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); > - dst_pixels[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); > + out_pixel[3] = 0xff; > + out_pixel[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); > + out_pixel[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); > + out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); > } > > -static void argb_u16_to_ARGB16161616(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > +static void argb_u16_to_ARGB16161616(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > { > - u16 *pixels = (u16 *)dst_pixels; > + u16 *pixel = (u16 *)out_pixel; > > - pixels[3] = cpu_to_le16(in_pixel->a); > - pixels[2] = cpu_to_le16(in_pixel->r); > - pixels[1] = cpu_to_le16(in_pixel->g); > - pixels[0] = cpu_to_le16(in_pixel->b); > + pixel[3] = cpu_to_le16(in_pixel->a); > + pixel[2] = cpu_to_le16(in_pixel->r); > + pixel[1] = cpu_to_le16(in_pixel->g); > + pixel[0] = cpu_to_le16(in_pixel->b); > } > > -static void argb_u16_to_XRGB16161616(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > +static void argb_u16_to_XRGB16161616(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > { > - u16 *pixels = (u16 *)dst_pixels; > + u16 *pixel = (u16 *)out_pixel; > > - pixels[3] = 0xffff; > - pixels[2] = cpu_to_le16(in_pixel->r); > - pixels[1] = cpu_to_le16(in_pixel->g); > - pixels[0] = cpu_to_le16(in_pixel->b); > + pixel[3] = 0xffff; > + pixel[2] = cpu_to_le16(in_pixel->r); > + pixel[1] = cpu_to_le16(in_pixel->g); > + pixel[0] = cpu_to_le16(in_pixel->b); > } > > -static void argb_u16_to_RGB565(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > +static void argb_u16_to_RGB565(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) > { > - u16 *pixels = (u16 *)dst_pixels; > + u16 *pixel = (u16 *)out_pixel; > > s64 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(31)); > s64 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(63)); > @@ -234,7 +234,7 @@ static void argb_u16_to_RGB565(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) > u16 g = drm_fixp2int(drm_fixp_div(fp_g, fp_g_ratio)); > u16 b = drm_fixp2int(drm_fixp_div(fp_b, fp_rb_ratio)); > > - *pixels = cpu_to_le16(r << 11 | g << 5 | b); > + *pixel = cpu_to_le16(r << 11 | g << 5 | b); > } > > /** > @@ -265,7 +265,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb, > * > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > */ > -void *get_pixel_conversion_function(u32 format) > +pixel_read_t get_pixel_read_function(u32 format) > { > switch (format) { > case DRM_FORMAT_ARGB8888: > @@ -279,7 +279,16 @@ void *get_pixel_conversion_function(u32 format) > case DRM_FORMAT_RGB565: > return &RGB565_to_argb_u16; > default: > - return NULL; > + /* > + * This is a bug in vkms_plane_atomic_check(). All the supported > + * format must: > + * - Be listed in vkms_formats in vkms_plane.c > + * - Have a pixel_read callback defined here > + */ > + WARN(true, > + "Pixel format %p4cc is not supported by VKMS planes. This is a kernel bug, atomic check must forbid this configuration.\n", > + &format); Well, if this is a kernel bug, shouldn't we use a BUG_ON()/BUG() macro? Best Regards, - Maíra > + return (pixel_read_t)NULL; > } > } > > @@ -290,7 +299,7 @@ void *get_pixel_conversion_function(u32 format) > * > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > */ > -void *get_pixel_write_function(u32 format) > +pixel_write_t get_pixel_write_function(u32 format) > { > switch (format) { > case DRM_FORMAT_ARGB8888: > @@ -304,6 +313,15 @@ void *get_pixel_write_function(u32 format) > case DRM_FORMAT_RGB565: > return &argb_u16_to_RGB565; > default: > - return NULL; > + /* > + * This is a bug in vkms_writeback_atomic_check. All the supported > + * format must: > + * - Be listed in vkms_wb_formats in vkms_writeback.c > + * - Have a pixel_write callback defined here > + */ > + WARN(true, > + "Pixel format %p4cc is not supported by VKMS writeback. This is a kernel bug, atomic check must forbid this configuration.\n", > + &format); > + return (pixel_write_t)NULL; > } > } > diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h > index cf59c2ed8e9a..3ecea4563254 100644 > --- a/drivers/gpu/drm/vkms/vkms_formats.h > +++ b/drivers/gpu/drm/vkms/vkms_formats.h > @@ -5,8 +5,8 @@ > > #include "vkms_drv.h" > > -void *get_pixel_conversion_function(u32 format); > +pixel_read_t get_pixel_read_function(u32 format); > > -void *get_pixel_write_function(u32 format); > +pixel_write_t get_pixel_write_function(u32 format); > > #endif /* _VKMS_FORMATS_H_ */ > diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c > index 21b5adfb44aa..10e9b23dab28 100644 > --- a/drivers/gpu/drm/vkms/vkms_plane.c > +++ b/drivers/gpu/drm/vkms/vkms_plane.c > @@ -125,7 +125,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane, > drm_rect_rotate(&frame_info->rotated, drm_rect_width(&frame_info->rotated), > drm_rect_height(&frame_info->rotated), frame_info->rotation); > > - vkms_plane_state->pixel_read = get_pixel_conversion_function(fmt); > + vkms_plane_state->pixel_read = get_pixel_read_function(fmt); > } > > static int vkms_plane_atomic_check(struct drm_plane *plane, >
Le 13/07/24 - 11:38, Maíra Canal a écrit : > On 5/16/24 10:04, Louis Chauvet wrote: > > Introduce two typedefs: pixel_read_t and pixel_write_t. It allows the > > compiler to check if the passed functions take the correct arguments. > > Such typedefs will help ensuring consistency across the code base in > > case of update of these prototypes. > > > > Rename input/output variable in a consistent way between read_line and > > write_line. > > > > A warn has been added in get_pixel_*_function to alert when an unsupported > > pixel format is requested. As those formats are checked before > > atomic_update callbacks, it should never happen. > > > > Document for those typedefs. > > > > Reviewed-by: Pekka Paalanen <pekka.paalanen@collabora.com> > > Signed-off-by: Louis Chauvet <louis.chauvet@bootlin.com> [...] > > /** > > @@ -265,7 +265,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb, > > * > > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > > */ > > -void *get_pixel_conversion_function(u32 format) > > +pixel_read_t get_pixel_read_function(u32 format) > > { > > switch (format) { > > case DRM_FORMAT_ARGB8888: > > @@ -279,7 +279,16 @@ void *get_pixel_conversion_function(u32 format) > > case DRM_FORMAT_RGB565: > > return &RGB565_to_argb_u16; > > default: > > - return NULL; > > + /* > > + * This is a bug in vkms_plane_atomic_check(). All the supported > > + * format must: > > + * - Be listed in vkms_formats in vkms_plane.c > > + * - Have a pixel_read callback defined here > > + */ > > + WARN(true, > > + "Pixel format %p4cc is not supported by VKMS planes. This is a kernel bug, atomic check must forbid this configuration.\n", > > + &format); > > Well, if this is a kernel bug, shouldn't we use a BUG_ON()/BUG() macro? I don't know what is the "rule" for this kind of bug. Yes it is a kernel bug, but it is recoverable and I can avoid a full kernel crash by using a "do nothing" callback. If you prefer I can replace this by: pr_err("Pixel format %p4cc is [...]", &format); BUG(); and drop the next commit. The kernel will crash if there is a mismatch between advertised supported formats and formats listed here. > Best Regards, > - Maíra > > > * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) > > */ > > -void *get_pixel_write_function(u32 format) > > +pixel_write_t get_pixel_write_function(u32 format) > > { > > switch (format) { > > case DRM_FORMAT_ARGB8888: > > @@ -304,6 +313,15 @@ void *get_pixel_write_function(u32 format) > > case DRM_FORMAT_RGB565: > > return &argb_u16_to_RGB565; > > default: > > - return NULL; > > + /* > > + * This is a bug in vkms_writeback_atomic_check. All the supported > > + * format must: > > + * - Be listed in vkms_wb_formats in vkms_writeback.c > > + * - Have a pixel_write callback defined here > > + */ > > + WARN(true, > > + "Pixel format %p4cc is not supported by VKMS writeback. This is a kernel bug, atomic check must forbid this configuration.\n", > > + &format); > > + return (pixel_write_t)NULL; I can do the same here. Thanks, Louis Chauvet [...]
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index 212f4ab6a71f..b1542b83b090 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -53,12 +53,31 @@ struct line_buffer { struct pixel_argb_u16 *pixels; }; +/** + * typedef pixel_write_t - These functions are used to read a pixel from a + * &struct pixel_argb_u16, convert it in a specific format and write it in the @dst_pixels + * buffer. + * + * @out_pixel: destination address to write the pixel + * @in_pixel: pixel to write + */ +typedef void (*pixel_write_t)(u8 *out_pixel, struct pixel_argb_u16 *in_pixel); + struct vkms_writeback_job { struct iosys_map data[DRM_FORMAT_MAX_PLANES]; struct vkms_frame_info wb_frame_info; - void (*pixel_write)(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel); + pixel_write_t pixel_write; }; +/** + * typedef pixel_read_t - These functions are used to read a pixel in the source frame, + * convert it to `struct pixel_argb_u16` and write it to @out_pixel. + * + * @in_pixel: pointer to the pixel to read + * @out_pixel: pointer to write the converted pixel + */ +typedef void (*pixel_read_t)(u8 *in_pixel, struct pixel_argb_u16 *out_pixel); + /** * struct vkms_plane_state - Driver specific plane state * @base: base plane state @@ -69,7 +88,7 @@ struct vkms_writeback_job { struct vkms_plane_state { struct drm_shadow_plane_state base; struct vkms_frame_info *frame_info; - void (*pixel_read)(u8 *src_buffer, struct pixel_argb_u16 *out_pixel); + pixel_read_t pixel_read; }; struct vkms_plane { diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c index f157c43da4d6..6b3e17374b19 100644 --- a/drivers/gpu/drm/vkms/vkms_formats.c +++ b/drivers/gpu/drm/vkms/vkms_formats.c @@ -75,7 +75,7 @@ static int get_x_position(const struct vkms_frame_info *frame_info, int limit, i * They are used in the vkms_compose_row() function to handle multiple formats. */ -static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) +static void ARGB8888_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) { /* * The 257 is the "conversion ratio". This number is obtained by the @@ -83,48 +83,48 @@ static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixe * the best color value in a pixel format with more possibilities. * A similar idea applies to others RGB color conversions. */ - out_pixel->a = (u16)src_pixels[3] * 257; - out_pixel->r = (u16)src_pixels[2] * 257; - out_pixel->g = (u16)src_pixels[1] * 257; - out_pixel->b = (u16)src_pixels[0] * 257; + out_pixel->a = (u16)in_pixel[3] * 257; + out_pixel->r = (u16)in_pixel[2] * 257; + out_pixel->g = (u16)in_pixel[1] * 257; + out_pixel->b = (u16)in_pixel[0] * 257; } -static void XRGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) +static void XRGB8888_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) { out_pixel->a = (u16)0xffff; - out_pixel->r = (u16)src_pixels[2] * 257; - out_pixel->g = (u16)src_pixels[1] * 257; - out_pixel->b = (u16)src_pixels[0] * 257; + out_pixel->r = (u16)in_pixel[2] * 257; + out_pixel->g = (u16)in_pixel[1] * 257; + out_pixel->b = (u16)in_pixel[0] * 257; } -static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) +static void ARGB16161616_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) { - u16 *pixels = (u16 *)src_pixels; + u16 *pixel = (u16 *)in_pixel; - out_pixel->a = le16_to_cpu(pixels[3]); - out_pixel->r = le16_to_cpu(pixels[2]); - out_pixel->g = le16_to_cpu(pixels[1]); - out_pixel->b = le16_to_cpu(pixels[0]); + out_pixel->a = le16_to_cpu(pixel[3]); + out_pixel->r = le16_to_cpu(pixel[2]); + out_pixel->g = le16_to_cpu(pixel[1]); + out_pixel->b = le16_to_cpu(pixel[0]); } -static void XRGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) +static void XRGB16161616_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) { - u16 *pixels = (u16 *)src_pixels; + u16 *pixel = (u16 *)in_pixel; out_pixel->a = (u16)0xffff; - out_pixel->r = le16_to_cpu(pixels[2]); - out_pixel->g = le16_to_cpu(pixels[1]); - out_pixel->b = le16_to_cpu(pixels[0]); + out_pixel->r = le16_to_cpu(pixel[2]); + out_pixel->g = le16_to_cpu(pixel[1]); + out_pixel->b = le16_to_cpu(pixel[0]); } -static void RGB565_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel) +static void RGB565_to_argb_u16(u8 *in_pixel, struct pixel_argb_u16 *out_pixel) { - u16 *pixels = (u16 *)src_pixels; + u16 *pixel = (u16 *)in_pixel; s64 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(31)); s64 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(63)); - u16 rgb_565 = le16_to_cpu(*pixels); + u16 rgb_565 = le16_to_cpu(*pixel); s64 fp_r = drm_int2fixp((rgb_565 >> 11) & 0x1f); s64 fp_g = drm_int2fixp((rgb_565 >> 5) & 0x3f); s64 fp_b = drm_int2fixp(rgb_565 & 0x1f); @@ -168,12 +168,12 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state /* * The following functions take one &struct pixel_argb_u16 and convert it to a specific format. - * The result is stored in @dst_pixels. + * The result is stored in @out_pixel. * * They are used in vkms_writeback_row() to convert and store a pixel from the src_buffer to * the writeback buffer. */ -static void argb_u16_to_ARGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) +static void argb_u16_to_ARGB8888(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) { /* * This sequence below is important because the format's byte order is @@ -185,43 +185,43 @@ static void argb_u16_to_ARGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel * | Addr + 2 | = Red channel * | Addr + 3 | = Alpha channel */ - dst_pixels[3] = DIV_ROUND_CLOSEST(in_pixel->a, 257); - dst_pixels[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); - dst_pixels[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); - dst_pixels[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); + out_pixel[3] = DIV_ROUND_CLOSEST(in_pixel->a, 257); + out_pixel[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); + out_pixel[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); + out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); } -static void argb_u16_to_XRGB8888(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) +static void argb_u16_to_XRGB8888(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) { - dst_pixels[3] = 0xff; - dst_pixels[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); - dst_pixels[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); - dst_pixels[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); + out_pixel[3] = 0xff; + out_pixel[2] = DIV_ROUND_CLOSEST(in_pixel->r, 257); + out_pixel[1] = DIV_ROUND_CLOSEST(in_pixel->g, 257); + out_pixel[0] = DIV_ROUND_CLOSEST(in_pixel->b, 257); } -static void argb_u16_to_ARGB16161616(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) +static void argb_u16_to_ARGB16161616(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) { - u16 *pixels = (u16 *)dst_pixels; + u16 *pixel = (u16 *)out_pixel; - pixels[3] = cpu_to_le16(in_pixel->a); - pixels[2] = cpu_to_le16(in_pixel->r); - pixels[1] = cpu_to_le16(in_pixel->g); - pixels[0] = cpu_to_le16(in_pixel->b); + pixel[3] = cpu_to_le16(in_pixel->a); + pixel[2] = cpu_to_le16(in_pixel->r); + pixel[1] = cpu_to_le16(in_pixel->g); + pixel[0] = cpu_to_le16(in_pixel->b); } -static void argb_u16_to_XRGB16161616(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) +static void argb_u16_to_XRGB16161616(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) { - u16 *pixels = (u16 *)dst_pixels; + u16 *pixel = (u16 *)out_pixel; - pixels[3] = 0xffff; - pixels[2] = cpu_to_le16(in_pixel->r); - pixels[1] = cpu_to_le16(in_pixel->g); - pixels[0] = cpu_to_le16(in_pixel->b); + pixel[3] = 0xffff; + pixel[2] = cpu_to_le16(in_pixel->r); + pixel[1] = cpu_to_le16(in_pixel->g); + pixel[0] = cpu_to_le16(in_pixel->b); } -static void argb_u16_to_RGB565(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) +static void argb_u16_to_RGB565(u8 *out_pixel, struct pixel_argb_u16 *in_pixel) { - u16 *pixels = (u16 *)dst_pixels; + u16 *pixel = (u16 *)out_pixel; s64 fp_rb_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(31)); s64 fp_g_ratio = drm_fixp_div(drm_int2fixp(65535), drm_int2fixp(63)); @@ -234,7 +234,7 @@ static void argb_u16_to_RGB565(u8 *dst_pixels, struct pixel_argb_u16 *in_pixel) u16 g = drm_fixp2int(drm_fixp_div(fp_g, fp_g_ratio)); u16 b = drm_fixp2int(drm_fixp_div(fp_b, fp_rb_ratio)); - *pixels = cpu_to_le16(r << 11 | g << 5 | b); + *pixel = cpu_to_le16(r << 11 | g << 5 | b); } /** @@ -265,7 +265,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb, * * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) */ -void *get_pixel_conversion_function(u32 format) +pixel_read_t get_pixel_read_function(u32 format) { switch (format) { case DRM_FORMAT_ARGB8888: @@ -279,7 +279,16 @@ void *get_pixel_conversion_function(u32 format) case DRM_FORMAT_RGB565: return &RGB565_to_argb_u16; default: - return NULL; + /* + * This is a bug in vkms_plane_atomic_check(). All the supported + * format must: + * - Be listed in vkms_formats in vkms_plane.c + * - Have a pixel_read callback defined here + */ + WARN(true, + "Pixel format %p4cc is not supported by VKMS planes. This is a kernel bug, atomic check must forbid this configuration.\n", + &format); + return (pixel_read_t)NULL; } } @@ -290,7 +299,7 @@ void *get_pixel_conversion_function(u32 format) * * @format: DRM_FORMAT_* value for which to obtain a conversion function (see [drm_fourcc.h]) */ -void *get_pixel_write_function(u32 format) +pixel_write_t get_pixel_write_function(u32 format) { switch (format) { case DRM_FORMAT_ARGB8888: @@ -304,6 +313,15 @@ void *get_pixel_write_function(u32 format) case DRM_FORMAT_RGB565: return &argb_u16_to_RGB565; default: - return NULL; + /* + * This is a bug in vkms_writeback_atomic_check. All the supported + * format must: + * - Be listed in vkms_wb_formats in vkms_writeback.c + * - Have a pixel_write callback defined here + */ + WARN(true, + "Pixel format %p4cc is not supported by VKMS writeback. This is a kernel bug, atomic check must forbid this configuration.\n", + &format); + return (pixel_write_t)NULL; } } diff --git a/drivers/gpu/drm/vkms/vkms_formats.h b/drivers/gpu/drm/vkms/vkms_formats.h index cf59c2ed8e9a..3ecea4563254 100644 --- a/drivers/gpu/drm/vkms/vkms_formats.h +++ b/drivers/gpu/drm/vkms/vkms_formats.h @@ -5,8 +5,8 @@ #include "vkms_drv.h" -void *get_pixel_conversion_function(u32 format); +pixel_read_t get_pixel_read_function(u32 format); -void *get_pixel_write_function(u32 format); +pixel_write_t get_pixel_write_function(u32 format); #endif /* _VKMS_FORMATS_H_ */ diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c index 21b5adfb44aa..10e9b23dab28 100644 --- a/drivers/gpu/drm/vkms/vkms_plane.c +++ b/drivers/gpu/drm/vkms/vkms_plane.c @@ -125,7 +125,7 @@ static void vkms_plane_atomic_update(struct drm_plane *plane, drm_rect_rotate(&frame_info->rotated, drm_rect_width(&frame_info->rotated), drm_rect_height(&frame_info->rotated), frame_info->rotation); - vkms_plane_state->pixel_read = get_pixel_conversion_function(fmt); + vkms_plane_state->pixel_read = get_pixel_read_function(fmt); } static int vkms_plane_atomic_check(struct drm_plane *plane,