diff mbox

[RFC,07/44] drm/i915: Disable 'get seqno' workaround for VLV

Message ID 1403803475-16337-8-git-send-email-John.C.Harrison@Intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

John Harrison June 26, 2014, 5:23 p.m. UTC
From: John Harrison <John.C.Harrison@Intel.com>

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.
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |    5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Jesse Barnes July 2, 2014, 5:51 p.m. UTC | #1
On Thu, 26 Jun 2014 18:23:58 +0100
John.C.Harrison@Intel.com wrote:

> From: John Harrison <John.C.Harrison@Intel.com>
> 
> 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.
> ---
>  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 279488a..bad5db0 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -1960,7 +1960,10 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
>  			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;
>  		ring->semaphore.sync_to = gen6_ring_sync;
>  		ring->semaphore.signal = gen6_signal;

Assuming this has been well tested:
Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>
Daniel Vetter July 7, 2014, 6:56 p.m. UTC | #2
On Wed, Jul 02, 2014 at 10:51:23AM -0700, Jesse Barnes wrote:
> On Thu, 26 Jun 2014 18:23:58 +0100
> John.C.Harrison@Intel.com wrote:
> 
> > From: John Harrison <John.C.Harrison@Intel.com>
> > 
> > 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.
> > ---
> >  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 279488a..bad5db0 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -1960,7 +1960,10 @@ int intel_init_render_ring_buffer(struct drm_device *dev)
> >  			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;
> >  		ring->semaphore.sync_to = gen6_ring_sync;
> >  		ring->semaphore.signal = gen6_signal;
> 
> Assuming this has been well tested:
> Reviewed-by: Jesse Barnes <jbarnes@virtuousgeek.org>

I have my doubts ... the seqno race is fairly hard to reproduce really and
needs some serious beating. Also highly timing dependent.

My best guess is that Oscar's irq handling race fixes fixed the underlying
bug on gen6+, so I think we should instead dare to rip out this w/a
completely and see what happens. Doing this on gen6+ will at least give us
serious amounts of test coverage.
-Daniel
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 279488a..bad5db0 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1960,7 +1960,10 @@  int intel_init_render_ring_buffer(struct drm_device *dev)
 			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;
 		ring->semaphore.sync_to = gen6_ring_sync;
 		ring->semaphore.signal = gen6_signal;