[Intel-gfx] [RFC 02/12] staging/android/sync: add sync_fence_create_dma

[John Harrison]

On 23/11/2015 13:27, Maarten Lankhorst wrote:
> Op 23-11-15 om 12:34 schreef John.C.Harrison at Intel.com:
>> From: Maarten Lankhorst <maarten.lankhorst at canonical.com>
>>
>> This allows users of dma fences to create a android fence.
>>
>> v2: Added kerneldoc. (Tvrtko Ursulin).
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at canonical.com>
>> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
>> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
>> Cc: Daniel Vetter <daniel at ffwll.ch>
>> Cc: Jesse Barnes <jbarnes at virtuousgeek.org>
>> Cc: devel at driverdev.osuosl.org
>> Cc: Riley Andrews <riandrews at android.com>
>> Cc: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
>> Cc: Arve Hjønnevåg <arve at android.com>
>> ---
>>   drivers/staging/android/sync.c | 13 +++++++++----
>>   drivers/staging/android/sync.h | 12 +++++++++++-
>>   2 files changed, 20 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/staging/android/sync.c b/drivers/staging/android/sync.c
>> index f83e00c..7f0e919 100644
>> --- a/drivers/staging/android/sync.c
>> +++ b/drivers/staging/android/sync.c
>> @@ -188,7 +188,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;
>>   
>> @@ -199,16 +199,21 @@ struct sync_fence *sync_fence_create(const char *name, struct sync_pt *pt)
>>   	fence->num_fences = 1;
>>   	atomic_set(&fence->status, 1);
>>   
>> -	fence->cbs[0].sync_pt = &pt->base;
>> +	fence->cbs[0].sync_pt = 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 61f8a3a..798cd56 100644
>> --- a/drivers/staging/android/sync.h
>> +++ b/drivers/staging/android/sync.h
>> @@ -250,10 +250,20 @@ 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(const char *name, struct sync_pt *pt);
> No it doesn't.
>> +/**
>> + * sync_fence_create_dma() - creates a sync fence from dma-fence
>> + * @name:	name of fence to create
>> + * @pt:	dma-fence to add to the fence
>> + *
>> + * Creates a fence containg @pt.  Once this is called, the fence takes
>> + * a reference on @pt.
>> + */
> No it doesn't.
This is your patch isn't it? Or is this something Tvrtko added on the 
way past? Either way, what should the correct description be? It takes a 
copy of the pointer to 'pt'. Is the comment meaning a reference in the 
sense of 'pass by reference', i.e. a pointer? But you are meaning a 
reference in the sense of incrementing a usage count?

>> +struct sync_fence *sync_fence_create_dma(const char *name, struct fence *pt);
>> +
>>   /*
>>    * API for sync_fence consumers
>>    */