[PATCH 1/2] drm/i915/gvt: Factor out audit and shadow from workload dispatch

[Du, Changbin]

On Wed, May 24, 2017 at 08:49:47PM +0800, 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.
> 
> 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);
> +
>  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;
> 
The 'out' lable is just followed, no goto need.

> +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;
> +	}
>  
I think you can handle shadow failure inside your intel_gvt_audit_and_shadow_workload.
So the api doesn't have side effect if it return failure.

And this the dispatch_workload() only has a single call to workload->prepare.
Then dispatch_workload fucntion turns out not neccessary.

>  	mutex_unlock(&dev_priv->drm.struct_mutex);
>  	return ret;
> -- 
> 2.7.4
> 
> _______________________________________________
> intel-gvt-dev mailing list
> intel-gvt-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gvt-dev

-- 
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/20170606/16d05508/attachment.sig>