| Message ID | 1424366285-29232-16-git-send-email-John.C.Harrison@Intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > The plan is to pass requests around as the basic submission tracking structure > rather than rings and contexts. This patch updates the i915_gem_object_sync() > code path. > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 +- > drivers/gpu/drm/i915/i915_gem.c | 7 ++++--- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- > drivers/gpu/drm/i915/intel_lrc.c | 2 +- > 4 files changed, 7 insertions(+), 6 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 375d4f9..bfd7b47 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -2744,7 +2744,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) > > int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); > int i915_gem_object_sync(struct drm_i915_gem_object *obj, > - struct intel_engine_cs *to); > + struct drm_i915_gem_request *to_req); > void i915_vma_move_to_active(struct i915_vma *vma, > struct intel_engine_cs *ring); > int i915_gem_dumb_create(struct drm_file *file_priv, > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 5897d54..c5b9bc7 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2956,7 +2956,7 @@ out: > * i915_gem_object_sync - sync an object to a ring. > * > * @obj: object which may be in use on another ring. > - * @to: ring we wish to use the object on. May be NULL. > + * @to_req: request we wish to use the object for. May be NULL. > * > * This code is meant to abstract object synchronization with the GPU. > * Calling with NULL implies synchronizing the object with the CPU > @@ -2966,8 +2966,9 @@ out: > */ > int > i915_gem_object_sync(struct drm_i915_gem_object *obj, > - struct intel_engine_cs *to) > + struct drm_i915_gem_request *to_req) > { > + struct intel_engine_cs *to = to_req ? to_req->ring : NULL; > struct intel_engine_cs *from; > u32 seqno; > int ret, idx; > @@ -3953,7 +3954,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > if (ret) > return ret; > > - ret = i915_gem_object_sync(obj, req->ring); > + ret = i915_gem_object_sync(obj, req); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 76f6dcf..2cd0579 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -838,7 +838,7 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req, > > list_for_each_entry(vma, vmas, exec_list) { > struct drm_i915_gem_object *obj = vma->obj; > - ret = i915_gem_object_sync(obj, req->ring); > + ret = i915_gem_object_sync(obj, req); > if (ret) > return ret; > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c > index 4b42346..0d88e9c 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -589,7 +589,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req, > list_for_each_entry(vma, vmas, exec_list) { > struct drm_i915_gem_object *obj = vma->obj; > > - ret = i915_gem_object_sync(obj, req->ring); > + ret = i915_gem_object_sync(obj, req); > if (ret) > return ret; > > This seems to be just fine but if the requests of any of the existing i915_gem_object_sync() call sites were to be null they would cause local null pointer exceptions in the calling functions even though i915_gem_object_sync() itself supports an incoming null request. There does not seem to be any call sites in the driver that actually passes or supports passing null requests into i915_gem_object_sync(). Does the driver do that anywhere today or how does that fit in? Aside from that open question: Reviewed-by: Tomas Elf <tomas.elf@intel.com> Thanks, Tomas
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 375d4f9..bfd7b47 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -2744,7 +2744,7 @@ static inline void i915_gem_object_unpin_pages(struct drm_i915_gem_object *obj) int __must_check i915_mutex_lock_interruptible(struct drm_device *dev); int i915_gem_object_sync(struct drm_i915_gem_object *obj, - struct intel_engine_cs *to); + struct drm_i915_gem_request *to_req); void i915_vma_move_to_active(struct i915_vma *vma, struct intel_engine_cs *ring); int i915_gem_dumb_create(struct drm_file *file_priv, diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 5897d54..c5b9bc7 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2956,7 +2956,7 @@ out: * i915_gem_object_sync - sync an object to a ring. * * @obj: object which may be in use on another ring. - * @to: ring we wish to use the object on. May be NULL. + * @to_req: request we wish to use the object for. May be NULL. * * This code is meant to abstract object synchronization with the GPU. * Calling with NULL implies synchronizing the object with the CPU @@ -2966,8 +2966,9 @@ out: */ int i915_gem_object_sync(struct drm_i915_gem_object *obj, - struct intel_engine_cs *to) + struct drm_i915_gem_request *to_req) { + struct intel_engine_cs *to = to_req ? to_req->ring : NULL; struct intel_engine_cs *from; u32 seqno; int ret, idx; @@ -3953,7 +3954,7 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, if (ret) return ret; - ret = i915_gem_object_sync(obj, req->ring); + ret = i915_gem_object_sync(obj, req); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 76f6dcf..2cd0579 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -838,7 +838,7 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req, list_for_each_entry(vma, vmas, exec_list) { struct drm_i915_gem_object *obj = vma->obj; - ret = i915_gem_object_sync(obj, req->ring); + ret = i915_gem_object_sync(obj, req); if (ret) return ret; diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 4b42346..0d88e9c 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -589,7 +589,7 @@ static int execlists_move_to_gpu(struct drm_i915_gem_request *req, list_for_each_entry(vma, vmas, exec_list) { struct drm_i915_gem_object *obj = vma->obj; - ret = i915_gem_object_sync(obj, req->ring); + ret = i915_gem_object_sync(obj, req); if (ret) return ret;