On Tue, Jan 30, 2018 at 03:23:32PM +0000, Shankar, Uma wrote: > > > >-----Original Message----- > >From: Ville Syrjälä [mailto:ville.syrjala@xxxxxxxxxxxxxxx] > >Sent: Friday, December 22, 2017 9:43 PM > >To: Shankar, Uma <uma.shankar@xxxxxxxxx> > >Cc: intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Lin, Johnson <johnson.lin@xxxxxxxxx>; > >Syrjala, Ville <ville.syrjala@xxxxxxxxx>; Lankhorst, Maarten > ><maarten.lankhorst@xxxxxxxxx> > >Subject: Re: [PATCH] drm/i915: Fix Limited Range Color Handling > > > >On Fri, Dec 22, 2017 at 05:54:31PM +0200, Ville Syrjälä wrote: > >> 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. > > > >I wonder if we could also get some igts for this limited range stuff. > >Maybe something like: > >1. grab crc from black/white screen with limited range 2. grab cdc from almost > >black/white screen with full range 3. compare the two > > > >Repeat with an identity csc matrix set. > > > >Not sure if we can get the crcs to match in these two cases though. > >Hard to say without trying it out. > > > > Hi Ville, > We will try this approach and update. Ideally crc's should match, will confirm the > behavior on hardware. I suspect it's not going to work on some older platforms because we use the magic bit in the pipe/port register to do the range compression. The effect of that bit may not be visible in the pipe crc. But starting from hsw it should be ok. > >> > >> > >> > - 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 > > > >-- > >Ville Syrjälä > >Intel OTC -- Ville Syrjälä Intel OTC _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
