[PATCH] drm/i915/gvt: implement per-vm mmio switching optimization
Zhenyu Wang
zhenyuw at linux.intel.com
Tue May 2 05:48:41 UTC 2017
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
> ---
> 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)
> {
> 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)
> {
> struct drm_i915_private *dev_priv = vgpu->gvt->dev_priv;
> struct render_mmio *mmio;
> @@ -348,3 +348,38 @@ void intel_gvt_restore_render_mmio(struct intel_vgpu *vgpu, int ring_id)
> mmio->value, v);
> }
> }
> +
> +/**
> + * 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.
> + 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.
> +
> + if (next) {
> + switch_mmio_to_vgpu(next, ring_id);
> + last_ring = ring_id;
> + }
> +}
> diff --git a/drivers/gpu/drm/i915/gvt/render.h b/drivers/gpu/drm/i915/gvt/render.h
> index dac1a3c..91db1d3 100644
> --- a/drivers/gpu/drm/i915/gvt/render.h
> +++ b/drivers/gpu/drm/i915/gvt/render.h
> @@ -36,8 +36,8 @@
> #ifndef __GVT_RENDER_H__
> #define __GVT_RENDER_H__
>
> -void intel_gvt_load_render_mmio(struct intel_vgpu *vgpu, int ring_id);
> +void intel_gvt_switch_mmio(struct intel_vgpu *pre,
> + struct intel_vgpu *next, int ring_id);
>
> -void intel_gvt_restore_render_mmio(struct intel_vgpu *vgpu, int ring_id);
>
> #endif
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index bada32b..1caabea 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -139,21 +139,36 @@ static int shadow_context_status_change(struct notifier_block *nb,
> struct intel_gvt *gvt = container_of(nb, struct intel_gvt,
> shadow_ctx_notifier_block[req->engine->id]);
> struct intel_gvt_workload_scheduler *scheduler = &gvt->scheduler;
> - struct intel_vgpu_workload *workload =
> - scheduler->current_workload[req->engine->id];
> + struct intel_vgpu_workload *workload;
> +
> + if (!is_gvt_request(req)) {
> + if (action == INTEL_CONTEXT_SCHEDULE_IN && scheduler->last_vgpu) {
> + /* Switch from vGPU to host. */
> + intel_gvt_switch_mmio(scheduler->last_vgpu, NULL,
> + req->engine->id);
> + }
> +
> + scheduler->last_vgpu = NULL;
> + return NOTIFY_OK;
> + }
>
> - if (!is_gvt_request(req) || unlikely(!workload))
> + workload = scheduler->current_workload[req->engine->id];
> + if (unlikely(!workload))
> return NOTIFY_OK;
>
> switch (action) {
> case INTEL_CONTEXT_SCHEDULE_IN:
> - intel_gvt_load_render_mmio(workload->vgpu,
> - workload->ring_id);
> + if (scheduler->last_vgpu != workload->vgpu) {
> + /* Switch from host to vGPU or from vGPU to vGPU. */
> + intel_gvt_switch_mmio(scheduler->last_vgpu,
> + workload->vgpu,
> + req->engine->id);
> + }
> +
> atomic_set(&workload->shadow_ctx_active, 1);
> + scheduler->last_vgpu = workload->vgpu;
> break;
> case INTEL_CONTEXT_SCHEDULE_OUT:
> - intel_gvt_restore_render_mmio(workload->vgpu,
> - workload->ring_id);
> /* If the status is -EINPROGRESS means this workload
> * doesn't meet any issue during dispatching so when
> * get the SCHEDULE_OUT set the status to be zero for
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.h b/drivers/gpu/drm/i915/gvt/scheduler.h
> index 2cd725c..8a06cd9 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.h
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.h
> @@ -38,6 +38,8 @@
>
> struct intel_gvt_workload_scheduler {
> struct intel_vgpu *current_vgpu;
> + /* last_vgpu can be null when running i915 context */
> + struct intel_vgpu *last_vgpu;
> struct intel_vgpu *next_vgpu;
> struct intel_vgpu_workload *current_workload[I915_NUM_ENGINES];
> bool need_reschedule;
> --
> 2.7.4
>
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev
--
Open Source Technology Center, Intel ltd.
$gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/intel-gvt-dev/attachments/20170502/475e4896/attachment.sig>
More information about the intel-gvt-dev
mailing list