On 02/26/2015 04:44 PM, Daniel Vetter wrote:
On Wed, Feb 25, 2015 at 04:47:24PM +0000, Tvrtko Ursulin wrote:From: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> By this patch all underlying bits have been implemented and this patch actually enables the feature. v2: Validate passed in fb modifiers to reject garbage. (Daniel Vetter) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx> Reviewed-by: Damien Lespiau <damien.lespiau@xxxxxxxxx> (v1) --- drivers/gpu/drm/i915/intel_display.c | 26 +++++++++++++++++++++----- 1 file changed, 21 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 1d50934..3232ddd 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -12783,6 +12783,19 @@ static int intel_framebuffer_init(struct drm_device *dev, WARN_ON(!mutex_is_locked(&dev->struct_mutex)); if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) { + /* Passed in modifier sanity checking. */ + switch (mode_cmd->modifier[0]) { + case DRM_FORMAT_MOD_NONE: + case I915_FORMAT_MOD_X_TILED: + case I915_FORMAT_MOD_Y_TILED: + case I915_FORMAT_MOD_Yf_TILED: + break; + default: + DRM_ERROR("Unsupported fb modifier 0x%llx!\n", + mode_cmd->modifier[0]); + return -EINVAL; + } + /* Enforce that fb modifier and tiling mode match, but only for * X-tiled. This is needed for FBC. */ if (!!(obj->tiling_mode == I915_TILING_X) != @@ -12790,6 +12803,14 @@ static int intel_framebuffer_init(struct drm_device *dev, DRM_DEBUG("tiling_mode doesn't match fb modifier\n"); return -EINVAL; } + + if (INTEL_INFO(dev)->gen < 9 && + (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED || + mode_cmd->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) { + DRM_DEBUG("Unsupported tiling 0x%llx!\n", + mode_cmd->modifier[0]); + return -EINVAL; + } } else { if (obj->tiling_mode == I915_TILING_X) mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; @@ -12799,11 +12820,6 @@ static int intel_framebuffer_init(struct drm_device *dev, } } - if (mode_cmd->modifier[0] == I915_FORMAT_MOD_Y_TILED) { - DRM_DEBUG("hardware does not support tiling Y\n"); - return -EINVAL; - }My idea was actually to put the switch here (reduces one indent level) and put the per-gen stuff into the switch statement too. I.e. /* Passed in modifier sanity checking. */ switch (mode_cmd->modifier[0]) { case I915_FORMAT_MOD_Y_TILED: case I915_FORMAT_MOD_Yf_TILED: if (INTEL_INFO(dev)->gen < 9) { DRM_DEBUG("Unsupported tiling 0x%llx!\n", mode_cmd->modifier[0]); return -EINVAL; } case DRM_FORMAT_MOD_NONE: case I915_FORMAT_MOD_X_TILED: break; default: DRM_ERROR("Unsupported fb modifier 0x%llx!\n", mode_cmd->modifier[0]); return -EINVAL; } That gives us a natural place for extensions later on if we need to add more special cases.
Yes I agree this patch being only on v2 was way disproportionate compared to some others from this series. ;)
Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
