OSDN Git Service

drm/i915: Apply LUT validation checks to platforms more accurately (v3)
authorMatt Roper <matthew.d.roper@intel.com>
Wed, 30 Jan 2019 18:10:22 +0000 (10:10 -0800)
committerMatt Roper <matthew.d.roper@intel.com>
Thu, 31 Jan 2019 01:09:45 +0000 (17:09 -0800)
Use of the new DRM_COLOR_LUT_NON_DECREASING test was a bit over-zealous;
it doesn't actually need to be applied to the degamma on "bdw-style"
platforms.  Likewise, we overlooked the fact that CHV should have that
test applied to the gamma LUT as well as the degamma LUT.

Rather than adding more complicated platform checking to
intel_color_check(), let's just store the appropriate set of LUT
validation flags for each platform in the intel_device_info structure.

v2:
 - Shuffle around LUT size tests so that the hardware-specific tests
   won't be applied to legacy gamma tables.  (Ville)
 - Add a debug message so that it will be easier to understand why an
   atomic transaction involving incorrectly-sized LUT's got rejected
   by the driver.

v3:
 - Switch size_t's to int's.  (Ville)

Fixes: 85e2d61e4976 ("drm/i915: Validate userspace-provided color management LUT's (v4)")
References: https://lists.freedesktop.org/archives/intel-gfx/2019-January/187634.html
Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Matt Roper <matthew.d.roper@intel.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190130181022.4291-1-matthew.d.roper@intel.com
drivers/gpu/drm/i915/i915_pci.c
drivers/gpu/drm/i915/intel_color.c
drivers/gpu/drm/i915/intel_device_info.h

index dd4aff2..6f7ad10 100644 (file)
 #define BDW_COLORS \
        .color = { .degamma_lut_size = 512, .gamma_lut_size = 512 }
 #define CHV_COLORS \
-       .color = { .degamma_lut_size = 65, .gamma_lut_size = 257 }
+       .color = { .degamma_lut_size = 65, .gamma_lut_size = 257, \
+                  .degamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
+                  .gamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING, \
+       }
 #define GLK_COLORS \
-       .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024 }
+       .color = { .degamma_lut_size = 0, .gamma_lut_size = 1024, \
+                  .degamma_lut_tests = DRM_COLOR_LUT_NON_DECREASING | \
+                                       DRM_COLOR_LUT_EQUAL_CHANNELS, \
+       }
 
 /* Keep in gen based order, and chronological order within a gen */
 
index bc75896..4b0044c 100644 (file)
@@ -605,48 +605,48 @@ void intel_color_load_luts(struct intel_crtc_state *crtc_state)
        dev_priv->display.load_luts(crtc_state);
 }
 
+static int check_lut_size(const struct drm_property_blob *lut, int expected)
+{
+       int len;
+
+       if (!lut)
+               return 0;
+
+       len = drm_color_lut_size(lut);
+       if (len != expected) {
+               DRM_DEBUG_KMS("Invalid LUT size; got %d, expected %d\n",
+                             len, expected);
+               return -EINVAL;
+       }
+
+       return 0;
+}
+
 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;
+       int gamma_length, degamma_length;
+       u32 gamma_tests, degamma_tests;
 
        degamma_length = INTEL_INFO(dev_priv)->color.degamma_lut_size;
        gamma_length = INTEL_INFO(dev_priv)->color.gamma_lut_size;
+       degamma_tests = INTEL_INFO(dev_priv)->color.degamma_lut_tests;
+       gamma_tests = INTEL_INFO(dev_priv)->color.gamma_lut_tests;
 
-       /*
-        * 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) >= 10)
-               tests |= DRM_COLOR_LUT_EQUAL_CHANNELS;
+       /* Always allow legacy gamma LUT with no further checking. */
+       if (crtc_state_is_legacy_gamma(crtc_state))
+               return 0;
 
-       if (drm_color_lut_check(crtc_state->base.degamma_lut, tests) != 0)
+       if (check_lut_size(crtc_state->base.degamma_lut, degamma_length) ||
+           check_lut_size(crtc_state->base.gamma_lut, gamma_length))
                return -EINVAL;
 
-       /*
-        * We allow both degamma & gamma luts at the right size or
-        * NULL.
-        */
-       if ((!crtc_state->base.degamma_lut ||
-            drm_color_lut_size(crtc_state->base.degamma_lut) == degamma_length) &&
-           (!crtc_state->base.gamma_lut ||
-            drm_color_lut_size(crtc_state->base.gamma_lut) == gamma_length))
-               return 0;
+       if (drm_color_lut_check(crtc_state->base.degamma_lut, degamma_tests) ||
+           drm_color_lut_check(crtc_state->base.gamma_lut, gamma_tests))
+               return -EINVAL;
 
-       /*
-        * We also allow no degamma lut/ctm and a gamma lut at the legacy
-        * size (256 entries).
-        */
-       if (crtc_state_is_legacy_gamma(crtc_state))
-               return 0;
 
-       return -EINVAL;
+       return 0;
 }
 
 void intel_color_init(struct intel_crtc *crtc)
index 957c652..7bf09ce 100644 (file)
@@ -189,6 +189,8 @@ struct intel_device_info {
        struct color_luts {
                u16 degamma_lut_size;
                u16 gamma_lut_size;
+               u32 degamma_lut_tests;
+               u32 gamma_lut_tests;
        } color;
 };