[PATCH v7 3/6] iio: core: Add new DMABUF interface infrastructure

[Paul Cercueil]

Le lundi 04 mars 2024 à 15:07 +0100, Christian König a écrit :
>  Am 04.03.24 um 14:59 schrieb Paul Cercueil:
>  
> > [SNIP]
> >  
> > >  
> > > >  
> > > > +	dma_to_ram = buffer->direction ==
> > > > IIO_BUFFER_DIRECTION_IN;
> > > > +
> > > > +	if (dma_to_ram) {
> > > > +		/*
> > > > +		 * If we're writing to the DMABUF, make sure
> > > > we
> > > > don't have
> > > > +		 * readers
> > > > +		 */
> > > > +		retl = dma_resv_wait_timeout(dmabuf->resv,
> > > > +					    
> > > > DMA_RESV_USAGE_READ,
> > > > true,
> > > > +					     timeout);
> > > > +		if (retl == 0)
> > > > +			retl = -EBUSY;
> > > > +		if (retl < 0) {
> > > > +			ret = (int)retl;
> > > > +			goto err_resv_unlock;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (buffer->access->lock_queue)
> > > > +		buffer->access->lock_queue(buffer);
> > > > +
> > > > +	ret = dma_resv_reserve_fences(dmabuf->resv, 1);
> > > > +	if (ret)
> > > > +		goto err_queue_unlock;
> > > > +
> > > > +	dma_resv_add_fence(dmabuf->resv, &fence->base,
> > > > +			   dma_resv_usage_rw(dma_to_ram));
> > > >  
> > >  
> > > That is incorrect use of the function dma_resv_usage_rw(). That
> > > function 
> > > is for retrieving fences and not adding them.
> > > 
> > > See the function implementation and comments, when you use it
> > > like
> > > this 
> > > you get exactly what you don't want.
> > > 
> > 
> > No, I get exactly what I want. If "dma_to_ram", I get
> > DMA_RESV_USAGE_READ, otherwise I get DMA_RESV_USAGE_WRITE.
> > 
> 
>  Ah, so basically !dma_to_ram means that you have a write access to
> the buffer?
>  

"dma_to_ram" means the data flows from the DMA to the RAM.

... Which means that it writes the buffer, so you are right, this is
wrong.

> > 
> > If you really don't like the macro, I can inline things here.
> 
>  Yeah, that would still be incorrect use.
>  
>  The dma__resv_usage_rw() is for retrieving the fences to sync to for
> read and write operations and should never be used together with
> dma_fence_resv_add().
> 

Ok, I'll inline it (and fix it) then.

Cheers,
-Paul