On Mon, Feb 18, 2013 at 05:31:14PM +0200, Ville Syrj?l? wrote: > On Mon, Feb 18, 2013 at 02:41:00PM +0200, Ville Syrj?l? wrote: > > On Mon, Feb 18, 2013 at 11:58:23AM +0200, Ville Syrj?l? 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 at linux.intel.com wrote: > > > > > From: Ville Syrj?l? <ville.syrjala at linux.intel.com> > > > > > > > > > > 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 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
