From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: fix hpd work vs. flush_work in the pageflip code deadlock Date: Mon, 2 Sep 2013 21:02:33 +0200 Message-ID: <20130902190233.GH9374@phenom.ffwll.local> References: <1378104541-22368-1-git-send-email-daniel.vetter@ffwll.ch> <20130902115205.GB6439@nuc-i3427.alporthouse.com> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Return-path: Content-Disposition: inline In-Reply-To: <20130902115205.GB6439@nuc-i3427.alporthouse.com> Sender: stable-owner@vger.kernel.org To: Chris Wilson , Daniel Vetter , Intel Graphics Development , Stuart Abercrombie , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Sep 02, 2013 at 12:52:05PM +0100, Chris Wilson wrote: > On Mon, Sep 02, 2013 at 08:49:01AM +0200, Daniel Vetter wrote: > > Historically we've run our own driver hotplug handling in our own > > work-queue, which then launched the drm core hotplug handling in the > > system workqueue. This is important since we flush our own driver > > workqueue in the pageflip code while hodling modeset locks, and only > > the drm hotplug code grabbed these locks. But with > > > > commit 69787f7da6b2adc4054357a661aaa1701a9ca76f > > Author: Daniel Vetter > > Date: Tue Oct 23 18:23:34 2012 +0000 > > > > drm: run the hpd irq event code directly > > > > this was changed and now we could deadlock in our flip handler if > > there's a hotplug work blocking the progress of the crucial unpin > > works. So this broke the careful deadlock avoidance implemented in > > > > commit b4a98e57fc27854b5938fc8b08b68e5e68b91e1f > > Author: Chris Wilson > > Date: Thu Nov 1 09:26:26 2012 +0000 > > > > drm/i915: Flush outstanding unpin tasks before pageflipping > > > > Since the rule thus far has been that work items on our own workqueue > > may never grab modeset locks simply restore that rule again. > > > > Cc: Chris Wilson > > Cc: Stuart Abercrombie > > Reported-by: Stuart Abercrombie > > References: http://permalink.gmane.org/gmane.comp.freedesktop.xorg.drivers.intel/26239 > > Cc: stable@vger.kernel.org > > Signed-off-by: Daniel Vetter > > That wins for simplicity, and it is indeed the only caller that requires > mode_config.lock, so > > Reviewed-by: Chris Wilson > > Bonus would a reminder in i915_drv.h to say that we cannot put items > that require mode_config.lock onto the wq, and that they should go onto > the global workqueue instead. I've merged the updated version, thanks for your review. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch