On Wed, Aug 10, 2016 at 12:23:15PM +0300, [email protected] wrote: > From: Ville Syrjälä <[email protected]> > > Currently we require the object to be X tiled if the fb is X > tiled. The argument is supposedly FBC GTT tracking. But > actually that no longer holds water since FBC supports > Y tiling as well on SKL+. > > A better rule IMO is to require that if there is a fence, the > fb modifier match the object tiling mode. But if the object is linear, > we can allow the fb modifier to be anything. The idea being that > if the user set the tiling mode on the object, presumably the intention > is to actually use the fence for CPU access. But if the tiling mode is > not set, the user has no intention of using a fence (and can't actually > since we disallow tiling mode changes when there are framebuffers > associated with the object). > > On gen2/3 we must keep to the rule that the object and fb > must be either both linear or both X tiled. No mixing allowed > since the display engine itself will use the fence if it's present. > > v2: Fix typos > v3: Rebase due to i915_gem_object_get_tiling() & co. > > Reviewed-by: Matthew Auld <[email protected]> > Signed-off-by: Ville Syrjälä <[email protected]>
So the reason why I didn't like this is that it makes the fbc checking more tricky. Someone might think that checking for X-tiling on just the drm_framebuffer is good enough and then we'd have some potentially funny bugs. But the fbc code still only looks at the gem bo tiling, so all is still fine. Reviewed-by: Daniel Vetter <[email protected]> > --- > drivers/gpu/drm/i915/intel_display.c | 31 ++++++++++++++++++++++++------- > 1 file changed, 24 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_display.c > b/drivers/gpu/drm/i915/intel_display.c > index d8855ba969be..ad9f8b303fbc 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -15150,23 +15150,26 @@ static int intel_framebuffer_init(struct drm_device > *dev, > struct drm_i915_gem_object *obj) > { > struct drm_i915_private *dev_priv = to_i915(dev); > + unsigned int tiling = i915_gem_object_get_tiling(obj); > int ret; > u32 pitch_limit, stride_alignment; > > WARN_ON(!mutex_is_locked(&dev->struct_mutex)); > > if (mode_cmd->flags & DRM_MODE_FB_MODIFIERS) { > - /* Enforce that fb modifier and tiling mode match, but only for > - * X-tiled. This is needed for FBC. */ > - if (!!(i915_gem_object_get_tiling(obj) == I915_TILING_X) != > - !!(mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED)) { > + /* > + * If there's a fence, enforce that > + * the fb modifier and tiling mode match. > + */ > + if (tiling != I915_TILING_NONE && > + tiling != > intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) { > DRM_DEBUG("tiling_mode doesn't match fb modifier\n"); > return -EINVAL; > } > } else { > - if (i915_gem_object_get_tiling(obj) == I915_TILING_X) > + if (tiling == I915_TILING_X) { > mode_cmd->modifier[0] = I915_FORMAT_MOD_X_TILED; > - else if (i915_gem_object_get_tiling(obj) == I915_TILING_Y) { > + } else if (tiling == I915_TILING_Y) { > DRM_DEBUG("No Y tiling for legacy addfb\n"); > return -EINVAL; > } > @@ -15190,6 +15193,16 @@ static int intel_framebuffer_init(struct drm_device > *dev, > return -EINVAL; > } > > + /* > + * gen2/3 display engine uses the fence if present, > + * so the tiling mode must match the fb modifier exactly. > + */ > + if (INTEL_INFO(dev_priv)->gen < 4 && > + tiling != intel_fb_modifier_to_tiling(mode_cmd->modifier[0])) { > + DRM_DEBUG("tiling_mode must match fb modifier exactly on > gen2/3\n"); > + return -EINVAL; > + } > + > stride_alignment = intel_fb_stride_alignment(dev_priv, > mode_cmd->modifier[0], > mode_cmd->pixel_format); > @@ -15209,7 +15222,11 @@ static int intel_framebuffer_init(struct drm_device > *dev, > return -EINVAL; > } > > - if (mode_cmd->modifier[0] == I915_FORMAT_MOD_X_TILED && > + /* > + * If there's a fence, enforce that > + * the fb pitch and fence stride match. > + */ > + if (tiling != I915_TILING_NONE && > mode_cmd->pitches[0] != i915_gem_object_get_stride(obj)) { > DRM_DEBUG("pitch (%d) must match tiling stride (%d)\n", > mode_cmd->pitches[0], > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
