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

[Gao, Ping A]

On 2017/6/29 16:38, Zhenyu Wang wrote:
> 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.

Sure, I will prepare a separated patch. Thanks!