| Message ID | 20231013131402.24072-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915: Fix LUT rounding | expand |
Hello Ville, > -----Original Message----- > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Ville > Syrjala > Sent: Friday, October 13, 2023 6:44 PM > To: intel-gfx@lists.freedesktop.org > Cc: dri-devel@lists.freedesktop.org > Subject: [PATCH 2/4] drm/i915: Adjust LUT rounding rules > > From: Ville Syrjälä <ville.syrjala@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@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? 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
On Mon, Nov 20, 2023 at 06:08:57AM +0000, Borah, Chaitanya Kumar wrote: > Hello Ville, > > > -----Original Message----- > > From: dri-devel <dri-devel-bounces@lists.freedesktop.org> On Behalf Of Ville > > Syrjala > > Sent: Friday, October 13, 2023 6:44 PM > > To: intel-gfx@lists.freedesktop.org > > Cc: dri-devel@lists.freedesktop.org > > Subject: [PATCH 2/4] drm/i915: Adjust LUT rounding rules > > > > From: Ville Syrjälä <ville.syrjala@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@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. > > 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 >
> -----Original Message----- > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > Sent: Monday, November 20, 2023 7:57 PM > To: Borah, Chaitanya Kumar <chaitanya.kumar.borah@intel.com> > Cc: intel-gfx@lists.freedesktop.org; dri-devel@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@lists.freedesktop.org> On Behalf > > > Of Ville Syrjala > > > Sent: Friday, October 13, 2023 6:44 PM > > > To: intel-gfx@lists.freedesktop.org > > > Cc: dri-devel@lists.freedesktop.org > > > Subject: [PATCH 2/4] drm/i915: Adjust LUT rounding rules > > > > > > From: Ville Syrjälä <ville.syrjala@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@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@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
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) { - 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)