Re: [PATCH 07/15] drm/i915: Defer default hardware context initialisation until first open

From: Dave Gordon <david.s.gordon@intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 07/15] drm/i915: Defer default hardware context initialisation until first open
Date: Fri, 19 Jun 2015 10:42:35 +0100	[thread overview]
Message-ID: <5583E40B.2060307@intel.com> (raw)
In-Reply-To: <20150616093533.GM28462@nuc-i3427.alporthouse.com>

On 16/06/15 10:35, Chris Wilson wrote:
> On Mon, Jun 15, 2015 at 07:36:25PM +0100, Dave Gordon wrote:
>> +static int i915_gem_context_first_open(struct drm_device *dev)
>> +{
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	int ret;
>> +
>> +	/*
>> +	 * We can't enable contexts until all firmware is loaded. This
>> +	 * call shouldn't return -EAGAIN because we pass wait=true, but
>> +	 * it can still fail with code -EIO if the GuC doesn't respond,
>> +	 * or -ENOEXEC if the GuC firmware image is invalid.
>> +	 */
>> +	ret = intel_guc_ucode_load(dev, true);
>> +	WARN_ON(ret == -EAGAIN);
>> +
>> +	/*
>> +	 * If an error occurred and GuC submission has been requested, we can
>> +	 * attempt recovery by disabling GuC submission and reinitialising
>> +	 * the GPU and driver. We then fail this open() anyway, but the next
>> +	 * attempt will find that GuC submission is already disabled, and so
>> +	 * proceed to complete context initialisation in non-GuC mode instead.
>> +	 */
>> +	if (ret && i915.enable_guc_submission) {
>> +		i915_handle_guc_error(dev, ret);
>> +		return ret;
>> +	}
> 
> This is still backwards. What we wanted was for the submission process
> to start up normally and then once the GuC loading succeeds, we then
> start submitting the backlog to the GuC. If the loading fails, we can
> then submit the backlog via execlists. It may be interesting to even
> start userspace before GuC finishes loading.

Absolutely. The latter is what this allows :)

(And its a requirement for Android, as on those platforms the f/w won't
become available until userspace is running).

But we're not going to keep stuff queued up in the rings. It would add
more complexity to manage the backlog and remember that we can accept
calls to add_request but not actually submit them. Also there would be
issue with any code that sent commands to the engines to program
registers via LRIs and then EITHER assumed that they had taken effect OR
waited for completion (because that would block indefinitely).

Instead, we've split hw_init() into early (MMIO) and late (context and
batch) phases, and deferred all of the latter iff we need GuC f/w to be
loaded before batch submission. When the late phase runs, it can submit
batches, and wait for completion if required, without blocking the
entire system as it would if called from driver_load().

It might even make sense as an overall strategy to defer MORE of the
driver initialisation process, so that the critical single-threaded
driver load during system startup does as little as possible.

> So this makes more sense as to why you have the tight integration with
> execlists then. I still don't think that justifies changing gen8 without
> reason.
> -Chris

It's tightly integrated because GuC submission *IS* execlist submission,
only with the GuC doing the actual poking of the ELSP and fielding the
resulting context-switch interrupts.

Deferred initialisation doesn't apply to Gen8, or anything else that
doesn't have and use GuC submission. The delayed-context-initialisation
codepath is only reachable if there is GuC firmware to be loaded.

.Dave.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx