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

[Du, Changbin]

On Tue, May 02, 2017 at 01:48:41PM +0800, Zhenyu Wang wrote:
> On 2017.05.02 12:52:15 +0800, changbin.du at gmail.com wrote:
> > 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>
> 
> no dup sign-off
> 
caused by different git config, will drop.

> > +/**
> > + * intel_gvt_switch_render_mmio - switch mmio context of specific engine
> > + * @pre: the last vGPU that own the engine
> > + * @next: the vGPU to switch to
> > + * @ring_id: specify the engine
> > + *
> > + * If pre is null indicates that host own the engine. If next is null
> > + * indicates that we are switching to host workload.
> > + */
> > +void intel_gvt_switch_mmio(struct intel_vgpu *pre,
> > +			   struct intel_vgpu *next, int ring_id)
> > +{
> > +
> > +	static enum intel_engine_id last_ring = -1;
> > +
> > +	if (WARN_ON((!pre && !next) || (pre && last_ring == -1)))
> > +		return;
> > +
> > +	gvt_dbg_render("switch mmio from %s to %s, last_ring %d, cur_ring %d\n",
> > +		       pre ? "vGPU" : "host", next ? "vGPU" : "HOST",
> > +		       last_ring, ring_id);
> > +
> > +	/**
> > +	 * TODO: Optimize for vGPU to vGPU switch by merging
> > +	 * switch_mmio_to_host() and switch_mmio_to_vgpu().
> > +	 */
> 
> Is this valid? For different vGPU switch, we still need to save old
> and load new.
>
For vgpu->vgpu, we can eliminate one cyle mem->mem copy after merging that two
functions.

> > +	if (pre)
> > +		switch_mmio_to_host(pre, last_ring);
> 
> Looks older interface name is better as switch_mmio_to_host() is also
> used for different vGPU switch, so not just for host.
> 
This is similar to above comment. We now implement vgpu->vgpu swith by:
  vgpu mmio context ---> host context --> vgpu2 context
If switch_mmio_to_host() and switch_mmio_to_vgpu() merged, we can do vgpu->vgpu
switch directly.

The origin name load/restore is some confusing, seems they are both restoring
sth.
switch_mmio_to_vgpu - switch mmio values from host's to a vgpu's
switch_mmio_to_host - switch mmio values from vgpu's to host's


-- 
Thanks,
Changbin Du
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20170502/c2373a71/attachment-0001.sig>