* [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client [not found] <0160927121915.GU28107@nuc-i3427.alporthouse.com> @ 2016-09-28 16:48 ` Tvrtko Ursulin 2016-09-28 17:09 ` Chris Wilson 2016-09-28 17:28 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev3) Patchwork 2016-09-29 8:28 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev4) Patchwork 2 siblings, 1 reply; 8+ messages in thread From: Tvrtko Ursulin @ 2016-09-28 16:48 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Several things we can clean up in this function: * Request and file passed in cannot be NULL. There is a single caller which makes it impossible so change that condition to a GEM_BUG_ON instead of a WARN with a return code. * Same is true for req->file_priv which is always zeroed before this function is called. * With the above we can remove the error return from the function making it void. * dev_private local variable was unused. * Call site in i915_gem_do_execbuffer can be simplified since there is no error handling any longer. v2: * Move next to the only caller. (Chris Wilson) * Remove useless asserts. (Joonas Lahtinen) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v1) --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 +++++++++++++++----- drivers/gpu/drm/i915/i915_gem_request.c | 25 ------------------------- drivers/gpu/drm/i915/i915_gem_request.h | 2 -- 3 files changed, 15 insertions(+), 32 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 33c85227643d..20dc7d90cecf 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1612,6 +1612,19 @@ eb_select_engine(struct drm_i915_private *dev_priv, return engine; } +static void i915_gem_request_add_to_client(struct drm_i915_gem_request *req, + struct drm_file *file) +{ + struct drm_i915_file_private *file_priv = file->driver_priv; + + GEM_BUG_ON(req->file_priv); + + spin_lock(&file_priv->mm.lock); + req->file_priv = file_priv; + list_add_tail(&req->client_list, &file_priv->mm.request_list); + spin_unlock(&file_priv->mm.lock); +} + static int i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct drm_file *file, @@ -1824,9 +1837,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, */ params->request->batch = params->batch; - ret = i915_gem_request_add_to_client(params->request, file); - if (ret) - goto err_request; + i915_gem_request_add_to_client(params->request, file); /* * Save assorted stuff away to pass through to *_submission(). @@ -1841,8 +1852,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, params->ctx = ctx; ret = execbuf_submit(params, args, &eb->vmas); -err_request: - __i915_add_request(params->request, ret == 0); + __i915_add_request(params->request, true); err_batch_unpin: /* diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 40978bc12ceb..490d3a40271a 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -115,31 +115,6 @@ const struct fence_ops i915_fence_ops = { .timeline_value_str = i915_fence_timeline_value_str, }; -int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, - struct drm_file *file) -{ - struct drm_i915_private *dev_private; - struct drm_i915_file_private *file_priv; - - WARN_ON(!req || !file || req->file_priv); - - if (!req || !file) - return -EINVAL; - - if (req->file_priv) - return -EINVAL; - - dev_private = req->i915; - file_priv = file->driver_priv; - - spin_lock(&file_priv->mm.lock); - req->file_priv = file_priv; - list_add_tail(&req->client_list, &file_priv->mm.request_list); - spin_unlock(&file_priv->mm.lock); - - return 0; -} - static inline void i915_gem_request_remove_from_client(struct drm_i915_gem_request *request) { diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h index 974bd7bcc801..df1a41bbf645 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.h +++ b/drivers/gpu/drm/i915/i915_gem_request.h @@ -155,8 +155,6 @@ static inline bool fence_is_i915(struct fence *fence) struct drm_i915_gem_request * __must_check i915_gem_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx); -int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, - struct drm_file *file); void i915_gem_request_retire_upto(struct drm_i915_gem_request *req); static inline u32 -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client 2016-09-28 16:48 ` [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client Tvrtko Ursulin @ 2016-09-28 17:09 ` Chris Wilson 2016-09-29 7:44 ` Tvrtko Ursulin 2016-09-29 7:53 ` [PATCH v3] " Tvrtko Ursulin 0 siblings, 2 replies; 8+ messages in thread From: Chris Wilson @ 2016-09-28 17:09 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx On Wed, Sep 28, 2016 at 05:48:23PM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Several things we can clean up in this function: > > * Request and file passed in cannot be NULL. There is > a single caller which makes it impossible so change > that condition to a GEM_BUG_ON instead of a WARN > with a return code. > > * Same is true for req->file_priv which is always > zeroed before this function is called. > > * With the above we can remove the error return > from the function making it void. > > * dev_private local variable was unused. > > * Call site in i915_gem_do_execbuffer can be > simplified since there is no error handling any > longer. > > v2: > * Move next to the only caller. (Chris Wilson) > * Remove useless asserts. (Joonas Lahtinen) > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v1) > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 +++++++++++++++----- > drivers/gpu/drm/i915/i915_gem_request.c | 25 ------------------------- > drivers/gpu/drm/i915/i915_gem_request.h | 2 -- > 3 files changed, 15 insertions(+), 32 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 33c85227643d..20dc7d90cecf 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1612,6 +1612,19 @@ eb_select_engine(struct drm_i915_private *dev_priv, > return engine; > } > > +static void i915_gem_request_add_to_client(struct drm_i915_gem_request *req, > + struct drm_file *file) Just add_to_client. > +{ > + struct drm_i915_file_private *file_priv = file->driver_priv; > + > + GEM_BUG_ON(req->file_priv); The error isn't associated with execbuf. The explosion will happen elsewhere and will be from a path that doesn't go near execbuf. > + spin_lock(&file_priv->mm.lock); > + req->file_priv = file_priv; > + list_add_tail(&req->client_list, &file_priv->mm.request_list); > + spin_unlock(&file_priv->mm.lock); > +} > + > static int > i915_gem_do_execbuffer(struct drm_device *dev, void *data, > struct drm_file *file, > @@ -1824,9 +1837,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > */ > params->request->batch = params->batch; > > - ret = i915_gem_request_add_to_client(params->request, file); > - if (ret) > - goto err_request; > + i915_gem_request_add_to_client(params->request, file); > > /* > * Save assorted stuff away to pass through to *_submission(). > @@ -1841,8 +1852,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > params->ctx = ctx; > > ret = execbuf_submit(params, args, &eb->vmas); > -err_request: > - __i915_add_request(params->request, ret == 0); > + __i915_add_request(params->request, true); We don't want to force the flush if submit fails either. * mutters about having to add back err_request: in the current series. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client 2016-09-28 17:09 ` Chris Wilson @ 2016-09-29 7:44 ` Tvrtko Ursulin 2016-09-29 7:53 ` [PATCH v3] " Tvrtko Ursulin 1 sibling, 0 replies; 8+ messages in thread From: Tvrtko Ursulin @ 2016-09-29 7:44 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx On 28/09/2016 18:09, Chris Wilson wrote: > On Wed, Sep 28, 2016 at 05:48:23PM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Several things we can clean up in this function: >> >> * Request and file passed in cannot be NULL. There is >> a single caller which makes it impossible so change >> that condition to a GEM_BUG_ON instead of a WARN >> with a return code. >> >> * Same is true for req->file_priv which is always >> zeroed before this function is called. >> >> * With the above we can remove the error return >> from the function making it void. >> >> * dev_private local variable was unused. >> >> * Call site in i915_gem_do_execbuffer can be >> simplified since there is no error handling any >> longer. >> >> v2: >> * Move next to the only caller. (Chris Wilson) >> * Remove useless asserts. (Joonas Lahtinen) >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v1) >> --- >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 20 +++++++++++++++----- >> drivers/gpu/drm/i915/i915_gem_request.c | 25 ------------------------- >> drivers/gpu/drm/i915/i915_gem_request.h | 2 -- >> 3 files changed, 15 insertions(+), 32 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> index 33c85227643d..20dc7d90cecf 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> @@ -1612,6 +1612,19 @@ eb_select_engine(struct drm_i915_private *dev_priv, >> return engine; >> } >> >> +static void i915_gem_request_add_to_client(struct drm_i915_gem_request *req, >> + struct drm_file *file) > Just add_to_client. Ok. >> +{ >> + struct drm_i915_file_private *file_priv = file->driver_priv; >> + >> + GEM_BUG_ON(req->file_priv); > The error isn't associated with execbuf. The explosion will happen > elsewhere and will be from a path that doesn't go near execbuf. I did not even spot it while looking. Will remove. >> + spin_lock(&file_priv->mm.lock); >> + req->file_priv = file_priv; >> + list_add_tail(&req->client_list, &file_priv->mm.request_list); >> + spin_unlock(&file_priv->mm.lock); >> +} >> + >> static int >> i915_gem_do_execbuffer(struct drm_device *dev, void *data, >> struct drm_file *file, >> @@ -1824,9 +1837,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, >> */ >> params->request->batch = params->batch; >> >> - ret = i915_gem_request_add_to_client(params->request, file); >> - if (ret) >> - goto err_request; >> + i915_gem_request_add_to_client(params->request, file); >> >> /* >> * Save assorted stuff away to pass through to *_submission(). >> @@ -1841,8 +1852,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, >> params->ctx = ctx; >> >> ret = execbuf_submit(params, args, &eb->vmas); >> -err_request: >> - __i915_add_request(params->request, ret == 0); >> + __i915_add_request(params->request, true); > We don't want to force the flush if submit fails either. Embarrassing. :I Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH v3] drm/i915: Cleanup i915_gem_request_add_to_client 2016-09-28 17:09 ` Chris Wilson 2016-09-29 7:44 ` Tvrtko Ursulin @ 2016-09-29 7:53 ` Tvrtko Ursulin 2016-09-29 8:08 ` Chris Wilson 1 sibling, 1 reply; 8+ messages in thread From: Tvrtko Ursulin @ 2016-09-29 7:53 UTC (permalink / raw) To: Intel-gfx From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Several things we can clean up in this function: * Request and file passed in cannot be NULL. There is a single caller which makes it impossible so change that condition to a GEM_BUG_ON instead of a WARN with a return code. * Same is true for req->file_priv which is always zeroed before this function is called. * With the above we can remove the error return from the function making it void. * dev_private local variable was unused. * Call site in i915_gem_do_execbuffer can be simplified since there is no error handling any longer. v2: * Move next to the only caller. (Chris Wilson) * Remove useless asserts. (Joonas Lahtinen) v3: * Shorten name, remove last standing assert and fix flushing after failed submit. (Chris Wilson) Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v1) --- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 ++++++++++++---- drivers/gpu/drm/i915/i915_gem_request.c | 25 ------------------------- drivers/gpu/drm/i915/i915_gem_request.h | 2 -- 3 files changed, 12 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 33c85227643d..f7b96391ff66 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -1612,6 +1612,17 @@ eb_select_engine(struct drm_i915_private *dev_priv, return engine; } +static void +add_to_client(struct drm_i915_gem_request *req, struct drm_file *file) +{ + struct drm_i915_file_private *file_priv = file->driver_priv; + + spin_lock(&file_priv->mm.lock); + req->file_priv = file_priv; + list_add_tail(&req->client_list, &file_priv->mm.request_list); + spin_unlock(&file_priv->mm.lock); +} + static int i915_gem_do_execbuffer(struct drm_device *dev, void *data, struct drm_file *file, @@ -1824,9 +1835,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, */ params->request->batch = params->batch; - ret = i915_gem_request_add_to_client(params->request, file); - if (ret) - goto err_request; + add_to_client(params->request, file); /* * Save assorted stuff away to pass through to *_submission(). @@ -1841,7 +1850,6 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, params->ctx = ctx; ret = execbuf_submit(params, args, &eb->vmas); -err_request: __i915_add_request(params->request, ret == 0); err_batch_unpin: diff --git a/drivers/gpu/drm/i915/i915_gem_request.c b/drivers/gpu/drm/i915/i915_gem_request.c index 40978bc12ceb..490d3a40271a 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.c +++ b/drivers/gpu/drm/i915/i915_gem_request.c @@ -115,31 +115,6 @@ const struct fence_ops i915_fence_ops = { .timeline_value_str = i915_fence_timeline_value_str, }; -int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, - struct drm_file *file) -{ - struct drm_i915_private *dev_private; - struct drm_i915_file_private *file_priv; - - WARN_ON(!req || !file || req->file_priv); - - if (!req || !file) - return -EINVAL; - - if (req->file_priv) - return -EINVAL; - - dev_private = req->i915; - file_priv = file->driver_priv; - - spin_lock(&file_priv->mm.lock); - req->file_priv = file_priv; - list_add_tail(&req->client_list, &file_priv->mm.request_list); - spin_unlock(&file_priv->mm.lock); - - return 0; -} - static inline void i915_gem_request_remove_from_client(struct drm_i915_gem_request *request) { diff --git a/drivers/gpu/drm/i915/i915_gem_request.h b/drivers/gpu/drm/i915/i915_gem_request.h index 974bd7bcc801..df1a41bbf645 100644 --- a/drivers/gpu/drm/i915/i915_gem_request.h +++ b/drivers/gpu/drm/i915/i915_gem_request.h @@ -155,8 +155,6 @@ static inline bool fence_is_i915(struct fence *fence) struct drm_i915_gem_request * __must_check i915_gem_request_alloc(struct intel_engine_cs *engine, struct i915_gem_context *ctx); -int i915_gem_request_add_to_client(struct drm_i915_gem_request *req, - struct drm_file *file); void i915_gem_request_retire_upto(struct drm_i915_gem_request *req); static inline u32 -- 2.7.4 _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH v3] drm/i915: Cleanup i915_gem_request_add_to_client 2016-09-29 7:53 ` [PATCH v3] " Tvrtko Ursulin @ 2016-09-29 8:08 ` Chris Wilson 2016-09-29 8:13 ` Tvrtko Ursulin 0 siblings, 1 reply; 8+ messages in thread From: Chris Wilson @ 2016-09-29 8:08 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: Intel-gfx On Thu, Sep 29, 2016 at 08:53:31AM +0100, Tvrtko Ursulin wrote: > From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > > Several things we can clean up in this function: > > * Request and file passed in cannot be NULL. There is > a single caller which makes it impossible so change > that condition to a GEM_BUG_ON instead of a WARN > with a return code. > > * Same is true for req->file_priv which is always > zeroed before this function is called. > > * With the above we can remove the error return > from the function making it void. > > * dev_private local variable was unused. > > * Call site in i915_gem_do_execbuffer can be > simplified since there is no error handling any > longer. > > v2: > * Move next to the only caller. (Chris Wilson) > * Remove useless asserts. (Joonas Lahtinen) > > v3: > * Shorten name, remove last standing assert and > fix flushing after failed submit. (Chris Wilson) > > Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v1) > --- > drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 ++++++++++++---- > drivers/gpu/drm/i915/i915_gem_request.c | 25 ------------------------- > drivers/gpu/drm/i915/i915_gem_request.h | 2 -- > 3 files changed, 12 insertions(+), 31 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > index 33c85227643d..f7b96391ff66 100644 > --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c > +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c > @@ -1612,6 +1612,17 @@ eb_select_engine(struct drm_i915_private *dev_priv, > return engine; > } > > +static void > +add_to_client(struct drm_i915_gem_request *req, struct drm_file *file) > +{ > + struct drm_i915_file_private *file_priv = file->driver_priv; > + > + spin_lock(&file_priv->mm.lock); > + req->file_priv = file_priv; > + list_add_tail(&req->client_list, &file_priv->mm.request_list); > + spin_unlock(&file_priv->mm.lock); > +} > + > static int > i915_gem_do_execbuffer(struct drm_device *dev, void *data, > struct drm_file *file, > @@ -1824,9 +1835,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, > */ > params->request->batch = params->batch; > > - ret = i915_gem_request_add_to_client(params->request, file); > - if (ret) > - goto err_request; > + add_to_client(params->request, file); So we add to the client list even if we fail to submit the batch? For that reason I put the call to add_to_client() in execbuf_submit. -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH v3] drm/i915: Cleanup i915_gem_request_add_to_client 2016-09-29 8:08 ` Chris Wilson @ 2016-09-29 8:13 ` Tvrtko Ursulin 0 siblings, 0 replies; 8+ messages in thread From: Tvrtko Ursulin @ 2016-09-29 8:13 UTC (permalink / raw) To: Chris Wilson, Tvrtko Ursulin, Intel-gfx On 29/09/2016 09:08, Chris Wilson wrote: > On Thu, Sep 29, 2016 at 08:53:31AM +0100, Tvrtko Ursulin wrote: >> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> >> Several things we can clean up in this function: >> >> * Request and file passed in cannot be NULL. There is >> a single caller which makes it impossible so change >> that condition to a GEM_BUG_ON instead of a WARN >> with a return code. >> >> * Same is true for req->file_priv which is always >> zeroed before this function is called. >> >> * With the above we can remove the error return >> from the function making it void. >> >> * dev_private local variable was unused. >> >> * Call site in i915_gem_do_execbuffer can be >> simplified since there is no error handling any >> longer. >> >> v2: >> * Move next to the only caller. (Chris Wilson) >> * Remove useless asserts. (Joonas Lahtinen) >> >> v3: >> * Shorten name, remove last standing assert and >> fix flushing after failed submit. (Chris Wilson) >> >> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> >> Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com> (v1) >> --- >> drivers/gpu/drm/i915/i915_gem_execbuffer.c | 16 ++++++++++++---- >> drivers/gpu/drm/i915/i915_gem_request.c | 25 ------------------------- >> drivers/gpu/drm/i915/i915_gem_request.h | 2 -- >> 3 files changed, 12 insertions(+), 31 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> index 33c85227643d..f7b96391ff66 100644 >> --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c >> @@ -1612,6 +1612,17 @@ eb_select_engine(struct drm_i915_private *dev_priv, >> return engine; >> } >> >> +static void >> +add_to_client(struct drm_i915_gem_request *req, struct drm_file *file) >> +{ >> + struct drm_i915_file_private *file_priv = file->driver_priv; >> + >> + spin_lock(&file_priv->mm.lock); >> + req->file_priv = file_priv; >> + list_add_tail(&req->client_list, &file_priv->mm.request_list); >> + spin_unlock(&file_priv->mm.lock); >> +} >> + >> static int >> i915_gem_do_execbuffer(struct drm_device *dev, void *data, >> struct drm_file *file, >> @@ -1824,9 +1835,7 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data, >> */ >> params->request->batch = params->batch; >> >> - ret = i915_gem_request_add_to_client(params->request, file); >> - if (ret) >> - goto err_request; >> + add_to_client(params->request, file); > So we add to the client list even if we fail to submit the batch? For > that reason I put the call to add_to_client() in execbuf_submit. Same as existing code, that was just trimming the useless bits. If you are reworking all this shortly lets drop this then. Regards, Tvrtko _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev3) [not found] <0160927121915.GU28107@nuc-i3427.alporthouse.com> 2016-09-28 16:48 ` [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client Tvrtko Ursulin @ 2016-09-28 17:28 ` Patchwork 2016-09-29 8:28 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev4) Patchwork 2 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2016-09-28 17:28 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: drm/i915: Cleanup i915_gem_request_add_to_client (rev3) URL : https://patchwork.freedesktop.org/series/12959/ State : success == Summary == Series 12959v3 drm/i915: Cleanup i915_gem_request_add_to_client https://patchwork.freedesktop.org/api/1.0/series/12959/revisions/3/mbox/ fi-bdw-5557u total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-bsw-n3050 total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-bxt-t5700 total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-byt-j1900 total:33 pass:27 dwarn:0 dfail:1 fail:0 skip:4 fi-byt-n2820 total:33 pass:23 dwarn:0 dfail:1 fail:0 skip:8 fi-hsw-4770 total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-hsw-4770r total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-ilk-650 total:33 pass:12 dwarn:0 dfail:1 fail:0 skip:19 fi-ivb-3520m total:33 pass:27 dwarn:0 dfail:1 fail:0 skip:4 fi-ivb-3770 total:33 pass:27 dwarn:0 dfail:1 fail:0 skip:4 fi-skl-6260u total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-skl-6700hq total:33 pass:25 dwarn:0 dfail:1 fail:0 skip:6 fi-skl-6700k total:33 pass:23 dwarn:1 dfail:1 fail:0 skip:7 fi-skl-6770hq total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-snb-2520m total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-snb-2600 total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 Results at /archive/results/CI_IGT_test/Patchwork_2588/ edf68a3aacccc2ad1ff8915fcc8011ad43d53be9 drm-intel-nightly: 2016y-09m-28d-15h-13m-34s UTC integration manifest effabab drm/i915: Cleanup i915_gem_request_add_to_client _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev4) [not found] <0160927121915.GU28107@nuc-i3427.alporthouse.com> 2016-09-28 16:48 ` [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client Tvrtko Ursulin 2016-09-28 17:28 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev3) Patchwork @ 2016-09-29 8:28 ` Patchwork 2 siblings, 0 replies; 8+ messages in thread From: Patchwork @ 2016-09-29 8:28 UTC (permalink / raw) To: Tvrtko Ursulin; +Cc: intel-gfx == Series Details == Series: drm/i915: Cleanup i915_gem_request_add_to_client (rev4) URL : https://patchwork.freedesktop.org/series/12959/ State : success == Summary == Series 12959v4 drm/i915: Cleanup i915_gem_request_add_to_client https://patchwork.freedesktop.org/api/1.0/series/12959/revisions/4/mbox/ fi-bdw-5557u total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-bsw-n3050 total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-bxt-t5700 total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-byt-j1900 total:33 pass:27 dwarn:0 dfail:1 fail:0 skip:4 fi-byt-n2820 total:33 pass:23 dwarn:0 dfail:1 fail:0 skip:8 fi-hsw-4770 total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-hsw-4770r total:33 pass:24 dwarn:0 dfail:1 fail:0 skip:7 fi-ilk-650 total:33 pass:12 dwarn:0 dfail:1 fail:0 skip:19 fi-ivb-3520m total:33 pass:27 dwarn:0 dfail:1 fail:0 skip:4 fi-ivb-3770 total:33 pass:27 dwarn:0 dfail:1 fail:0 skip:4 fi-skl-6260u total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-skl-6700hq total:33 pass:25 dwarn:0 dfail:1 fail:0 skip:6 fi-skl-6700k total:33 pass:23 dwarn:1 dfail:1 fail:0 skip:7 fi-skl-6770hq total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-snb-2520m total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 fi-snb-2600 total:33 pass:26 dwarn:0 dfail:1 fail:0 skip:5 Results at /archive/results/CI_IGT_test/Patchwork_2592/ edf68a3aacccc2ad1ff8915fcc8011ad43d53be9 drm-intel-nightly: 2016y-09m-28d-15h-13m-34s UTC integration manifest 987fded drm/i915: Cleanup i915_gem_request_add_to_client _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/intel-gfx ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-09-29 8:28 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <0160927121915.GU28107@nuc-i3427.alporthouse.com> 2016-09-28 16:48 ` [PATCH v2] drm/i915: Cleanup i915_gem_request_add_to_client Tvrtko Ursulin 2016-09-28 17:09 ` Chris Wilson 2016-09-29 7:44 ` Tvrtko Ursulin 2016-09-29 7:53 ` [PATCH v3] " Tvrtko Ursulin 2016-09-29 8:08 ` Chris Wilson 2016-09-29 8:13 ` Tvrtko Ursulin 2016-09-28 17:28 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev3) Patchwork 2016-09-29 8:28 ` ✓ Fi.CI.BAT: success for drm/i915: Cleanup i915_gem_request_add_to_client (rev4) Patchwork
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.