From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mika Kuoppala Subject: Re: [PATCH] drm/i915: Advance seqno upon reseting the GPU following a hang Date: Tue, 14 May 2013 15:31:41 +0300 Message-ID: <878v3h4uaa.fsf@gaia.fi.intel.com> References: <1368019770-4653-1-git-send-email-chris@chris-wilson.co.uk> <87bo8f3u11.fsf@gaia.fi.intel.com> <20130514103447.GC9239@cantiga.alporthouse.com> 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 B4AB3E6170 for ; Tue, 14 May 2013 05:32:06 -0700 (PDT) In-Reply-To: <20130514103447.GC9239@cantiga.alporthouse.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 Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Chris Wilson writes: > On Mon, May 13, 2013 at 04:10:18PM +0300, Mika Kuoppala wrote: >> Chris Wilson writes: >> > + >> > + intel_ring_init_seqno(ring, seqno); >> > + for (i = 0; i < ARRAY_SIZE(ring->sync_seqno); i++) >> > + ring->sync_seqno[i] = 0; >> > } >> >> I remember pondering about resetting sync_seqno's >> inside intel_ring_init_seqno(). Is there reason >> not to? > > Not a strong one. Conceptually the ring->sync_seqno[] belong to the other > rings, so I felt it was clumsy for intel_ring_init_seqno() to falsely > claim ownership and reset its own sync_seqno. But I think we can > refactor away those qualms with a comment. The existing intel_ring_init_seqno() already clumsily resets the sync registers of other rings. As we can't and wont initialize anything but all of the ring seqnos at once, the existing code could be more explicit on that. But for this patch: Reviewed-by: Mika Kuoppala