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. -Daniel > - > stride_alignment = intel_fb_stride_alignment(dev, mode_cmd->modifier[0], > mode_cmd->pixel_format); > if (mode_cmd->pitches[0] & (stride_alignment - 1)) { > -- > 2.3.0 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx