[Intel-gfx] Losing write domains and breaking flushing list

[Keith Packard]

On Tue, 2008-10-28 at 11:07 -0700, Eric Anholt wrote:

> NAK for this reversion of a bugfix.  Suppose you have a batchbuffer that
> sees the following state:
> 
> obj->domains starts at (RENDER, RENDER)
> reloc1 = (0, TEXTURE)		flush_domains = RENDER
> 				obj->domains = (0, TEXTURE|RENDER)
> (batchbuffer has an MI_FLUSH here)
> reloc2 = (RENDER, RENDER)	obj->domains = (RENDER, RENDER)

Yeah, oops. Thanks for reading this patch closely. That change was
unintentionally stuck in this patch.

> obj->domains should be (RENDER, RENDER) at the end of this batchbuffer's
> execution.  However, since you removed this add_request, the
> flush_domains = RENDER that now gets add_requested after batchbuffer
> execution will cause the state at the end to be (0, RENDER).

We should figure out how to avoid emitting two sequence numbers for each
request then; it's just noise in the ring. I know it means losing some
granularity in waiting for flush operations to complete, but I don't
think I care.

> For flush_domains & ~(I915_GEM_DOMAIN_CPU | I915_GEM_DOMAIN_GTT) we
> already did bo_wait_rendering inside i915_gem_object_set_pending_domain,
> right? 

Only for GPU->CPU transitions. For pure GPU->GPU transitions, we aren't
calling drm_bo_wait_rendering from set_pendign_domain.

However, I think you've found an inconsistency here that we should
resolve somehow. I think we want to pend all of the
drm_bo_wait_rendering calls until we get to set_current_domain, right?
That would make them happen after any GPU flush operations had been
pended, and so we'd really just wait for a single flush to complete that
would cover all of the GPU->CPU transitioning buffers.

On second thought, we only ever handle a single buffer in the GPU->CPU
case anyway, so it probably doesn't matter, aside from making things
look a little cleaner.

>  It seems like this new block at the end either adds an
> irrelevant request to the command stream, or it returns too early since
> the job of getting the domains flushed and ready for the CPU isn't done
> yet.

The CPU case is handled in set_pending_domain. But, in that case we will
be adding another flush, which seems wasteful. Again, doing the
drm_bo_wait_rendering in set_current_domain would let us piggy-back all
of the synchronization on a single flush operation quite easily.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.freedesktop.org/archives/intel-gfx/attachments/20081028/bc05588f/attachment.sig>