From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH v2 1/3] drm/i915: Wake up pending_flip_queue as part of reset handling Date: Mon, 18 Feb 2013 17:39:52 +0100 Message-ID: <20130218163952.GP5813@phenom.ffwll.local> 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> <20130218153114.GP9135@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mail-wi0-f170.google.com (mail-wi0-f170.google.com [209.85.212.170]) by gabe.freedesktop.org (Postfix) with ESMTP id 38A2BE63DC for ; Mon, 18 Feb 2013 08:37:35 -0800 (PST) Received: by mail-wi0-f170.google.com with SMTP id hm11so3963114wib.1 for ; Mon, 18 Feb 2013 08:37:34 -0800 (PST) Content-Disposition: inline In-Reply-To: <20130218153114.GP9135@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: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Mon, Feb 18, 2013 at 05:31:14PM +0200, Ville Syrj=E4l=E4 wrote: > 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= .com 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 ma= ke > > > 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= woken 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? I'd vote for the second approach of first waking everyone up, and then doing the modeset updates. I guess you could even use drm_modeset_lock_all, not really worth optimizing things here imo. Also please add a comment to explain this ordering constraint in the reset code. -Daniel -- = Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch