2024-01-10 17:51:25

by Arthur Grillo

[permalink] [raw]
Subject: [PATCH v2 0/7] Add YUV formats to VKMS

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]>
---
Changes in v2:
- Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test
file (Maxime)
- Link to v1: https://lore.kernel.org/r/[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/Makefile | 1 +
drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
drivers/gpu/drm/vkms/tests/Makefile | 3 +
drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 ++++++++++++++++
drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
drivers/gpu/drm/vkms/vkms_formats.c | 247 ++++++++++++++++++++++----
drivers/gpu/drm/vkms/vkms_formats.h | 9 +
drivers/gpu/drm/vkms/vkms_plane.c | 26 ++-
drivers/gpu/drm/vkms/vkms_writeback.c | 5 -
11 files changed, 426 insertions(+), 49 deletions(-)
---
base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
change-id: 20231226-vkms-yuv-6f7859f32df8

Best regards,
--
Arthur Grillo <[email protected]>



2024-01-10 17:51:36

by Arthur Grillo

[permalink] [raw]
Subject: [PATCH v2 1/7] drm/vkms: Use drm_frame directly

Remove intermidiary variables and access the variables directly from
drm_frame. These changes should be noop.

Signed-off-by: Arthur Grillo <[email protected]>
---
drivers/gpu/drm/vkms/vkms_drv.h | 3 ---
drivers/gpu/drm/vkms/vkms_formats.c | 12 +++++++-----
drivers/gpu/drm/vkms/vkms_plane.c | 3 ---
drivers/gpu/drm/vkms/vkms_writeback.c | 5 -----
4 files changed, 7 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 8f5710debb1e..b4b357447292 100644
--- a/drivers/gpu/drm/vkms/vkms_drv.h
+++ b/drivers/gpu/drm/vkms/vkms_drv.h
@@ -31,9 +31,6 @@ struct vkms_frame_info {
struct drm_rect rotated;
struct iosys_map map[DRM_FORMAT_MAX_PLANES];
unsigned int rotation;
- unsigned int offset;
- unsigned int pitch;
- unsigned int cpp;
};

struct pixel_argb_u16 {
diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
index 36046b12f296..172830a3936a 100644
--- a/drivers/gpu/drm/vkms/vkms_formats.c
+++ b/drivers/gpu/drm/vkms/vkms_formats.c
@@ -11,8 +11,10 @@

static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, int y)
{
- return frame_info->offset + (y * frame_info->pitch)
- + (x * frame_info->cpp);
+ struct drm_framebuffer *fb = frame_info->fb;
+
+ return fb->offsets[0] + (y * fb->pitches[0])
+ + (x * fb->format->cpp[0]);
}

/*
@@ -131,12 +133,12 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
u8 *src_pixels = get_packed_src_addr(frame_info, y);
int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);

- for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp) {
+ for (size_t x = 0; x < limit; x++, src_pixels += frame_info->fb->format->cpp[0]) {
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->cpp * y;
+ + frame_info->fb->format->cpp[0] * y;

plane->pixel_read(src_pixels, &out_pixels[x_pos]);
}
@@ -223,7 +225,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb,
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);

- for (size_t x = 0; x < x_limit; x++, dst_pixels += frame_info->cpp)
+ for (size_t x = 0; x < x_limit; x++, dst_pixels += frame_info->fb->format->cpp[0])
wb->pixel_write(dst_pixels, &in_pixels[x]);
}

diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
index e5c625ab8e3e..8f2c6ea419a3 100644
--- a/drivers/gpu/drm/vkms/vkms_plane.c
+++ b/drivers/gpu/drm/vkms/vkms_plane.c
@@ -125,9 +125,6 @@ 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);

- frame_info->offset = fb->offsets[0];
- frame_info->pitch = fb->pitches[0];
- frame_info->cpp = fb->format->cpp[0];
vkms_plane_state->pixel_read = get_pixel_conversion_function(fmt);
}

diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
index bc724cbd5e3a..c8582df1f739 100644
--- a/drivers/gpu/drm/vkms/vkms_writeback.c
+++ b/drivers/gpu/drm/vkms/vkms_writeback.c
@@ -149,11 +149,6 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
crtc_state->active_writeback = active_wb;
crtc_state->wb_pending = true;
spin_unlock_irq(&output->composer_lock);
-
- wb_frame_info->offset = fb->offsets[0];
- wb_frame_info->pitch = fb->pitches[0];
- wb_frame_info->cpp = fb->format->cpp[0];
-
drm_writeback_queue_job(wb_conn, connector_state);
active_wb->pixel_write = get_pixel_write_function(wb_format);
drm_rect_init(&wb_frame_info->src, 0, 0, crtc_width, crtc_height);

--
2.43.0


2024-01-10 17:55:21

by Arthur Grillo

[permalink] [raw]
Subject: [PATCH v2 6/7] drm/vkms: Drop YUV formats TODO

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


2024-01-15 15:06:54

by Sebastian Wick

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] Add YUV formats to VKMS

On Wed, Jan 10, 2024 at 02:44:00PM -0300, Arthur Grillo wrote:
> 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.

H.273 CICP [1] has concise descriptions of the required values for most
widely used formats and the colour python framework [2] also can be used
to quickly get to the desired information most of the time.

[1]: https://www.itu.int/rec/T-REC-H.273
[2]: https://www.colour-science.org/

> 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]>
> ---
> Changes in v2:
> - Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test
> file (Maxime)
> - Link to v1: https://lore.kernel.org/r/[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/Makefile | 1 +
> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
> drivers/gpu/drm/vkms/tests/Makefile | 3 +
> drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 ++++++++++++++++
> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
> drivers/gpu/drm/vkms/vkms_formats.c | 247 ++++++++++++++++++++++----
> drivers/gpu/drm/vkms/vkms_formats.h | 9 +
> drivers/gpu/drm/vkms/vkms_plane.c | 26 ++-
> drivers/gpu/drm/vkms/vkms_writeback.c | 5 -
> 11 files changed, 426 insertions(+), 49 deletions(-)
> ---
> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
> change-id: 20231226-vkms-yuv-6f7859f32df8
>
> Best regards,
> --
> Arthur Grillo <[email protected]>
>


2024-02-01 17:43:54

by Louis Chauvet

[permalink] [raw]
Subject: Re: [PATCH v2 1/7] drm/vkms: Use drm_frame directly

Le 10/01/24 - 14:44, Arthur Grillo a ?crit :
> Remove intermidiary variables and access the variables directly from
> drm_frame. These changes should be noop.
>
> Signed-off-by: Arthur Grillo <[email protected]>
> ---
> drivers/gpu/drm/vkms/vkms_drv.h | 3 ---
> drivers/gpu/drm/vkms/vkms_formats.c | 12 +++++++-----
> drivers/gpu/drm/vkms/vkms_plane.c | 3 ---
> drivers/gpu/drm/vkms/vkms_writeback.c | 5 -----
> 4 files changed, 7 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 8f5710debb1e..b4b357447292 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -31,9 +31,6 @@ struct vkms_frame_info {
> struct drm_rect rotated;
> struct iosys_map map[DRM_FORMAT_MAX_PLANES];
> unsigned int rotation;
> - unsigned int offset;
> - unsigned int pitch;
> - unsigned int cpp;
> };
>
> struct pixel_argb_u16 {
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index 36046b12f296..172830a3936a 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -11,8 +11,10 @@
>
> static size_t pixel_offset(const struct vkms_frame_info *frame_info, int x, int y)
> {
> - return frame_info->offset + (y * frame_info->pitch)
> - + (x * frame_info->cpp);
> + struct drm_framebuffer *fb = frame_info->fb;
> +
> + return fb->offsets[0] + (y * fb->pitches[0])
> + + (x * fb->format->cpp[0]);
> }
>
> /*
> @@ -131,12 +133,12 @@ void vkms_compose_row(struct line_buffer *stage_buffer, struct vkms_plane_state
> u8 *src_pixels = get_packed_src_addr(frame_info, y);
> int limit = min_t(size_t, drm_rect_width(&frame_info->dst), stage_buffer->n_pixels);
>
> - for (size_t x = 0; x < limit; x++, src_pixels += frame_info->cpp) {
> + for (size_t x = 0; x < limit; x++, src_pixels += frame_info->fb->format->cpp[0]) {
> 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->cpp * y;
> + + frame_info->fb->format->cpp[0] * y;
>
> plane->pixel_read(src_pixels, &out_pixels[x_pos]);
> }
> @@ -223,7 +225,7 @@ void vkms_writeback_row(struct vkms_writeback_job *wb,
> 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);
>
> - for (size_t x = 0; x < x_limit; x++, dst_pixels += frame_info->cpp)
> + for (size_t x = 0; x < x_limit; x++, dst_pixels += frame_info->fb->format->cpp[0])
> wb->pixel_write(dst_pixels, &in_pixels[x]);
> }
>
> diff --git a/drivers/gpu/drm/vkms/vkms_plane.c b/drivers/gpu/drm/vkms/vkms_plane.c
> index e5c625ab8e3e..8f2c6ea419a3 100644
> --- a/drivers/gpu/drm/vkms/vkms_plane.c
> +++ b/drivers/gpu/drm/vkms/vkms_plane.c
> @@ -125,9 +125,6 @@ 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);
>
> - frame_info->offset = fb->offsets[0];
> - frame_info->pitch = fb->pitches[0];
> - frame_info->cpp = fb->format->cpp[0];
> vkms_plane_state->pixel_read = get_pixel_conversion_function(fmt);
> }
>
> diff --git a/drivers/gpu/drm/vkms/vkms_writeback.c b/drivers/gpu/drm/vkms/vkms_writeback.c
> index bc724cbd5e3a..c8582df1f739 100644
> --- a/drivers/gpu/drm/vkms/vkms_writeback.c
> +++ b/drivers/gpu/drm/vkms/vkms_writeback.c
> @@ -149,11 +149,6 @@ static void vkms_wb_atomic_commit(struct drm_connector *conn,
> crtc_state->active_writeback = active_wb;
> crtc_state->wb_pending = true;
> spin_unlock_irq(&output->composer_lock);
> -
> - wb_frame_info->offset = fb->offsets[0];
> - wb_frame_info->pitch = fb->pitches[0];
> - wb_frame_info->cpp = fb->format->cpp[0];
> -
> drm_writeback_queue_job(wb_conn, connector_state);
> active_wb->pixel_write = get_pixel_write_function(wb_format);
> drm_rect_init(&wb_frame_info->src, 0, 0, crtc_width, crtc_height);
>
> --
> 2.43.0
>

Reviewed-by: Louis Chauvet <[email protected]>

2024-02-01 17:46:53

by Louis Chauvet

[permalink] [raw]
Subject: Re: [PATCH v2 6/7] drm/vkms: Drop YUV formats TODO

Le 10/01/24 - 14:44, Arthur Grillo a ?crit :
> 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
>

(Sorry Arthur for the double mail, I miss the reply-all in the previous
mail)

Reviewed-by: Louis Chauvet <[email protected]>

2024-02-28 23:50:31

by Arthur Grillo

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] Add YUV formats to VKMS



On 15/01/24 12:06, Sebastian Wick wrote:
> On Wed, Jan 10, 2024 at 02:44:00PM -0300, Arthur Grillo wrote:
>> 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.
>
> H.273 CICP [1] has concise descriptions of the required values for most
> widely used formats and the colour python framework [2] also can be used
> to quickly get to the desired information most of the time.
>
> [1]: https://www.itu.int/rec/T-REC-H.273
> [2]: https://www.colour-science.org/

I want to thank you for suggesting the python framework, it helped
immensely now that I'm changing the precision from 8-bit to 32-bit[1].

If I'd known about this framework while developing this patch, I
would've saved myself a few weeks of pain and suffering recreating a
part of this library XD.

[1]: https://lore.kernel.org/all/[email protected]/

Best Regards,
~Arthur Grillo

>
>> 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]>
>> ---
>> Changes in v2:
>> - Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test
>> file (Maxime)
>> - Link to v1: https://lore.kernel.org/r/[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/Makefile | 1 +
>> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
>> drivers/gpu/drm/vkms/tests/Makefile | 3 +
>> drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 ++++++++++++++++
>> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
>> drivers/gpu/drm/vkms/vkms_formats.c | 247 ++++++++++++++++++++++----
>> drivers/gpu/drm/vkms/vkms_formats.h | 9 +
>> drivers/gpu/drm/vkms/vkms_plane.c | 26 ++-
>> drivers/gpu/drm/vkms/vkms_writeback.c | 5 -
>> 11 files changed, 426 insertions(+), 49 deletions(-)
>> ---
>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
>> change-id: 20231226-vkms-yuv-6f7859f32df8
>>
>> Best regards,
>> --
>> Arthur Grillo <[email protected]>
>>
>

2024-02-29 17:52:26

by Sebastian Wick

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] Add YUV formats to VKMS

On Wed, Feb 28, 2024 at 08:42:41PM -0300, Arthur Grillo wrote:
>
>
> On 15/01/24 12:06, Sebastian Wick wrote:
> > On Wed, Jan 10, 2024 at 02:44:00PM -0300, Arthur Grillo wrote:
> >> 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.
> >
> > H.273 CICP [1] has concise descriptions of the required values for most
> > widely used formats and the colour python framework [2] also can be used
> > to quickly get to the desired information most of the time.
> >
> > [1]: https://www.itu.int/rec/T-REC-H.273
> > [2]: https://www.colour-science.org/
>
> I want to thank you for suggesting the python framework, it helped
> immensely now that I'm changing the precision from 8-bit to 32-bit[1].
>
> If I'd known about this framework while developing this patch, I
> would've saved myself a few weeks of pain and suffering recreating a
> part of this library XD.

I'm glad this is useful for you! We also used it for our color-and-hdr
project https://gitlab.freedesktop.org/pq/color-and-hdr/.

> [1]: https://lore.kernel.org/all/[email protected]/
>
> Best Regards,
> ~Arthur Grillo
>
> >
> >> 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]>
> >> ---
> >> Changes in v2:
> >> - Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test
> >> file (Maxime)
> >> - Link to v1: https://lore.kernel.org/r/[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/Makefile | 1 +
> >> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
> >> drivers/gpu/drm/vkms/tests/Makefile | 3 +
> >> drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 ++++++++++++++++
> >> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
> >> drivers/gpu/drm/vkms/vkms_formats.c | 247 ++++++++++++++++++++++----
> >> drivers/gpu/drm/vkms/vkms_formats.h | 9 +
> >> drivers/gpu/drm/vkms/vkms_plane.c | 26 ++-
> >> drivers/gpu/drm/vkms/vkms_writeback.c | 5 -
> >> 11 files changed, 426 insertions(+), 49 deletions(-)
> >> ---
> >> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
> >> change-id: 20231226-vkms-yuv-6f7859f32df8
> >>
> >> Best regards,
> >> --
> >> Arthur Grillo <[email protected]>
> >>
> >
>


2024-02-29 18:44:45

by Arthur Grillo

[permalink] [raw]
Subject: Re: [PATCH v2 0/7] Add YUV formats to VKMS



On 29/02/24 14:52, Sebastian Wick wrote:
> On Wed, Feb 28, 2024 at 08:42:41PM -0300, Arthur Grillo wrote:
>>
>>
>> On 15/01/24 12:06, Sebastian Wick wrote:
>>> On Wed, Jan 10, 2024 at 02:44:00PM -0300, Arthur Grillo wrote:
>>>> 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.
>>>
>>> H.273 CICP [1] has concise descriptions of the required values for most
>>> widely used formats and the colour python framework [2] also can be used
>>> to quickly get to the desired information most of the time.
>>>
>>> [1]: https://www.itu.int/rec/T-REC-H.273
>>> [2]: https://www.colour-science.org/
>>
>> I want to thank you for suggesting the python framework, it helped
>> immensely now that I'm changing the precision from 8-bit to 32-bit[1].
>>
>> If I'd known about this framework while developing this patch, I
>> would've saved myself a few weeks of pain and suffering recreating a
>> part of this library XD.
>
> I'm glad this is useful for you! We also used it for our color-and-hdr
> project https://gitlab.freedesktop.org/pq/color-and-hdr/.

Cool project! I'll be sure to give look!

Best Regards,
~Arthur Grillo

>
>> [1]: https://lore.kernel.org/all/[email protected]/
>>
>> Best Regards,
>> ~Arthur Grillo
>>
>>>
>>>> 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]>
>>>> ---
>>>> Changes in v2:
>>>> - Use EXPORT_SYMBOL_IF_KUNIT instead of including the .c test
>>>> file (Maxime)
>>>> - Link to v1: https://lore.kernel.org/r/[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/Makefile | 1 +
>>>> drivers/gpu/drm/vkms/tests/.kunitconfig | 4 +
>>>> drivers/gpu/drm/vkms/tests/Makefile | 3 +
>>>> drivers/gpu/drm/vkms/tests/vkms_format_test.c | 156 ++++++++++++++++
>>>> drivers/gpu/drm/vkms/vkms_drv.h | 6 +-
>>>> drivers/gpu/drm/vkms/vkms_formats.c | 247 ++++++++++++++++++++++----
>>>> drivers/gpu/drm/vkms/vkms_formats.h | 9 +
>>>> drivers/gpu/drm/vkms/vkms_plane.c | 26 ++-
>>>> drivers/gpu/drm/vkms/vkms_writeback.c | 5 -
>>>> 11 files changed, 426 insertions(+), 49 deletions(-)
>>>> ---
>>>> base-commit: eeb8e8d9f124f279e80ae679f4ba6e822ce4f95f
>>>> change-id: 20231226-vkms-yuv-6f7859f32df8
>>>>
>>>> Best regards,
>>>> --
>>>> Arthur Grillo <[email protected]>
>>>>
>>>
>>
>