From 80075d492f8773209e26d11d6bb13ba624ef95a4 Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Wed, 9 Oct 2013 21:23:52 +0200 Subject: [PATCH] drm/i915: prevent tiling changes on framebuffer backing storage MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Assuming that all framebuffer related metadata is invariant simplifies our userspace input data checking. And current userspace always first updates the tiling of an object before creating a framebuffer with it. This allows us to upconvert a check in pin_and_fence to a WARN. In the future it should also be helpful to know which buffer objects are potential scanout targets for e.g. frontbuffer rendering tracking and similar things. Note that SNA shipped for one prerelease with code which will be broken through this patch. But users shouldn't notice since it's purely an optimization and will transparently fall back to allocating a new fb. i-g-t also had offending code (now fixed), but we don't really care about breaking the test-suite. Cc: Ville Syrjälä Reviewed-by: Ville Syrjälä Grumpily-reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/i915_drv.h | 3 +++ drivers/gpu/drm/i915/i915_gem_tiling.c | 2 +- drivers/gpu/drm/i915/intel_display.c | 7 +++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index e2bf930c2e91..759de9f123fd 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1597,6 +1597,9 @@ struct drm_i915_gem_object { /** Current tiling stride for the object, if it's tiled. */ uint32_t stride; + /** References from framebuffers, locks out tiling changes. */ + unsigned long framebuffer_references; + /** Record of address bit 17 of each page at last unbind. */ unsigned long *bit_17; diff --git a/drivers/gpu/drm/i915/i915_gem_tiling.c b/drivers/gpu/drm/i915/i915_gem_tiling.c index ac9ebe98f8b0..b13905348048 100644 --- a/drivers/gpu/drm/i915/i915_gem_tiling.c +++ b/drivers/gpu/drm/i915/i915_gem_tiling.c @@ -308,7 +308,7 @@ i915_gem_set_tiling(struct drm_device *dev, void *data, return -EINVAL; } - if (obj->pin_count) { + if (obj->pin_count || obj->framebuffer_references) { drm_gem_object_unreference_unlocked(&obj->base); return -EBUSY; } diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 88e560070bec..97ee553a81e5 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1921,10 +1921,7 @@ intel_pin_and_fence_fb_obj(struct drm_device *dev, alignment = 0; break; case I915_TILING_Y: - /* Despite that we check this in framebuffer_init userspace can - * screw us over and change the tiling after the fact. Only - * pinned buffers can't change their tiling. */ - DRM_DEBUG_DRIVER("Y tiled not allowed for scan out buffers\n"); + WARN(1, "Y tiled bo slipped through, driver bug!\n"); return -EINVAL; default: BUG(); @@ -9962,6 +9959,7 @@ static void intel_setup_outputs(struct drm_device *dev) void intel_framebuffer_fini(struct intel_framebuffer *fb) { drm_framebuffer_cleanup(&fb->base); + WARN_ON(!fb->obj->framebuffer_references--); drm_gem_object_unreference_unlocked(&fb->obj->base); } @@ -10088,6 +10086,7 @@ int intel_framebuffer_init(struct drm_device *dev, drm_helper_mode_fill_fb_struct(&intel_fb->base, mode_cmd); intel_fb->obj = obj; + intel_fb->obj->framebuffer_references++; ret = drm_framebuffer_init(dev, &intel_fb->base, &intel_fb_funcs); if (ret) { -- 2.34.1