From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v2 1/3] drm/i915: Wake up pending_flip_queue as part of reset handling Date: Mon, 18 Feb 2013 17:31:14 +0200 Message-ID: <20130218153114.GP9135@intel.com> References: <1360940866-22435-1-git-send-email-ville.syrjala@linux.intel.com> <1360940866-22435-2-git-send-email-ville.syrjala@linux.intel.com> <20130215235314.GB18708@cantiga.alporthouse.com> <20130218095822.GG9135@intel.com> <20130218124100.GO9135@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id B2A0BE637A for ; Mon, 18 Feb 2013 07:31:19 -0800 (PST) Content-Disposition: inline In-Reply-To: <20130218124100.GO9135@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Chris Wilson , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Feb 18, 2013 at 02:41:00PM +0200, Ville Syrj=E4l=E4 wrote: > On Mon, Feb 18, 2013 at 11:58:23AM +0200, Ville Syrj=E4l=E4 wrote: > > On Fri, Feb 15, 2013 at 11:53:14PM +0000, Chris Wilson wrote: > > > On Fri, Feb 15, 2013 at 05:07:44PM +0200, ville.syrjala@linux.intel.c= om wrote: > > > > From: Ville Syrj=E4l=E4 > > > > = > > > > Someone may be waiting for a flip that will never complete due to a= GPU > > > > reset. Wake up all such waiters after the GPU reset processing has > > > > finished. > > > > = > > > > v2: Dropped the wake_up_all() from i915_handle_error() since > > > > we no longer wait for pending flips with struct_mutex held. > > > = > > > Isn't the wake_up(pending_flip_queue) superseded by performing the > > > explicit do_intel_finish_page_flip() in patch 3? > > = > > Yes that's correct. But I actually forgot to remove the wake_up patch > > from my tree when I tested this. I'll run a few more tests just to make > > sure it still works. > = > I just tried it w/o the wake_up_all() and unfortunately it hung :( > = > Need to think about it a bit more I suppose. Well after a bit more though I think I know what happened: 1. page flip submitted on crtc which is not the first crtc in the list 2. mode set operation on any crtc (will grab all crtc locks) 3. reset handling loops over crtcs 3.1 first crtc doesn't have a pending flip, so pending_flip_queue is not w= oken up 3.2 code tries to grab first crtc mutex to update the base address -> deadlock So either I need to keep the wake_up_all() call in the reset handling, or I need to do two loops over the crtcs, first loop would complete all page flips, and second loop would update the base address registers. I don't really care which way we go. Anyone else have a preference? -- = Ville Syrj=E4l=E4 Intel OTC