* [PATCH] drm/i915: correctly order the ring init sequence
@ 2012-08-07 7:54 Daniel Vetter
2012-08-07 12:32 ` Jani Nikula
0 siblings, 1 reply; 4+ messages in thread
From: Daniel Vetter @ 2012-08-07 7:54 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, stable
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.
Cc: stable@vger.kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
Tested-by: Yang Guang <guang.a.yang@intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: correctly order the ring init sequence
2012-08-07 7:54 [PATCH] drm/i915: correctly order the ring init sequence Daniel Vetter
@ 2012-08-07 12:32 ` Jani Nikula
2012-08-07 12:31 ` Daniel Vetter
2012-08-07 12:36 ` Jani Nikula
0 siblings, 2 replies; 4+ messages in thread
From: Jani Nikula @ 2012-08-07 12:32 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, stable
On Tue, 07 Aug 2012, Daniel Vetter <daniel.vetter@ffwll.ch> 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 <jani.nikula@intel.com>
>
> Cc: stable@vger.kernel.org
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
> Tested-by: Yang Guang <guang.a.yang@intel.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
> 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: correctly order the ring init sequence
2012-08-07 12:32 ` Jani Nikula
@ 2012-08-07 12:31 ` Daniel Vetter
2012-08-07 12:36 ` Jani Nikula
1 sibling, 0 replies; 4+ messages in thread
From: Daniel Vetter @ 2012-08-07 12:31 UTC (permalink / raw)
To: Jani Nikula; +Cc: Daniel Vetter, Intel Graphics Development, stable
On Tue, Aug 07, 2012 at 03:32:42PM +0300, Jani Nikula wrote:
> On Tue, 07 Aug 2012, Daniel Vetter <daniel.vetter@ffwll.ch> 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 <jani.nikula@intel.com>
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 <guang.a.yang@intel.com> Signed-off-by: Daniel Vetter
> > <daniel.vetter@ffwll.ch> --- 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
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: correctly order the ring init sequence
2012-08-07 12:32 ` Jani Nikula
2012-08-07 12:31 ` Daniel Vetter
@ 2012-08-07 12:36 ` Jani Nikula
1 sibling, 0 replies; 4+ messages in thread
From: Jani Nikula @ 2012-08-07 12:36 UTC (permalink / raw)
To: Intel Graphics Development; +Cc: Daniel Vetter, stable
On Tue, 07 Aug 2012, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 07 Aug 2012, Daniel Vetter <daniel.vetter@ffwll.ch> 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,
For the record,
15:14 danvet "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."
>
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
>
>
>>
>> Cc: stable@vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
>> Tested-by: Yang Guang <guang.a.yang@intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>> ---
>> 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
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2012-08-07 12:32 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-08-07 7:54 [PATCH] drm/i915: correctly order the ring init sequence Daniel Vetter
2012-08-07 12:32 ` Jani Nikula
2012-08-07 12:31 ` Daniel Vetter
2012-08-07 12:36 ` Jani Nikula
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox