On Mon, Apr 25, 2016 at 07:33:21PM -0300, Gustavo Padovan wrote:
> +static const char *fence_collection_get_timeline_name(struct fence *fence)
> +{
> +     return "no context";

"unbound" to distinguish from fence contexts within a timeline?

> +static bool fence_collection_enable_signaling(struct fence *fence)
> +{
> +     struct fence_collection *collection = to_fence_collection(fence);
> +     int i;
> +
> +     for (i = 0 ; i < collection->num_fences ; i++) {
> +             if (fence_add_callback(collection->fences[i].fence,
> +                                    &collection->fences[i].cb,
> +                                    collection_check_cb_func)) {
> +                     atomic_dec(&collection->num_pending_fences);
> +                     return false;

Don't stop, we need to enable all the others!

> +             }
> +     }
> +
> +     return !!atomic_read(&collection->num_pending_fences);

Redundant !!

> +}
> +
> +static bool fence_collection_signaled(struct fence *fence)
> +{
> +     struct fence_collection *collection = to_fence_collection(fence);
> +
> +     return (atomic_read(&collection->num_pending_fences) == 0);

Redundant ()

> +static signed long fence_collection_wait(struct fence *fence, bool intr,
> +                                      signed long timeout)
> +{

What advantage does this have over fence_default_wait? You enable
signaling on all, then wait sequentially. The code looks redundant and
could just use fence_default_wait instead.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Reply via email to