Re: [PATCH 03/16] dma-buf: specify usage while adding fences to dma_resv obj v6

["Christian König" <christian.koenig@amd.com>]

Am 06.04.22 um 14:35 schrieb Daniel Vetter:
> On Wed, Apr 06, 2022 at 02:32:22PM +0200, Daniel Vetter wrote:
>> On Wed, Apr 06, 2022 at 09:51:19AM +0200, Christian König wrote:
>>> [SNIP]
>>> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>>> index 53f7c78628a4..98bb5c9239de 100644
>>> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>>> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
>>> @@ -202,14 +202,10 @@ static void submit_attach_object_fences(struct etnaviv_gem_submit *submit)
>>>   
>>>   	for (i = 0; i < submit->nr_bos; i++) {
>>>   		struct drm_gem_object *obj = &submit->bos[i].obj->base;
>>> +		bool write = submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE;
>>>   
>>> -		if (submit->bos[i].flags & ETNA_SUBMIT_BO_WRITE)
>>> -			dma_resv_add_excl_fence(obj->resv,
>>> -							  submit->out_fence);
>>> -		else
>>> -			dma_resv_add_shared_fence(obj->resv,
>>> -							    submit->out_fence);
>>> -
>>> +		dma_resv_add_fence(obj->resv, submit->out_fence, write ?
>>> +				   DMA_RESV_USAGE_WRITE : DMA_RESV_USAGE_READ);
>> Iirc I had some suggestions to use dma_resv_usage_rw here and above. Do
>> these happen in later patches? There's also a few more of these later on.

That won't work. dma_resv_usage_rw() returns the usage as necessary for 
dependencies. In other words write return DMA_RESV_USAGE_READ and read 
return DMA_RESV_USAGE_WRITE.

What we could do is to add a dma_resv_add_fence_rw() wrapper which does 
the necessary ?: in a central place.

>>>   
>>> diff --git a/drivers/gpu/drm/lima/lima_gem.c b/drivers/gpu/drm/lima/lima_gem.c
>>> index e0a11ee0e86d..cb3bfccc930f 100644
>>> --- a/drivers/gpu/drm/lima/lima_gem.c
>>> +++ b/drivers/gpu/drm/lima/lima_gem.c
>>> @@ -367,7 +367,7 @@ int lima_gem_submit(struct drm_file *file, struct lima_submit *submit)
>>>   		if (submit->bos[i].flags & LIMA_SUBMIT_BO_WRITE)
>>>   			dma_resv_add_excl_fence(lima_bo_resv(bos[i]), fence);
>>>   		else
>>> -			dma_resv_add_shared_fence(lima_bo_resv(bos[i]), fence);
>>> +			dma_resv_add_fence(lima_bo_resv(bos[i]), fence);
> Correction on the r-b, I'm still pretty sure that this won't compile at
> all.

Grrr, I've forgot to add CONFIG_OF to my compile build config.

BTW: Do we have a tool for compile test coverage of patches? E.g. 
something which figures out if a build created an o file for each c file 
a patch touched?

Christian.

> -Daniel
>