| Message ID | 20180629075348.27358-14-chris@chris-wilson.co.uk (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 29/06/2018 08:53, Chris Wilson wrote: > Currently all callers are responsible for adding the vma to the active > timeline and then exporting its fence. Combine the two operations into > i915_vma_move_to_active() to move all the extra handling from the > callers to the single site. > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 47 +++++++++---------- > drivers/gpu/drm/i915/selftests/huge_pages.c | 4 -- > .../drm/i915/selftests/i915_gem_coherency.c | 4 -- > .../gpu/drm/i915/selftests/i915_gem_context.c | 4 -- > 4 files changed, 21 insertions(+), 38 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index c2dd9b4cdace..91f20445147f 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1166,15 +1166,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb, > > GEM_BUG_ON(!reservation_object_test_signaled_rcu(batch->resv, true)); > i915_vma_move_to_active(batch, rq, 0); > - reservation_object_lock(batch->resv, NULL); > - reservation_object_add_excl_fence(batch->resv, &rq->fence); > - reservation_object_unlock(batch->resv); Here now the exclusive fence is not added any more due flags being zero. > i915_vma_unpin(batch); > > i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); > - reservation_object_lock(vma->resv, NULL); > - reservation_object_add_excl_fence(vma->resv, &rq->fence); > - reservation_object_unlock(vma->resv); > > rq->batch = batch; > > @@ -1771,25 +1765,6 @@ static int eb_relocate(struct i915_execbuffer *eb) > return eb_relocate_slow(eb); > } > > -static void eb_export_fence(struct i915_vma *vma, > - struct i915_request *rq, > - unsigned int flags) > -{ > - struct reservation_object *resv = vma->resv; > - > - /* > - * Ignore errors from failing to allocate the new fence, we can't > - * handle an error right now. Worst case should be missed > - * synchronisation leading to rendering corruption. > - */ > - reservation_object_lock(resv, NULL); > - if (flags & EXEC_OBJECT_WRITE) > - reservation_object_add_excl_fence(resv, &rq->fence); > - else if (reservation_object_reserve_shared(resv) == 0) > - reservation_object_add_shared_fence(resv, &rq->fence); > - reservation_object_unlock(resv); > -} > - > static int eb_move_to_gpu(struct i915_execbuffer *eb) > { > const unsigned int count = eb->buffer_count; > @@ -1844,7 +1819,6 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb) > struct i915_vma *vma = eb->vma[i]; > > i915_vma_move_to_active(vma, eb->request, flags); > - eb_export_fence(vma, eb->request, flags); > > __eb_unreserve_vma(vma, flags); > vma->exec_flags = NULL; > @@ -1884,6 +1858,25 @@ static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec) > return true; > } > > +static void export_fence(struct i915_vma *vma, > + struct i915_request *rq, > + unsigned int flags) > +{ > + struct reservation_object *resv = vma->resv; > + > + /* > + * Ignore errors from failing to allocate the new fence, we can't > + * handle an error right now. Worst case should be missed > + * synchronisation leading to rendering corruption. > + */ > + reservation_object_lock(resv, NULL); > + if (flags & EXEC_OBJECT_WRITE) > + reservation_object_add_excl_fence(resv, &rq->fence); > + else if (reservation_object_reserve_shared(resv) == 0) > + reservation_object_add_shared_fence(resv, &rq->fence); > + reservation_object_unlock(resv); > +} > + > void i915_vma_move_to_active(struct i915_vma *vma, > struct i915_request *rq, > unsigned int flags) > @@ -1921,6 +1914,8 @@ void i915_vma_move_to_active(struct i915_vma *vma, > > if (flags & EXEC_OBJECT_NEEDS_FENCE) > i915_gem_active_set(&vma->last_fence, rq); > + > + export_fence(vma, rq, flags); > } > > static int i915_reset_gen7_sol_offsets(struct i915_request *rq) > diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c > index b5e87fcdcdae..358fc81f6c99 100644 > --- a/drivers/gpu/drm/i915/selftests/huge_pages.c > +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c > @@ -998,10 +998,6 @@ static int gpu_write(struct i915_vma *vma, > > i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); > > - reservation_object_lock(vma->resv, NULL); > - reservation_object_add_excl_fence(vma->resv, &rq->fence); > - reservation_object_unlock(vma->resv); > - > err_request: > i915_request_add(rq); > > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c > index a4900091ae3d..11427aae0853 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c > @@ -225,10 +225,6 @@ static int gpu_set(struct drm_i915_gem_object *obj, > i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); > i915_vma_unpin(vma); > > - reservation_object_lock(obj->resv, NULL); > - reservation_object_add_excl_fence(obj->resv, &rq->fence); > - reservation_object_unlock(obj->resv); > - > i915_request_add(rq); > > return 0; > diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c > index 3169b72180d0..a78f8bdb8222 100644 > --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c > @@ -179,10 +179,6 @@ static int gpu_fill(struct drm_i915_gem_object *obj, > i915_vma_move_to_active(vma, rq, 0); > i915_vma_unpin(vma); > > - reservation_object_lock(obj->resv, NULL); > - reservation_object_add_excl_fence(obj->resv, &rq->fence); Here as well this won't be added due zero flags. > - reservation_object_unlock(obj->resv); > - > i915_request_add(rq); > > return 0; > There is one i915_vma_move_to_active in render state and one in gvt which will now have some extra processing/allocations, do we care? Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index c2dd9b4cdace..91f20445147f 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1166,15 +1166,9 @@ static int __reloc_gpu_alloc(struct i915_execbuffer *eb, GEM_BUG_ON(!reservation_object_test_signaled_rcu(batch->resv, true)); i915_vma_move_to_active(batch, rq, 0); - reservation_object_lock(batch->resv, NULL); - reservation_object_add_excl_fence(batch->resv, &rq->fence); - reservation_object_unlock(batch->resv); i915_vma_unpin(batch); i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); - reservation_object_lock(vma->resv, NULL); - reservation_object_add_excl_fence(vma->resv, &rq->fence); - reservation_object_unlock(vma->resv); rq->batch = batch; @@ -1771,25 +1765,6 @@ static int eb_relocate(struct i915_execbuffer *eb) return eb_relocate_slow(eb); } -static void eb_export_fence(struct i915_vma *vma, - struct i915_request *rq, - unsigned int flags) -{ - struct reservation_object *resv = vma->resv; - - /* - * Ignore errors from failing to allocate the new fence, we can't - * handle an error right now. Worst case should be missed - * synchronisation leading to rendering corruption. - */ - reservation_object_lock(resv, NULL); - if (flags & EXEC_OBJECT_WRITE) - reservation_object_add_excl_fence(resv, &rq->fence); - else if (reservation_object_reserve_shared(resv) == 0) - reservation_object_add_shared_fence(resv, &rq->fence); - reservation_object_unlock(resv); -} - static int eb_move_to_gpu(struct i915_execbuffer *eb) { const unsigned int count = eb->buffer_count; @@ -1844,7 +1819,6 @@ static int eb_move_to_gpu(struct i915_execbuffer *eb) struct i915_vma *vma = eb->vma[i]; i915_vma_move_to_active(vma, eb->request, flags); - eb_export_fence(vma, eb->request, flags); __eb_unreserve_vma(vma, flags); vma->exec_flags = NULL; @@ -1884,6 +1858,25 @@ static bool i915_gem_check_execbuffer(struct drm_i915_gem_execbuffer2 *exec) return true; } +static void export_fence(struct i915_vma *vma, + struct i915_request *rq, + unsigned int flags) +{ + struct reservation_object *resv = vma->resv; + + /* + * Ignore errors from failing to allocate the new fence, we can't + * handle an error right now. Worst case should be missed + * synchronisation leading to rendering corruption. + */ + reservation_object_lock(resv, NULL); + if (flags & EXEC_OBJECT_WRITE) + reservation_object_add_excl_fence(resv, &rq->fence); + else if (reservation_object_reserve_shared(resv) == 0) + reservation_object_add_shared_fence(resv, &rq->fence); + reservation_object_unlock(resv); +} + void i915_vma_move_to_active(struct i915_vma *vma, struct i915_request *rq, unsigned int flags) @@ -1921,6 +1914,8 @@ void i915_vma_move_to_active(struct i915_vma *vma, if (flags & EXEC_OBJECT_NEEDS_FENCE) i915_gem_active_set(&vma->last_fence, rq); + + export_fence(vma, rq, flags); } static int i915_reset_gen7_sol_offsets(struct i915_request *rq) diff --git a/drivers/gpu/drm/i915/selftests/huge_pages.c b/drivers/gpu/drm/i915/selftests/huge_pages.c index b5e87fcdcdae..358fc81f6c99 100644 --- a/drivers/gpu/drm/i915/selftests/huge_pages.c +++ b/drivers/gpu/drm/i915/selftests/huge_pages.c @@ -998,10 +998,6 @@ static int gpu_write(struct i915_vma *vma, i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); - reservation_object_lock(vma->resv, NULL); - reservation_object_add_excl_fence(vma->resv, &rq->fence); - reservation_object_unlock(vma->resv); - err_request: i915_request_add(rq); diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c index a4900091ae3d..11427aae0853 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_coherency.c @@ -225,10 +225,6 @@ static int gpu_set(struct drm_i915_gem_object *obj, i915_vma_move_to_active(vma, rq, EXEC_OBJECT_WRITE); i915_vma_unpin(vma); - reservation_object_lock(obj->resv, NULL); - reservation_object_add_excl_fence(obj->resv, &rq->fence); - reservation_object_unlock(obj->resv); - i915_request_add(rq); return 0; diff --git a/drivers/gpu/drm/i915/selftests/i915_gem_context.c b/drivers/gpu/drm/i915/selftests/i915_gem_context.c index 3169b72180d0..a78f8bdb8222 100644 --- a/drivers/gpu/drm/i915/selftests/i915_gem_context.c +++ b/drivers/gpu/drm/i915/selftests/i915_gem_context.c @@ -179,10 +179,6 @@ static int gpu_fill(struct drm_i915_gem_object *obj, i915_vma_move_to_active(vma, rq, 0); i915_vma_unpin(vma); - reservation_object_lock(obj->resv, NULL); - reservation_object_add_excl_fence(obj->resv, &rq->fence); - reservation_object_unlock(obj->resv); - i915_request_add(rq); return 0;
Currently all callers are responsible for adding the vma to the active timeline and then exporting its fence. Combine the two operations into i915_vma_move_to_active() to move all the extra handling from the callers to the single site. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 47 +++++++++---------- drivers/gpu/drm/i915/selftests/huge_pages.c | 4 -- .../drm/i915/selftests/i915_gem_coherency.c | 4 -- .../gpu/drm/i915/selftests/i915_gem_context.c | 4 -- 4 files changed, 21 insertions(+), 38 deletions(-)