On Mon, Jun 27, 2016 at 04:29:22PM -0300, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
>
> Create sync_file->fence to abstract the type of fence we are using for
> each sync_file. If only one fence is present we use a normal struct fence
> but if there is more fences to be added to the sync_file a fence_array
> is created.
>
> This change cleans up sync_file a bit. We don't need to have sync_file_cb
> array anymore. Instead, as we always have one fence, only one fence
> callback is registered per sync_file.
>
> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Christian König <christian.koenig@xxxxxxx>
> Signed-off-by: Gustavo Padovan <gustavo.padovan@xxxxxxxxxxxxxxx>
> ---
> @@ -76,21 +76,19 @@ struct sync_file *sync_file_create(struct fence *fence)
> {
> struct sync_file *sync_file;
>
> - sync_file = sync_file_alloc(offsetof(struct sync_file, cbs[1]));
> + sync_file = sync_file_alloc();
> if (!sync_file)
> return NULL;
>
> - sync_file->num_fences = 1;
> + sync_file->fence = fence;
> +
> atomic_set(&sync_file->status, 1);
sync_file->status => fence_is_signaled(sync_file->fence);
Both should just be an atomic read, except fence_is_signaled() will then
do a secondary poll.
> snprintf(sync_file->name, sizeof(sync_file->name), "%s-%s%llu-%d",
> fence->ops->get_driver_name(fence),
> fence->ops->get_timeline_name(fence), fence->context,
> fence->seqno);
>
> - sync_file->cbs[0].fence = fence;
> - sync_file->cbs[0].sync_file = sync_file;
> - if (fence_add_callback(fence, &sync_file->cbs[0].cb,
> - fence_check_cb_func))
> + if (fence_add_callback(fence, &sync_file->cb, fence_check_cb_func))
> atomic_dec(&sync_file->status);
>
> return sync_file;
> @@ -121,14 +119,42 @@ err:
> return NULL;
> }
>
> -static void sync_file_add_pt(struct sync_file *sync_file, int *i,
> - struct fence *fence)
> +static int sync_file_set_fence(struct sync_file *sync_file,
> + struct fence **fences, int num_fences)
> {
> - sync_file->cbs[*i].fence = fence;
> - sync_file->cbs[*i].sync_file = sync_file;
> + struct fence_array *array;
> +
> + if (num_fences == 1) {
> + sync_file->fence = fences[0];
This steals the references.
> + } else {
> + array = fence_array_create(num_fences, fences,
> + fence_context_alloc(1), 1, false);
This creates a reference.
When we call fence_put(sync_fence->fence) we release a reference we
never owned if num_fences == 1.
> + struct fence **fences, **a_fences, **b_fences;
> + int i, i_a, i_b, num_fences, a_num_fences, b_num_fences;
>
> + a_fences = get_fences(a, &a_num_fences);
> + b_fences = get_fences(b, &b_num_fences);
> + num_fences = a_num_fences + b_num_fences;
> +
> + fences = kcalloc(num_fences, sizeof(**fences), GFP_KERNEL);
Just sizeof(*fences) (you want to allocate an array of pointers, not an
array of fence structs).
-Chris
--
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
