Re: [Intel-gfx] [PATCH] drm/i915/selftests: Wait request to finish

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 18/10/2021 04:42, Oak Zeng wrote:
> Caller of function igt_gpu_fill_dw assumes when this function
> returns, the gpu command to fill dword is already finished.
> Caller usually checks the GPU filling result immediately, see
> the usage in function __igt_write_huge. If there is no wait,
> when function igt_gpu_fill_dw returns, the GPU command is not
> guaranteed to be finished. So wait for request to finish.

There is more than one caller and the first one I looked at already does 
wait so the above is not fully accurate.

I your example of __igt_write_huge you have i915_gem_object_prepare_read 
called before the assert which does object wait, so that too is not a 
missing wait.

I haven't checked the rest. So perhaps say what failure you are trying 
to fix with a link to a CI report or something?

> 
> Signed-off-by: Oak Zeng <oak.zeng@intel.com>
> ---
>   drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c
> index b35c1219c852..5fa595ed30b1 100644
> --- a/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c
> +++ b/drivers/gpu/drm/i915/gem/selftests/igt_gem_utils.c
> @@ -157,6 +157,7 @@ int igt_gpu_fill_dw(struct intel_context *ce,
>   	if (err)
>   		i915_request_set_error_once(rq, err);
>   	i915_request_add(rq);
> +	err = i915_request_wait(rq, 0, HZ/5);

For you reference this exploded in CI because as soon as 
i915_request_add() you are giving your rq reference to the backend and 
i915_request_wait can/will therefore ooops. If you used this pattern 
somewhere you'd need to do:

i915_request_get()
i915_request_add()
i915_request_wait()
i915_request_put()

Regards,

Tvrtko

>   err_batch:
>   	i915_vma_unpin_and_release(&batch, 0);
>   	return err;
>