From ab8d66752a9c28cd6c94fa173feacdfc1554aa03 Mon Sep 17 00:00:00 2001 From: Tvrtko Ursulin Date: Mon, 2 Feb 2015 15:44:15 +0000 Subject: [PATCH] drm/i915: Track old framebuffer instead of object Daniel Vetter spotted a bug while reviewing some of my refactoring in this are of the code. I'll quote: """ > @@ -9764,6 +9768,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, > work->event = event; > work->crtc = crtc; > work->old_fb_obj = intel_fb_obj(old_fb); > + work->old_tiling_mode = to_intel_framebuffer(old_fb)->tiling_mode; Hm, that's actually an interesting bugfix - currently userspace could be sneaky and destroy the old fb immediately after the flip completes and the change the tiling of the underlying object before the unpin work had a chance to run (needs some fudgin with rt prios to starve workers to make this work though). Imo the right fix is to hold a reference onto the fb and not the underlying gem object. With that tiling is guaranteed not to change. """ This patch tries to implement the above proposed change. Signed-off-by: Tvrtko Ursulin Cc: Daniel Vetter Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_display.c | 14 +++++++------- drivers/gpu/drm/i915/intel_drv.h | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ebf973c303b7..213b870ae06e 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -9055,9 +9055,9 @@ static void intel_unpin_work_fn(struct work_struct *__work) enum pipe pipe = to_intel_crtc(work->crtc)->pipe; mutex_lock(&dev->struct_mutex); - intel_unpin_fb_obj(work->old_fb_obj); + intel_unpin_fb_obj(intel_fb_obj(work->old_fb)); drm_gem_object_unreference(&work->pending_flip_obj->base); - drm_gem_object_unreference(&work->old_fb_obj->base); + drm_framebuffer_unreference(work->old_fb); intel_fbc_update(dev); @@ -9760,7 +9760,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, work->event = event; work->crtc = crtc; - work->old_fb_obj = intel_fb_obj(old_fb); + work->old_fb = old_fb; INIT_WORK(&work->work, intel_unpin_work_fn); ret = drm_crtc_vblank_get(crtc); @@ -9796,7 +9796,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, goto cleanup; /* Reference the objects for the scheduled work. */ - drm_gem_object_reference(&work->old_fb_obj->base); + drm_framebuffer_reference(work->old_fb); drm_gem_object_reference(&obj->base); crtc->primary->fb = fb; @@ -9818,7 +9818,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, if (IS_VALLEYVIEW(dev)) { ring = &dev_priv->ring[BCS]; - if (obj->tiling_mode != work->old_fb_obj->tiling_mode) + if (obj->tiling_mode != intel_fb_obj(work->old_fb)->tiling_mode) /* vlv: DISPLAY_FLIP fails to change tiling */ ring = NULL; } else if (IS_IVYBRIDGE(dev) || IS_HASWELL(dev)) { @@ -9859,7 +9859,7 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc, work->flip_queued_vblank = drm_vblank_count(dev, intel_crtc->pipe); work->enable_stall_check = true; - i915_gem_track_fb(work->old_fb_obj, obj, + i915_gem_track_fb(intel_fb_obj(work->old_fb), obj, INTEL_FRONTBUFFER_PRIMARY(pipe)); intel_fbc_disable(dev); @@ -9875,7 +9875,7 @@ cleanup_unpin: cleanup_pending: atomic_dec(&intel_crtc->unpin_work_count); crtc->primary->fb = old_fb; - drm_gem_object_unreference(&work->old_fb_obj->base); + drm_framebuffer_unreference(work->old_fb); drm_gem_object_unreference(&obj->base); mutex_unlock(&dev->struct_mutex); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index eef79ccd0b7c..f048f8bb8beb 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -710,7 +710,7 @@ intel_get_crtc_for_plane(struct drm_device *dev, int plane) struct intel_unpin_work { struct work_struct work; struct drm_crtc *crtc; - struct drm_i915_gem_object *old_fb_obj; + struct drm_framebuffer *old_fb; struct drm_i915_gem_object *pending_flip_obj; struct drm_pending_vblank_event *event; atomic_t pending; -- 2.20.1