All of lore.kernel.org
 help / color / mirror / Atom feed
From: Alex Hung <alex.hung@amd.com>
To: Harry Wentland <harry.wentland@amd.com>,
	dri-devel@lists.freedesktop.org,  amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH v4 02/11] drm/vkms: Fix limited-range YCbCr to RGB conversion scaling
Date: Sat, 25 Jul 2026 00:02:27 -0600	[thread overview]
Message-ID: <c741aa10-fa0f-4563-9985-d4da17193830@amd.com> (raw)
In-Reply-To: <20260722134607.136293-3-harry.wentland@amd.com>

Reviewed-by: Alex Hung <alex.hung@amd.com>

On 7/22/26 07:45, Harry Wentland wrote:
> The limited-/studio-range YCbCr to RGB conversion matrices were generated
> with colour.matrix_YCbCr(is_legal=True, bits=8), which normalises the
> narrow range by 2^bits (256 for 8-bit) rather than by the full-range
> maximum 2^bits - 1 (255). As a result the luma scale was 256/219 and the
> chroma scale 256/224 instead of the correct 255/219 and 255/224.
> 
> This over-scales every limited-range conversion by a factor of 256/255,
> producing an error of up to ~3/255 (8-bit) and causing IGT tests that
> compare software-computed surfaces against VKMS-processed ones to fail.
> 
> Recompute the three limited-range matrices from first principles rather
> than relying on colour's is_legal scaling:
> 
>   1. Start from the standard ITU-R YCbCr -> RGB relations for the luma
>      weights Kr, Kb (Kg = 1 - Kr - Kb) of each encoding (BT.601, BT.709
>      and BT.2020 non-constant luminance). These are the same relations
>      that produce the existing full-range matrices.
>   2. Expand the studio input range to full range relative to a full-range
>      maximum of 2^n - 1: the luma coefficient by 255/(235 - 16) and the
>      chroma coefficients by 255/(240 - 16). This matches the DRM UAPI
>      definition and IGT's igt_ycbcr_to_rgb_matrix().
>   3. Convert each coefficient to S31.32 fixed point (round(coeff * 2^32)).
> 
> The full-range matrices (is_legal=False) are unaffected as they use exact
> 1.0/2.0 scaling.
> 
> Update the limited-range reference values in the vkms-format KUnit test
> accordingly: the expected YCbCr inputs now use the studio-range encoding
> consistent with the corrected matrices (8-bit value v placed at v * 257,
> i.e. relative to a full-range maximum of 255).
> 
> Fixes: fe22d21e9342 ("drm/vkms: Add YUV support")
> Assisted-by: Copilot:claude-opus-4.8
> Signed-off-by: Harry Wentland <harry.wentland@amd.com>
> ---
>   drivers/gpu/drm/vkms/tests/vkms_format_test.c | 36 ++++++------
>   drivers/gpu/drm/vkms/vkms_formats.c           | 55 +++++++++++++------
>   2 files changed, 55 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> index a7788fbc45dc..789c59d07ffb 100644
> --- a/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> +++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> @@ -101,12 +101,12 @@ static struct yuv_u16_to_argb_u16_case yuv_u16_to_argb_u16_cases[] = {
>   		.range = DRM_COLOR_YCBCR_LIMITED_RANGE,
>   		.n_colors = 6,
>   		.colors = {
> -			{ "white", { 0xeb00, 0x8000, 0x8000 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> -			{ "gray",  { 0x7dee, 0x8000, 0x8000 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> -			{ "black", { 0x1000, 0x8000, 0x8000 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> -			{ "red",   { 0x517b, 0x5a34, 0xf000 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> -			{ "green", { 0x908e, 0x35cc, 0x2237 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> -			{ "blue",  { 0x28f7, 0xf000, 0x6dc9 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> +			{ "white", { 0xebeb, 0x8080, 0x8080 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> +			{ "gray",  { 0x7e6c, 0x8080, 0x8080 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> +			{ "black", { 0x1010, 0x8080, 0x8080 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> +			{ "red",   { 0x51cd, 0x5a8e, 0xf0f0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> +			{ "green", { 0x911e, 0x3602, 0x2259 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> +			{ "blue",  { 0x2920, 0xf0f0, 0x6e37 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
>   		}
>   	},
>   	/*
> @@ -151,12 +151,12 @@ static struct yuv_u16_to_argb_u16_case yuv_u16_to_argb_u16_cases[] = {
>   		.range = DRM_COLOR_YCBCR_LIMITED_RANGE,
>   		.n_colors = 6,
>   		.colors = {
> -			{ "white", { 0xeb00, 0x8000, 0x8000 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> -			{ "gray",  { 0x7dee, 0x8000, 0x8000 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> -			{ "black", { 0x1000, 0x8000, 0x8000 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> -			{ "red",   { 0x3e8f, 0x6656, 0xf000 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> -			{ "green", { 0xaca1, 0x29aa, 0x1a45 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> -			{ "blue",  { 0x1fd0, 0xf000, 0x75bb }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> +			{ "white", { 0xebeb, 0x8080, 0x8080 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> +			{ "gray",  { 0x7e6c, 0x8080, 0x8080 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> +			{ "black", { 0x1010, 0x8080, 0x8080 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> +			{ "red",   { 0x3ece, 0x66bc, 0xf0f0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> +			{ "green", { 0xad4e, 0x29d4, 0x1a5f }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> +			{ "blue",  { 0x1ff0, 0xf0f0, 0x7631 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
>   		}
>   	},
>   	/*
> @@ -201,12 +201,12 @@ static struct yuv_u16_to_argb_u16_case yuv_u16_to_argb_u16_cases[] = {
>   		.range = DRM_COLOR_YCBCR_LIMITED_RANGE,
>   		.n_colors = 6,
>   		.colors = {
> -			{ "white", { 0xeb00, 0x8000, 0x8000 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> -			{ "gray",  { 0x7dee, 0x8000, 0x8000 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> -			{ "black", { 0x1000, 0x8000, 0x8000 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> -			{ "red",   { 0x4988, 0x60b9, 0xf000 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> -			{ "green", { 0xa47b, 0x2f47, 0x1902 }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> -			{ "blue",  { 0x1cfd, 0xf000, 0x76fe }, { 0xffff, 0x0000, 0x0000, 0xffff }},
> +			{ "white", { 0xebeb, 0x8080, 0x8080 }, { 0xffff, 0xffff, 0xffff, 0xffff }},
> +			{ "gray",  { 0x7e6c, 0x8080, 0x8080 }, { 0xffff, 0x8080, 0x8080, 0x8080 }},
> +			{ "black", { 0x1010, 0x8080, 0x8080 }, { 0xffff, 0x0000, 0x0000, 0x0000 }},
> +			{ "red",   { 0x49d2, 0x611a, 0xf0f0 }, { 0xffff, 0xffff, 0x0000, 0x0000 }},
> +			{ "green", { 0xa520, 0x2f76, 0x191b }, { 0xffff, 0x0000, 0xffff, 0x0000 }},
> +			{ "blue",  { 0x1d1a, 0xf0f0, 0x7775 }, { 0xffff, 0x0000, 0x0000, 0xffff }},
>   		}
>   	},
>   };
> diff --git a/drivers/gpu/drm/vkms/vkms_formats.c b/drivers/gpu/drm/vkms/vkms_formats.c
> index dfb8e13cba87..4d5fcaeb82c5 100644
> --- a/drivers/gpu/drm/vkms/vkms_formats.c
> +++ b/drivers/gpu/drm/vkms/vkms_formats.c
> @@ -788,15 +788,36 @@ static const struct conversion_matrix yuv_bt601_full = {
>   };
>   
>   /*
> - * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.601"],
> - *                                  is_legal = True,
> - *                                  bits = 8) * 2**32).astype(int)
> + * BT.601 limited-/studio-range YCbCr to full-range RGB.
> + *
> + * The coefficients are derived as follows:
> + *
> + *  1. Take the standard ITU-R YCbCr -> RGB relations for luma weights
> + *     Kr, Kb (Kg = 1 - Kr - Kb), with Y in [0, 1] and Cb, Cr in
> + *     [-0.5, 0.5]. For BT.601 Kr = 0.299 and Kb = 0.114:
> + *
> + *       R = Y                                + 2 * (1 - Kr)          * Cr
> + *       G = Y - 2 * (1 - Kb) * Kb / Kg * Cb - 2 * (1 - Kr) * Kr / Kg * Cr
> + *       B = Y + 2 * (1 - Kb)          * Cb
> + *
> + *     These are exactly the yuv_bt601_full coefficients above.
> + *
> + *  2. Expand the studio input range to full range, relative to a
> + *     full-range maximum of 2^n - 1 (255 for 8-bit): the luma
> + *     coefficient is scaled by 255/(235 - 16) and the chroma
> + *     coefficients by 255/(240 - 16). This matches the DRM UAPI
> + *     definition and IGT's igt_ycbcr_to_rgb_matrix(). Note this differs
> + *     from colour.matrix_YCbCr(is_legal=True), which normalises by 2^n
> + *     and is thus off by a factor of 256/255.
> + *
> + *  3. Convert each coefficient to S31.32 fixed point, i.e.
> + *     round(coeff * 2^32).
>    */
>   static const struct conversion_matrix yuv_bt601_limited = {
>   	.matrix = {
> -		{ 5020601039, 0,           6881764740 },
> -		{ 5020601039, -1689204679, -3505362278 },
> -		{ 5020601039, 8697922339,  0 },
> +		{ 5000989317, 0,           6854882848 },
> +		{ 5000989317, -1682606224, -3491669458 },
> +		{ 5000989317, 8663946082,  0 },
>   	},
>   	.y_offset = 16,
>   };
> @@ -816,15 +837,14 @@ static const struct conversion_matrix yuv_bt709_full = {
>   };
>   
>   /*
> - * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.709"],
> - *                                  is_legal = True,
> - *                                  bits = 8) * 2**32).astype(int)
> + * BT.709 limited-range YCbCr to full-range RGB (Kr = 0.2126, Kb = 0.0722).
> + * Derived as described for yuv_bt601_limited.
>    */
>   static const struct conversion_matrix yuv_bt709_limited = {
>   	.matrix = {
> -		{ 5020601039, 0,          7729959424 },
> -		{ 5020601039, -919487572, -2297803934 },
> -		{ 5020601039, 9108275786, 0 },
> +		{ 5000989317, 0,          7699764272 },
> +		{ 5000989317, -915895824, -2288828138 },
> +		{ 5000989317, 9072696586, 0 },
>   	},
>   	.y_offset = 16,
>   };
> @@ -844,15 +864,14 @@ static const struct conversion_matrix yuv_bt2020_full = {
>   };
>   
>   /*
> - * numpy.around(colour.matrix_YCbCr(K=colour.WEIGHTS_YCBCR["ITU-R BT.2020"],
> - *                                  is_legal = True,
> - *                                  bits = 8) * 2**32).astype(int)
> + * BT.2020 non-constant-luminance limited-range YCbCr to full-range RGB
> + * (Kr = 0.2627, Kb = 0.0593). Derived as described for yuv_bt601_limited.
>    */
>   static const struct conversion_matrix yuv_bt2020_limited = {
>   	.matrix = {
> -		{ 5020601039, 0,          7238124312 },
> -		{ 5020601039, -807714626, -2804506279 },
> -		{ 5020601039, 9234915964, 0 },
> +		{ 5000989317, 0,          7209850391 },
> +		{ 5000989317, -804559491, -2793551177 },
> +		{ 5000989317, 9198842076, 0 },
>   	},
>   	.y_offset = 16,
>   };


  reply	other threads:[~2026-07-25  6:02 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-22 13:45 [PATCH v4 00/11] YUV conversion colorop with amdgpu and VKMS Harry Wentland
2026-07-22 13:45 ` [PATCH v4 01/11] drm/colorop: Add DRM_COLOROP_FIXED_MATRIX Harry Wentland
2026-07-22 13:55   ` sashiko-bot
2026-07-25  6:02   ` Alex Hung
2026-07-28 15:02   ` Robert Mader
2026-07-28 16:00     ` Borah, Chaitanya Kumar
2026-07-28 15:31   ` Robert Mader
2026-07-22 13:45 ` [PATCH v4 02/11] drm/vkms: Fix limited-range YCbCr to RGB conversion scaling Harry Wentland
2026-07-25  6:02   ` Alex Hung [this message]
2026-07-28 15:11   ` Robert Mader
2026-07-22 13:45 ` [PATCH v4 03/11] drm/vkms: Add KUnit test for YCbCr to RGB conversion matrices Harry Wentland
2026-07-22 14:01   ` sashiko-bot
2026-07-25  6:02   ` Alex Hung
2026-07-28 15:17   ` Robert Mader
2026-07-22 13:46 ` [PATCH v4 04/11] drm/vkms: Add fixed matrix colorop to color pipeline Harry Wentland
2026-07-22 13:46 ` [PATCH v4 05/11] drm/vkms: Add atomic check and matrix handling for fixed matrix colorop Harry Wentland
2026-07-22 13:59   ` sashiko-bot
2026-07-22 13:46 ` [PATCH v4 06/11] drm/amd/display: Add fixed matrix colorop to color pipeline Harry Wentland
2026-07-22 14:03   ` sashiko-bot
2026-07-28 16:00   ` Robert Mader
2026-07-22 13:46 ` [PATCH v4 07/11] drm/amd/display: Implement fixed matrix colorop color space mapping Harry Wentland
2026-07-22 14:14   ` sashiko-bot
2026-07-28 16:01   ` Robert Mader
2026-07-22 13:46 ` [PATCH v4 08/11] drm/amd/display: Use GAMCOR for first TF if YUV conversion is needed Harry Wentland
2026-07-25  6:07   ` Alex Hung
2026-07-28 16:01   ` Robert Mader
2026-07-22 13:46 ` [PATCH v4 09/11] drm/amd/display: Check actual state during commit_tail Harry Wentland
2026-07-22 14:08   ` sashiko-bot
2026-07-28 16:02   ` Robert Mader
2026-07-22 13:46 ` [PATCH v4 10/11] drm/amd/display: Set color_space to plane_infos Harry Wentland
2026-07-22 14:13   ` sashiko-bot
2026-07-28 16:03   ` Robert Mader
2026-07-22 13:46 ` [PATCH v4 11/11] drm/amd/display: Force GAMCOR for subsampled surfaces with PQ/Gamma22/HLG Harry Wentland
2026-07-25  6:11   ` Alex Hung
2026-07-28 16:04   ` Robert Mader
2026-07-28 14:58 ` [PATCH v4 00/11] YUV conversion colorop with amdgpu and VKMS Robert Mader

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c741aa10-fa0f-4563-9985-d4da17193830@amd.com \
    --to=alex.hung@amd.com \
    --cc=amd-gfx@lists.freedesktop.org \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=harry.wentland@amd.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.