public inbox for intel-gfx@lists.freedesktop.org
 help / color / mirror / Atom feed
From: Maarten Lankhorst <maarten.lankhorst@canonical.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Android sync points for i915
Date: Fri, 01 Aug 2014 08:23:49 +0200	[thread overview]
Message-ID: <53DB3275.6010102@canonical.com> (raw)
In-Reply-To: <1406833088-25801-2-git-send-email-jbarnes@virtuousgeek.org>

Hey,
On 31-07-14 20:58, Jesse Barnes wrote:
> Expose an ioctl to create Android fences based on the Android sync point
> infrastructure (which in turn is based on DMA-buf fences).  Just a
> sketch at this point, no testing has been done.
>
> There are a couple of goals here:
>   1) allow applications and libraries to create fences without an
>      associated buffer
>   2) re-use a common API so userspace doesn't have to impedance mismatch
>      between different driver implementations too much
>   3) allow applications and libraries to use explicit synchronization if
>      they choose by exposing fences directly
>
Do you really need to use android timelines here? I think you should be 
able to use fence directly, and export android userspace fences anyway, 
you could easily wrap them.

~Maarten

Untested uncompiled patch for that below, looking from the documentation I may have
caused a memory leak by taking a reference on the sync_pt..
---
diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
index ba0d69e..201b088 100644
--- a/drivers/staging/android/sync.c
+++ b/drivers/staging/android/sync.c
@@ -179,7 +179,7 @@ static void fence_check_cb_func(struct fence *f, struct fence_cb *cb)
 }
 
 /* TODO: implement a create which takes more that one sync_pt */
-struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
+struct sync_fence *sync_fence_create_dma(const char *name, struct fence *pt)
 {
 	struct sync_fence *fence;
 
@@ -190,16 +190,21 @@ struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
 	fence->num_fences = 1;
 	atomic_set(&fence->status, 1);
 
-	fence_get(&pt->base);
-	fence->cbs[0].sync_pt = &pt->base;
+	fence->cbs[0].sync_pt = fence_get(pt);
 	fence->cbs[0].fence = fence;
-	if (fence_add_callback(&pt->base, &fence->cbs[0].cb, fence_check_cb_func))
+	if (fence_add_callback(pt, &fence->cbs[0].cb, fence_check_cb_func))
 		atomic_dec(&fence->status);
 
 	sync_fence_debug_add(fence);
 
 	return fence;
 }
+EXPORT_SYMBOL(sync_fence_create_dma);
+
+struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
+{
+	return sync_fence_create_dma(name, &pt->base);
+}
 EXPORT_SYMBOL(sync_fence_create);
 
 struct sync_fence *sync_fence_fdget(int fd)
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index bcdd119..6d1682a 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -242,8 +242,9 @@ void sync_pt_free(struct sync_pt *pt);
  * @pt:		sync_pt to add to the fence
  *
  * Creates a fence containg @pt.  Once this is called, the fence takes
- * ownership of @pt.
+ * a reference on @pt.
  */
+struct sync_fence *sync_fence_create_dma(const char *name, struct fence *pt);
 struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt);
 
 /*

  reply	other threads:[~2014-08-01  6:34 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-07-31 18:58 [RFC] Sync points/fences for i915 Jesse Barnes
2014-07-31 18:58 ` [PATCH] drm/i915: Android sync points " Jesse Barnes
2014-08-01  6:23   ` Maarten Lankhorst [this message]
2014-08-04 23:18     ` [PATCH] drm/i915: Android sync points for i915 v2 Jesse Barnes
2014-08-05  7:44       ` Daniel Vetter
2014-08-05 14:59         ` Jesse Barnes
2014-08-05 15:00           ` Maarten Lankhorst
2014-08-05 15:08           ` Daniel Vetter
2014-08-05 16:05             ` Jesse Barnes
2014-08-05 16:08               ` Daniel Vetter
2014-08-05 16:32                 ` Jesse Barnes
2014-08-05 17:09                 ` Jesse Barnes
2014-08-05 17:43                   ` Daniel Vetter
2014-08-05 17:52                     ` Jesse Barnes
2014-08-05  8:09       ` Maarten Lankhorst
2014-08-05 15:03         ` Jesse Barnes
2014-08-05 15:09           ` Daniel Vetter
2014-08-01  9:04   ` [PATCH] drm/i915: Android sync points for i915 Tvrtko Ursulin
2014-08-01 16:02     ` Jesse Barnes

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=53DB3275.6010102@canonical.com \
    --to=maarten.lankhorst@canonical.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=jbarnes@virtuousgeek.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox