[Intel-gfx] [PATCH 02/10] drm/i915/gvt: Pin the per-engine GVT shadow contexts

[Chris Wilson]

Quoting Zhenyu Wang (2019-04-26 07:04:45)
> On 2019.04.25 17:23:44 +0100, Chris Wilson wrote:
> > Quoting Chris Wilson (2019-04-25 06:42:02)
> > > Our eventual goal is to rid request construction of struct_mutex, with
> > > the short term step of lifting the struct_mutex requirements into the
> > > higher levels (i.e. the caller must ensure that the context is already
> > > pinned into the GTT). In this patch, we pin GVT's shadow context upon
> > > allocation and so keep them pinned into the GGTT for as long as the
> > > virtual machine is alive, and so we can use the simpler request
> > > construction path safe in the knowledge that the hard work is already
> > > done.
> > > 
> > > Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> > > Cc: Zhenyu Wang <zhenyuw at linux.intel.com>
> > 
> > Hi Zhenyu, could you check through this patch and make sure I haven't
> > broken gvt in the process?
> > 
> > The end result is that the gvt shadow context is always pinned into the
> > ggtt, avoids any eviction/shrinking, and so allows gvt to use the faster
> > paths for request allocation.
> >
> 
> I did found an issue with below extra change required.
> 
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index 012564cac752..d4dc0e189547 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -895,11 +895,6 @@ static void complete_current_workload(struct intel_gvt *gvt, int ring_id)
>                                 intel_vgpu_trigger_virtual_event(vgpu, event);
>                 }
>  
> -               /* unpin shadow ctx as the shadow_ctx update is done */
> -               mutex_lock(&rq->i915->drm.struct_mutex);
> -               intel_context_unpin(rq->hw_context);
> -               mutex_unlock(&rq->i915->drm.struct_mutex);
> -
>                 i915_request_put(fetch_and_zero(&workload->req));
>         }
>  
> Previously we tried extra pin to make sure GVT still can access shadow
> context after request finish, then we did above extra unpin after done
> with update to guest context. Now this is not required and actually
> caused wrong unpin.

Oops. I forgot to grep for rq->hw_context (and I forgot about earlier
discussions for why that existed :)

One less struct_mutex for the win. The end goal for this is that gvt
will never need struct_mutex (nor anything else in the driver!)
 
> With this change this series can pass my sanity test. So with this added.
> 
> Acked-by: Zhenyu Wang <zhenyuw at linux.intel.com>

Ta,
-Chris