On Tue, 07 Aug 2012, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Tue, 07 Aug 2012, Daniel Vetter <daniel.vetter at 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 at intel.com>
>
>
>>
>> Cc: stable at vger.kernel.org
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=50522
>> Tested-by: Yang Guang <guang.a.yang at intel.com>
>> Signed-off-by: Daniel Vetter <daniel.vetter at 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 at lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
