[PATCH] drm/i915/gvt: implement per-vm mmio switching optimization

[Dong, Chuanxiao]

> -----Original Message-----
> From: intel-gvt-dev [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On
> Behalf Of Du, Changbin
> Sent: Tuesday, May 2, 2017 4:38 PM
> To: Dong, Chuanxiao <chuanxiao.dong at intel.com>
> Cc: Du, Changbin <changbin.du at intel.com>; intel-gvt-
> dev at lists.freedesktop.org; Du at freedesktop.org; changbin.du at gmail.com
> Subject: Re: [PATCH] drm/i915/gvt: implement per-vm mmio switching
> optimization
> 
> On Tue, May 02, 2017 at 12:53:28AM -0700, Dong, Chuanxiao wrote:
> >
> >
> > > -----Original Message-----
> > > From: intel-gvt-dev
> > > [mailto:intel-gvt-dev-bounces at lists.freedesktop.org] On Behalf Of
> > > changbin.du at gmail.com
> > > Sent: Tuesday, May 2, 2017 12:52 PM
> > > To: intel-gvt-dev at lists.freedesktop.org
> > > Cc: Du, Changbin <changbin.du at intel.com>; Du at freedesktop.org
> > > Subject: [PATCH] drm/i915/gvt: implement per-vm mmio switching
> > > optimization
> > >
> > > From: Changbin Du <changbin.du at intel.com>
> > >
> > > Commit ab9da627906a ("drm/i915: make context status notifier head be
> > > per
> > > engine") gives us a chance to inspect every single request. Then we
> > > can eliminate unnecessary mmio switching for same vGPU. We only need
> > > mmio switching for different VMs (including host).
> > >
> > > This patch introduced a new general API intel_gvt_switch_mmio() to
> > > replace the old intel_gvt_load/restore_render_mmio(). This function
> > > save the last ring id internally which will be used for next switch.
> > >
> > > This optimization is very useful if only one guest has plenty of
> > > workloads and the host is mostly idle. The best case is no mmio switching
> will happen.
> > >
> > > Signed-off-by: Changbin Du <changbin.du at intel.com>
> > > Signed-off-by: Du, Changbin <changbin.du at intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/gvt/render.c    | 39
> > > ++++++++++++++++++++++++++++++++++--
> > >  drivers/gpu/drm/i915/gvt/render.h    |  4 ++--
> > >  drivers/gpu/drm/i915/gvt/scheduler.c | 29
> > > ++++++++++++++++++++------- drivers/gpu/drm/i915/gvt/scheduler.h |
> > > 2 ++
> > >  4 files changed, 63 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gvt/render.c
> > > b/drivers/gpu/drm/i915/gvt/render.c
> > > index c6e7972..260ef83 100644
> > > --- a/drivers/gpu/drm/i915/gvt/render.c
> > > +++ b/drivers/gpu/drm/i915/gvt/render.c
> > > @@ -260,7 +260,7 @@ static void restore_mocs(struct intel_vgpu
> > > *vgpu, int
> > > ring_id)
> > >
> > >  #define CTX_CONTEXT_CONTROL_VAL	0x03
> > >
> > > -void intel_gvt_load_render_mmio(struct intel_vgpu *vgpu, int
> > > ring_id)
> > > +static void switch_mmio_to_vgpu(struct intel_vgpu *vgpu, int
> > > +ring_id)
> >
> > As it is per-vm switching, is it really OK to only save/restore the mmio
> according to the specific ring instead of all the rings? As after switch to VGPU,
> other rings may also be used by VGPU.
> >
> Seems this aways is our defaut policy so far. We only switch the ring which
> will be used, why switch other rings?

The current switch policy is per request. If this request will use RCS, only RCS is save/restore as other rings won't be touched. After switch to per-vm, a VM is possible that send a batch of requests to i915, and these requests may use different rings. So my understanding is after switch to per-vm, only save/restore for one ring is not enough.

Thanks
Chuanxiao 

> 
> 
> > >  {
> > >  	struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> > >  	struct render_mmio *mmio;
> > > @@ -312,7 +312,7 @@ void intel_gvt_load_render_mmio(struct
> > > intel_vgpu *vgpu, int ring_id)
> > >  	handle_tlb_pending_event(vgpu, ring_id);  }
> > >
> > > -void intel_gvt_restore_render_mmio(struct intel_vgpu *vgpu, int
> > > ring_id)
> > > +static void switch_mmio_to_host(struct intel_vgpu *vgpu, int
> > > +ring_id)
> >
> > The same concern as the above.
> >
> > Thanks
> > Chuanxiao
> >
> 
> --
> Thanks,
> Changbin Du