| Message ID | 20181213215526.31991-3-matthew.d.roper@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add gamma/degamma LUT validation helper | expand |
>-----Original Message----- >From: dri-devel [mailto:dri-devel-bounces@lists.freedesktop.org] On Behalf Of >Matt Roper >Sent: Friday, December 14, 2018 3:25 AM >To: dri-devel@lists.freedesktop.org; intel-gfx@lists.freedesktop.org >Cc: Shankar, Uma <uma.shankar@intel.com>; Sharma, Swati2 ><swati2.sharma@intel.com> >Subject: [PATCH v2 2/2] drm/i915: Validate userspace-provided color >management LUT's (v2) > >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. Looks ok to me. Reviewed-By: Uma Shankar <uma.shankar@intel.com> >Cc: Uma Shankar <uma.shankar@intel.com> >Cc: Swati Sharma <swati2.sharma@intel.com> >Signed-off-by: Matt Roper <matthew.d.roper@intel.com> >--- > drivers/gpu/drm/i915/intel_color.c | 19 +++++++++++++++++++ > 1 file changed, 19 insertions(+) > >diff --git a/drivers/gpu/drm/i915/intel_color.c >b/drivers/gpu/drm/i915/intel_color.c >index 37fd9ddf762e..5ad4459a5f3c 100644 >--- a/drivers/gpu/drm/i915/intel_color.c >+++ b/drivers/gpu/drm/i915/intel_color.c >@@ -609,10 +609,29 @@ 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_INCREASING; > > 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 we just test that it's a proper size multiple >+ * (tablesize % entrysize == 0). >+ */ >+ if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11) >+ tests |= DRM_COLOR_LUT_EQUAL_CHANNELS; >+ >+ if (drm_color_lut_check(crtc_state->base.degamma_lut, tests) != 0) >+ return -EINVAL; >+ if (drm_color_lut_check(crtc_state->base.gamma_lut, 0) != 0) >+ return -EINVAL; >+ > /* > * We allow both degamma & gamma luts at the right size or > * NULL. >-- >2.14.4 > >_______________________________________________ >dri-devel mailing list >dri-devel@lists.freedesktop.org >https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff --git a/drivers/gpu/drm/i915/intel_color.c b/drivers/gpu/drm/i915/intel_color.c index 37fd9ddf762e..5ad4459a5f3c 100644 --- a/drivers/gpu/drm/i915/intel_color.c +++ b/drivers/gpu/drm/i915/intel_color.c @@ -609,10 +609,29 @@ 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_INCREASING; 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 we just test that it's a proper size multiple + * (tablesize % entrysize == 0). + */ + if (IS_GEMINILAKE(dev_priv) || INTEL_GEN(dev_priv) >= 11) + tests |= DRM_COLOR_LUT_EQUAL_CHANNELS; + + if (drm_color_lut_check(crtc_state->base.degamma_lut, tests) != 0) + return -EINVAL; + if (drm_color_lut_check(crtc_state->base.gamma_lut, 0) != 0) + return -EINVAL; + /* * We allow both degamma & gamma luts at the right size or * NULL.
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. Cc: Uma Shankar <uma.shankar@intel.com> Cc: Swati Sharma <swati2.sharma@intel.com> Signed-off-by: Matt Roper <matthew.d.roper@intel.com> --- drivers/gpu/drm/i915/intel_color.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)