On Fri, Dec 22, 2017 at 08:34:47PM +0530, Uma Shankar wrote: > From: Johnson Lin <johnson.lin@xxxxxxxxx> > > 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@xxxxxxxxx> > Signed-off-by: Uma Shankar <uma.shankar@xxxxxxxxx> > --- > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx