[Intel-gfx] [PATCH 2/4] drm/i915: Adjust LUT rounding rules

[Borah, Chaitanya Kumar]

> -----Original Message-----
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> Sent: Monday, November 20, 2023 7:57 PM
> To: Borah, Chaitanya Kumar <chaitanya.kumar.borah at intel.com>
> Cc: intel-gfx at lists.freedesktop.org; dri-devel at lists.freedesktop.org
> Subject: Re: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
> 
> On Mon, Nov 20, 2023 at 06:08:57AM +0000, Borah, Chaitanya Kumar wrote:
> > Hello Ville,
> >
> > > -----Original Message-----
> > > From: dri-devel <dri-devel-bounces at lists.freedesktop.org> On Behalf
> > > Of Ville Syrjala
> > > Sent: Friday, October 13, 2023 6:44 PM
> > > To: intel-gfx at lists.freedesktop.org
> > > Cc: dri-devel at lists.freedesktop.org
> > > Subject: [PATCH 2/4] drm/i915: Adjust LUT rounding rules
> > >
> > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > >
> > > drm_color_lut_extract() rounding was changed to follow the OpenGL
> > > int<-
> > > >float conversion rules. Adjust intel_color_lut_pack() to match.
> > >
> > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/display/intel_color.c | 14 ++++++--------
> > >  1 file changed, 6 insertions(+), 8 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c
> > > b/drivers/gpu/drm/i915/display/intel_color.c
> > > index 2a2a163ea652..b01f463af861 100644
> > > --- a/drivers/gpu/drm/i915/display/intel_color.c
> > > +++ b/drivers/gpu/drm/i915/display/intel_color.c
> > > @@ -785,14 +785,12 @@ static void chv_assign_csc(struct
> > > intel_crtc_state
> > > *crtc_state)
> > >  /* convert hw value with given bit_precision to lut property val */
> > > static u32
> > > intel_color_lut_pack(u32 val, int bit_precision)  {
> >
> > Is this operation unique to Intel. Should there be a drm helper for this?
> 
> If some other driver gains gamma readout support they could probably use
> something like this. The other option would be to rework the current helper
> to allow conversions both ways.
> 

The function name could be a minor inconvenience but anyway until that time arrives.

LGTM.

Reviewed-by: Chaitanya Kumar Borah <chaitanya.kumar.borah at intel.com>

> >
> > Regards
> >
> > Chaitanya
> >
> > > -	u32 max = 0xffff >> (16 - bit_precision);
> > > -
> > > -	val = clamp_val(val, 0, max);
> > > -
> > > -	if (bit_precision < 16)
> > > -		val <<= 16 - bit_precision;
> > > -
> > > -	return val;
> > > +	if (bit_precision > 16)
> > > +		return DIV_ROUND_CLOSEST_ULL(mul_u32_u32(val, (1 << 16)
> > > - 1),
> > > +					     (1 << bit_precision) - 1);
> > > +	else
> > > +		return DIV_ROUND_CLOSEST(val * ((1 << 16) - 1),
> > > +					 (1 << bit_precision) - 1);
> > >  }
> > >
> > >  static u32 i9xx_lut_8(const struct drm_color_lut *color)
> > > --
> > > 2.41.0
> >
> 
> --
> Ville Syrjälä
> Intel