On Tue, Dec 18, 2018 at 09:51:58AM -0800, Matt Roper wrote:
> We currently program userspace-provided gamma and degamma LUT's into our
> hardware without really checking to see whether they satisfy our
> hardware's rules. We should try to catch tables that are invalid for
> our hardware early and reject the atomic transaction.
>
> All of our platforms that accept a degamma LUT expect that the entries
> in the LUT are always flat or increasing, never decreasing. Also, our
> GLK and ICL platforms only accept degamma tables with r=g=b entries; so
> we should also add the relevant checks for that in anticipation of
> degamma support landing for those platforms.
>
> v2:
> - Use new API (single check function with bitmask of tests to apply)
> - Call helper for our gamma table as well (with no additional tests
> specified) so that the table size will be validated.
>
> v3:
> - Don't call on the gamma table since the LUT size is already tested at
> property blob upload and we don't have any additional hardware
> constraints for that LUT.
>
> v4:
> - Apply equal color channel check on gen10 as well; the bspec has some
> strange tagging for CNL platforms, but this appears to apply there as
> well. (Ville)
>
> Cc: Uma Shankar <uma.shankar@xxxxxxxxx>
> Cc: Swati Sharma <swati2.sharma@xxxxxxxxx>
> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> Signed-off-by: Matt Roper <matthew.d.roper@xxxxxxxxx>
> Reviewed-by: Uma Shankar <uma.shankar@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_color.c | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c
> index 37fd9ddf762e..e3ffbb0ad9a6 100644
> --- a/drivers/gpu/drm/i915/intel_color.c
> +++ b/drivers/gpu/drm/i915/intel_color.c
> @@ -609,10 +609,26 @@ int intel_color_check(struct intel_crtc_state *crtc_state)
> {
> struct drm_i915_private *dev_priv = to_i915(crtc_state->base.crtc->dev);
> size_t gamma_length, degamma_length;
> + uint32_t tests = DRM_COLOR_LUT_NON_DECREASING;
>
> degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
> gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
>
> + /*
> + * All of our platforms mandate that the degamma curve be
> + * non-decreasing.
This is actually not true. Only interpolated gamma modes require a
non-decreasing curve. The split gamma mode used on pre-glk is not
interpolated.
Also both CHV CGM gamma and degamma are interpolated, so we should
rather be checking gamma as well here.
> Additionally, GLK and gen11 only accept a single
> + * value for red, green, and blue in the degamma table. Make sure
> + * userspace didn't try to pass us something we can't handle.
> + *
> + * We don't have any extra hardware constraints on the gamma table,
> + * so no need to explicitly check it.
> + */
> + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 10)
> + tests |= DRM_COLOR_LUT_EQUAL_CHANNELS;
> +
> + if (drm_color_lut_check(crtc_state->base.degamma_lut, tests) != 0)
> + return -EINVAL;
> +
> /*
> * We allow both degamma & gamma luts at the right size or
> * NULL.
> --
> 2.14.4
--
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
