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

[Yu Dai <yu.dai@intel.com>]

On 07/28/2015 06:59 AM, Dave Gordon wrote:
> 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.
>

Aye. -Alex

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