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

[Dave Gordon <david.s.gordon@intel.com>]

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.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx