Re: [PATCH 11/13 v4] drm/i915: Integrate GuC-based command submission

Dave Gordon
2015-07-28T13:59:19Z

On 27/07/15 16:57, O'Rourke, Tom wrote:
> On Thu, Jul 09, 2015 at 07:29:12PM +0100, Dave Gordon wrote:
>> From: Alex Dai <yu.dai@intel.com>
>>
>> GuC-based submission is mostly the same as execlist mode, up to
>> intel_logical_ring_advance_and_submit(), where the context being
>> dispatched would be added to the execlist queue; at this point
>> we submit the context to the GuC backend instead.
>>
>> There are, however, a few other changes also required, notably:
>> 1. Contexts must be pinned at GGTT addresses accessible by the GuC
>> i.e. NOT in the range [0..WOPCM_SIZE), so we have to add the
>> PIN_OFFSET_BIAS flag to the relevant GGTT-pinning calls.
>>
>> 2. The GuC's TLB must be invalidated after a context is pinned at
>> a new GGTT address.

[snip]

>> static int gen8_init_rcs_context(struct drm_i915_gem_request *req)
>> {
>> + struct drm_i915_private *dev_priv = req->i915;
>> int ret;
>>
>> + /* Invalidate GuC TLB. */
>> + if (i915.enable_guc_submission)
>> + I915_WRITE(GEN8_GTCR, GEN8_GTCR_INVALIDATE);
>> +
>
> [TOR:] This invalidation is in the init_context for render
> ring but not the other rings. Is this needed for other
> rings? Or, should this invalidation happen at a different
> level? It seems this may depend on the on render ring being
> initialized first.
>
> Thanks,
> Tom

Hi Tom,

it looks like this is redundant here in the case where its called from
the non-default-context case of intel_lr_context_deferred_create(); but
when called from i915_gem_init_hw() [via i915_gem_context_enable()] it
wouldn't be, because the GuC TLB wouldn't have been flushed since the
default context was pinned [which is in a completely different path
through intel_lr_context_deferred_create()!].

However, if we add a TLB flush just after that point, we can remove this
one here, with several advantages:
* firstly, that path is taken just once, rather than every time a new
render context is created, and
* secondly, each flush can be specifically associated with a pin-to-gtt
call that includes the (PIN_OFFSET_BIAS | GUC_WOPCM_SIZE_VALUE) flags,
showing that the pinned object is of interest to the GuC.

I'll also move the existing TLB flushes in guc_submission.c and
guc_loader.c so that they're also just after the relevant 'pin' calls.

Thanks,
.Dave.