[PATCH v2] drm/i915/gvt: refactor function intel_vgpu_submit_execlist

[Zhi Wang]

Thanks for the patch! LGTM. :P

于 05/04/17 18:36, changbin.du at intel.com 写道:
> From: Changbin Du <changbin.du at intel.com>
>
> The function intel_vgpu_submit_execlist could be more simpler. It
> actually does:
>    1) validate the submission. The first context must be valid,
>       and all two must be privilege_access.
>    2) submit valid contexts. The first one need emulate schedule_in.
>
> We do not need a bitmap, valid desc copy valid_desc. Local variable
> emulate_schedule_in also can be optimized out.
>
> v2: dump desc content in err msg (Zhi Wang)
>
> Signed-off-by: Changbin Du <changbin.du at intel.com>
> Reviewed-by: Zhi Wang <zhi.a.wang at intel.com>
> ---
>   drivers/gpu/drm/i915/gvt/execlist.c | 56 +++++++++++++++----------------------
>   1 file changed, 23 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gvt/execlist.c b/drivers/gpu/drm/i915/gvt/execlist.c
> index dca989e..8bba38f 100644
> --- a/drivers/gpu/drm/i915/gvt/execlist.c
> +++ b/drivers/gpu/drm/i915/gvt/execlist.c
> @@ -708,53 +708,43 @@ static int submit_context(struct intel_vgpu *vgpu, int ring_id,
>   int intel_vgpu_submit_execlist(struct intel_vgpu *vgpu, int ring_id)
>   {
>   	struct intel_vgpu_execlist *execlist = &vgpu->execlist[ring_id];
> -	struct execlist_ctx_descriptor_format *desc[2], valid_desc[2];
> -	unsigned long valid_desc_bitmap = 0;
> -	bool emulate_schedule_in = true;
> -	int ret;
> -	int i;
> +	struct execlist_ctx_descriptor_format desc[2];
> +	int i, ret;
>
> -	memset(valid_desc, 0, sizeof(valid_desc));
> +	desc[0] = *get_desc_from_elsp_dwords(&execlist->elsp_dwords, 1);
> +	desc[1] = *get_desc_from_elsp_dwords(&execlist->elsp_dwords, 0);
>
> -	desc[0] = get_desc_from_elsp_dwords(&execlist->elsp_dwords, 1);
> -	desc[1] = get_desc_from_elsp_dwords(&execlist->elsp_dwords, 0);
> +	if (!desc[0].valid) {
> +		gvt_vgpu_err("invalid elsp submission, desc0 is invalid\n");
> +		goto inv_desc;
> +	}
>
> -	for (i = 0; i < 2; i++) {
> -		if (!desc[i]->valid)
> +	for (i = 0; i < ARRAY_SIZE(desc); i++) {
> +		if (!desc[i].valid)
>   			continue;
> -
> -		if (!desc[i]->privilege_access) {
> +		if (!desc[i].privilege_access) {
>   			gvt_vgpu_err("unexpected GGTT elsp submission\n");
> -			return -EINVAL;
> +			goto inv_desc;
>   		}
> -
> -		/* TODO: add another guest context checks here. */
> -		set_bit(i, &valid_desc_bitmap);
> -		valid_desc[i] = *desc[i];
> -	}
> -
> -	if (!valid_desc_bitmap) {
> -		gvt_vgpu_err("no valid desc in a elsp submission\n");
> -		return -EINVAL;
> -	}
> -
> -	if (!test_bit(0, (void *)&valid_desc_bitmap) &&
> -			test_bit(1, (void *)&valid_desc_bitmap)) {
> -		gvt_vgpu_err("weird elsp submission, desc 0 is not valid\n");
> -		return -EINVAL;
>   	}
>
>   	/* submit workload */
> -	for_each_set_bit(i, (void *)&valid_desc_bitmap, 2) {
> -		ret = submit_context(vgpu, ring_id, &valid_desc[i],
> -				emulate_schedule_in);
> +	for (i = 0; i < ARRAY_SIZE(desc); i++) {
> +		if (!desc[i].valid)
> +			continue;
> +		ret = submit_context(vgpu, ring_id, &desc[i], i == 0);
>   		if (ret) {
> -			gvt_vgpu_err("fail to schedule workload\n");
> +			gvt_vgpu_err("failed to submit desc %d\n", i);
>   			return ret;
>   		}
> -		emulate_schedule_in = false;
>   	}
> +
>   	return 0;
> +
> +inv_desc:
> +	gvt_vgpu_err("descriptors content: desc0 %08x %08x desc1 %08x %08x\n",
> +		     desc[0].udw, desc[0].ldw, desc[1].udw, desc[1].ldw);
> +	return -EINVAL;
>   }
>
>   static void init_vgpu_execlist(struct intel_vgpu *vgpu, int ring_id)
>