[Intel-gfx] [PATCH 12/18] drm/i915: Unify request submission

[Dave Gordon]

On 27/07/16 19:09, Chris Wilson wrote:
> On Wed, Jul 27, 2016 at 06:51:35PM +0100, Dave Gordon wrote:
>>>> @@ -1006,6 +1005,10 @@ int i915_guc_submission_enable(struct drm_i915_private *dev_priv)
>>>> 	host2guc_sample_forcewake(guc, client);
>>>> 	guc_init_doorbell_hw(guc);
>>>>
>>>> +	/* Take over from manual control of ELSP (execlists) */
>>>> +	for_each_engine(engine, dev_priv)
>>>> +		engine->submit_request = i915_guc_submit;
>>
>> This doesn't get undone in i915_guc_submission_disable().
>> That will prevent the runtime fallback from working.
>
> I honestly wasn't sure if that was supported. (runtime enabling would be
> nice...)

Any time the GuC (re)loading process fails, it will revert (forever) to 
execlists mode. At present there's no way to switch back to GuC mode 
thereafter. Of course, we don't actually *expect* it to fail on a 
reload, but we did observe this in action a while back, before we 
discovered why reload on resume didn't always work. That's fixed now, 
but the fallback is still there just to make sure that any undiscovered 
issues around GuC reload don't leave you with a blank screen and an 
unusable machine.

> Would calling a hypothetical intel_execlists_submission_enable() be ok?
> -Chris

Something like that, I guess, although since GuC submission mode is 
treated as execlist mode for almost all purposes then _enable() and its 
presumed complement _disable() probably aren't the right terms (we don't 
want to suggest that enabling GuC mode involves disabling execlist 
mode). Perhaps _activate() for a function which simply (re)installs the 
vfunc pointers, with no corresponding _deactivate() required, as 
activating a new mode implicitly deactivates the old mode (i.e. makes it 
unreachable). That little loop can then be moved into its own trivial 
guc_submission_activate() function for clarity; and the GuC code would 
call intel_execlists_submission_activate() as part of reverting from GuC 
to execlists (at the moment it relies on just clearing 
enable_guc_submission, but presumably you'd like as few runtime tests of 
that as possible).

.Dave.