On 10/12/14 10:42, Daniel Vetter wrote:
> On Tue, Dec 09, 2014 at 12:59:08PM +0000, John.C.Harrison@xxxxxxxxx wrote:
>> From: Dave Gordon <david.s.gordon@xxxxxxxxx>
>>
>> There is a workaround for a hardware bug when reading the seqno from the status
>> page. The bug does not exist on VLV however, the workaround was still being
>> applied.
>
> Given how much trouble the missed seqno fun cause us I'd like more
> justification. What kind of (stress)-testing has been done here? And
> you're sure you've never seen the little dmesg notice that the kernel
> switched to polling?
> -Daniel
This was necessary during VLV scheduler/preemption work, where we
stressed the command streamer and IRQ path probably more than any of the
usual tests, since we were looking for out-of-sequence anomalies.
The comments in gen6_ring_get_seqno() say it's to fix a bug on ivb/snb,
and nothing in the BSpec suggests it's needed on later chips.
I think the problem was really in the fact that the workaround
implemented in gen6_ring_get_seqno() was reading a nonshadowed register,
therefore triggering forcewake activity, which was pretty buggy; so we
took out the gen6-specific code before finding that there were still
other problems with forcewake.
[Aside]
The original intention appears to have been to generate a single I/O
READ cycle, forcing all pending inbound DMA so be flushed to memory, so
that the subsequent memory read would see the latest value. But now, it
doesn't just generate a single cycle but a whole flurry of writes and
polling reads, because reading ACTHD requires forcewake (unless that was
wrong, and has now been fixed). So should gen6_ring_get_seqno() instead
read a register that doesn't require forcewake? I wouldn't object to
using it in perpetuity if it really only read one register!
[/aside]
.Dave.
>> Change-Id: Ic781fdb31e1f794ce1fa8a6d0d5ee379756c5db6
>> Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
>> ---
>> drivers/gpu/drm/i915/intel_ringbuffer.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> index 3887f1a..f990ce4 100644
>> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
>> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
>> @@ -2367,7 +2367,10 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>> ring->irq_get = gen6_ring_get_irq;
>> ring->irq_put = gen6_ring_put_irq;
>> ring->irq_enable_mask = GT_RENDER_USER_INTERRUPT;
>> - ring->get_seqno = gen6_ring_get_seqno;
>> + if (IS_VALLEYVIEW(dev))
>> + ring->get_seqno = ring_get_seqno;
>> + else
>> + ring->get_seqno = gen6_ring_get_seqno;
>> ring->set_seqno = ring_set_seqno;
>> if (i915_semaphore_is_enabled(dev)) {
>> ring->semaphore.sync_to = gen6_ring_sync;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
