| Message ID | 1354626725-8521-4-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>: > If wrap just happened we need to prevent emitting waits for > pre wrap values. Detect this and emit no-ops instead. > > v2: Use olr > seqno to detect wrap instead of *seqno == 0 > as suggested by Chris Wilson. This commit introduces a bug on Haswell. Now when I'm typing my password on GDM the screen keeps doing wrong rendering. It "blinks blue". After logging in I don't see more problems. > > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/intel_ringbuffer.c | 21 ++++++++++++++++----- > 1 file changed, 16 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c > index 36e1e13a..2305af5 100644 > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c > @@ -626,11 +626,22 @@ gen6_ring_sync(struct intel_ring_buffer *waiter, > if (ret) > return ret; > > - intel_ring_emit(waiter, > - dw1 | signaller->semaphore_register[waiter->id]); > - intel_ring_emit(waiter, seqno); > - intel_ring_emit(waiter, 0); > - intel_ring_emit(waiter, MI_NOOP); > + BUG_ON(!waiter->outstanding_lazy_request); > + > + /* If seqno wrap happened, omit the wait with no-ops */ > + if (likely(waiter->outstanding_lazy_request > seqno)) { > + intel_ring_emit(waiter, > + dw1 | > + signaller->semaphore_register[waiter->id]); > + intel_ring_emit(waiter, seqno); > + intel_ring_emit(waiter, 0); > + intel_ring_emit(waiter, MI_NOOP); > + } else { > + intel_ring_emit(waiter, MI_NOOP); > + intel_ring_emit(waiter, MI_NOOP); > + intel_ring_emit(waiter, MI_NOOP); > + intel_ring_emit(waiter, MI_NOOP); > + } > intel_ring_advance(waiter); > > return 0; > -- > 1.7.9.5 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>: >> If wrap just happened we need to prevent emitting waits for >> pre wrap values. Detect this and emit no-ops instead. >> >> v2: Use olr > seqno to detect wrap instead of *seqno == 0 >> as suggested by Chris Wilson. > > This commit introduces a bug on Haswell. Now when I'm typing my > password on GDM the screen keeps doing wrong rendering. It "blinks > blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems. Just now I've taken out "drm/i915: Set initial seqno value close to wrap boundary" since QA complained that it regresses things. Does that help for you, too? -Daniel
Hi 2012/12/6 Daniel Vetter <daniel@ffwll.ch>: > On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote: >> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>: >>> If wrap just happened we need to prevent emitting waits for >>> pre wrap values. Detect this and emit no-ops instead. >>> >>> v2: Use olr > seqno to detect wrap instead of *seqno == 0 >>> as suggested by Chris Wilson. >> >> This commit introduces a bug on Haswell. Now when I'm typing my >> password on GDM the screen keeps doing wrong rendering. It "blinks >> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems. > > Just now I've taken out "drm/i915: Set initial seqno value close to > wrap boundary" since QA complained that it regresses things. Does that > help for you, too? It helps: besides the "wrong rendering at GDM screen" I was also getting GPU hangs (when starting X, when running dmesg, when alt+tab, etc), and it seems with today's dinq I don't get the gpu hangs anymore. I still get the "wrong rendering" problem and it goes away if we revert the "Don't emit semaphore wait if wrap happened". > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch
On Thu, Dec 6, 2012 at 12:41 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > 2012/12/6 Daniel Vetter <daniel@ffwll.ch>: >> On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote: >>> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>: >>>> If wrap just happened we need to prevent emitting waits for >>>> pre wrap values. Detect this and emit no-ops instead. >>>> >>>> v2: Use olr > seqno to detect wrap instead of *seqno == 0 >>>> as suggested by Chris Wilson. >>> >>> This commit introduces a bug on Haswell. Now when I'm typing my >>> password on GDM the screen keeps doing wrong rendering. It "blinks >>> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems. >> >> Just now I've taken out "drm/i915: Set initial seqno value close to >> wrap boundary" since QA complained that it regresses things. Does that >> help for you, too? > > It helps: besides the "wrong rendering at GDM screen" I was also > getting GPU hangs (when starting X, when running dmesg, when alt+tab, > etc), and it seems with today's dinq I don't get the gpu hangs > anymore. I still get the "wrong rendering" problem and it goes away if > we revert the "Don't emit semaphore wait if wrap happened". Ok, looks like we have still some fish left to fry here. I've backed out the 2nd patch, too. And I guess we need some more tests in i-g-t to check semaphore correctness, we seem to have some serious gaps ... -Daniel
On Thu, 6 Dec 2012 13:14:09 +0100, Daniel Vetter <daniel@ffwll.ch> wrote: > On Thu, Dec 6, 2012 at 12:41 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > > 2012/12/6 Daniel Vetter <daniel@ffwll.ch>: > >> On Wed, Dec 5, 2012 at 9:44 PM, Paulo Zanoni <przanoni@gmail.com> wrote: > >>> 2012/12/4 Mika Kuoppala <mika.kuoppala@linux.intel.com>: > >>>> If wrap just happened we need to prevent emitting waits for > >>>> pre wrap values. Detect this and emit no-ops instead. > >>>> > >>>> v2: Use olr > seqno to detect wrap instead of *seqno == 0 > >>>> as suggested by Chris Wilson. > >>> > >>> This commit introduces a bug on Haswell. Now when I'm typing my > >>> password on GDM the screen keeps doing wrong rendering. It "blinks > >>> blue". After logging in I don't see more prodrm/i915: Set initial seqno value close to wrap boundaryblems. > >> > >> Just now I've taken out "drm/i915: Set initial seqno value close to > >> wrap boundary" since QA complained that it regresses things. Does that > >> help for you, too? > > > > It helps: besides the "wrong rendering at GDM screen" I was also > > getting GPU hangs (when starting X, when running dmesg, when alt+tab, > > etc), and it seems with today's dinq I don't get the gpu hangs > > anymore. I still get the "wrong rendering" problem and it goes away if > > we revert the "Don't emit semaphore wait if wrap happened". > > Ok, looks like we have still some fish left to fry here. I've backed > out the 2nd patch, too. And I guess we need some more tests in i-g-t > to check semaphore correctness, we seem to have some serious gaps ... I wouldn't back that out too quickly as it seems that Paulo has some debugging to do first. A few WARNs would be a good start... -Chris
diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 36e1e13a..2305af5 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -626,11 +626,22 @@ gen6_ring_sync(struct intel_ring_buffer *waiter, if (ret) return ret; - intel_ring_emit(waiter, - dw1 | signaller->semaphore_register[waiter->id]); - intel_ring_emit(waiter, seqno); - intel_ring_emit(waiter, 0); - intel_ring_emit(waiter, MI_NOOP); + BUG_ON(!waiter->outstanding_lazy_request); + + /* If seqno wrap happened, omit the wait with no-ops */ + if (likely(waiter->outstanding_lazy_request > seqno)) { + intel_ring_emit(waiter, + dw1 | + signaller->semaphore_register[waiter->id]); + intel_ring_emit(waiter, seqno); + intel_ring_emit(waiter, 0); + intel_ring_emit(waiter, MI_NOOP); + } else { + intel_ring_emit(waiter, MI_NOOP); + intel_ring_emit(waiter, MI_NOOP); + intel_ring_emit(waiter, MI_NOOP); + intel_ring_emit(waiter, MI_NOOP); + } intel_ring_advance(waiter); return 0;
If wrap just happened we need to prevent emitting waits for pre wrap values. Detect this and emit no-ops instead. v2: Use olr > seqno to detect wrap instead of *seqno == 0 as suggested by Chris Wilson. Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/intel_ringbuffer.c | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-)