On 10/09/2015 06:29 AM, David Woodhouse wrote:
> On Fri, 2015-09-04 at 09:59 -0700, Jesse Barnes wrote:
>>
>> @@ -2286,6 +2287,10 @@ struct drm_i915_gem_request {
>> /** Execlists no. of times this request has been sent to the ELSP */
>> int elsp_submitted;
>>
>> + /* core fence obj for this request, may be exported */
>> + struct fence fence;
>
> As discussed, this doesn't work as-is. The final fence_put() will
> attempt to free(&req->fence). Unless you have a .release method in your
> fence ops, which you don't.
>
> I suppose we could tie up a .release method with the existing release
> method for the drm_i915_gem_request.
>
> As things stand, though, bad things are happening. This makes it go
> away and at least lets me get on with testing.
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8ef19e2..2d0c93c 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2297,7 +2298,7 @@ struct drm_i915_gem_request {
> int elsp_submitted;
>
> /* core fence obj for this request, may be exported */
> - struct fence fence;
> + struct fence *fence;
>
> wait_queue_t wait;
> };
> diff --git a/drivers/gpu/drm/i915/i915_sync.c b/drivers/gpu/drm/i915/i915_sync.c
> index 085f1f9..6ffe273 100644
> --- a/drivers/gpu/drm/i915/i915_sync.c
> +++ b/drivers/gpu/drm/i915/i915_sync.c
> @@ -58,7 +58,12 @@ struct i915_sync_timeline {
> * allow non-RCS fences (need ring/context association)
> */
>
> -#define to_i915_request(x) container_of(x, struct drm_i915_gem_request, fence)
> +struct foo {
> + struct fence fence;
> + struct drm_i915_gem_request *req;
> +};
> +
> +#define to_i915_request(x) (((struct foo *)(x))->req)
>
> static const char *i915_fence_get_driver_name(struct fence *fence)
> {
> @@ -81,10 +86,10 @@ static int i915_fence_ring_check(wait_queue_t *wait, unsigned mode, int flags,
> if (!i915_gem_request_completed(req, false))
> return 0;
>
> - fence_signal_locked(&req->fence);
> + fence_signal_locked(req->fence);
>
> __remove_wait_queue(&ring->irq_queue, wait);
> - fence_put(&req->fence);
> + fence_put(req->fence);
> ring->irq_put(ring);
>
> return 0;
> @@ -200,6 +205,15 @@ struct fence *i915_fence_create_ring(struct intel_engine_cs *ring,
> if (ret)
> return ERR_PTR(ret);
>
> + request->fence = kmalloc(sizeof(struct foo), GFP_KERNEL);
> + if (!request->fence) {
> + ret = -ENOMEM;
> + goto err_cancel;
> + }
> + /* I have no clue how this is *supposed* to work and no real interest
> + in finding out. Just stop hurting me please. */
> + ((struct foo *)request->fence)->req = request;
> +
> if (i915.enable_execlists) {
> ringbuf = ctx->engine[ring->id].ringbuf;
> } else
> @@ -270,10 +284,10 @@ struct fence *i915_fence_create_ring(struct intel_engine_cs *ring,
> round_jiffies_up_relative(HZ));
> intel_mark_busy(dev_priv->dev);
>
> - fence_init(&request->fence, &i915_fence_ring_ops, &fence_lock,
> + fence_init(request->fence, &i915_fence_ring_ops, &fence_lock,
> ctx->user_handle, request->seqno);
>
> - return &request->fence;
> + return request->fence;
>
> err_cancel:
> i915_gem_request_cancel(request);
> @@ -306,10 +320,10 @@ static struct fence *i915_fence_create_display(struct intel_context *ctx)
>
> req = ring->outstanding_lazy_request;
>
> - fence_init(&req->fence, &i915_fence_ops, &fence_lock,
> + fence_init(req->fence, &i915_fence_ops, &fence_lock,
> ctx->user_handle, req->seqno);
>
> - return &req->fence;
> + return req->fence;
> }
> #endif
Yeah this is definitely better than what I had (untested code and all
that). But the actual signaling and such still needs work. I had a
question for Maarten on that actually; today it doesn't look like the
fence would enabling signaling at the right point, so I had to add
something. But I'll look and see what the latest is here from John H; I
know his Android code worked, so it would probably be best to just use that.
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
