Re: [PATCH 14/53] drm/i915: Update pin_to_display_plane() to do explicit request management

Tomas Elf
2015-03-05T16:23:01Z

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
>