| Message ID | 20211018034256.816113-1-oak.zeng@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/selftests: Wait request to finish | expand |
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; >
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. AFAIK the existing callers only expect that the request will be scheduled to run at some point, like in the case of __igt_write_huge(), before we read the page contents via the CPU, there is an object_wait(). Also in the shrink_thp test I guess we intentionally don't want to wait directly in the test, since part of what we are testing is that the shrinker ensures the object is idle before unbinding it. > > 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); > err_batch: > i915_vma_unpin_and_release(&batch, 0); > return err; >
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); err_batch: i915_vma_unpin_and_release(&batch, 0); return err;
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. 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(+)