* [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START @ 2014-08-07 14:11 Chris Wilson 2014-08-07 14:17 ` Chris Wilson 2014-08-07 14:26 ` Ville Syrjälä 0 siblings, 2 replies; 6+ messages in thread From: Chris Wilson @ 2014-08-07 14:11 UTC (permalink / raw) To: intel-gfx Ville found an old w/a documented for g4x that suggested that we need to reset the HEAD after writing START. This is a useful fixup for some of the g4x ring initialisation woes, but as usual, not all. References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 45e3ec927051..0a37ac0d9c0d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs *ring) * also enforces ordering), otherwise the hw might lose the new ring * register values. */ I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); + + /* WaClearRingBufHeadRegAtInit:ctg,elk */ + if (I915_READ_HEAD(ring)) { + DRM_DEBUG("%s initialization failed [head=%08x], fudging\n", + ring->name, I915_READ_HEAD(ring)); + I915_WRITE_HEAD(ring, 0); + (void)I915_READ_HEAD(ring); + } + I915_WRITE_CTL(ring, ((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES) | RING_VALID); -- 2.1.0.rc1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START 2014-08-07 14:11 [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START Chris Wilson @ 2014-08-07 14:17 ` Chris Wilson 2014-08-07 14:26 ` Ville Syrjälä 1 sibling, 0 replies; 6+ messages in thread From: Chris Wilson @ 2014-08-07 14:17 UTC (permalink / raw) To: intel-gfx On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote: > Ville found an old w/a documented for g4x that suggested that we need to > reset the HEAD after writing START. This is a useful fixup for some of > the g4x ring initialisation woes, but as usual, not all. Note that a slightly different form of this patch has been used by the reporters of bug76584 without much sucess. (My gm45 works with the old patch - it prints out the debugging information that the fudge was applied.)) -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START 2014-08-07 14:11 [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START Chris Wilson 2014-08-07 14:17 ` Chris Wilson @ 2014-08-07 14:26 ` Ville Syrjälä 2014-08-07 14:35 ` Chris Wilson 2014-08-07 14:39 ` Chris Wilson 1 sibling, 2 replies; 6+ messages in thread From: Ville Syrjälä @ 2014-08-07 14:26 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote: > Ville found an old w/a documented for g4x that suggested that we need to > reset the HEAD after writing START. This is a useful fixup for some of > the g4x ring initialisation woes, but as usual, not all. > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++++ > 1 file changed, 9 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 45e3ec927051..0a37ac0d9c0d 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs *ring) > * also enforces ordering), otherwise the hw might lose the new ring > * register values. */ > I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); > + > + /* WaClearRingBufHeadRegAtInit:ctg,elk */ > + if (I915_READ_HEAD(ring)) { > + DRM_DEBUG("%s initialization failed [head=%08x], fudging\n", > + ring->name, I915_READ_HEAD(ring)); > + I915_WRITE_HEAD(ring, 0); > + (void)I915_READ_HEAD(ring); > + } Just a clarification that I don't know if this is the right w/a name. There's no description in the database, but it would seem to fit the symptoms of HEAD going wild on its own. The spec says this: RING_START: "Writing this register also causes the Head Offset to be reset to zero, and the Wrap Count to be reset to zero." RING_HEAD: "Ring Buffer Head Offsets must be properly programmed before ring is enable" So my theory is that the RING_START write actually corruptsa RING_HEAD instead of clearing it on g4x, and thus we need to rewrite it once more after writing RING_START, but before we enable the ring. As far as the patch goes, I think I'd just do the write unconditionally in case the corruption is somehow magical and doesn't appear until the ring is started. After all it can't do any harm as the RING_START write is already supposed to clear it. Not sure if there should be some posting reads between the steps just for a bit of extra paranoia. > + > I915_WRITE_CTL(ring, > ((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES) > | RING_VALID); > -- > 2.1.0.rc1 -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START 2014-08-07 14:26 ` Ville Syrjälä @ 2014-08-07 14:35 ` Chris Wilson 2014-08-07 14:39 ` Chris Wilson 1 sibling, 0 replies; 6+ messages in thread From: Chris Wilson @ 2014-08-07 14:35 UTC (permalink / raw) To: Ville Syrjälä; +Cc: intel-gfx On Thu, Aug 07, 2014 at 05:26:20PM +0300, Ville Syrjälä wrote: > On Thu, Aug 07, 2014 at 03:11:17PM +0100, Chris Wilson wrote: > > Ville found an old w/a documented for g4x that suggested that we need to > > reset the HEAD after writing START. This is a useful fixup for some of > > the g4x ring initialisation woes, but as usual, not all. > > > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/intel_ringbuffer.c | 9 +++++++++ > > 1 file changed, 9 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > > index 45e3ec927051..0a37ac0d9c0d 100644 > > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > > @@ -537,6 +537,15 @@ static int init_ring_common(struct intel_engine_cs *ring) > > * also enforces ordering), otherwise the hw might lose the new ring > > * register values. */ > > I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); > > + > > + /* WaClearRingBufHeadRegAtInit:ctg,elk */ > > + if (I915_READ_HEAD(ring)) { > > + DRM_DEBUG("%s initialization failed [head=%08x], fudging\n", > > + ring->name, I915_READ_HEAD(ring)); > > + I915_WRITE_HEAD(ring, 0); > > + (void)I915_READ_HEAD(ring); > > + } > > Just a clarification that I don't know if this is the right w/a name. > There's no description in the database, but it would seem to fit the > symptoms of HEAD going wild on its own. > > The spec says this: > > RING_START: > "Writing this register also causes the Head Offset to be reset to zero, > and the Wrap Count to be reset to zero." > > RING_HEAD: > "Ring Buffer Head Offsets must be properly programmed before ring is > enable" > > So my theory is that the RING_START write actually corruptsa RING_HEAD > instead of clearing it on g4x, and thus we need to rewrite it once more > after writing RING_START, but before we enable the ring. > > As far as the patch goes, I think I'd just do the write unconditionally > in case the corruption is somehow magical and doesn't appear until the > ring is started. After all it can't do any harm as the RING_START write > is already supposed to clear it. Not sure if there should be some posting > reads between the steps just for a bit of extra paranoia. The read there is mostly for debugging (it's nice to know when w/a are being applied after all), but it also helps with ordering. As a compromise: /* WaClearRingBufHeadRegAtInit:ctg,elk */ if (I95_READ_HEAD()) DRM_DEBUG(); I915_WRITE_HEAD(ring, 0); -Chris -- Chris Wilson, Intel Open Source Technology Centre ^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START 2014-08-07 14:26 ` Ville Syrjälä 2014-08-07 14:35 ` Chris Wilson @ 2014-08-07 14:39 ` Chris Wilson 2014-08-07 16:05 ` Ville Syrjälä 1 sibling, 1 reply; 6+ messages in thread From: Chris Wilson @ 2014-08-07 14:39 UTC (permalink / raw) To: intel-gfx Ville found an old w/a documented for g4x that suggested that we need to reset the HEAD after writing START. This is a useful fixup for some of the g4x ring initialisation woes, but as usual, not all. v2: Do the rewrite unconditionally anyway References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 45e3ec927051..26ec25afc02a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -537,6 +537,14 @@ static int init_ring_common(struct intel_engine_cs *ring) * also enforces ordering), otherwise the hw might lose the new ring * register values. */ I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); + + /* WaClearRingBufHeadRegAtInit:ctg,elk */ + if (I915_READ_HEAD(ring)) + DRM_DEBUG("%s initialization failed [head=%08x], fudging\n", + ring->name, I915_READ_HEAD(ring)); + I915_WRITE_HEAD(ring, 0); + (void)I915_READ_HEAD(ring); + I915_WRITE_CTL(ring, ((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES) | RING_VALID); -- 2.1.0.rc1 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START 2014-08-07 14:39 ` Chris Wilson @ 2014-08-07 16:05 ` Ville Syrjälä 0 siblings, 0 replies; 6+ messages in thread From: Ville Syrjälä @ 2014-08-07 16:05 UTC (permalink / raw) To: Chris Wilson; +Cc: intel-gfx On Thu, Aug 07, 2014 at 03:39:54PM +0100, Chris Wilson wrote: > Ville found an old w/a documented for g4x that suggested that we need to > reset the HEAD after writing START. This is a useful fixup for some of > the g4x ring initialisation woes, but as usual, not all. > > v2: Do the rewrite unconditionally anyway > > References: https://bugs.freedesktop.org/show_bug.cgi?id=76554 > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Yeah looks sensible to me. So based on my gut feeling just from the seeing the w/a name this is: Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 45e3ec927051..26ec25afc02a 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -537,6 +537,14 @@ static int init_ring_common(struct intel_engine_cs *ring) > * also enforces ordering), otherwise the hw might lose the new ring > * register values. */ > I915_WRITE_START(ring, i915_gem_obj_ggtt_offset(obj)); > + > + /* WaClearRingBufHeadRegAtInit:ctg,elk */ > + if (I915_READ_HEAD(ring)) > + DRM_DEBUG("%s initialization failed [head=%08x], fudging\n", > + ring->name, I915_READ_HEAD(ring)); > + I915_WRITE_HEAD(ring, 0); > + (void)I915_READ_HEAD(ring); > + > I915_WRITE_CTL(ring, > ((ringbuf->size - PAGE_SIZE) & RING_NR_PAGES) > | RING_VALID); > -- > 2.1.0.rc1 -- Ville Syrjälä Intel OTC ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2014-08-07 16:06 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-08-07 14:11 [PATCH] drm/i915: Reset the HEAD pointer for the ring after writing START Chris Wilson 2014-08-07 14:17 ` Chris Wilson 2014-08-07 14:26 ` Ville Syrjälä 2014-08-07 14:35 ` Chris Wilson 2014-08-07 14:39 ` Chris Wilson 2014-08-07 16:05 ` Ville Syrjälä
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.