[Intel-gfx] [PATCH 3/4] drm/i915: add workaround to gen7_render_ring_flush
Daniel Vetter
daniel at ffwll.ch
Tue Aug 28 17:17:50 CEST 2012
On Tue, Aug 28, 2012 at 10:10:13AM -0300, Paulo Zanoni wrote:
> 2012/8/28 Daniel Vetter <daniel at ffwll.ch>:
> > On Fri, Aug 17, 2012 at 06:35:43PM -0300, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >>
> >> The combination of this commit + the next one will prevent a lot of
> >> gpu hangs.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni at intel.com>
> >
> > tbh I'm not happy with the justification in the commit message here. If
> > there's anything about this in Bspec, I want a full reference, if this is
> > just due to the simulator, please say so (and maybe paste the complaint
> > fulsim raises). If it's just empirical evidence please say which machines
> > this affects (since afaict it's only confirmed to help on hsw).
>
> It's from Bspec. See the page that describes the PIPE_CONTROL
> register. Look for a piece of text that's very similar to the comment
> I wrote. It affects IVB and newer.
>
> This is a regression. If you're not happy with this solution, you can
> revert all the commits that happened in gen6_render_ring_flush since
> we stopped applying the gen6 workarounds to IVB and newer. This
> regression makes Haswell a *pain* to use: you're typing on the
> terminal and then suddenly everything freezes because there's a GPU
> hang. Then 5 seconds later it happens again. Then 2 minutes later,
> again. Then again.
Ok, after some flaming in private on irc we've resolved our differences.
I've applied the first 2 patches as-is and squashed the later 2 workaround
patches together, with some bikeshedding applied (mostly to cite the bspec
in the commit message, cite the comment that introduced the regression and
also mentioned that this restores hsw stability).
Thanks for tracking this down and writing the patches.
Yours, Daniel
>
> Cheers,
> Paulo
>
> >
> > [Also directed at Ben.]
> >
> > Thanks, Daniel
> >> ---
> >> drivers/gpu/drm/i915/intel_ringbuffer.c | 24 ++++++++++++++++++++++++
> >> 1 file changed, 24 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> index dc5272b..9895a6e 100644
> >> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> >> @@ -263,6 +263,25 @@ gen6_render_ring_flush(struct intel_ring_buffer *ring,
> >> }
> >>
> >> static int
> >> +gen7_render_ring_cs_stall_wa(struct intel_ring_buffer *ring)
> >> +{
> >> + int ret;
> >> +
> >> + ret = intel_ring_begin(ring, 4);
> >> + if (ret)
> >> + return ret;
> >> +
> >> + intel_ring_emit(ring, GFX_OP_PIPE_CONTROL(4));
> >> + intel_ring_emit(ring, PIPE_CONTROL_CS_STALL |
> >> + PIPE_CONTROL_STALL_AT_SCOREBOARD);
> >> + intel_ring_emit(ring, 0);
> >> + intel_ring_emit(ring, 0);
> >> + intel_ring_advance(ring);
> >> +
> >> + return 0;
> >> +}
> >> +
> >> +static int
> >> gen7_render_ring_flush(struct intel_ring_buffer *ring,
> >> u32 invalidate_domains, u32 flush_domains)
> >> {
> >> @@ -295,6 +314,11 @@ gen7_render_ring_flush(struct intel_ring_buffer *ring,
> >> * TLB invalidate requires a post-sync write.
> >> */
> >> flags |= PIPE_CONTROL_QW_WRITE;
> >> +
> >> + /* Workaround: we must issue a pipe_control with CS-stall bit
> >> + * set before a pipe_control command that has the state cache
> >> + * invalidate bit set. */
> >> + gen7_render_ring_cs_stall_wa(ring);
> >> }
> >>
> >> ret = intel_ring_begin(ring, 4);
> >> --
> >> 1.7.11.2
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx at lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
> > --
> > Daniel Vetter
> > Mail: daniel at ffwll.ch
> > Mobile: +41 (0)79 365 57 48
>
>
>
> --
> Paulo Zanoni
--
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48
More information about the Intel-gfx
mailing list