On Fri, Feb 27, 2015 at 09:45:27AM +0000, Tvrtko Ursulin wrote: > > 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 <[email protected]> > >> > >>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 <[email protected]> > >>Reviewed-by: Damien Lespiau <[email protected]> (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. ;)
Well I've obviously failed to explain properly in my review to v1 to avoid a v3 ;-) -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/intel-gfx
