[Intel-gfx] [PATCH 25/55] drm/i915: Update i915_gem_object_sync() to take a request structure

[Chris Wilson]

On Mon, Jun 22, 2015 at 10:03:20PM +0200, Daniel Vetter wrote:
> On Thu, Jun 18, 2015 at 05:16:15PM +0100, John Harrison wrote:
> > No, that is the whole point. If you have a non-null '*to_req' then 'to' must
> > be non-null (and specifically must be the ring that '*to_req' is
> > referencing). However, it is valid to have a non-null 'to' and a null
> > '*to_req'.  In the case of MMIO flips, the page flip itself does not require
> > a request as it does not go through the ring. However, it still passes in
> > 'i915_gem_request_get_ring(obj->last_write_req)' as the ring to synchronise
> > to. Thus it is potentially passing in a valid to pointer but without wanting
> > to pre-allocate a request object. If the synchronisation requires writing a
> > semaphore to the ring then a request will be created internally and passed
> > back out for the page flip code to submit (and to re-use in the case of
> > non-MMIO flips). But if the synchronisation is a no-op then no request ever
> > gets created or submitting and nothing touches the ring at all.

Your API is very badly designed.
 
> We use mmio flips by default if there's any ring switching going on, which
> means except when a user sets a silly debug module option this will never
> happend. Which means it's not too pretty to carry this complication around
> for no real use at all. Otoh the flip code is in a massive churn because
> of atomic, so not much point in cleaning that out if it'll all disapear
> anyway. I'll smash a patch on top to note this TODO.

It's only a complication of bad design, again.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre