linux-kernel - Re: [PATCH AUTOSEL 6.1 04/53] drm: Fix color LUT rounding

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Za6ano6dg0Mau7OI@intel.com>
Date: Mon, 22 Jan 2024 18:50:00 +0200
From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
To: Sasha Levin <sashal@...nel.org>
Cc: linux-kernel@...r.kernel.org, stable@...r.kernel.org,
	Chaitanya Kumar Borah <chaitanya.kumar.borah@...el.com>,
	Jani Nikula <jani.nikula@...el.com>,
	Maxime Ripard <mripard@...nel.org>,
	maarten.lankhorst@...ux.intel.com, tzimmermann@...e.de,
	airlied@...il.com, daniel@...ll.ch, dri-devel@...ts.freedesktop.org
Subject: Re: [PATCH AUTOSEL 6.1 04/53] drm: Fix color LUT rounding

On Mon, Jan 22, 2024 at 10:08:05AM -0500, Sasha Levin wrote:
> From: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> 
> [ Upstream commit c6fbb6bca10838485b820e8a26c23996f77ce580 ]

Why is this being backported?

> 
> The current implementation of drm_color_lut_extract()
> generates weird results. Eg. if we go through all the
> values for 16->8bpc conversion we see the following pattern:
> 
> in            out (count)
>    0 -   7f ->  0 (128)
>   80 -  17f ->  1 (256)
>  180 -  27f ->  2 (256)
>  280 -  37f ->  3 (256)
> ...
> fb80 - fc7f -> fc (256)
> fc80 - fd7f -> fd (256)
> fd80 - fe7f -> fe (256)
> fe80 - ffff -> ff (384)
> 
> So less values map to 0 and more values map 0xff, which
> doesn't seem particularly great.
> 
> To get just the same number of input values to map to
> the same output values we'd just need to drop the rounding
> entrirely. But perhaps a better idea would be to follow the
> OpenGL int<->float conversion rules, in which case we get
> the following results:
> 
> in            out (count)
>    0 -   80 ->  0 (129)
>   81 -  181 ->  1 (257)
>  182 -  282 ->  2 (257)
>  283 -  383 ->  3 (257)
> ...
> fc7c - fd7c -> fc (257)
> fd7d - fe7d -> fd (257)
> fe7e - ff7e -> fe (257)
> ff7f - ffff -> ff (129)
> 
> Note that since the divisor is constant the compiler
> is able to optimize away the integer division in most
> cases. The only exception is the _ULL() case on 32bit
> architectures since that gets emitted as inline asm
> via do_div() and thus the compiler doesn't get to
> optimize it.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@...ux.intel.com>
> Link: https://patchwork.freedesktop.org/patch/msgid/20231013131402.24072-2-ville.syrjala@linux.intel.com
> Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah@...el.com>
> Reviewed-by: Jani Nikula <jani.nikula@...el.com>
> Acked-by: Maxime Ripard <mripard@...nel.org>
> Signed-off-by: Sasha Levin <sashal@...nel.org>
> ---
>  include/drm/drm_color_mgmt.h | 19 ++++++++-----------
>  1 file changed, 8 insertions(+), 11 deletions(-)
> 
> diff --git a/include/drm/drm_color_mgmt.h b/include/drm/drm_color_mgmt.h
> index 81c298488b0c..54b2b2467bfd 100644
> --- a/include/drm/drm_color_mgmt.h
> +++ b/include/drm/drm_color_mgmt.h
> @@ -36,20 +36,17 @@ struct drm_plane;
>   *
>   * Extract a degamma/gamma LUT value provided by user (in the form of
>   * &drm_color_lut entries) and round it to the precision supported by the
> - * hardware.
> + * hardware, following OpenGL int<->float conversion rules
> + * (see eg. OpenGL 4.6 specification - 2.3.5 Fixed-Point Data Conversions).
>   */
>  static inline u32 drm_color_lut_extract(u32 user_input, int bit_precision)
>  {
> -	u32 val = user_input;
> -	u32 max = 0xffff >> (16 - bit_precision);
> -
> -	/* Round only if we're not using full precision. */
> -	if (bit_precision < 16) {
> -		val += 1UL << (16 - bit_precision - 1);
> -		val >>= 16 - bit_precision;
> -	}
> -
> -	return clamp_val(val, 0, max);
> +	if (bit_precision > 16)
> +		return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(user_input, (1 << bit_precision) - 1),
> +					     (1 << 16) - 1);
> +	else
> +		return DIV_ROUND_CLOSEST(user_input * ((1 << bit_precision) - 1),
> +					 (1 << 16) - 1);
>  }
>  
>  u64 drm_color_ctm_s31_32_to_qm_n(u64 user_input, u32 m, u32 n);
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel