This patchset aims to add support for additional buffer YUV formats.
More specifically, it adds support to:
Semi-planar formats:
- NV12
- NV16
- NV24
- NV21
- NV61
- NV42
Planar formats:
- YUV440
- YUV422
- YUV444
- YVU440
- YVU422
- YVU444
These formats have more than one plane, and most have chroma
subsampling. These properties don't have support on VKMS, so I had to
work on this before.
To ensure that the conversions from YUV to RGB are working, I wrote a
KUnit test. As the work from Harry on creating KUnit tests on VKMS[1] is
not yet merged, I took the setup part (Kconfig entry and .kunitfile)
from it.
Furthermore, I couldn't find any sources with the conversion matrices,
so I had to work out the values myself based on the ITU papers[2][3][4].
So, I'm not 100% sure if the values are accurate. I'd appreciate some
input if anyone has more knowledge in this area.
Also, I used two IGT tests to check if the formats were having a correct
conversion (all with the --extended flag):
- kms_plane@pixel_format
- kms_plane@pixel_format_source_clamping.
The nonsubsampled formats don't have support on IGT, so I sent a patch
fixing this[5].
Currently, this patchset does not add those formats to the writeback, as
it would require a rewrite of how the conversions are done (similar to
what was done on a previous patch[6]). So, I would like to review this
patchset before I start the work on this other part.
[1]: https://lore.kernel.org/all/[email protected]/
[2]: https://www.itu.int/rec/R-REC-BT.601-7-201103-I/en
[3]: https://www.itu.int/rec/R-REC-BT.709-6-201506-I/en
[4]: https://www.itu.int/rec/R-REC-BT.2020-2-201510-I/en
[5]: https://lists.freedesktop.org/archives/igt-dev/2024-January/066937.html
[6]: https://lore.kernel.org/dri-devel/[email protected]/
Signed-off-by: Arthur Grillo <[email protected]>
---
Arthur Grillo (7):
drm/vkms: Use drm_frame directly
drm/vkms: Add support for multy-planar framebuffers
drm/vkms: Add range and encoding properties to pixel_read function
drm/vkms: Add chroma subsampling
drm/vkms: Add YUV support
drm/vkms: Drop YUV formats TODO
drm/vkms: Create KUnit tests for YUV conversions
Documentation/gpu/vkms.rst | 3 +-
drivers/gpu/drm/vkms/Kconfig | 15 ++
drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
drivers/gpu/drm/vkms/tests/vkms_format_test.c | 151 ++++++++++++++++
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_formats.c | 250 ++++++++++++++++++++++----
drivers/gpu/drm/vkms/vkms_plane.c | 26 ++-
drivers/gpu/drm/vkms/vkms_writeback.c | 5 -
8 files changed, 411 insertions(+), 49 deletions(-)
---
base-commit: 0c75d52190b8bfa22cdb66e07148aea599c4535d
change-id: 20231226-vkms-yuv-6f7859f32df8
Best regards,
--
Arthur Grillo <[email protected]>
Add support to multy-planar formats by adding an index argument to the
framebuffer data access functions.
Also, give all the planes to the conversion functions. This, for now,
should be noop, as all the supported formats have only one plane.
Signed-off-by: Arthur Grillo <[email protected]>
---
drivers/gpu/drm/vkms/vkms_drv.h | 2 +-
drivers/gpu/drm/vkms/vkms_formats.c | 73 +++++++++++++++++++++----------------
2 files changed, 42 insertions(+), 33 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index b4b357447292..c38590562e4b 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -56,7 +56,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);
+ void (*pixel_read)(u8 **src_buffer, struct pixel_argb_u16 *out_pixel);
};
struct vkms_plane {
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 172830a3936a..5566a7cd7bb4 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -9,12 +9,12 @@
#include "vkms_formats.h"
-static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, int y)
+static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, int y, size_t index)
{
struct drm_framebuffer *fb = frame_info->fb;
- return fb->offsets[0] + (y * fb->pitches[0])
- + (x * fb->format->cpp[0]);
+ return fb->offsets[index] + (y * fb->pitches[index])
+ + (x * fb->format->cpp[index]);
}
/*
@@ -23,27 +23,25 @@ static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, int
* @frame_info: Buffer metadata
* @x: The x(width) coordinate of the 2D buffer
* @y: The y(Heigth) coordinate of the 2D buffer
+ * @index: The index of the plane on the 2D buffer
*
* Takes the information stored in the frame_info, a pair of coordinates, and
- * returns the address of the first color channel.
- * This function assumes the channels are packed together, i.e. a color channel
- * comes immediately after another in the memory. And therefore, this function
- * doesn't work for YUV with chroma subsampling (e.g. YUV420 and NV21).
+ * returns the address of the first color channel on the desired index.
*/
static void *packed_pixels_addr(const struct vkms_frame_info *frame_info,
- int x, int y)
+ int x, int y, size_t index)
{
- size_t offset = pixel_offset(frame_info, x, y);
+ size_t offset = pixel_offset(frame_info, x, y, index);
return (u8 *)frame_info->map[0].vaddr + offset;
}
-static void *get_packed_src_addr(const struct vkms_frame_info *frame_info, int y)
+static void *get_packed_src_addr(const struct vkms_frame_info *frame_info, int y, size_t index)
{
int x_src = frame_info->src.x1 >> 16;
int y_src = y - frame_info->rotated.y1 + (frame_info->src.y1 >> 16);
- return packed_pixels_addr(frame_info, x_src, y_src);
+ return packed_pixels_addr(frame_info, x_src, y_src, index);
}
static int get_x_position(const struct vkms_frame_info *frame_info, int limit, int x)
@@ -53,7 +51,7 @@ static int get_x_position(const struct vkms_frame_info *frame_info, int limit, i
return x;
}
-static void ARGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
+static void ARGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
{
/*
* The 257 is the "conversion ratio". This number is obtained by the
@@ -61,23 +59,23 @@ 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)src_pixels[0][3] * 257;
+ out_pixel->r = (u16)src_pixels[0][2] * 257;
+ out_pixel->g = (u16)src_pixels[0][1] * 257;
+ out_pixel->b = (u16)src_pixels[0][0] * 257;
}
-static void XRGB8888_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
+static void XRGB8888_to_argb_u16(u8 **src_pixels, 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)src_pixels[0][2] * 257;
+ out_pixel->g = (u16)src_pixels[0][1] * 257;
+ out_pixel->b = (u16)src_pixels[0][0] * 257;
}
-static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
+static void ARGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
{
- u16 *pixels = (u16 *)src_pixels;
+ u16 *pixels = (u16 *)src_pixels[0];
out_pixel->a = le16_to_cpu(pixels[3]);
out_pixel->r = le16_to_cpu(pixels[2]);
@@ -85,9 +83,9 @@ static void ARGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_
out_pixel->b = le16_to_cpu(pixels[0]);
}
-static void XRGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
+static void XRGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
{
- u16 *pixels = (u16 *)src_pixels;
+ u16 *pixels = (u16 *)src_pixels[0];
out_pixel->a = (u16)0xffff;
out_pixel->r = le16_to_cpu(pixels[2]);
@@ -95,9 +93,9 @@ static void XRGB16161616_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_
out_pixel->b = le16_to_cpu(pixels[0]);
}
-static void RGB565_to_argb_u16(u8 *src_pixels, struct pixel_argb_u16 *out_pixel)
+static void RGB565_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
{
- u16 *pixels = (u16 *)src_pixels;
+ u16 *pixels = (u16 *)src_pixels[0];
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));
@@ -130,17 +128,28 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
{
struct pixel_argb_u16 *out_pixels = stage_buffer->pixels;
struct vkms_frame_info *frame_info = plane->frame_info;
- u8 *src_pixels = get_packed_src_addr(frame_info, y);
+ const struct drm_format_info *frame_format = frame_info->fb->format;
int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);
+ u8 *src_pixels[DRM_FORMAT_MAX_PLANES];
- for (size_t x = 0; x < limit; x++, src_pixels += frame_info->fb->format->cpp[0]) {
+ for (size_t i = 0; i < frame_format->num_planes; i++)
+ src_pixels[i] = get_packed_src_addr(frame_info, y, i);
+
+ for (size_t x = 0; x < limit; x++) {
int x_pos = get_x_position(frame_info, limit, x);
- if (drm_rotation_90_or_270(frame_info->rotation))
- src_pixels = get_packed_src_addr(frame_info, x + frame_info->rotated.y1)
- + frame_info->fb->format->cpp[0] * y;
+ if (drm_rotation_90_or_270(frame_info->rotation)) {
+ for (size_t i = 0; i < frame_format->num_planes; i++) {
+ src_pixels[i] = get_packed_src_addr(frame_info,
+ x + frame_info->rotated.y1, i);
+ src_pixels[i] += frame_format->cpp[i] * y;
+ }
+ }
plane->pixel_read(src_pixels, &out_pixels[x_pos]);
+
+ for (size_t i = 0; i < frame_format->num_planes; i++)
+ src_pixels[i] += frame_format->cpp[i];
}
}
@@ -221,7 +230,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb,
{
struct vkms_frame_info *frame_info = &wb->wb_frame_info;
int x_dst = frame_info->dst.x1;
- u8 *dst_pixels = packed_pixels_addr(frame_info, x_dst, y);
+ u8 *dst_pixels = packed_pixels_addr(frame_info, x_dst, y, 0);
struct pixel_argb_u16 *in_pixels = src_buffer->pixels;
int x_limit = min_t(size_t, drm_rect_width(&frame_info->dst), src_buffer->n_pixels);
--
2.43.0
Create range and encoding properties. This should be noop, as none of
the conversion functions need those properties.
Signed-off-by: Arthur Grillo <[email protected]>
---
drivers/gpu/drm/vkms/vkms_drv.h | 3 ++-
drivers/gpu/drm/vkms/vkms_formats.c | 20 ++++++++++++++------
drivers/gpu/drm/vkms/vkms_plane.c | 9 +++++++++
3 files changed, 25 insertions(+), 7 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index c38590562e4b..51349a0c32d8 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -56,7 +56,8 @@ 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);
+ void (*pixel_read)(u8 **src_buffer, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding enconding, enum drm_color_range range);
};
struct vkms_plane {
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 5566a7cd7bb4..0156372aa1ef 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -51,7 +51,8 @@ static int get_x_position(const struct vkms_frame_info *frame_info, int limit, i
return x;
}
-static void ARGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
+static void ARGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
{
/*
* The 257 is the "conversion ratio". This number is obtained by the
@@ -65,7 +66,8 @@ static void ARGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pix
out_pixel->b = (u16)src_pixels[0][0] * 257;
}
-static void XRGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
+static void XRGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
{
out_pixel->a = (u16)0xffff;
out_pixel->r = (u16)src_pixels[0][2] * 257;
@@ -73,7 +75,8 @@ static void XRGB8888_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pix
out_pixel->b = (u16)src_pixels[0][0] * 257;
}
-static void ARGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
+static void ARGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
{
u16 *pixels = (u16 *)src_pixels[0];
@@ -83,7 +86,8 @@ static void ARGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out
out_pixel->b = le16_to_cpu(pixels[0]);
}
-static void XRGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
+static void XRGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
{
u16 *pixels = (u16 *)src_pixels[0];
@@ -93,7 +97,8 @@ static void XRGB16161616_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out
out_pixel->b = le16_to_cpu(pixels[0]);
}
-static void RGB565_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel)
+static void RGB565_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
{
u16 *pixels = (u16 *)src_pixels[0];
@@ -132,6 +137,9 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);
u8 *src_pixels[DRM_FORMAT_MAX_PLANES];
+ enum drm_color_encoding encoding = plane->base.base.color_encoding;
+ enum drm_color_range range = plane->base.base.color_range;
+
for (size_t i = 0; i < frame_format->num_planes; i++)
src_pixels[i] = get_packed_src_addr(frame_info, y, i);
@@ -146,7 +154,7 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
}
}
- plane->pixel_read(src_pixels, &out_pixels[x_pos]);
+ plane->pixel_read(src_pixels, &out_pixels[x_pos], encoding, range);
for (size_t i = 0; i < frame_format->num_planes; i++)
src_pixels[i] += frame_format->cpp[i];
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index 8f2c6ea419a3..e87c80575b7d 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -212,5 +212,14 @@ struct vkms_plane *vkms_plane_init(struct vkms_device *vkmsdev,
drm_plane_create_rotation_property(&plane->base, DRM_MODE_ROTATE_0,
DRM_MODE_ROTATE_MASK | DRM_MODE_REFLECT_MASK);
+ drm_plane_create_color_properties(&plane->base,
+ BIT(DRM_COLOR_YCBCR_BT601) |
+ BIT(DRM_COLOR_YCBCR_BT709) |
+ BIT(DRM_COLOR_YCBCR_BT2020),
+ BIT(DRM_COLOR_YCBCR_LIMITED_RANGE) |
+ BIT(DRM_COLOR_YCBCR_FULL_RANGE),
+ DRM_COLOR_YCBCR_BT601,
+ DRM_COLOR_YCBCR_FULL_RANGE);
+
return plane;
}
--
2.43.0
Add support to chroma subsampling. This should be noop, as all supported
formats do not have chroma subsampling.
Signed-off-by: Arthur Grillo <[email protected]>
---
drivers/gpu/drm/vkms/vkms_formats.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 0156372aa1ef..098ed16f2104 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -26,12 +26,15 @@ static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, int
* @index: The index of the plane on the 2D buffer
*
* Takes the information stored in the frame_info, a pair of coordinates, and
- * returns the address of the first color channel on the desired index.
+ * returns the address of the first color channel on the desired index. The
+ * format's specific subsample is applied.
*/
static void *packed_pixels_addr(const struct vkms_frame_info *frame_info,
int x, int y, size_t index)
{
- size_t offset = pixel_offset(frame_info, x, y, index);
+ int vsub = index == 0 ? 1 : frame_info->fb->format->vsub;
+ int hsub = index == 0 ? 1 : frame_info->fb->format->hsub;
+ size_t offset = pixel_offset(frame_info, x / hsub, y / vsub, index);
return (u8 *)frame_info->map[0].vaddr + offset;
}
@@ -146,18 +149,23 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
for (size_t x = 0; x < limit; x++) {
int x_pos = get_x_position(frame_info, limit, x);
+ bool shoud_inc = !((x + 1) % frame_format->num_planes);
+
if (drm_rotation_90_or_270(frame_info->rotation)) {
for (size_t i = 0; i < frame_format->num_planes; i++) {
src_pixels[i] = get_packed_src_addr(frame_info,
x + frame_info->rotated.y1, i);
- src_pixels[i] += frame_format->cpp[i] * y;
+ if (!i || shoud_inc)
+ src_pixels[i] += frame_format->cpp[i] * y;
}
}
plane->pixel_read(src_pixels, &out_pixels[x_pos], encoding, range);
- for (size_t i = 0; i < frame_format->num_planes; i++)
- src_pixels[i] += frame_format->cpp[i];
+ for (size_t i = 0; i < frame_format->num_planes; i++) {
+ if (!i || shoud_inc)
+ src_pixels[i] += frame_format->cpp[i];
+ }
}
}
--
2.43.0
Add support to the YUV formats bellow:
- NV12
- NV16
- NV24
- NV21
- NV61
- NV42
- YUV420
- YUV422
- YUV444
- YVU420
- YVU422
- YVU444
The conversion matrices of each encoding and range were obtained by
rounding the values of the original conversion matrices multiplied by
2^8. This is done to avoid the use of fixed point operations.
Signed-off-by: Arthur Grillo <[email protected]>
---
drivers/gpu/drm/vkms/vkms_formats.c | 151 ++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/vkms/vkms_plane.c | 14 +++-
2 files changed, 164 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 098ed16f2104..b654b6661a20 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -119,6 +119,141 @@ static void RGB565_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel
out_pixel->b = drm_fixp2int_round(drm_fixp_mul(fp_b, fp_rb_ratio));
}
+struct pixel_yuv_u8 {
+ u8 y, u, v;
+};
+
+static void ycbcr2rgb(const s16 m[3][3], u8 y, u8 cb, u8 cr, u8 y_offset, u8 *r, u8 *g, u8 *b)
+{
+ s32 y_16, cb_16, cr_16;
+ s32 r_16, g_16, b_16;
+
+ y_16 = y - y_offset;
+ cb_16 = cb - 128;
+ cr_16 = cr - 128;
+
+ r_16 = m[0][0] * y_16 + m[0][1] * cb_16 + m[0][2] * cr_16;
+ g_16 = m[1][0] * y_16 + m[1][1] * cb_16 + m[1][2] * cr_16;
+ b_16 = m[2][0] * y_16 + m[2][1] * cb_16 + m[2][2] * cr_16;
+
+ *r = clamp(r_16, 0, 0xffff) >> 8;
+ *g = clamp(g_16, 0, 0xffff) >> 8;
+ *b = clamp(b_16, 0, 0xffff) >> 8;
+}
+
+static void yuv_u8_to_argb_u16(struct pixel_argb_u16 *argb_u16, const struct pixel_yuv_u8 *yuv_u8,
+ enum drm_color_encoding encoding, enum drm_color_range range)
+{
+ static const s16 bt601_full[3][3] = {
+ {256, 0, 359},
+ {256, -88, -183},
+ {256, 454, 0},
+ };
+ static const s16 bt601[3][3] = {
+ {298, 0, 409},
+ {298, -100, -208},
+ {298, 516, 0},
+ };
+ static const s16 rec709_full[3][3] = {
+ {256, 0, 408},
+ {256, -48, -120},
+ {256, 476, 0 },
+ };
+ static const s16 rec709[3][3] = {
+ {298, 0, 459},
+ {298, -55, -136},
+ {298, 541, 0},
+ };
+ static const s16 bt2020_full[3][3] = {
+ {256, 0, 377},
+ {256, -42, -146},
+ {256, 482, 0},
+ };
+ static const s16 bt2020[3][3] = {
+ {298, 0, 430},
+ {298, -48, -167},
+ {298, 548, 0},
+ };
+
+ u8 r = 0;
+ u8 g = 0;
+ u8 b = 0;
+ bool full = range == DRM_COLOR_YCBCR_FULL_RANGE;
+ unsigned int y_offset = full ? 0 : 16;
+
+ switch (encoding) {
+ case DRM_COLOR_YCBCR_BT601:
+ ycbcr2rgb(full ? bt601_full : bt601,
+ yuv_u8->y, yuv_u8->u, yuv_u8->v, y_offset, &r, &g, &b);
+ break;
+ case DRM_COLOR_YCBCR_BT709:
+ ycbcr2rgb(full ? rec709_full : rec709,
+ yuv_u8->y, yuv_u8->u, yuv_u8->v, y_offset, &r, &g, &b);
+ break;
+ case DRM_COLOR_YCBCR_BT2020:
+ ycbcr2rgb(full ? bt2020_full : bt2020,
+ yuv_u8->y, yuv_u8->u, yuv_u8->v, y_offset, &r, &g, &b);
+ break;
+ default:
+ pr_warn_once("Not supported color encoding\n");
+ break;
+ }
+
+ argb_u16->r = r * 257;
+ argb_u16->g = g * 257;
+ argb_u16->b = b * 257;
+}
+
+static void semi_planar_yuv_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding,
+ enum drm_color_range range)
+{
+ struct pixel_yuv_u8 yuv_u8;
+
+ yuv_u8.y = src_pixels[0][0];
+ yuv_u8.u = src_pixels[1][0];
+ yuv_u8.v = src_pixels[1][1];
+
+ yuv_u8_to_argb_u16(out_pixel, &yuv_u8, encoding, range);
+}
+
+static void semi_planar_yvu_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding,
+ enum drm_color_range range)
+{
+ struct pixel_yuv_u8 yuv_u8;
+
+ yuv_u8.y = src_pixels[0][0];
+ yuv_u8.v = src_pixels[1][0];
+ yuv_u8.u = src_pixels[1][1];
+
+ yuv_u8_to_argb_u16(out_pixel, &yuv_u8, encoding, range);
+}
+
+static void planar_yuv_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
+{
+ struct pixel_yuv_u8 yuv_u8;
+
+ yuv_u8.y = src_pixels[0][0];
+ yuv_u8.u = src_pixels[1][0];
+ yuv_u8.v = src_pixels[2][0];
+
+ yuv_u8_to_argb_u16(out_pixel, &yuv_u8, encoding, range);
+}
+
+static void planar_yvu_to_argb_u16(u8 **src_pixels, struct pixel_argb_u16 *out_pixel,
+ enum drm_color_encoding encoding, enum drm_color_range range)
+{
+ struct pixel_yuv_u8 yuv_u8;
+
+ yuv_u8.y = src_pixels[0][0];
+ yuv_u8.v = src_pixels[1][0];
+ yuv_u8.u = src_pixels[2][0];
+
+ yuv_u8_to_argb_u16(out_pixel, &yuv_u8, encoding, range);
+}
+
/**
* vkms_compose_row - compose a single row of a plane
* @stage_buffer: output line with the composed pixels
@@ -267,6 +402,22 @@ void *get_pixel_conversion_function(u32 format)
return &XRGB16161616_to_argb_u16;
case DRM_FORMAT_RGB565:
return &RGB565_to_argb_u16;
+ case DRM_FORMAT_NV12:
+ case DRM_FORMAT_NV16:
+ case DRM_FORMAT_NV24:
+ return &semi_planar_yuv_to_argb_u16;
+ case DRM_FORMAT_NV21:
+ case DRM_FORMAT_NV61:
+ case DRM_FORMAT_NV42:
+ return &semi_planar_yvu_to_argb_u16;
+ case DRM_FORMAT_YUV420:
+ case DRM_FORMAT_YUV422:
+ case DRM_FORMAT_YUV444:
+ return &planar_yuv_to_argb_u16;
+ case DRM_FORMAT_YVU420:
+ case DRM_FORMAT_YVU422:
+ case DRM_FORMAT_YVU444:
+ return &planar_yvu_to_argb_u16;
default:
return NULL;
}
diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index e87c80575b7d..932736fc3ee9 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -17,7 +17,19 @@ static const u32 vkms_formats[] = {
DRM_FORMAT_XRGB8888,
DRM_FORMAT_XRGB16161616,
DRM_FORMAT_ARGB16161616,
- DRM_FORMAT_RGB565
+ DRM_FORMAT_RGB565,
+ DRM_FORMAT_NV12,
+ DRM_FORMAT_NV16,
+ DRM_FORMAT_NV24,
+ DRM_FORMAT_NV21,
+ DRM_FORMAT_NV61,
+ DRM_FORMAT_NV42,
+ DRM_FORMAT_YUV420,
+ DRM_FORMAT_YUV422,
+ DRM_FORMAT_YUV444,
+ DRM_FORMAT_YVU420,
+ DRM_FORMAT_YVU422,
+ DRM_FORMAT_YVU444
};
static struct drm_plane_state *
--
2.43.0
VKMS has support for YUV formats now. Remove the task from the TODO
list.
Signed-off-by: Arthur Grillo <[email protected]>
---
Documentation/gpu/vkms.rst | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/Documentation/gpu/vkms.rst b/Documentation/gpu/vkms.rst
index ba04ac7c2167..13b866c3617c 100644
--- a/Documentation/gpu/vkms.rst
+++ b/Documentation/gpu/vkms.rst
@@ -122,8 +122,7 @@ There's lots of plane features we could add support for:
- Scaling.
-- Additional buffer formats, especially YUV formats for video like NV12.
- Low/high bpp RGB formats would also be interesting.
+- Additional buffer formats. Low/high bpp RGB formats would be interesting.
- Async updates (currently only possible on cursor plane using the legacy
cursor api).
--
2.43.0
Create KUnit tests to test the conversion between YUV and RGB. Test each
conversion and range combination with some common colors.
Signed-off-by: Arthur Grillo <[email protected]>
---
drivers/gpu/drm/vkms/Kconfig | 15 +++
drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
drivers/gpu/drm/vkms/tests/vkms_format_test.c | 151 ++++++++++++++++++++++++++
drivers/gpu/drm/vkms/vkms_formats.c | 4 +
4 files changed, 174 insertions(+)
diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
index b9ecdebecb0b..5b0094ad41eb 100644
--- a/drivers/gpu/drm/vkms/Kconfig
+++ b/drivers/gpu/drm/vkms/Kconfig
@@ -13,3 +13,18 @@ config DRM_VKMS
a VKMS.
If M is selected the module will be called vkms.
+
+config DRM_VKMS_KUNIT_TESTS
+ tristate "Tests for VKMS" if !KUNIT_ALL_TESTS
+ depends on DRM_VKMS && KUNIT
+ default KUNIT_ALL_TESTS
+ help
+ This builds unit tests for VKMS. This option is not useful for
+ distributions or general kernels, but only for kernel
+ developers working on VKMS.
+
+ For more information on KUnit and unit tests in general,
+ please refer to the KUnit documentation in
+ Documentation/dev-tools/kunit/.
+
+ If in doubt, say "N".
diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig b/drivers/gpu/drm/vkms/tests/.kunitconfig
new file mode 100644
index 000000000000..70e378228cbd
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/.kunitconfig
@@ -0,0 +1,4 @@
+CONFIG_KUNIT=y
+CONFIG_DRM=y
+CONFIG_DRM_VKMS=y
+CONFIG_DRM_VKMS_KUNIT_TESTS=y
diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
new file mode 100644
index 000000000000..c902cdd2d7f2
--- /dev/null
+++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
@@ -0,0 +1,151 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <kunit/test.h>
+
+#include <drm/drm_fixed.h>
+
+#include "../../drm_crtc_internal.h"
+
+#define TEST_BUFF_SIZE 50
+
+struct yuv_u8_to_argb_u16_case {
+ enum drm_color_encoding encoding;
+ enum drm_color_range range;
+ size_t n_colors;
+ struct format_pair {
+ char *name;
+ struct pixel_yuv_u8 yuv;
+ struct pixel_argb_u16 argb;
+ } colors[TEST_BUFF_SIZE];
+};
+
+static struct yuv_u8_to_argb_u16_case yuv_u8_to_argb_u16_cases[] = {
+ {
+ .encoding = DRM_COLOR_YCBCR_BT601,
+ .range = DRM_COLOR_YCBCR_FULL_RANGE,
+ .n_colors = 6,
+ .colors = {
+ {"white", {0xff, 0x80, 0x80}, {0x0000, 0xffff, 0xffff, 0xffff}},
+ {"gray", {0x80, 0x80, 0x80}, {0x0000, 0x8000, 0x8000, 0x8000}},
+ {"black", {0x00, 0x80, 0x80}, {0x0000, 0x0000, 0x0000, 0x0000}},
+ {"red", {0x4c, 0x55, 0xff}, {0x0000, 0xffff, 0x0000, 0x0000}},
+ {"green", {0x96, 0x2c, 0x15}, {0x0000, 0x0000, 0xffff, 0x0000}},
+ {"blue", {0x1d, 0xff, 0x6b}, {0x0000, 0x0000, 0x0000, 0xffff}},
+ },
+ },
+ {
+ .encoding = DRM_COLOR_YCBCR_BT601,
+ .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ .n_colors = 6,
+ .colors = {
+ {"white", {0xeb, 0x80, 0x80}, {0x0000, 0xffff, 0xffff, 0xffff}},
+ {"gray", {0x7e, 0x80, 0x80}, {0x0000, 0x8000, 0x8000, 0x8000}},
+ {"black", {0x10, 0x80, 0x80}, {0x0000, 0x0000, 0x0000, 0x0000}},
+ {"red", {0x51, 0x5a, 0xf0}, {0x0000, 0xffff, 0x0000, 0x0000}},
+ {"green", {0x91, 0x36, 0x22}, {0x0000, 0x0000, 0xffff, 0x0000}},
+ {"blue", {0x29, 0xf0, 0x6e}, {0x0000, 0x0000, 0x0000, 0xffff}},
+ },
+ },
+ {
+ .encoding = DRM_COLOR_YCBCR_BT709,
+ .range = DRM_COLOR_YCBCR_FULL_RANGE,
+ .n_colors = 4,
+ .colors = {
+ {"white", {0xff, 0x80, 0x80}, {0x0000, 0xffff, 0xffff, 0xffff}},
+ {"gray", {0x80, 0x80, 0x80}, {0x0000, 0x8000, 0x8000, 0x8000}},
+ {"black", {0x00, 0x80, 0x80}, {0x0000, 0x0000, 0x0000, 0x0000}},
+ {"red", {0x35, 0x63, 0xff}, {0x0000, 0xffff, 0x0000, 0x0000}},
+ {"green", {0xb6, 0x1e, 0x0c}, {0x0000, 0x0000, 0xffff, 0x0000}},
+ {"blue", {0x12, 0xff, 0x74}, {0x0000, 0x0000, 0x0000, 0xffff}},
+ },
+ },
+ {
+ .encoding = DRM_COLOR_YCBCR_BT709,
+ .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ .n_colors = 4,
+ .colors = {
+ {"white", {0xeb, 0x80, 0x80}, {0x0000, 0xffff, 0xffff, 0xffff}},
+ {"gray", {0x7e, 0x80, 0x80}, {0x0000, 0x8000, 0x8000, 0x8000}},
+ {"black", {0x10, 0x80, 0x80}, {0x0000, 0x0000, 0x0000, 0x0000}},
+ {"red", {0x3f, 0x66, 0xf0}, {0x0000, 0xffff, 0x0000, 0x0000}},
+ {"green", {0xad, 0x2a, 0x1a}, {0x0000, 0x0000, 0xffff, 0x0000}},
+ {"blue", {0x20, 0xf0, 0x76}, {0x0000, 0x0000, 0x0000, 0xffff}},
+ },
+ },
+ {
+ .encoding = DRM_COLOR_YCBCR_BT2020,
+ .range = DRM_COLOR_YCBCR_FULL_RANGE,
+ .n_colors = 4,
+ .colors = {
+ {"white", {0xff, 0x80, 0x80}, {0x0000, 0xffff, 0xffff, 0xffff}},
+ {"gray", {0x80, 0x80, 0x80}, {0x0000, 0x8000, 0x8000, 0x8000}},
+ {"black", {0x00, 0x80, 0x80}, {0x0000, 0x0000, 0x0000, 0x0000}},
+ {"red", {0x43, 0x5c, 0xff}, {0x0000, 0xffff, 0x0000, 0x0000}},
+ {"green", {0xad, 0x24, 0x0b}, {0x0000, 0x0000, 0xffff, 0x0000}},
+ {"blue", {0x0f, 0xff, 0x76}, {0x0000, 0x0000, 0x0000, 0xffff}},
+ },
+ },
+ {
+ .encoding = DRM_COLOR_YCBCR_BT2020,
+ .range = DRM_COLOR_YCBCR_LIMITED_RANGE,
+ .n_colors = 4,
+ .colors = {
+ {"white", {0xeb, 0x80, 0x80}, {0x0000, 0xffff, 0xffff, 0xffff}},
+ {"gray", {0x7e, 0x80, 0x80}, {0x0000, 0x8000, 0x8000, 0x8000}},
+ {"black", {0x10, 0x80, 0x80}, {0x0000, 0x0000, 0x0000, 0x0000}},
+ {"red", {0x4a, 0x61, 0xf0}, {0x0000, 0xffff, 0x0000, 0x0000}},
+ {"green", {0xa4, 0x2f, 0x19}, {0x0000, 0x0000, 0xffff, 0x0000}},
+ {"blue", {0x1d, 0xf0, 0x77}, {0x0000, 0x0000, 0x0000, 0xffff}},
+ },
+ },
+};
+
+static void vkms_format_test_yuv_u8_to_argb_u16(struct kunit *test)
+{
+ const struct yuv_u8_to_argb_u16_case *param = test->param_value;
+ struct pixel_argb_u16 argb;
+
+ for (size_t i = 0; i < param->n_colors; i++) {
+ const struct format_pair *color = ¶m->colors[i];
+
+ yuv_u8_to_argb_u16(&argb, &color->yuv, param->encoding, param->range);
+
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.a, color->argb.a), 257,
+ "On the A channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.a, argb.a);
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.r, color->argb.r), 257,
+ "On the R channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.r, argb.r);
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.g, color->argb.g), 257,
+ "On the G channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.g, argb.g);
+ KUNIT_EXPECT_LE_MSG(test, abs_diff(argb.b, color->argb.b), 257,
+ "On the B channel of the color %s expected 0x%04x, got 0x%04x",
+ color->name, color->argb.b, argb.b);
+ }
+}
+
+
+static void vkms_format_test_yuv_u8_to_argb_u16_case_desc(struct yuv_u8_to_argb_u16_case *t,
+ char *desc)
+{
+ snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s - %s",
+ drm_get_color_encoding_name(t->encoding), drm_get_color_range_name(t->range));
+}
+
+KUNIT_ARRAY_PARAM(yuv_u8_to_argb_u16, yuv_u8_to_argb_u16_cases,
+ vkms_format_test_yuv_u8_to_argb_u16_case_desc);
+
+static struct kunit_case vkms_format_test_cases[] = {
+ KUNIT_CASE_PARAM(vkms_format_test_yuv_u8_to_argb_u16, yuv_u8_to_argb_u16_gen_params),
+ {}
+};
+
+static struct kunit_suite vkms_format_test_suite = {
+ .name = "vkms-format",
+ .test_cases = vkms_format_test_cases,
+};
+
+kunit_test_suite(vkms_format_test_suite);
+
+MODULE_LICENSE("GPL");
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index b654b6661a20..11df990a0fa9 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -440,3 +440,7 @@ void *get_pixel_write_function(u32 format)
return NULL;
}
}
+
+#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS
+#include "tests/vkms_format_test.c"
+#endif
--
2.43.0
Hi Arthur,
On Fri, Jan 05, 2024 at 01:35:08PM -0300, Arthur Grillo wrote:
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index b654b6661a20..11df990a0fa9 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -440,3 +440,7 @@ void *get_pixel_write_function(u32 format)
> return NULL;
> }
> }
> +
> +#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS
> +#include "tests/vkms_format_test.c"
> +#endif
I assume this is due to testing a static function?
If so, the preferred way nowadays is to use EXPORT_SYMBOL_IF_KUNIT or
EXPORT_SYMBOL_FOR_TESTS_ONLY if it's DRM/KMS only.
Maxime
On 08/01/24 07:15, Maxime Ripard wrote:
> Hi Arthur,
>
> On Fri, Jan 05, 2024 at 01:35:08PM -0300, Arthur Grillo wrote:
>> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
>> index b654b6661a20..11df990a0fa9 100644
>> --- a/drivers/gpu/drm/vkms/vkms_formats.c
>> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
>> @@ -440,3 +440,7 @@ void *get_pixel_write_function(u32 format)
>> return NULL;
>> }
>> }
>> +
>> +#ifdef CONFIG_DRM_VKMS_KUNIT_TESTS
>> +#include "tests/vkms_format_test.c"
>> +#endif
>
> I assume this is due to testing a static function?
Yeah, you're right.
>
> If so, the preferred way nowadays is to use EXPORT_SYMBOL_IF_KUNIT or
> EXPORT_SYMBOL_FOR_TESTS_ONLY if it's DRM/KMS only.
Oh, I didn't know about that. I think I will use EXPORT_SYMBOL_IF_KUNIT
as you can use VISIBLE_IF_KUNIT to maintain the function static if the
test is not used.
~Arthur Grillo
>
> Maxime