Re: [PATCH 7/9] drm/i915: add fences to the request struct

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1.1: Type: text/plain, Size: 3475 bytes --]

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
 
-- 
dwmw2


[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 5691 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx