Re: [RFC 09/21] drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno

From: Daniel Vetter <daniel@ffwll.ch>
To: John Harrison <John.C.Harrison@Intel.com>
Cc: Intel-GFX@Lists.FreeDesktop.Org
Subject: Re: [RFC 09/21] drm/i915: Make 'i915_gem_check_olr' actually check by request not seqno
Date: Mon, 3 Nov 2014 11:51:36 +0100	[thread overview]
Message-ID: <20141103105136.GH26941@phenom.ffwll.local> (raw)
In-Reply-To: <544FA1D0.8050307@Intel.com>

On Tue, Oct 28, 2014 at 02:01:52PM +0000, John Harrison wrote:
> On 19/10/2014 13:55, Daniel Vetter wrote:
> >On Mon, Oct 06, 2014 at 03:15:13PM +0100, John.C.Harrison@Intel.com wrote:
> >>From: John Harrison <John.C.Harrison@Intel.com>
> >To thin commit message.
> >
> >Also I wonder whethere we should track the olr state more explicitly in
> >the request structure instead of jumping through all these hoops. And
> >explicit olr state for a request might also help to clarify control flow
> >and checks in a bunch of places.
> 
> What would be nicer would be to get rid of the OLR completely. If code wants
> to write to the ring then it should request a request structure at the
> start, pass that request in to all the ring writing code and submit the same
> request structure at the end. The whole idea of some random request floating
> around in the ether scooping up any ring accesses that happen to occur with
> no idea about ownership, sequence, etc. just seems highly dodgy. Indeed it
> definitely causes issues with the scheduler with things like page flips
> being mixed up in completely unrelated batch buffer submissions! I believe
> that is one of the things Chris did in his re-write the universe patch.

Oh OLR is highly dodgy ;-)

It is unfortunately a bit of work to fix things up, especially without a
rewrite-the-universe patch. Perhaps:
1. Explicitly preallocate the request, still store it at the same place.
2. Pass the explicit request to add_request, add any missing add_requests
call where we currently relied on the olr to scoop up register writes.
3. Shovel the request pointer from the engine to the ring (since that's
where we need it it for execlist), that means we need to allocate the
request for a specific engine. Steps 1&3 might be better if switched,
dunno.
4. Sprinkle asserts over all ring functions to make sure the request is
there already.
5. Throw away olr logic.
6. Optional, refactoring over the next few years: Use requests as the
primary cmd submission object everywhere.

Comments?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx