Message ID | 1424366285-29232-10-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> > > Rather than just having a local request variable in the execbuff code, the > request pointer is now stored in the execbuff params structure. Also added > explicit cleanup of the request (plus wiping the OLR to match) in the error > case. This means that the execbuff code is no longer dependent upon the OLR > keeping track of the request so as to not leak it when things do go wrong. Note > that in the success case, the i915_add_request() at the end of the submission > function will tidy up the request and clear the OLR. > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 13 +++++++++++-- > 2 files changed, 12 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 90223f208..678b190 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1653,6 +1653,7 @@ struct i915_execbuffer_params { > struct intel_engine_cs *ring; > struct drm_i915_gem_object *batch_obj; > struct intel_context *ctx; > + struct drm_i915_gem_request *request; > }; > > struct drm_i915_private { > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 37dcc6f..10462f6 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1353,7 +1353,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > struct i915_address_space *vm; > struct i915_execbuffer_params params_master; /* XXX: will be removed later */ > struct i915_execbuffer_params *params = ¶ms_master; > - struct drm_i915_gem_request *request; > const u32 ctx_id = i915_execbuffer2_get_context_id(*args); > u32 dispatch_flags; > int ret; > @@ -1532,7 +1531,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > params->batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm); > > /* Allocate a request for this batch buffer nice and early. */ > - ret = dev_priv->gt.alloc_request(ring, ctx, &request); > + ret = dev_priv->gt.alloc_request(ring, ctx, ¶ms->request); > if (ret) > goto err; > > @@ -1565,6 +1564,16 @@ err: > i915_gem_context_unreference(ctx); > eb_destroy(eb); > > + /* > + * If the request was created but not successfully submitted then it > + * must be freed again. If it was submitted then it is being tracked > + * on the active request list and no clean up is required here. > + */ > + if (ret && params->request) { > + i915_gem_request_unreference(params->request); > + ring->outstanding_lazy_request = NULL; > + } > + > mutex_unlock(&dev->struct_mutex); > > pre_mutex_err: > 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 90223f208..678b190 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1653,6 +1653,7 @@ struct i915_execbuffer_params { struct intel_engine_cs *ring; struct drm_i915_gem_object *batch_obj; struct intel_context *ctx; + struct drm_i915_gem_request *request; }; struct drm_i915_private { diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 37dcc6f..10462f6 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1353,7 +1353,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct i915_address_space *vm; struct i915_execbuffer_params params_master; /* XXX: will be removed later */ struct i915_execbuffer_params *params = ¶ms_master; - struct drm_i915_gem_request *request; const u32 ctx_id = i915_execbuffer2_get_context_id(*args); u32 dispatch_flags; int ret; @@ -1532,7 +1531,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, params->batch_obj_vm_offset = i915_gem_obj_offset(batch_obj, vm); /* Allocate a request for this batch buffer nice and early. */ - ret = dev_priv->gt.alloc_request(ring, ctx, &request); + ret = dev_priv->gt.alloc_request(ring, ctx, ¶ms->request); if (ret) goto err; @@ -1565,6 +1564,16 @@ err: i915_gem_context_unreference(ctx); eb_destroy(eb); + /* + * If the request was created but not successfully submitted then it + * must be freed again. If it was submitted then it is being tracked + * on the active request list and no clean up is required here. + */ + if (ret && params->request) { + i915_gem_request_unreference(params->request); + ring->outstanding_lazy_request = NULL; + } + mutex_unlock(&dev->struct_mutex); pre_mutex_err: