Commit e4c0d531 authored by Matt Roper's avatar Matt Roper

drm/i915: Apply LUT validation checks to platforms more accurately (v3)

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: 85e2d61e ("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: default avatarMatt Roper <matthew.d.roper@intel.com>
Reviewed-by: default avatarVille Syrjälä <ville.syrjala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20190130181022.4291-1-matthew.d.roper@intel.com
parent c0550305
......@@ -67,9 +67,15 @@
#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 */
......
......@@ -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)
......
......@@ -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;
};
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment