Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

[Colin Cross]

On Thu, Mar 23, 2017 at 4:56 PM, Dylan Baker  wrote:
> Quoting Colin Cross (2017-03-23 15:14:16)
>> On Thu, Mar 23, 2017 at 2:56 PM, Greg Hackmann  wrote:
>> > On 03/22/2017 02:48 PM, Rob Clark wrote:
>> >>
>> >> On Wed, Mar 22, 2017 at 4:10 PM, Dylan Baker  wrote:
>> >>>
>> >>> Quoting Rob Clark (2017-03-22 10:07:54)
>> >>>>
>> >>>> I guess an interesting question (from someone who doesn't know meson
>> >>>> yet) would be whether meson could slurp in the Makefile.sources type
>> >>>> stuff that we have, which are shared so far between
>> >>>> android/scons/autotools (and for the most part, kept developers from
>> >>>> having to care *too* much about the different build systems)
>> >>>
>> >>>
>> >>> Jason and I have talked about that too. I'd suggested that we could write
>> >>> a
>> >>> module for meson to read makefile.sources (since we're surely not the
>> >>> only
>> >>> project that would benefit from such a module), except that android is
>> >>> moving to
>> >>> blueprint[1] instead of android.mk files. As far as I can tell blueprint
>> >>> doesn't
>> >>> support using makefile.sources, so it seems somewhat moot in a world of
>> >>> blueprint for android, meson for *.
>> >>
>> >>
>> >> I guess even if it is only a temporary thing, something that could
>> >> slurp in Makefile.sources seems like it would be useful for a
>> >> transition period.
>> >>
>> >> I'm not totally up to speed on android/blueprint stuff.. but even some
>> >> simplified or different "here-are-my-sources" type file that could be
>> >> shared across build systems would be useful.  Meson sounds a bit more
>> >> extensible so maybe there is some potential to adapt to whatever
>> >> android forces on us ;-)
>> >
>> >
>> > +ccross from the Android build team can hopefully provide some input here.
>>
>> For cases like this, we generally add a python script that translates
>> the build files into something Blueprint understands, and rerun it
>> each time we import into the project.
>>
>> Alternatively, Blueprint efficiently supports globbing for sources, so
>> if all the source files are always listed in Makefile.sources (which
>> seems to be true in our copy of libdrm except for intel/test_decode.c)
>> then we could use globs and ignore the makefiles, possibly manually
>> excluding a few files.
>
> I'm hoping you can clarify a couple of questions I have about blueprint:
> 1) android is moving to blueprint from android.mk files?

Yes, in a phased transition.  We support both for now.

> 2) is there a publicly available project that uses blueprint I could look at?
>The documentation I've been able to find is pretty sparse.

Blueprint is a framework for writing build systems, and Soong is
AOSP's Blueprint build system.  See
https://android.googlesource.com/platform/build/soong/+/master/README.md
for documentation on Soong.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [Mesa-dev] [RFC libdrm 0/2] Replace the build system with meson

[Colin Cross]

On Thu, Mar 23, 2017 at 2:56 PM, Greg Hackmann  wrote:
> On 03/22/2017 02:48 PM, Rob Clark wrote:
>>
>> On Wed, Mar 22, 2017 at 4:10 PM, Dylan Baker  wrote:
>>>
>>> Quoting Rob Clark (2017-03-22 10:07:54)

 I guess an interesting question (from someone who doesn't know meson
 yet) would be whether meson could slurp in the Makefile.sources type
 stuff that we have, which are shared so far between
 android/scons/autotools (and for the most part, kept developers from
 having to care *too* much about the different build systems)
>>>
>>>
>>> Jason and I have talked about that too. I'd suggested that we could write
>>> a
>>> module for meson to read makefile.sources (since we're surely not the
>>> only
>>> project that would benefit from such a module), except that android is
>>> moving to
>>> blueprint[1] instead of android.mk files. As far as I can tell blueprint
>>> doesn't
>>> support using makefile.sources, so it seems somewhat moot in a world of
>>> blueprint for android, meson for *.
>>
>>
>> I guess even if it is only a temporary thing, something that could
>> slurp in Makefile.sources seems like it would be useful for a
>> transition period.
>>
>> I'm not totally up to speed on android/blueprint stuff.. but even some
>> simplified or different "here-are-my-sources" type file that could be
>> shared across build systems would be useful.  Meson sounds a bit more
>> extensible so maybe there is some potential to adapt to whatever
>> android forces on us ;-)
>
>
> +ccross from the Android build team can hopefully provide some input here.

For cases like this, we generally add a python script that translates
the build files into something Blueprint understands, and rerun it
each time we import into the project.

Alternatively, Blueprint efficiently supports globbing for sources, so
if all the source files are always listed in Makefile.sources (which
seems to be true in our copy of libdrm except for intel/test_decode.c)
then we could use globs and ignore the makefiles, possibly manually
excluding a few files.
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Colin Cross]

On Thu, Jun 19, 2014 at 5:28 AM, Daniel Vetter  wrote:
> On Thu, Jun 19, 2014 at 1:48 PM, Thierry Reding
>  wrote:
>>> > With these changes, can we pull the android sync logic out of
>>> > drivers/staging/ now?
>>>
>>> Afaik the google guys never really looked at this and acked it. So I'm not
>>> sure whether they'll follow along. The other issue I have as the
>>> maintainer of gfx driver is that I don't want to implement support for two
>>> different sync object primitives (once for dma-buf and once for android
>>> syncpts), and my impression thus far has been that even with this we're
>>> not there.
>>>
>>> I'm trying to get our own android guys to upstream their i915 syncpts
>>> support, but thus far I haven't managed to convince them to throw people's
>>> time at this.
>>
>> This has been discussed a fair bit internally recently and some of our
>> GPU experts have raised concerns that this may result in seriously
>> degraded performance in our proprietary graphics stack. Now I don't care
>> very much for the proprietary graphics stack, but by extension I would
>> assume that the same restrictions are relevant for any open-source
>> driver as well.
>>
>> I'm still trying to fully understand all the implications and at the
>> same time get some of the people who raised concerns to join in this
>> discussion. As I understand it the concern is mostly about explicit vs.
>> implicit synchronization and having this mechanism in the kernel will
>> implicitly synchronize all accesses to these buffers even in cases where
>> it's not needed (read vs. write locks, etc.). In one particular instance
>> it was even mentioned that this kind of implicit synchronization can
>> lead to deadlocks in some use-cases (this was mentioned for Android
>> compositing, but I suspect that the same may happen for Wayland or X
>> compositors).
>
> Well the implicit fences here actually can't deadlock. That's the
> entire point behind using ww mutexes. I've also heard tons of
> complaints about implicit enforced syncing (especially from opencl
> people), but in the end drivers and always expose unsynchronized
> access for specific cases. We do that in i915 for upload buffers and
> other fun stuff. This is about shared stuff across different drivers
> and different processes.
>
> I also expect that i915 will loose implicit syncing in a few upcoming
> hw modes because explicit syncing is a more natural fit there.
>
> All that isn't about the discussion at hand imo since no matter what
> i915 needs to have on internal representation for a bit of gpu work,
> and afaics right now we don't have that. With this patch android
> syncpts use Maarten's fences internally, but I can't freely exchange
> one for the other. So in i915 I still expect to get stuck with both of
> them, which is one too many.
>
> The other issue (and I haven't dug into details that much) I have with
> syncpts are some of the interface choices. Apparently you can commit a
> fence after creation (or at least the hw composer interface works like
> that) which means userspace can construct deadlocks with android
> syncpts if I'm not super careful in my driver. I haven't seen any
> generic code to do that, so I presume everyone just blindly trusts
> surface-flinger to not do that. Speaks of the average quality of an
> android gfx driver if the kernel is less trusted than the compositor
> in userspace ...

Android sync is designed not to allow userspace to deadlock the
kernel, a sync_pt should only get created by the kernel when it has
received a chunk of work that it expects to complete in the near
future.  The CONFIG_SW_SYNC_USER driver violates that by allowing
userspace to create and signal arbitrary sync points, but that is
intended only for testing sync.

> There's a few other things like exposing timestamps (which are tricky
> to do right, our driver is littered with wrong attempts) and other
> details.

Timestamps are necessary for vsync synchronization to reduce the frame latency.

> Finally I've never seen anyone from google or any android product guy
> push a real driver enabling for syncpts to upstream, and google itself
> has a bit a history of constantly exchanging their gfx framework for
> the next best thing. So I really doubt this is worthwhile to pursue in
> upstream with our essentially eternal api guarantees. At least until
> we see serious uptake from vendors and gfx driver guys. Unfortunately
> the Intel android folks are no exception here and haven't pushed
> anything like this in my direction yet at all. Despite multiple pokes
> from my side.

As far as I know, every SoC vendor that supports android is using sync
now, but none of them have succeeded in pushing their drivers upstream
for a variety of other reasons (interfaces only used by closed source
userspaces, KMS/DRM vs ADF, ION, etc.).

> So from my side I think we should move ahead with Maarten's work and
> figure the android side out once there's real interest.

As long as that doesn't involve r

[REPOST PATCH 4/8] android: convert sync to fence api, v5

[Colin Cross]

On Wed, Jun 18, 2014 at 11:37 PM, Daniel Vetter  wrote:
> On Wed, Jun 18, 2014 at 06:15:56PM -0700, Greg KH wrote:
>> On Wed, Jun 18, 2014 at 12:37:11PM +0200, Maarten Lankhorst wrote:
>> > Just to show it's easy.
>> >
>> > Android syncpoints can be mapped to a timeline. This removes the need
>> > to maintain a separate api for synchronization. I've left the android
>> > trace events in place, but the core fence events should already be
>> > sufficient for debugging.
>> >
>> > v2:
>> > - Call fence_remove_callback in sync_fence_free if not all fences have 
>> > fired.
>> > v3:
>> > - Merge Colin Cross' bugfixes, and the android fence merge optimization.
>> > v4:
>> > - Merge with the upstream fixes.
>> > v5:
>> > - Fix small style issues pointed out by Thomas Hellstrom.
>> >
>> > Signed-off-by: Maarten Lankhorst 
>> > Acked-by: John Stultz 
>> > ---
>> >  drivers/staging/android/Kconfig  |1
>> >  drivers/staging/android/Makefile |2
>> >  drivers/staging/android/sw_sync.c|6
>> >  drivers/staging/android/sync.c   |  913 
>> > +++---
>> >  drivers/staging/android/sync.h   |   79 ++-
>> >  drivers/staging/android/sync_debug.c |  247 +
>> >  drivers/staging/android/trace/sync.h |   12
>> >  7 files changed, 609 insertions(+), 651 deletions(-)
>> >  create mode 100644 drivers/staging/android/sync_debug.c
>>
>> With these changes, can we pull the android sync logic out of
>> drivers/staging/ now?
>
> Afaik the google guys never really looked at this and acked it. So I'm not
> sure whether they'll follow along. The other issue I have as the
> maintainer of gfx driver is that I don't want to implement support for two
> different sync object primitives (once for dma-buf and once for android
> syncpts), and my impression thus far has been that even with this we're
> not there.

We have tested these patches to use dma fences to back the android
sync driver and not found any major issues.  However, my understanding
is that dma fences are designed for implicit sync, and explicit sync
through the android sync driver is bolted on the side to share code.
Android is not moving away from explicit sync, but we do wrap all of
our userspace sync accesses through libsync
(https://android.googlesource.com/platform/system/core/+/master/libsync/sync.c,
ignore the sw_sync parts), so if the kernel supported a slightly
different userspace explicit sync interface we could adapt to it
fairly easily.  All we require is that individual kernel drivers need
to be able to accept work alongisde an fd to wait on, and to return an
fd that will signal when the work is done, and that userspace has some
way to merge two of those fds, wait on an fd, and get some debugging
info from an fd.  However, this patch set doesn't do that, it has no
way to export a dma fence as an fd except through the android sync
driver, so it is not yet ready to fully replace android sync.

> I'm trying to get our own android guys to upstream their i915 syncpts
> support, but thus far I haven't managed to convince them to throw people's
> time at this.
>
> It looks like a step into the right direction, but until I have the proof
> in the form of i915 patches that I won't have to support 2 gfx fencing
> frameworks I'm opposed to de-staging android syncpts. Ofc someone else
> could do that too, but besides i915 I don't see a full-fledged (modeset
> side only kinda doesn't count) upstream gfx driver shipping on android.
> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH] dma-buf: avoid using IS_ERR_OR_NULL

[Colin Cross]

On Fri, Feb 7, 2014 at 8:43 AM, Greg Kroah-Hartman
 wrote:
> On Sat, Dec 21, 2013 at 07:42:17AM -0500, Rob Clark wrote:
>> On Fri, Dec 20, 2013 at 7:43 PM, Colin Cross  wrote:
>> > dma_buf_map_attachment and dma_buf_vmap can return NULL or
>> > ERR_PTR on a error.  This encourages a common buggy pattern in
>> > callers:
>> > sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
>> > if (IS_ERR_OR_NULL(sgt))
>> > return PTR_ERR(sgt);
>> >
>> > This causes the caller to return 0 on an error.  IS_ERR_OR_NULL
>> > is almost always a sign of poorly-defined error handling.
>> >
>> > This patch converts dma_buf_map_attachment to always return
>> > ERR_PTR, and fixes the callers that incorrectly handled NULL.
>> > There are a few more callers that were not checking for NULL
>> > at all, which would have dereferenced a NULL pointer later.
>> > There are also a few more callers that correctly handled NULL
>> > and ERR_PTR differently, I left those alone but they could also
>> > be modified to delete the NULL check.
>> >
>> > This patch also converts dma_buf_vmap to always return NULL.
>> > All the callers to dma_buf_vmap only check for NULL, and would
>> > have dereferenced an ERR_PTR and panic'd if one was ever
>> > returned. This is not consistent with the rest of the dma buf
>> > APIs, but matches the expectations of all of the callers.
>> >
>> > Signed-off-by: Colin Cross 
>> > ---
>> >  drivers/base/dma-buf.c | 18 +++---
>> >  drivers/gpu/drm/drm_prime.c|  2 +-
>> >  drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |  2 +-
>> >  drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
>> >  4 files changed, 14 insertions(+), 10 deletions(-)
>> >
>> > diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
>> > index 1e16cbd61da2..cfe1d8bc7bb8 100644
>> > --- a/drivers/base/dma-buf.c
>> > +++ b/drivers/base/dma-buf.c
>> > @@ -251,9 +251,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
>> >   * @dmabuf:[in]buffer to attach device to.
>> >   * @dev:   [in]device to be attached.
>> >   *
>> > - * Returns struct dma_buf_attachment * for this attachment; may return 
>> > negative
>> > - * error codes.
>> > - *
>> > + * Returns struct dma_buf_attachment * for this attachment; returns 
>> > ERR_PTR on
>> > + * error.
>> >   */
>> >  struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
>> >   struct device *dev)
>> > @@ -319,9 +318,8 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
>> >   * @attach:[in]attachment whose scatterlist is to be returned
>> >   * @direction: [in]direction of DMA transfer
>> >   *
>> > - * Returns sg_table containing the scatterlist to be returned; may return 
>> > NULL
>> > - * or ERR_PTR.
>> > - *
>> > + * Returns sg_table containing the scatterlist to be returned; returns 
>> > ERR_PTR
>> > + * on error.
>> >   */
>> >  struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
>> > enum dma_data_direction direction)
>> > @@ -334,6 +332,8 @@ struct sg_table *dma_buf_map_attachment(struct 
>> > dma_buf_attachment *attach,
>> > return ERR_PTR(-EINVAL);
>> >
>> > sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
>> > +   if (!sg_table)
>> > +   sg_table = ERR_PTR(-ENOMEM);
>> >
>> > return sg_table;
>> >  }
>> > @@ -544,6 +544,8 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
>> >   * These calls are optional in drivers. The intended use for them
>> >   * is for mapping objects linear in kernel space for high use objects.
>> >   * Please attempt to use kmap/kunmap before thinking about these 
>> > interfaces.
>> > + *
>> > + * Returns NULL on error.
>> >   */
>> >  void *dma_buf_vmap(struct dma_buf *dmabuf)
>> >  {
>> > @@ -566,7 +568,9 @@ void *dma_buf_vmap(struct dma_buf *dmabuf)
>> > BUG_ON(dmabuf->vmap_ptr);
>> >
>> > ptr = dmabuf->ops->vmap(dmabuf);
>> > -   if (IS_ERR_OR_NULL(ptr))
>> > +   if (WARN_ON_ONCE(IS_ERR(ptr)))
>>
>> since vmap is optional, the WARN_ON might be a bit strong..  although
>> it would be a bit strange for an exporter to supply a vmap fxn which
>> always returned NULL, not sure about that.  Just thought I'd mention
>> it in case anyone else had an opinion about that.
>
> Yeah, I don't like this, it could cause unnecessary reports of problems.

The WARN_ON_ONCE is only if the vmap op returns ERR_PTR, not if it
returns NULL.  This is designed to catch vmap ops that don't follow
the spec, so I would call them necessary reports, but I can take it
out if you still disagree.

[PATCH] dma-buf: avoid using IS_ERR_OR_NULL

[Colin Cross]

dma_buf_map_attachment and dma_buf_vmap can return NULL or
ERR_PTR on a error.  This encourages a common buggy pattern in
callers:
sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
if (IS_ERR_OR_NULL(sgt))
return PTR_ERR(sgt);

This causes the caller to return 0 on an error.  IS_ERR_OR_NULL
is almost always a sign of poorly-defined error handling.

This patch converts dma_buf_map_attachment to always return
ERR_PTR, and fixes the callers that incorrectly handled NULL.
There are a few more callers that were not checking for NULL
at all, which would have dereferenced a NULL pointer later.
There are also a few more callers that correctly handled NULL
and ERR_PTR differently, I left those alone but they could also
be modified to delete the NULL check.

This patch also converts dma_buf_vmap to always return NULL.
All the callers to dma_buf_vmap only check for NULL, and would
have dereferenced an ERR_PTR and panic'd if one was ever
returned. This is not consistent with the rest of the dma buf
APIs, but matches the expectations of all of the callers.

Signed-off-by: Colin Cross 
---
 drivers/base/dma-buf.c | 18 +++---
 drivers/gpu/drm/drm_prime.c|  2 +-
 drivers/gpu/drm/exynos/exynos_drm_dmabuf.c |  2 +-
 drivers/media/v4l2-core/videobuf2-dma-contig.c |  2 +-
 4 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/drivers/base/dma-buf.c b/drivers/base/dma-buf.c
index 1e16cbd61da2..cfe1d8bc7bb8 100644
--- a/drivers/base/dma-buf.c
+++ b/drivers/base/dma-buf.c
@@ -251,9 +251,8 @@ EXPORT_SYMBOL_GPL(dma_buf_put);
  * @dmabuf:[in]buffer to attach device to.
  * @dev:   [in]device to be attached.
  *
- * Returns struct dma_buf_attachment * for this attachment; may return negative
- * error codes.
- *
+ * Returns struct dma_buf_attachment * for this attachment; returns ERR_PTR on
+ * error.
  */
 struct dma_buf_attachment *dma_buf_attach(struct dma_buf *dmabuf,
  struct device *dev)
@@ -319,9 +318,8 @@ EXPORT_SYMBOL_GPL(dma_buf_detach);
  * @attach:[in]attachment whose scatterlist is to be returned
  * @direction: [in]direction of DMA transfer
  *
- * Returns sg_table containing the scatterlist to be returned; may return NULL
- * or ERR_PTR.
- *
+ * Returns sg_table containing the scatterlist to be returned; returns ERR_PTR
+ * on error.
  */
 struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
enum dma_data_direction direction)
@@ -334,6 +332,8 @@ struct sg_table *dma_buf_map_attachment(struct 
dma_buf_attachment *attach,
return ERR_PTR(-EINVAL);

sg_table = attach->dmabuf->ops->map_dma_buf(attach, direction);
+   if (!sg_table)
+   sg_table = ERR_PTR(-ENOMEM);

return sg_table;
 }
@@ -544,6 +544,8 @@ EXPORT_SYMBOL_GPL(dma_buf_mmap);
  * These calls are optional in drivers. The intended use for them
  * is for mapping objects linear in kernel space for high use objects.
  * Please attempt to use kmap/kunmap before thinking about these interfaces.
+ *
+ * Returns NULL on error.
  */
 void *dma_buf_vmap(struct dma_buf *dmabuf)
 {
@@ -566,7 +568,9 @@ void *dma_buf_vmap(struct dma_buf *dmabuf)
BUG_ON(dmabuf->vmap_ptr);

ptr = dmabuf->ops->vmap(dmabuf);
-   if (IS_ERR_OR_NULL(ptr))
+   if (WARN_ON_ONCE(IS_ERR(ptr)))
+   ptr = NULL;
+   if (!ptr)
goto out_unlock;

dmabuf->vmap_ptr = ptr;
diff --git a/drivers/gpu/drm/drm_prime.c b/drivers/gpu/drm/drm_prime.c
index 56805c39c906..bb516fdd195d 100644
--- a/drivers/gpu/drm/drm_prime.c
+++ b/drivers/gpu/drm/drm_prime.c
@@ -471,7 +471,7 @@ struct drm_gem_object *drm_gem_prime_import(struct 
drm_device *dev,
get_dma_buf(dma_buf);

sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
-   if (IS_ERR_OR_NULL(sgt)) {
+   if (IS_ERR(sgt)) {
ret = PTR_ERR(sgt);
goto fail_detach;
}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c 
b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
index 59827cc5e770..c786cd4f457b 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_dmabuf.c
@@ -224,7 +224,7 @@ struct drm_gem_object *exynos_dmabuf_prime_import(struct 
drm_device *drm_dev,
get_dma_buf(dma_buf);

sgt = dma_buf_map_attachment(attach, DMA_BIDIRECTIONAL);
-   if (IS_ERR_OR_NULL(sgt)) {
+   if (IS_ERR(sgt)) {
ret = PTR_ERR(sgt);
goto err_buf_detach;
}
diff --git a/drivers/media/v4l2-core/videobuf2-dma-contig.c 
b/drivers/media/v4l2-core/videobuf2-dma-contig.c
index 33d3871d1e13..880be0782dd9 100644
--- a/drivers/media/v4l2-core/videobuf2-dma-contig.c
+++ b/drivers/media/v4l2-core/videobuf2-dma-contig.c
@@ -719,7 +719,7 @@ static int vb2_dc

[PATCH 1/7] sched: allow try_to_wake_up to be used internally outside of core.c

[Colin Cross]

On Mon, Jan 13, 2014 at 4:31 AM, Maarten Lankhorst
 wrote:
> The kernel fence implementation doesn't use event queues, but needs
> to perform the same wake up. The symbol is not exported, since the
> fence implementation is not built as a module.
>
> Signed-off-by: Maarten Lankhorst 
> ---
>  include/linux/wait.h |1 +
>  kernel/sched/core.c  |2 +-
>  2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/wait.h b/include/linux/wait.h
> index eaa00b10abaa..c54e3ef50134 100644
> --- a/include/linux/wait.h
> +++ b/include/linux/wait.h
> @@ -12,6 +12,7 @@
>  typedef struct __wait_queue wait_queue_t;
>  typedef int (*wait_queue_func_t)(wait_queue_t *wait, unsigned mode, int 
> flags, void *key);
>  int default_wake_function(wait_queue_t *wait, unsigned mode, int flags, void 
> *key);
> +int try_to_wake_up(struct task_struct *p, unsigned int state, int 
> wake_flags);
>
>  struct __wait_queue {
> unsigned intflags;
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index a88f4a485c5e..f41d317042dd 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -1578,7 +1578,7 @@ static void ttwu_queue(struct task_struct *p, int cpu)
>   * Return: %true if @p was woken up, %false if it was already running.
>   * or @state didn't match @p's state.
>   */
> -static int
> +int
>  try_to_wake_up(struct task_struct *p, unsigned int state, int wake_flags)
>  {
> unsigned long flags;
>

wake_up_state is already available in linux/sched.h, can you use that?

[Linaro-mm-sig] thoughts of looking at android fences

[Colin Cross]

On Wed, Oct 30, 2013 at 5:17 AM, Maarten Lankhorst
 wrote:
> op 24-10-13 14:13, Maarten Lankhorst schreef:
>> So I actually tried to implement it now. I killed all the deprecated members 
>> and assumed a linear timeline.
>> This means that syncpoints can only be added at the end, not in between. In 
>> particular it means sw_sync
>> might be slightly broken.
>>
>> I only tested it with a simple program I wrote called ufence.c, it's in 
>> drivers/staging/android/ufence.c in the following tree:
>>
>> http://cgit.freedesktop.org/~mlankhorst/linux
>>
>> the "rfc: convert android to fence api" has all the changes from my 
>> dma-fence proposal to what android would need,
>> it also converts the userspace fence api to use the dma-fence api.
>>
>> sync_pt is implemented as fence too. This meant not having to convert all of 
>> android right away, though I did make some changes.
>> I killed the deprecated members and made all the fence calls forward to the 
>> sync_timeline_ops. dup and compare are no longer used.
>>
>> I haven't given this a spin on a full android kernel, only with the 
>> components that are in mainline kernel under staging and my dumb test 
>> program.
>>
>> ~Maarten
>>
>> PS: The nomenclature is very confusing. I want to rename dma-fence to 
>> syncpoint, but I want some feedback from the android devs first. :)
>>
> Come on, any feedback? I want to move the discussion forward.
>
> ~Maarten

I experimented with it a little on a device that uses sync and came
across a few bugs:
1.  sync_timeline_signal needs to call __fence_signal on all signaled
points on the timeline, not just the first
2.  fence_add_callback doesn't always initialize cb.node
3.  sync_fence_wait should take ms
4.  sync_print_pt status printing was incorrect
5.  there is a deadlock:
   sync_print_obj takes obj->child_list_lock
   sync_print_pt
   fence_is_signaled
   fence_signal takes fence->lock == obj->child_list_lock
6.  freeing a timeline before all the fences holding points on that
timeline have timed out results in a crash

With the attached patch to fix these issues, our libsync and sync_test
give the same results as with our sync code.  I haven't tested against
the full Android framework yet.

The compare op and timeline ordering is critical to the efficiency of
sync points on Android.  The compare op is used when merging fences to
drop all but the latest point on the same timeline.  This is necessary
for example when the same buffer is submitted to the display on
multiple frames, like when there is a live wallpaper in the background
updating at 60 fps and a static screen of widgets on top of it.  The
static widget buffer is submitted on every frame, returning a new
fence each time.  The compositor merges the new fence with the fence
for the previous buffer, and because they are on the same timeline it
merges down to a single point.  I experimented with disabling the
merge optimization on a Nexus 10, and found that leaving the screen on
running a live wallpaper eventually resulted in 100k outstanding sync
points.
-- next part --
A non-text attachment was scrubbed...
Name: 0001-dma-fence-fixes.patch
Type: text/x-patch
Size: 4962 bytes
Desc: not available
URL: