Re: [RFC v2 1/8] dma-buf/fence: add fence_collection fences

[Chris Wilson]

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