From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mika Kuoppala Subject: Re: [PATCH] drm/i915: Preallocate next seqno before touching the ring Date: Tue, 27 Nov 2012 16:30:30 +0200 Message-ID: <87haobce49.fsf@gaia.fi.intel.com> References: <1354009730-15796-1-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 mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id 4ABE0E5CFE for ; Tue, 27 Nov 2012 06:30:34 -0800 (PST) In-Reply-To: <1354009730-15796-1-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 , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, 27 Nov 2012 09:48:50 +0000, Chris Wilson wrote: > Based on the work by Mika Kuoppala, we realised that we need to handle > seqno wraparound prior to committing our changes to the ring. The most > obvious point then is to grab the seqno inside intel_ring_begin(), and > then to reuse that seqno for all ring operations until the next request. > As intel_ring_begin() can fail, the callers must already be prepared to > handle such failure and so we can safely add further checks. > > This patch looks like it should be split up into the interface > changes and the tweaks to move seqno wrapping from the execbuffer into > the core seqno increment. However, I found no easy way to break it into > incremental steps without introducing further broken behaviour. > > v2: Mika found a silly mistake and a subtle error in the existing code; > inside i915_gem_retire_requests() we were resetting the sync_seqno of > the target ring based on the seqno from this ring - which are only > related by the order of their allocation, not retirement. Hence we were > applying the optimisation that the rings were synchronised too early, > fortunately the only real casualty there is the handling of seqno > wrapping. > > Signed-off-by: Chris Wilson > Cc: Mika Kuoppala > Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=863861 Reviewed-by: Mika Kuoppala