[Intel-gfx] [PATCH] drm/i915: Asynchronously initialise the GPU state

[Chris Wilson]

On Wed, Jul 01, 2015 at 04:07:08PM +0200, Daniel Vetter wrote:
> On Wed, Jul 01, 2015 at 02:17:28PM +0100, Chris Wilson wrote:
> > On Wed, Jul 01, 2015 at 03:07:18PM +0200, Daniel Vetter wrote:
> > > On Wed, Jul 01, 2015 at 10:27:21AM +0100, Chris Wilson wrote:
> > > > Dave Gordon made the good suggestion that once the ringbuffers were
> > > > setup, the actual queuing of commands to program the initial GPU state
> > > > could be deferred. Since that initial state contains instructions for
> > > > setting up the first power context, we want to execute that as earlier
> > > > as possible, preferrably in the background to userspace. Then when
> > > > userspace does wake up, the first time it opens the device we just need
> > > > to flush the work to be sure that our commands are queued before any of
> > > > userspace's. (Hooking into the device open should mean we have to check
> > > > less often than say hooking into execbuffer.)
> > > > 
> > > > Suggested-by: Dave Gordon <david.s.gordon at intel.com>
> > > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > > Cc: Dave Gordon <david.s.gordon at intel.com>
> > > 
> > > Just before this gets a bit out of hand with various patches floating
> > > around ... I really meant it when I said that we should have a proper
> > > design discussion about this in Jesse's meeting first.
> > 
> > What more is there to design? Asynchronously loading the submission port
> > is orthogonal to the task of queuing requests for it, and need not block
> > request construction (be it kernel or userspace). Dave just identified
> > some work that we didn't need to do during module load. I don't think he
> > would propose using it for loading guc firmware, that would just be
> > silly...
> 
> set_wedged in your patch doesn't have the wakeup to kick waiters.

True. But is has to be impossible for a waiter to exist at this point,
or else the entire async GPU init is broken. These commands have to be
the first requests we send to the GPU. Everything else must wait before
it is allowed to start queuing.

> And
> maybe we want to be somewhat more synchronous with with init fail than gpu
> hangs, for userspace to make better decisions.

The init is still synchronous with userspace using the device, just (and
this is no change) the only communication with userspace that GEM
initialisation failed is the wedged GPU.

> Also we still have that
> issue that sometimes an -EIO escapes into modeset code.

But there are no new wait requests running conncurrent with GEM init,
so this patch doesn't alter that.

> And yes this is
> mean to provide the async init for the request firmware.

It is an inappropriate juncture for async request firmware. I can keep
repeating that enabling the submission ports is orthogonal to setting up
the CS engines and allowing requests to be queued, because it is...

There is no need to modify the higher levels for async GuC
initialisation. The serialisation there is when to start feeding requests
into the submission port. At the moment we do that immediately when it 
is idle - but the GuC is not idle until it loaded, as soon as it is
loaded it can simply feed in the first set of requests and start on
its merry way. This also allows GuC failure also always to transparently
fallback to execlists.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre