[Intel-gfx] [PATCH] drm/i915: Fix Limited Range Color Handling

[Ville Syrjälä]

On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote:
> From: Johnson Lin <johnson.lin at intel.com>
> 
> Some panels support limited range output (16-235) compared
> to full range RGB values (0-255). Also userspace can control
> the RGB range using "Broadcast RGB" property. Currently the
> code to handle full range to limited range is broken. This
> patch fixes the same by properly scaling down all the full
> range co-efficients with limited range scaling factor.
> 
> Signed-off-by: Johnson Lin <johnson.lin at intel.com>
> Signed-off-by: Uma Shankar <uma.shankar at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_color.c |   16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index aa66e95..777ce26 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -94,16 +94,24 @@ static void ctm_mult_by_limited(uint64_t *result, int64_t *input)
>  	for (i = 0; i < 9; i++)
>  		result[i] = 0;

This can go.

>  
> -	for (i = 0; i < 3; i++) {
> -		int64_t user_coeff = input[i * 3 + i];
> +	for (i = 0; i < 9; i++) {
> +		int64_t user_coeff = input[i];

That's not a two's complement number so should probably be u64.

>  		uint64_t limited_coeff = CTM_COEFF_LIMITED_RANGE >> 2;
>  		uint64_t abs_coeff = clamp_val(CTM_COEFF_ABS(user_coeff),
>  					       0,
>  					       CTM_COEFF_4_0 - 1) >> 2;

Seems to me that we should be able to drop the >>2 from the limited_coeff
since it's always < 1.0, make both of these u32 and then use
'mul_u32_u32() >> 30' to get the answer.

>  
> -		result[i * 3 + i] = (limited_coeff * abs_coeff) >> 27;
> +		/*
> +		 * By scaling every co-efficient with limited range (235-16)
> +		 * vs full range (0-255) the final o/p will be scaled down to
> +		 * fit in the limited range supported by the panel.
> +		 * Need to shift the multiplication result by 28 as the floating
> +		 * count expected is of 32bits, multiplication here is done in
> +		 * U2.30 so result need to be right shifted by 60-32 = 28
> +		 */
> +		result[i] = (limited_coeff * abs_coeff) >> 28;
>  		if (CTM_COEFF_NEGATIVE(user_coeff))

And this can be replaced with just

result[i] |= user_coeff & CTM_COEFF_SIGN;

to eliminate the silly branch.


> -			result[i * 3 + i] |= CTM_COEFF_SIGN;
> +			result[i] |= CTM_COEFF_SIGN;
>  	}
>  }
>  
> -- 
> 1.7.9.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC