From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH] drm/i915: correctly order the ring init sequence Date: Tue, 7 Aug 2012 14:31:32 +0200 Message-ID: <20120807123132.GC10377@phenom.ffwll.local> References: <1344326054-10244-1-git-send-email-daniel.vetter@ffwll.ch> <87y5lqevbp.fsf@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-bk0-f49.google.com (mail-bk0-f49.google.com [209.85.214.49]) by gabe.freedesktop.org (Postfix) with ESMTP id 252BA9E77F for ; Tue, 7 Aug 2012 05:31:13 -0700 (PDT) Received: by bkcji2 with SMTP id ji2so1637263bkc.36 for ; Tue, 07 Aug 2012 05:31:13 -0700 (PDT) Content-Disposition: inline In-Reply-To: <87y5lqevbp.fsf@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: Jani Nikula Cc: Daniel Vetter , Intel Graphics Development , stable@vger.kernel.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Aug 07, 2012 at 03:32:42PM +0300, Jani Nikula wrote: > On Tue, 07 Aug 2012, Daniel Vetter wrote: > > We may only start to set up the new register values after having > > confirmed that the ring is truely off. Otherwise the hw might lose the > > newly written register values. This is caught later on in the init > > sequence, when we check whether the register writes have stuck. > > With or without (up to you) the comment clarification discussed in IRC, > > Reviewed-by: Jani Nikula I'll fixup the comment when applying, but for reference the new proposal is: "Initialize the ring. This must happen _after_ we've cleared the ring registers with the above sequence (the readback of the HEAD registers also enforces ordering), otherwise the hw might lose the new ring register values." Thanks for the review. -Daniel > > > > > > Cc: stable@vger.kernel.org Bugzilla: > > https://bugs.freedesktop.org/show_bug.cgi?id=50522 Tested-by: Yang > > Guang Signed-off-by: Daniel Vetter > > --- drivers/gpu/drm/i915/intel_ringbuffer.c | > > 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c > > b/drivers/gpu/drm/i915/intel_ringbuffer.c index bf0195a..5b19917 > > 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ > > b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -289,8 +289,6 @@ static > > int init_ring_common(struct intel_ring_buffer *ring) > > I915_WRITE_HEAD(ring, 0); ring->write_tail(ring, 0); > > > > - /* Initialize the ring. */ - I915_WRITE_START(ring, > > obj->gtt_offset); head = I915_READ_HEAD(ring) & HEAD_ADDR; > > > > /* G45 ring initialization fails to reset head to zero */ @@ > > -316,6 +314,10 @@ static int init_ring_common(struct > > intel_ring_buffer *ring) } } > > > > + /* Initialize the ring. This must happen _after_ we have confirmed > > that + * the ring is off (with the above head == 0 check), > > otherwise the hw + * might lose the new ring register values. */ + > > I915_WRITE_START(ring, obj->gtt_offset); I915_WRITE_CTL(ring, > > ((ring->size - PAGE_SIZE) & RING_NR_PAGES) | RING_VALID); -- 1.7.10.4 > > > > _______________________________________________ Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48