| Message ID | 20170310101835.29845-1-ander.conselvan.de.oliveira@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 10, 2017 at 12:18:34PM +0200, Ander Conselvan de Oliveira wrote: > The 33rd entry in the pre-CSC gamma table in Geminilake can represent a > value of 1.0 as 17 bits fixed point with one integer bit. However, the > table was generated such that the value of 1.0 would be 0.ffff with > all the intervals scaled accordingly. For instance, 0.5 mapped to > 0.7fff instead of 0.8000. Actually all the gamma tables except the legacy 8bpc gamma have this 1.0 thing. And yet we program them all to <1.0. But I guess in most cases that doesn't cause any issues. But I think this is definitely something someone should look at. One problem is that the uapi we got in the end uses 0.16 representation instead of the 8.24 that I thought we were going with. So the conversion isn't going to as nice as it could be I suppose. But yeah, this patch seems trouble free since we don't yet expose the pre-csc gamma to userspace. Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > For a reason that is not clear to the author, the rounding seems to be > different when a cursor plane is used, leading to some seemingly random > failures of the kms_cursor_crc igt tests. The differences weren't > perceptible at 8bpc with images captured by a Chamelium device, but did > cause CRC mismatches. > > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> > --- > drivers/gpu/drm/i915/intel_color.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c > index b9e5266d..306c6b0 100644 > --- a/drivers/gpu/drm/i915/intel_color.c > +++ b/drivers/gpu/drm/i915/intel_color.c > @@ -465,14 +465,14 @@ static void glk_load_degamma_lut(struct drm_crtc_state *state) > * different values per channel, so this just loads a linear table. > */ > for (i = 0; i < lut_size; i++) { > - uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1); > + uint32_t v = (i * (1 << 16)) / (lut_size - 1); > > I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v); > } > > /* Clamp values > 1.0. */ > while (i++ < 35) > - I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1); > + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16)); > } > > static void glk_load_luts(struct drm_crtc_state *state) > -- > 2.9.3 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Fri, 2017-03-10 at 13:18 +0000, Patchwork wrote: > == Series Details == > > Series: drm/i915/glk: Improve rounding caused by pre-CSC gamma tables > URL : https://patchwork.freedesktop.org/series/21049/ > State : success Pushed. Thanks for reviewing. Ander > > == Summary == > > Series 21049v1 drm/i915/glk: Improve rounding caused by pre-CSC gamma tables > https://patchwork.freedesktop.org/api/1.0/series/21049/revisions/1/mbox/ > > fi-bdw-5557u total:278 pass:267 dwarn:0 dfail:0 fail:0 skip:11 time: 459s > fi-bsw-n3050 total:278 pass:239 dwarn:0 dfail:0 fail:0 skip:39 time: 610s > fi-bxt-j4205 total:278 pass:259 dwarn:0 dfail:0 fail:0 skip:19 time: 536s > fi-bxt-t5700 total:278 pass:258 dwarn:0 dfail:0 fail:0 skip:20 time: 596s > fi-byt-j1900 total:278 pass:251 dwarn:0 dfail:0 fail:0 skip:27 time: 508s > fi-byt-n2820 total:278 pass:247 dwarn:0 dfail:0 fail:0 skip:31 time: 504s > fi-hsw-4770 total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 444s > fi-hsw-4770r total:278 pass:262 dwarn:0 dfail:0 fail:0 skip:16 time: 436s > fi-ilk-650 total:278 pass:228 dwarn:0 dfail:0 fail:0 skip:50 time: 437s > fi-ivb-3520m total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 508s > fi-ivb-3770 total:278 pass:260 dwarn:0 dfail:0 fail:0 skip:18 time: 500s > fi-kbl-7500u total:278 pass:259 dwarn:1 dfail:0 fail:0 skip:18 time: 479s > fi-skl-6260u total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 510s > fi-skl-6700hq total:278 pass:261 dwarn:0 dfail:0 fail:0 skip:17 time: 591s > fi-skl-6700k total:278 pass:256 dwarn:4 dfail:0 fail:0 skip:18 time: 498s > fi-skl-6770hq total:278 pass:268 dwarn:0 dfail:0 fail:0 skip:10 time: 558s > fi-snb-2520m total:278 pass:250 dwarn:0 dfail:0 fail:0 skip:28 time: 556s > fi-snb-2600 total:278 pass:249 dwarn:0 dfail:0 fail:0 skip:29 time: 426s > > d8d69f76555feef19e3e4b601378446604d90da5 drm-tip: 2017y-03m-10d-11h-50m-04s UTC integration manifest > f2f76a8 drm/i915/glk: Improve rounding caused by pre-CSC gamma tables > > == Logs == > > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4134/ > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index b9e5266d..306c6b0 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -465,14 +465,14 @@ static void glk_load_degamma_lut(struct drm_crtc_state *state) * different values per channel, so this just loads a linear table. */ for (i = 0; i < lut_size; i++) { - uint32_t v = (i * ((1 << 16) - 1)) / (lut_size - 1); + uint32_t v = (i * (1 << 16)) / (lut_size - 1); I915_WRITE(PRE_CSC_GAMC_DATA(pipe), v); } /* Clamp values > 1.0. */ while (i++ < 35) - I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16) - 1); + I915_WRITE(PRE_CSC_GAMC_DATA(pipe), (1 << 16)); } static void glk_load_luts(struct drm_crtc_state *state)
The 33rd entry in the pre-CSC gamma table in Geminilake can represent a value of 1.0 as 17 bits fixed point with one integer bit. However, the table was generated such that the value of 1.0 would be 0.ffff with all the intervals scaled accordingly. For instance, 0.5 mapped to 0.7fff instead of 0.8000. For a reason that is not clear to the author, the rounding seems to be different when a cursor plane is used, leading to some seemingly random failures of the kms_cursor_crc igt tests. The differences weren't perceptible at 8bpc with images captured by a Chamelium device, but did cause CRC mismatches. Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)