On Mon, Dec 17, 2018 at 02:44:15PM -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. > > Cc: Uma Shankar <uma.shankar@xxxxxxxxx> > Cc: Swati Sharma <swati2.sharma@xxxxxxxxx> > 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..bdbe474ad9b2 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. 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) >= 11) > + tests |= DRM_COLOR_LUT_EQUAL_CHANNELS; >= 10 ? > + > + 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 > > _______________________________________________ > dri-devel mailing list > dri-devel@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/dri-devel -- Ville Syrjälä Intel _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel