dri-devel Archive on 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: 47+ 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-30 14:27       ` Pekka Paalanen
2026-07-30 14:45         ` Borah, Chaitanya Kumar
2026-07-31 10:01         ` Robert Mader
2026-08-03  9:42           ` Borah, Chaitanya Kumar
2026-07-28 15:31   ` Robert Mader
2026-07-30 10:59   ` Pekka Paalanen
2026-07-30 20:27     ` Harry Wentland
2026-07-31  4:34       ` Borah, Chaitanya Kumar
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-30 13:46   ` Pekka Paalanen
2026-07-31 15:30     ` Harry Wentland
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-30 14:19     ` Pekka Paalanen
2026-07-31 15:32       ` Harry Wentland
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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox