| Message ID | 1424366285-29232-15-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> > > Added explicit creation creation and submission of the request structure to the Nitpick: "creation creation" > display object pinning code. This removes any reliance on the OLR keeping track > of the request and the unknown randomness that can ensue with other work > becoming part of the same request. > > v2: Added semaphore enabled check to prevent allocating a pointless request > structure in the case where the sync just calls wait_rendering(). > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 4c29177..5897d54 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3943,9 +3943,24 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > int ret; > > if (pipelined != i915_gem_request_get_ring(obj->last_read_req)) { > - ret = i915_gem_object_sync(obj, pipelined); > - if (ret) > - return ret; > + if (!pipelined || !i915_semaphore_is_enabled(obj->base.dev)) { > + ret = i915_gem_object_wait_rendering(obj, false); > + } else { The call to i915_gem_object_wait_rendering() was taken from the implementation of i915_gem_object_sync() below. Ripping out code from the function you're calling just to do it in advance is not very nice. Just imagine a scenario where the i915_gem_object_sync() implementation were to change but the code outside the function would not. We need to figure out a better way of doing this. In fact, allocating and managing a request that never gets to be used inside the i915_gem_object_sync() function could be a better way to go since it poses less potential future problem even though it's more wasteful. But there are probably better ways of doing this. Thanks, Tomas > + struct drm_i915_private *dev_priv = pipelined->dev->dev_private; > + struct drm_i915_gem_request *req; > + > + ret = dev_priv->gt.alloc_request(pipelined, pipelined->default_context, &req); > + if (ret) > + return ret; > + > + ret = i915_gem_object_sync(obj, req->ring); > + if (ret) > + return ret; > + > + ret = i915_add_request_no_flush(req->ring); > + if (ret) > + return ret; > + } > } > > /* Mark the pin_display early so that we account for the >
On 19/02/2015 17:17, John.C.Harrison@Intel.com wrote: > From: John Harrison <John.C.Harrison@Intel.com> > > Added explicit creation creation and submission of the request structure to the > display object pinning code. This removes any reliance on the OLR keeping track > of the request and the unknown randomness that can ensue with other work > becoming part of the same request. > > v2: Added semaphore enabled check to prevent allocating a pointless request > structure in the case where the sync just calls wait_rendering(). > > For: VIZ-5115 > Signed-off-by: John Harrison <John.C.Harrison@Intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 21 ++++++++++++++++++--- > 1 file changed, 18 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 4c29177..5897d54 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -3943,9 +3943,24 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, > int ret; > > if (pipelined != i915_gem_request_get_ring(obj->last_read_req)) { > - ret = i915_gem_object_sync(obj, pipelined); > - if (ret) > - return ret; > + if (!pipelined || !i915_semaphore_is_enabled(obj->base.dev)) { > + ret = i915_gem_object_wait_rendering(obj, false); ... Also, the ret value here is never checked and returned. It's overwritten a few lines further down. Thanks, Tomas > + } else { > + struct drm_i915_private *dev_priv = pipelined->dev->dev_private; > + struct drm_i915_gem_request *req; > + > + ret = dev_priv->gt.alloc_request(pipelined, pipelined->default_context, &req); > + if (ret) > + return ret; > + > + ret = i915_gem_object_sync(obj, req->ring); > + if (ret) > + return ret; > + > + ret = i915_add_request_no_flush(req->ring); > + if (ret) > + return ret; > + } > } > > /* Mark the pin_display early so that we account for the >
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 4c29177..5897d54 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -3943,9 +3943,24 @@ i915_gem_object_pin_to_display_plane(struct drm_i915_gem_object *obj, int ret; if (pipelined != i915_gem_request_get_ring(obj->last_read_req)) { - ret = i915_gem_object_sync(obj, pipelined); - if (ret) - return ret; + if (!pipelined || !i915_semaphore_is_enabled(obj->base.dev)) { + ret = i915_gem_object_wait_rendering(obj, false); + } else { + struct drm_i915_private *dev_priv = pipelined->dev->dev_private; + struct drm_i915_gem_request *req; + + ret = dev_priv->gt.alloc_request(pipelined, pipelined->default_context, &req); + if (ret) + return ret; + + ret = i915_gem_object_sync(obj, req->ring); + if (ret) + return ret; + + ret = i915_add_request_no_flush(req->ring); + if (ret) + return ret; + } } /* Mark the pin_display early so that we account for the