[Intel-gfx] [PATCH 1/2] drm/i915: Move drm_framebuffer_unreference out of struct_mutex for flips

[Daniel Vetter]

On Mon, Feb 16, 2015 at 02:31:49PM +0000, Chris Wilson wrote:
> intel_user_framebuffer_destroy() requires the struct_mutex for its
> object bookkeeping, so this means that all calls to
> drm_framebuffer_reference must be held without that lock.

                 ^unref... you mean I guess? Same type in patch 2.
> 
> Regression from commit ab8d66752a9c28cd6c94fa173feacdfc1554aa03
> Author: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Date:   Mon Feb 2 15:44:15 2015 +0000
> 
>     drm/i915: Track old framebuffer instead of object
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89166
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

Tvrtko can you please review these two?

Thanks, Daniel

> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3b0fe9f1f3c9..6e1da7da5cca 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9107,7 +9107,6 @@ static void intel_unpin_work_fn(struct work_struct *__work)
>  	mutex_lock(&dev->struct_mutex);
>  	intel_unpin_fb_obj(intel_fb_obj(work->old_fb));
>  	drm_gem_object_unreference(&work->pending_flip_obj->base);
> -	drm_framebuffer_unreference(work->old_fb);
>  
>  	intel_fbc_update(dev);
>  
> @@ -9116,6 +9115,7 @@ static void intel_unpin_work_fn(struct work_struct *__work)
>  	mutex_unlock(&dev->struct_mutex);
>  
>  	intel_frontbuffer_flip_complete(dev, INTEL_FRONTBUFFER_PRIMARY(pipe));
> +	drm_framebuffer_unreference(work->old_fb);
>  
>  	BUG_ON(atomic_read(&to_intel_crtc(work->crtc)->unpin_work_count) == 0);
>  	atomic_dec(&to_intel_crtc(work->crtc)->unpin_work_count);
> @@ -9778,10 +9778,6 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  	if (atomic_read(&intel_crtc->unpin_work_count) >= 2)
>  		flush_workqueue(dev_priv->wq);
>  
> -	ret = i915_mutex_lock_interruptible(dev);
> -	if (ret)
> -		goto cleanup;
> -
>  	/* Reference the objects for the scheduled work. */
>  	drm_framebuffer_reference(work->old_fb);
>  	drm_gem_object_reference(&obj->base);
> @@ -9791,6 +9787,10 @@ static int intel_crtc_page_flip(struct drm_crtc *crtc,
>  
>  	work->pending_flip_obj = obj;
>  
> +	ret = i915_mutex_lock_interruptible(dev);
> +	if (ret)
> +		goto cleanup;
> +
>  	atomic_inc(&intel_crtc->unpin_work_count);
>  	intel_crtc->reset_counter = atomic_read(&dev_priv->gpu_error.reset_counter);
>  
> @@ -9855,13 +9855,14 @@ cleanup_unpin:
>  	intel_unpin_fb_obj(obj);
>  cleanup_pending:
>  	atomic_dec(&intel_crtc->unpin_work_count);
> +	mutex_unlock(&dev->struct_mutex);
> +cleanup:
>  	crtc->primary->fb = old_fb;
>  	update_state_fb(crtc->primary);
> +
> +	drm_gem_object_unreference_unlocked(&obj->base);
>  	drm_framebuffer_unreference(work->old_fb);
> -	drm_gem_object_unreference(&obj->base);
> -	mutex_unlock(&dev->struct_mutex);
>  
> -cleanup:
>  	spin_lock_irq(&dev->event_lock);
>  	intel_crtc->unpin_work = NULL;
>  	spin_unlock_irq(&dev->event_lock);
> -- 
> 2.1.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch