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
