On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> Tidy up the for loops that handle waiting for read/write vs read-only
> access.
>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> ---
> drivers/gpu/drm/i915/i915_gem.c | 158 +++++++++++++++++++---------------------
> 1 file changed, 75 insertions(+), 83 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 3f6b69dcaccb..2d86a0c3f295 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1339,6 +1339,23 @@ put_rpm:
> return ret;
> }
>
> +static void
> +i915_gem_object_retire_request(struct drm_i915_gem_object *obj,
> + struct drm_i915_gem_request *req)
> +{
> + int idx = req->engine->id;
> +
> + if (i915_gem_active_peek(&obj->last_read[idx],
> + &obj->base.dev->struct_mutex) == req)
> + i915_gem_object_retire__read(obj, idx);
> + else if (i915_gem_active_peek(&obj->last_write,
> + &obj->base.dev->struct_mutex) == req)
> + i915_gem_object_retire__write(obj);
If these functions will use same mutex (be it different than
struct_mutex) in all invocations, I'd make an alias for it.
> +
> + if (!i915_reset_in_progress(&req->i915->gpu_error))
> + i915_gem_request_retire_upto(req);
> +}
> +
> /**
> * Ensures that all rendering to the object has completed and the object is
> * safe to unbind from the GTT or access from the CPU.
> @@ -1349,39 +1366,34 @@ int
> i915_gem_object_wait_rendering(struct drm_i915_gem_object *obj,
> bool readonly)
> {
> - struct drm_i915_gem_request *request;
> struct reservation_object *resv;
> - int ret, i;
> + struct i915_gem_active *active;
> + unsigned long active_mask;
> + int idx, ret;
>
We could do early exit here based on the active_mask, like with the
nonblocking version.
> - if (readonly) {
> - request = i915_gem_active_peek(&obj->last_write,
> - &obj->base.dev->struct_mutex);
> - if (request) {
> - ret = i915_wait_request(request);
> - if (ret)
> - return ret;
> + lockdep_assert_held(&obj->base.dev->struct_mutex);
>
> - i = request->engine->id;
> - if (i915_gem_active_peek(&obj->last_read[i],
> - &obj->base.dev->struct_mutex) == request)
> - i915_gem_object_retire__read(obj, i);
> - else
> - i915_gem_object_retire__write(obj);
> - }
> + if (!readonly) {
Not sure why not just swap and keep this if (readonly)...
> + active = obj->last_read;
> + active_mask = obj->active;
> } else {
> - for (i = 0; i < I915_NUM_ENGINES; i++) {
> - request = i915_gem_active_peek(&obj->last_read[i],
> - &obj->base.dev->struct_mutex);
> - if (!request)
> - continue;
> + active_mask = 1;
Wouldn't we have RENDER_RING define for this and other instances?
With above addressed,
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Regards, Joonas
--
Joonas Lahtinen
Open Source Technology Center
Intel Corporation
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
