[PATCH 1/2] drm/i915/gvt: Factor out audit and shadow from workload dispatch
Zhi Wang
zhi.a.wang at intel.com
Mon Jun 5 08:33:53 UTC 2017
Better move context pin out of intel_gvt_audit_and_shadow_workload(). It
looks a bit weird that only context unpin is out there while context pin
is in another function.
On 05/24/17 20:49, Ping Gao wrote:
> The workload audit and shadow could execute in earlier ELSP
> writing stage for performance consideration, it's need to factor
> out it from workload dispatch first.
>
To perform the workload scan and shadow in ELSP writing stage for
performance consideration, the workload scan and shadow stuffs should be
factored out from dispatch_workload().
> Signed-off-by: Ping Gao <ping.a.gao at intel.com>
> ---
> drivers/gpu/drm/i915/gvt/gvt.h | 2 ++
> drivers/gpu/drm/i915/gvt/scheduler.c | 44 ++++++++++++++++++++++++------------
> 2 files changed, 32 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 07ecd73..accd9c4 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -455,6 +455,8 @@ int intel_vgpu_init_opregion(struct intel_vgpu *vgpu, u32 gpa);
> int intel_vgpu_emulate_opregion_request(struct intel_vgpu *vgpu, u32 swsci);
> void populate_pvinfo_page(struct intel_vgpu *vgpu);
>
> +int intel_gvt_audit_and_shadow_workload(struct intel_vgpu_workload *workload);
> +
Looks we can figure out a better name here. The actual work (scan and
shadow) is done in intel_gvt_scan_and_shadow_workload()/cmd_parser.c.
Maybe we can rename it
(intel_gvt_scan_and_shadow_workload()/cmd_parser.c) to
intel_gvt_scan_and_shadow_ringbuffer()/cmd_parser.c, as its work is
actually in ring buffer level. While the work we did here should be
workload level (lrc context/wa context/ring buffer), you can use the
name (intel_gvt_scan_and_shadow_workload()/scheduler.c) here.
Do not forget to add some special comments for doxygen while you expose
an API. :P
> struct intel_gvt_ops {
> int (*emulate_cfg_read)(struct intel_vgpu *, unsigned int, void *,
> unsigned int);
> diff --git a/drivers/gpu/drm/i915/gvt/scheduler.c b/drivers/gpu/drm/i915/gvt/scheduler.c
> index e63b1d8..d6bfdfe 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -172,7 +172,8 @@ static int shadow_context_status_change(struct notifier_block *nb,
> return NOTIFY_OK;
> }
>
> -static int dispatch_workload(struct intel_vgpu_workload *workload)
> +
> +int intel_gvt_audit_and_shadow_workload(struct intel_vgpu_workload *workload)
> {
> int ring_id = workload->ring_id;
> struct i915_gem_context *shadow_ctx = workload->vgpu->shadow_ctx;
> @@ -183,15 +184,10 @@ static int dispatch_workload(struct intel_vgpu_workload *workload)
> struct intel_ring *ring;
> int ret;
>
> - gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n",
> - ring_id, workload);
> -
> shadow_ctx->desc_template &= ~(0x3 << GEN8_CTX_ADDRESSING_MODE_SHIFT);
> shadow_ctx->desc_template |= workload->ctx_desc.addressing_mode <<
> GEN8_CTX_ADDRESSING_MODE_SHIFT;
>
> - mutex_lock(&dev_priv->drm.struct_mutex);
> -
> /* pin shadow context by gvt even the shadow context will be pinned
> * when i915 alloc request. That is because gvt will update the guest
> * context from shadow context when workload is completed, and at that
> @@ -234,6 +230,29 @@ static int dispatch_workload(struct intel_vgpu_workload *workload)
> if (ret)
> goto out;
>
> +out:
> + workload->status = ret;
> + return ret;
> +}
> +
> +
> +static int dispatch_workload(struct intel_vgpu_workload *workload)
> +{
> + int ring_id = workload->ring_id;
> + struct i915_gem_context *shadow_ctx = workload->vgpu->shadow_ctx;
> + struct drm_i915_private *dev_priv = workload->vgpu->gvt->dev_priv;
> + struct intel_engine_cs *engine = dev_priv->engine[ring_id];
> + int ret = 0;
> +
> + gvt_dbg_sched("ring id %d prepare to dispatch workload %p\n",
> + ring_id, workload);
> +
> + mutex_lock(&dev_priv->drm.struct_mutex);
> +
> + ret = intel_gvt_audit_and_shadow_workload(workload);
> + if (ret)
> + goto out;
> +
> if (workload->prepare) {
> ret = workload->prepare(workload);
> if (ret)
> @@ -243,16 +262,13 @@ static int dispatch_workload(struct intel_vgpu_workload *workload)
> gvt_dbg_sched("ring id %d submit workload to i915 %p\n",
> ring_id, workload->req);
>
> - ret = 0;
> - workload->dispatched = true;
> out:
> - if (ret)
> - workload->status = ret;
> -
> - if (!IS_ERR_OR_NULL(rq))
> - i915_add_request(rq);
> - else
> + if (ret) {
> engine->context_unpin(engine, shadow_ctx);
> + } else {
> + i915_add_request(workload->req);
> + workload->dispatched = true;
> + }
>
> mutex_unlock(&dev_priv->drm.struct_mutex);
> return ret;
>
More information about the intel-gvt-dev
mailing list