On Mon, Jun 15, 2015 at 08:53:02AM +0100, Chris Wilson wrote: > On Mon, Jun 15, 2015 at 09:44:15AM +0300, Jani Nikula wrote: > > On Wed, 03 Jun 2015, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > > > On Wed, Jun 03, 2015 at 03:43:32PM +0200, Lukas Wunner wrote: > > >> Hi, > > >> > > >> a deadlock was introduced by commit 60a5ca015ffd2aacfe5674b5a401cd2a37159e07 > > >> > > >> Author: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > >> Date: Fri Jun 13 11:10:53 2014 +0300 > > >> > > >> drm/i915: Add locking around framebuffer_references-- > > >> > > >> > > >> The commit amended intel_display.c:intel_user_framebuffer_destroy() with > > >> mutex_lock(&dev->struct_mutex). > > >> > > >> A few weeks prior Chris Wilson had amended intel_fbdev.c:intelfb_create() > > >> with a call to drm_framebuffer_unreference() while &dev->struct_mutex is > > >> locked (commit edd586fe705e819bc711b5ed7194a0b6f9f1a7e1, "drm/i915: Discard > > >> BIOS framebuffers too small to accommodate chosen mode"). > > >> > > > > > > Just move the mutex_lock down a step. > > > > Lucas, did you try this? > > There's a goto unlock that also needed to be disabled, such as Your previous patch placed the mutex_lock before the goto out_unlock - I fail to see what has been broken with that version? Can you resubmit that as a proper patch with sob and Lucas' t-b? Thanks, Daniel > > diff --git a/drivers/gpu/drm/i915/intel_fbdev.c b/drivers/gpu/drm/i915/intel_fbdev.c > index dda99c0d6be1..fc7ec5138fb7 100644 > --- a/drivers/gpu/drm/i915/intel_fbdev.c > +++ b/drivers/gpu/drm/i915/intel_fbdev.c > @@ -213,8 +213,6 @@ static int intelfb_create(struct drm_fb_helper *helper, > bool prealloc = false; > int ret; > > - mutex_lock(&dev->struct_mutex); > - > if (intel_fb && > (sizes->fb_width > intel_fb->base.width || > sizes->fb_height > intel_fb->base.height)) { > @@ -229,7 +227,7 @@ static int intelfb_create(struct drm_fb_helper *helper, > DRM_DEBUG_KMS("no BIOS fb, allocating a new one\n"); > ret = intelfb_alloc(helper, sizes); > if (ret) > - goto out_unlock; > + return ret; > intel_fb = ifbdev->fb; > } else { > DRM_DEBUG_KMS("re-using BIOS fb\n"); > @@ -241,6 +239,7 @@ static int intelfb_create(struct drm_fb_helper *helper, > obj = intel_fb->obj; > vma = i915_gem_obj_to_ggtt(obj, NULL); > > + mutex_lock(&dev->struct_mutex); > info = framebuffer_alloc(0, &dev->pdev->dev); > if (!info) { > ret = -ENOMEM; > @@ -311,7 +310,6 @@ static int intelfb_create(struct drm_fb_helper *helper, > out_unpin: > drm_gem_object_unreference(&obj->base); > -out_unlock: > mutex_unlock(&dev->struct_mutex); > return ret; > } > > -- > Chris Wilson, Intel Open Source Technology Centre > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx