Re: [PATCH] dma-buf: Document non-dynamic exporter expectations better

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jun 21, 2021 at 05:53:46PM +0200, Christian König wrote:
> Am 21.06.21 um 17:17 schrieb Daniel Vetter:
> > Christian and me realized we have a pretty massive disconnect about
> > different interpretations of what dma_resv is used for by different
> > drivers. The discussion is much, much bigger than this change here,
> > but this is an important one:
> > 
> > Non-dynamic exporters must guarantee that the memory they return is
> > ready for use. They cannot expect importers to wait for the exclusive
> > fence. Only dynamic importers are required to obey the dma_resv fences
> > strictly (and more patches are needed to define exactly what this
> > means).
> > 
> > Christian has patches to update nouvea, radeon and amdgpu. The only
> > other driver using both ttm and supporting dma-buf export is qxl,
> > which only uses synchronous ttm_bo_move.
> > 
> > v2: To hammer this in document that dynamic importers _must_ wait for
> > the exclusive fence after having called dma_buf_map_attachment.
> > 
> > Cc: Christian König <ckoenig.leichtzumerken@xxxxxxxxx>
> > Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
> 
> Reviewed-by: Christian König <christian.koenig@xxxxxxx>

Applied to drm-misc-next, thanks for taking a look. Maybe when you merge
the actual bugfixes link to this patch as an explanation in each commit
message:

References: https://lore.kernel.org/dri-devel/20210621151758.2347474-1-daniel.vetter@xxxxxxxx/

That helps a bit with your rather terse commit messages.
-Daniel

> 
> > ---
> >   drivers/dma-buf/dma-buf.c |  3 +++
> >   include/linux/dma-buf.h   | 15 +++++++++++++++
> >   2 files changed, 18 insertions(+)
> > 
> > diff --git a/drivers/dma-buf/dma-buf.c b/drivers/dma-buf/dma-buf.c
> > index e3ba5db5f292..65cbd7f0f16a 100644
> > --- a/drivers/dma-buf/dma-buf.c
> > +++ b/drivers/dma-buf/dma-buf.c
> > @@ -956,6 +956,9 @@ EXPORT_SYMBOL_GPL(dma_buf_unpin);
> >    * the underlying backing storage is pinned for as long as a mapping exists,
> >    * therefore users/importers should not hold onto a mapping for undue amounts of
> >    * time.
> > + *
> > + * Important: Dynamic importers must wait for the exclusive fence of the struct
> > + * dma_resv attached to the DMA-BUF first.
> >    */
> >   struct sg_table *dma_buf_map_attachment(struct dma_buf_attachment *attach,
> >   					enum dma_data_direction direction)
> > diff --git a/include/linux/dma-buf.h b/include/linux/dma-buf.h
> > index 342585bd6dff..92eec38a03aa 100644
> > --- a/include/linux/dma-buf.h
> > +++ b/include/linux/dma-buf.h
> > @@ -96,6 +96,12 @@ struct dma_buf_ops {
> >   	 * This is called automatically for non-dynamic importers from
> >   	 * dma_buf_attach().
> >   	 *
> > +	 * Note that similar to non-dynamic exporters in their @map_dma_buf
> > +	 * callback the driver must guarantee that the memory is available for
> > +	 * use and cleared of any old data by the time this function returns.
> > +	 * Drivers which pipeline their buffer moves internally must wait for
> > +	 * all moves and clears to complete.
> > +	 *
> >   	 * Returns:
> >   	 *
> >   	 * 0 on success, negative error code on failure.
> > @@ -144,6 +150,15 @@ struct dma_buf_ops {
> >   	 * This is always called with the dmabuf->resv object locked when
> >   	 * the dynamic_mapping flag is true.
> >   	 *
> > +	 * Note that for non-dynamic exporters the driver must guarantee that
> > +	 * that the memory is available for use and cleared of any old data by
> > +	 * the time this function returns.  Drivers which pipeline their buffer
> > +	 * moves internally must wait for all moves and clears to complete.
> > +	 * Dynamic exporters do not need to follow this rule: For non-dynamic
> > +	 * importers the buffer is already pinned through @pin, which has the
> > +	 * same requirements. Dynamic importers otoh are required to obey the
> > +	 * dma_resv fences.
> > +	 *
> >   	 * Returns:
> >   	 *
> >   	 * A &sg_table scatter list of or the backing storage of the DMA buffer,
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux