Skip to content

Commit

Permalink
drm/i915: Start returning an error from i915_vma_move_to_active()
Browse files Browse the repository at this point in the history
Handling such a late error in request construction is tricky, but to
accommodate future patches which may allocate here, we potentially could
err. To handle the error after already adjusting global state to track
the new request, we must finish and submit the request. But we don't
want to use the request as not everything is being tracked by it, so we
opt to cancel the commands inside the request.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180706103947.15919-3-chris@chris-wilson.co.uk
  • Loading branch information
Chris Wilson committed Jul 6, 2018
1 parent 6dd7526 commit a523697
Show file tree
Hide file tree
Showing 12 changed files with 78 additions and 29 deletions.
6 changes: 5 additions & 1 deletion drivers/gpu/drm/i915/gvt/scheduler.c
Original file line number Diff line number Diff line change
Expand Up @@ -476,7 +476,11 @@ static int prepare_shadow_batch_buffer(struct intel_vgpu_workload *workload)
i915_gem_obj_finish_shmem_access(bb->obj);
bb->accessing = false;

i915_vma_move_to_active(bb->vma, workload->req, 0);
ret = i915_vma_move_to_active(bb->vma,
workload->req,
0);
if (ret)
goto err;
}
}
return 0;
Expand Down
6 changes: 3 additions & 3 deletions drivers/gpu/drm/i915/i915_drv.h
Original file line number Diff line number Diff line change
Expand Up @@ -3090,9 +3090,9 @@ i915_gem_obj_finish_shmem_access(struct drm_i915_gem_object *obj)
}

int __must_check i915_mutex_lock_interruptible(struct drm_device *dev);
void i915_vma_move_to_active(struct i915_vma *vma,
struct i915_request *rq,
unsigned int flags);
int __must_check i915_vma_move_to_active(struct i915_vma *vma,
struct i915_request *rq,
unsigned int flags);
int i915_gem_dumb_create(struct drm_file *file_priv,
struct drm_device *dev,
struct drm_mode_create_dumb *args);
Expand Down
25 changes: 18 additions & 7 deletions drivers/gpu/drm/i915/i915_gem_execbuffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -1165,12 +1165,16 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
goto err_request;

GEM_BUG_ON(!reservation_object_test_signaled_rcu(batch->resv, true));
i915_vma_move_to_active(batch, rq, 0);
i915_vma_unpin(batch);
err = i915_vma_move_to_active(batch, rq, 0);
if (err)
goto skip_request;

i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
if (err)
goto skip_request;

rq->batch = batch;
i915_vma_unpin(batch);

cache->rq = rq;
cache->rq_cmd = cmd;
Expand All @@ -1179,6 +1183,8 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb,
/* Return with batch mapping (cmd) still pinned */
return 0;

skip_request:
i915_request_skip(rq, err);
err_request:
i915_request_add(rq);
err_unpin:
Expand Down Expand Up @@ -1818,7 +1824,11 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb)
unsigned int flags = eb->flags[i];
struct i915_vma *vma = eb->vma[i];

i915_vma_move_to_active(vma, eb->request, flags);
err = i915_vma_move_to_active(vma, eb->request, flags);
if (unlikely(err)) {
i915_request_skip(eb->request, err);
return err;
}

__eb_unreserve_vma(vma, flags);
vma->exec_flags = NULL;
Expand Down Expand Up @@ -1877,9 +1887,9 @@ static void export_fence(struct i915_vma *vma,
reservation_object_unlock(resv);
}

void i915_vma_move_to_active(struct i915_vma *vma,
struct i915_request *rq,
unsigned int flags)
int i915_vma_move_to_active(struct i915_vma *vma,
struct i915_request *rq,
unsigned int flags)
{
struct drm_i915_gem_object *obj = vma->obj;
const unsigned int idx = rq->engine->id;
Expand Down Expand Up @@ -1916,6 +1926,7 @@ void i915_vma_move_to_active(struct i915_vma *vma,
i915_gem_active_set(&vma->last_fence, rq);

export_fence(vma, rq, flags);
return 0;
}

static int i915_reset_gen7_sol_offsets(struct i915_request *rq)
Expand Down
2 changes: 1 addition & 1 deletion drivers/gpu/drm/i915/i915_gem_render_state.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,7 @@ int i915_gem_render_state_emit(struct i915_request *rq)
goto err_unpin;
}

i915_vma_move_to_active(so.vma, rq, 0);
err = i915_vma_move_to_active(so.vma, rq, 0);
err_unpin:
i915_vma_unpin(so.vma);
err_vma:
Expand Down
9 changes: 7 additions & 2 deletions drivers/gpu/drm/i915/selftests/huge_pages.c
Original file line number Diff line number Diff line change
Expand Up @@ -985,7 +985,10 @@ static int gpu_write(struct i915_vma *vma,
goto err_request;
}

i915_vma_move_to_active(batch, rq, 0);
err = i915_vma_move_to_active(batch, rq, 0);
if (err)
goto err_request;

i915_gem_object_set_active_reference(batch->obj);
i915_vma_unpin(batch);
i915_vma_close(batch);
Expand All @@ -996,7 +999,9 @@ static int gpu_write(struct i915_vma *vma,
if (err)
goto err_request;

i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
if (err)
i915_request_skip(rq, err);

err_request:
i915_request_add(rq);
Expand Down
4 changes: 2 additions & 2 deletions drivers/gpu/drm/i915/selftests/i915_gem_coherency.c
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,12 @@ static int gpu_set(struct drm_i915_gem_object *obj,
}
intel_ring_advance(rq, cs);

i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
i915_vma_unpin(vma);

i915_request_add(rq);

return 0;
return err;
}

static bool always_valid(struct drm_i915_private *i915)
Expand Down
12 changes: 10 additions & 2 deletions drivers/gpu/drm/i915/selftests/i915_gem_context.c
Original file line number Diff line number Diff line change
Expand Up @@ -170,18 +170,26 @@ static int gpu_fill(struct drm_i915_gem_object *obj,
if (err)
goto err_request;

i915_vma_move_to_active(batch, rq, 0);
err = i915_vma_move_to_active(batch, rq, 0);
if (err)
goto skip_request;

err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
if (err)
goto skip_request;

i915_gem_object_set_active_reference(batch->obj);
i915_vma_unpin(batch);
i915_vma_close(batch);

i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
i915_vma_unpin(vma);

i915_request_add(rq);

return 0;

skip_request:
i915_request_skip(rq, err);
err_request:
i915_request_add(rq);
err_batch:
Expand Down
7 changes: 4 additions & 3 deletions drivers/gpu/drm/i915/selftests/i915_gem_object.c
Original file line number Diff line number Diff line change
Expand Up @@ -464,13 +464,14 @@ static int make_obj_busy(struct drm_i915_gem_object *obj)
return PTR_ERR(rq);
}

i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);

i915_request_add(rq);

i915_gem_object_set_active_reference(obj);
__i915_gem_object_release_unless_active(obj);
i915_vma_unpin(vma);
return 0;

return err;
}

static bool assert_mmap_offset(struct drm_i915_private *i915,
Expand Down
8 changes: 6 additions & 2 deletions drivers/gpu/drm/i915/selftests/i915_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -675,7 +675,9 @@ static int live_all_engines(void *arg)
i915_gem_object_set_active_reference(batch->obj);
}

i915_vma_move_to_active(batch, request[id], 0);
err = i915_vma_move_to_active(batch, request[id], 0);
GEM_BUG_ON(err);

i915_request_get(request[id]);
i915_request_add(request[id]);
}
Expand Down Expand Up @@ -785,7 +787,9 @@ static int live_sequential_engines(void *arg)
GEM_BUG_ON(err);
request[id]->batch = batch;

i915_vma_move_to_active(batch, request[id], 0);
err = i915_vma_move_to_active(batch, request[id], 0);
GEM_BUG_ON(err);

i915_gem_object_set_active_reference(batch->obj);
i915_vma_get(batch);

Expand Down
11 changes: 9 additions & 2 deletions drivers/gpu/drm/i915/selftests/intel_hangcheck.c
Original file line number Diff line number Diff line change
Expand Up @@ -130,13 +130,19 @@ static int emit_recurse_batch(struct hang *h,
if (err)
goto unpin_vma;

i915_vma_move_to_active(vma, rq, 0);
err = i915_vma_move_to_active(vma, rq, 0);
if (err)
goto unpin_hws;

if (!i915_gem_object_has_active_reference(vma->obj)) {
i915_gem_object_get(vma->obj);
i915_gem_object_set_active_reference(vma->obj);
}

i915_vma_move_to_active(hws, rq, 0);
err = i915_vma_move_to_active(hws, rq, 0);
if (err)
goto unpin_hws;

if (!i915_gem_object_has_active_reference(hws->obj)) {
i915_gem_object_get(hws->obj);
i915_gem_object_set_active_reference(hws->obj);
Expand Down Expand Up @@ -205,6 +211,7 @@ static int emit_recurse_batch(struct hang *h,

err = rq->engine->emit_bb_start(rq, vma->node.start, PAGE_SIZE, flags);

unpin_hws:
i915_vma_unpin(hws);
unpin_vma:
i915_vma_unpin(vma);
Expand Down
11 changes: 9 additions & 2 deletions drivers/gpu/drm/i915/selftests/intel_lrc.c
Original file line number Diff line number Diff line change
Expand Up @@ -104,13 +104,19 @@ static int emit_recurse_batch(struct spinner *spin,
if (err)
goto unpin_vma;

i915_vma_move_to_active(vma, rq, 0);
err = i915_vma_move_to_active(vma, rq, 0);
if (err)
goto unpin_hws;

if (!i915_gem_object_has_active_reference(vma->obj)) {
i915_gem_object_get(vma->obj);
i915_gem_object_set_active_reference(vma->obj);
}

i915_vma_move_to_active(hws, rq, 0);
err = i915_vma_move_to_active(hws, rq, 0);
if (err)
goto unpin_hws;

if (!i915_gem_object_has_active_reference(hws->obj)) {
i915_gem_object_get(hws->obj);
i915_gem_object_set_active_reference(hws->obj);
Expand All @@ -134,6 +140,7 @@ static int emit_recurse_batch(struct spinner *spin,

err = rq->engine->emit_bb_start(rq, vma->node.start, PAGE_SIZE, 0);

unpin_hws:
i915_vma_unpin(hws);
unpin_vma:
i915_vma_unpin(vma);
Expand Down
6 changes: 4 additions & 2 deletions drivers/gpu/drm/i915/selftests/intel_workarounds.c
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,10 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
goto err_pin;
}

err = i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);
if (err)
goto err_req;

srm = MI_STORE_REGISTER_MEM | MI_SRM_LRM_GLOBAL_GTT;
if (INTEL_GEN(ctx->i915) >= 8)
srm++;
Expand All @@ -67,8 +71,6 @@ read_nonprivs(struct i915_gem_context *ctx, struct intel_engine_cs *engine)
}
intel_ring_advance(rq, cs);

i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE);

i915_gem_object_get(result);
i915_gem_object_set_active_reference(result);

Expand Down

0 comments on commit a523697

Please sign in to comment.