[Intel-gfx] [PATCH 2/4] drm/i915: fixup seqno allocation logic for lazy_request
chris at chris-wilson.co.uk
Wed Jan 25 15:17:46 CET 2012
On Wed, 25 Jan 2012 14:03:58 +0100, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> Currently we reserve seqnos only when we emit the request to the ring
> (by bumping dev_priv->next_seqno), but start using it much earlier for
> ring->oustanding_lazy_request. When 2 threads compete for the gpu and
> run on two different rings (e.g. ddx on blitter vs. compositor)
> hilarity ensued, especially when we get constantly interrupted while
> reserving buffers.
> Breakage seems to have been introduced in
> commit 6f392d548658a17600da7faaf8a5df25ee5f01f6
> Author: Chris Wilson <chris at chris-wilson.co.uk>
> Date: Sat Aug 7 11:01:22 2010 +0100
> drm/i915: Use a common seqno for all rings.
> This patch fixes up the seqno reservation logic by moving it into
> i915_gem_next_request_seqno. The ring->add_request functions now
> superflously still return the new seqno through a pointer, that will
> be refactored in the next patch.
> v2: Keep i915_gem_get_seqno (but move it to i915_gem.c) to make it
> clear that we only have one seqno counter for all rings. Suggested by
> Chris Wilson.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45181
> Tested-by: Nicolas Kalkhof nkalkhof()at()web.de
> Signed-Off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> static int
> render_ring_flush(struct intel_ring_buffer *ring,
> u32 invalidate_domains,
> @@ -467,7 +453,7 @@ gen6_add_request(struct intel_ring_buffer *ring,
> mbox1_reg = ring->signal_mbox;
> mbox2_reg = ring->signal_mbox;
> - *seqno = i915_gem_get_seqno(ring->dev);
> + *seqno = ring->outstanding_lazy_request;
In discussing this patch with Daniel, I made the mistake of reading that
as i915_gem_get_next_request_seqno() instead of get_seqno(). I'd suggest
the patch makes that change and hide the ugly ring->o_l_r. Then since we
do i915_gem_get_next_request_seqno() both here and in the caller, it
becomes much clearer that we are able to remove it.
Daniel, apologies for the confusion!
Chris Wilson, Intel Open Source Technology Centre
More information about the Intel-gfx