From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 07/21] drm/i915: Track fence setup separately from fenced object lifetime Date: Sat, 16 Apr 2011 15:20:42 +0200 Message-ID: <20110416132041.GD3498@viiv.ffwll.ch> References: <1302945465-32115-1-git-send-email-chris@chris-wilson.co.uk> <1302945465-32115-8-git-send-email-chris@chris-wilson.co.uk> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-ww0-f43.google.com (mail-ww0-f43.google.com [74.125.82.43]) by gabe.freedesktop.org (Postfix) with ESMTP id F387E9E7A1 for ; Sat, 16 Apr 2011 06:20:47 -0700 (PDT) Received: by wwb17 with SMTP id 17so3471764wwb.12 for ; Sat, 16 Apr 2011 06:20:46 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1302945465-32115-8-git-send-email-chris@chris-wilson.co.uk> 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 Cc: Andy Whitcroft , Daniel Vetter , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Sat, Apr 16, 2011 at 10:17:31AM +0100, Chris Wilson wrote: > This fixes a bookkeeping error causing an OOPS whilst waiting for an > object to finish using a fence. Now we can simply wait for the fence to > be written independent of the objects currently inhabiting it (past, > present and future). > > A large amount of the change is to delay updating the information about > the fence on bo until after we successfully write, or queue the write to, > the register. This avoids the complication of undoing a partial change > should we fail in pipelining the change. Reordering the accounting and consistently using setup_seqno/ring to track fence changes looks correct. I still see a few minor issues with the pipelined fencing code that should be addressed in later patches: - setup_seqno/ring is not being cleanup up anywhere and might go stale. The hack I've used in one of my stabs at this was to loop over all fences at the end of retire_requests. - The semantics of last_fenced_seqno/ring are a bit hairy. I think ideal would be if the following would always hold: * an object may never change it's ring if last_fenced_seqno != 0. A call to flush fence is required to change the ring. * if last_fenced_seqno != 0, then always last_rendering_seqno != 0 (i.e. if the execbuffer fails we still move all objects with pipelined fencing setups to the active list). These two combined should give: last_fenced_ring != NULL implies last_fenced_ring == last_rendering_ring allowing us to simplify a few things. - The obj->fenced_gpu_access optimization got killed in a previous patch. We could resurrect that by clearing it in process_flushing_list. With that change (and perhaps some movement of the assignement of fenced_gpu_access) we could consolidate the last_fenced_seqno/ring tracking. Reviewed-by: Daniel Vetter -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48