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

[Zhenyu Wang]

On 2017.06.29 12:22:42 +0800, Ping Gao wrote:
> 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().
> 
> v2:Put context pin before i915_add_request;
>    Refine the comments;
>    Rename some APIs;
> 
> v3:workload->status should set only when error happens.
> v4:i915_add_request is must to have after i915_gem_request_alloc.
> 
> Signed-off-by: Ping Gao <ping.a.gao at intel.com>
> Reviewed-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
>  drivers/gpu/drm/i915/gvt/cmd_parser.c |  2 +-
>  drivers/gpu/drm/i915/gvt/cmd_parser.h |  2 +-
>  drivers/gpu/drm/i915/gvt/gvt.h        |  2 +
>  drivers/gpu/drm/i915/gvt/scheduler.c  | 83 +++++++++++++++++++++--------------
>  4 files changed, 54 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.c b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> index 51241de5..4ddae9b 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.c
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.c
> @@ -2637,7 +2637,7 @@ static int shadow_workload_ring_buffer(struct intel_vgpu_workload *workload)
>  	return 0;
>  }
>  
> -int intel_gvt_scan_and_shadow_workload(struct intel_vgpu_workload *workload)
> +int intel_gvt_scan_and_shadow_ringbuffer(struct intel_vgpu_workload *workload)
>  {
>  	int ret;
>  	struct intel_vgpu *vgpu = workload->vgpu;
> diff --git a/drivers/gpu/drm/i915/gvt/cmd_parser.h b/drivers/gpu/drm/i915/gvt/cmd_parser.h
> index bed3351..2867036 100644
> --- a/drivers/gpu/drm/i915/gvt/cmd_parser.h
> +++ b/drivers/gpu/drm/i915/gvt/cmd_parser.h
> @@ -42,7 +42,7 @@ void intel_gvt_clean_cmd_parser(struct intel_gvt *gvt);
>  
>  int intel_gvt_init_cmd_parser(struct intel_gvt *gvt);
>  
> -int intel_gvt_scan_and_shadow_workload(struct intel_vgpu_workload *workload);
> +int intel_gvt_scan_and_shadow_ringbuffer(struct intel_vgpu_workload *workload);
>  
>  int intel_gvt_scan_and_shadow_wa_ctx(struct intel_shadow_wa_ctx *wa_ctx);
>  
> diff --git a/drivers/gpu/drm/i915/gvt/gvt.h b/drivers/gpu/drm/i915/gvt/gvt.h
> index 3a74e79..ba53ad1 100644
> --- a/drivers/gpu/drm/i915/gvt/gvt.h
> +++ b/drivers/gpu/drm/i915/gvt/gvt.h
> @@ -470,6 +470,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_scan_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 0e2e36a..7929c02 100644
> --- a/drivers/gpu/drm/i915/gvt/scheduler.c
> +++ b/drivers/gpu/drm/i915/gvt/scheduler.c
> @@ -184,42 +184,27 @@ static int shadow_context_status_change(struct notifier_block *nb,
>  	return NOTIFY_OK;
>  }
>  
> -static int dispatch_workload(struct intel_vgpu_workload *workload)
> +/**
> + * intel_gvt_scan_and_shadow_workload - audit the workload by scanning and
> + * shadow it as well, include ringbuffer,wa_ctx and ctx.
> + * @workload: an abstract entity for each execlist submission.
> + *
> + * This function is called before the workload submitting to i915, to make
> + * sure the content of the workload is valid.
> + */
> +int intel_gvt_scan_and_shadow_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];
>  	struct drm_i915_gem_request *rq;
>  	struct intel_vgpu *vgpu = workload->vgpu;
> -	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);
> -

Could you add a lockdep assert that caller must hold this mutex? maybe in a following patch,
I might just apply this series now.

-- 
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/20170629/d47a90e2/attachment.sig>