RE: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
Thanks for the review, Inki Dae -Original Message- From: linux-fbdev-ow...@vger.kernel.org [mailto:linux-fbdev- ow...@vger.kernel.org] On Behalf Of Konrad Rzeszutek Wilk Sent: Wednesday, August 21, 2013 4:22 AM To: Inki Dae Cc: dri-de...@lists.freedesktop.org; linux-fb...@vger.kernel.org; linux- arm-ker...@lists.infradead.org; linux-media@vger.kernel.org; linaro- ker...@lists.linaro.org; kyungmin.p...@samsung.com; myungjoo@samsung.com Subject: Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework On Tue, Aug 13, 2013 at 06:19:35PM +0900, Inki Dae wrote: This patch adds a buffer synchronization framework based on DMA BUF[1] and and based on ww-mutexes[2] for lock mechanism. The purpose of this framework is to provide not only buffer access control to CPU and DMA but also easy-to-use interfaces for device drivers and user application. This framework can be used for all dma devices using system memory as dma buffer, especially for most ARM based SoCs. Changelog v6: - Fix sync lock to multiple reads. - Add select system call support. . Wake up poll_wait when a dmabuf is unlocked. - Remove unnecessary the use of mutex lock. - Add private backend ops callbacks. . This ops has one callback for device drivers to clean up their sync object resource when the sync object is freed. For this, device drivers should implement the free callback properly. - Update document file. Changelog v5: - Rmove a dependence on reservation_object: the reservation_object is used to hook up to ttm and dma-buf for easy sharing of reservations across devices. However, the dmabuf sync can be used for all dma devices; v4l2 and drm based drivers, so doesn't need the reservation_object anymore. With regared to this, it adds 'void *sync' to dma_buf structure. - All patches are rebased on mainline, Linux v3.10. Changelog v4: - Add user side interface for buffer synchronization mechanism and update descriptions related to the user side interface. Changelog v3: - remove cache operation relevant codes and update document file. Changelog v2: - use atomic_add_unless to avoid potential bug. - add a macro for checking valid access type. - code clean. The mechanism of this framework has the following steps, 1. Register dmabufs to a sync object - A task gets a new sync object and can add one or more dmabufs that the task wants to access. This registering should be performed when a device context or an event context such as a page flip event is created or before CPU accesses a shared buffer. dma_buf_sync_get(a sync object, a dmabuf); 2. Lock a sync object - A task tries to lock all dmabufs added in its own sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA and DMA. Taking a lock means that others cannot access all locked dmabufs until the task that locked the corresponding dmabufs, unlocks all the locked dmabufs. This locking should be performed before DMA or CPU accesses these dmabufs. dma_buf_sync_lock(a sync object); 3. Unlock a sync object - The task unlocks all dmabufs added in its own sync object. The unlock means that the DMA or CPU accesses to the dmabufs have been completed so that others may access them. This unlocking should be performed after DMA or CPU has completed accesses to the dmabufs. dma_buf_sync_unlock(a sync object); 4. Unregister one or all dmabufs from a sync object - A task unregisters the given dmabufs from the sync object. This means that the task dosen't want to lock the dmabufs. The unregistering should be performed after DMA or CPU has completed accesses to the dmabufs or when dma_buf_sync_lock() is failed. dma_buf_sync_put(a sync object, a dmabuf); dma_buf_sync_put_all(a sync object); The described steps may be summarized as: get - lock - CPU or DMA access to a buffer/s - unlock - put This framework includes the following two features. 1. read (shared) and write (exclusive) locks - A task is required to declare the access type when the task tries to register a dmabuf; READ, WRITE, READ DMA, or WRITE DMA. The below is example codes, struct dmabuf_sync *sync; sync = dmabuf_sync_init(...); ... dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R); ... And the below can be used as access types: DMA_BUF_ACCESS_R - CPU will access a buffer for read. DMA_BUF_ACCESS_W - CPU will access a buffer for read or write. DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read
Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
+EXPORT_SYMBOL(is_dmabuf_sync_supported); _GPL ? I would also prefix it with 'dmabuf_is_sync_supported' just to make all of the libraries call start with 'dmabuf' Seems better. Will change it to dmabuf_is_sync_supported, and use EXPORT_SYMBOL_GPL. One thing thought - while I suggest that you use GPL variant I think you should check who the consumers are. As in, if nvidia wants to use it it might make their lawyers unhappy - and in turn means that their engineers won't be able to use these symbols. So - if there is a strong argument to not have it GPL - then please say so. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
On Tue, Aug 13, 2013 at 06:19:35PM +0900, Inki Dae wrote:
This patch adds a buffer synchronization framework based on DMA BUF[1]
and and based on ww-mutexes[2] for lock mechanism.
The purpose of this framework is to provide not only buffer access control
to CPU and DMA but also easy-to-use interfaces for device drivers and
user application. This framework can be used for all dma devices using
system memory as dma buffer, especially for most ARM based SoCs.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
The mechanism of this framework has the following steps,
1. Register dmabufs to a sync object - A task gets a new sync object and
can add one or more dmabufs that the task wants to access.
This registering should be performed when a device context or an event
context such as a page flip event is created or before CPU accesses a
shared
buffer.
dma_buf_sync_get(a sync object, a dmabuf);
2. Lock a sync object - A task tries to lock all dmabufs added in its own
sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead
lock issue and for race condition between CPU and CPU, CPU and DMA, and
DMA
and DMA. Taking a lock means that others cannot access all locked dmabufs
until the task that locked the corresponding dmabufs, unlocks all the
locked
dmabufs.
This locking should be performed before DMA or CPU accesses these dmabufs.
dma_buf_sync_lock(a sync object);
3. Unlock a sync object - The task unlocks all dmabufs added in its own
sync
object. The unlock means that the DMA or CPU accesses to the dmabufs have
been completed so that others may access them.
This unlocking should be performed after DMA or CPU has completed accesses
to the dmabufs.
dma_buf_sync_unlock(a sync object);
4. Unregister one or all dmabufs from a sync object - A task unregisters
the given dmabufs from the sync object. This means that the task dosen't
want to lock the dmabufs.
The unregistering should be performed after DMA or CPU has completed
accesses to the dmabufs or when dma_buf_sync_lock() is failed.
dma_buf_sync_put(a sync object, a dmabuf);
dma_buf_sync_put_all(a sync object);
The described steps may be summarized as:
get - lock - CPU or DMA access to a buffer/s - unlock - put
This framework includes the following two features.
1. read (shared) and write (exclusive) locks - A task is required to
declare
the access type when the task tries to register a dmabuf;
READ, WRITE, READ DMA, or WRITE DMA.
The below is example codes,
struct dmabuf_sync *sync;
sync = dmabuf_sync_init(...);
...
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
...
And the below can be used as access types:
DMA_BUF_ACCESS_R - CPU will access a buffer for read.
DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
write.
2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
A task may never try to unlock a buffer after taking a lock to the buffer.
In this case, a timer handler to the corresponding sync object is called
in five (default) seconds and then the timed-out buffer is unlocked by
work
queue handler to avoid lockups and to enforce resources of the buffer.
The below is how to use interfaces for device driver:
1. Allocate and Initialize a sync object:
static void xxx_dmabuf_sync_free(void *priv)
{
[PATCH 1/2] [RFC PATCH v6] dmabuf-sync: Add a buffer synchronization framework
This patch adds a buffer synchronization framework based on DMA BUF[1]
and and based on ww-mutexes[2] for lock mechanism.
The purpose of this framework is to provide not only buffer access control
to CPU and DMA but also easy-to-use interfaces for device drivers and
user application. This framework can be used for all dma devices using
system memory as dma buffer, especially for most ARM based SoCs.
Changelog v6:
- Fix sync lock to multiple reads.
- Add select system call support.
. Wake up poll_wait when a dmabuf is unlocked.
- Remove unnecessary the use of mutex lock.
- Add private backend ops callbacks.
. This ops has one callback for device drivers to clean up their
sync object resource when the sync object is freed. For this,
device drivers should implement the free callback properly.
- Update document file.
Changelog v5:
- Rmove a dependence on reservation_object: the reservation_object is used
to hook up to ttm and dma-buf for easy sharing of reservations across
devices. However, the dmabuf sync can be used for all dma devices; v4l2
and drm based drivers, so doesn't need the reservation_object anymore.
With regared to this, it adds 'void *sync' to dma_buf structure.
- All patches are rebased on mainline, Linux v3.10.
Changelog v4:
- Add user side interface for buffer synchronization mechanism and update
descriptions related to the user side interface.
Changelog v3:
- remove cache operation relevant codes and update document file.
Changelog v2:
- use atomic_add_unless to avoid potential bug.
- add a macro for checking valid access type.
- code clean.
The mechanism of this framework has the following steps,
1. Register dmabufs to a sync object - A task gets a new sync object and
can add one or more dmabufs that the task wants to access.
This registering should be performed when a device context or an event
context such as a page flip event is created or before CPU accesses a shared
buffer.
dma_buf_sync_get(a sync object, a dmabuf);
2. Lock a sync object - A task tries to lock all dmabufs added in its own
sync object. Basically, the lock mechanism uses ww-mutex[1] to avoid dead
lock issue and for race condition between CPU and CPU, CPU and DMA, and DMA
and DMA. Taking a lock means that others cannot access all locked dmabufs
until the task that locked the corresponding dmabufs, unlocks all the locked
dmabufs.
This locking should be performed before DMA or CPU accesses these dmabufs.
dma_buf_sync_lock(a sync object);
3. Unlock a sync object - The task unlocks all dmabufs added in its own sync
object. The unlock means that the DMA or CPU accesses to the dmabufs have
been completed so that others may access them.
This unlocking should be performed after DMA or CPU has completed accesses
to the dmabufs.
dma_buf_sync_unlock(a sync object);
4. Unregister one or all dmabufs from a sync object - A task unregisters
the given dmabufs from the sync object. This means that the task dosen't
want to lock the dmabufs.
The unregistering should be performed after DMA or CPU has completed
accesses to the dmabufs or when dma_buf_sync_lock() is failed.
dma_buf_sync_put(a sync object, a dmabuf);
dma_buf_sync_put_all(a sync object);
The described steps may be summarized as:
get - lock - CPU or DMA access to a buffer/s - unlock - put
This framework includes the following two features.
1. read (shared) and write (exclusive) locks - A task is required to declare
the access type when the task tries to register a dmabuf;
READ, WRITE, READ DMA, or WRITE DMA.
The below is example codes,
struct dmabuf_sync *sync;
sync = dmabuf_sync_init(...);
...
dmabuf_sync_get(sync, dmabuf, DMA_BUF_ACCESS_R);
...
And the below can be used as access types:
DMA_BUF_ACCESS_R - CPU will access a buffer for read.
DMA_BUF_ACCESS_W - CPU will access a buffer for read or write.
DMA_BUF_ACCESS_DMA_R - DMA will access a buffer for read
DMA_BUF_ACCESS_DMA_W - DMA will access a buffer for read or
write.
2. Mandatory resource releasing - a task cannot hold a lock indefinitely.
A task may never try to unlock a buffer after taking a lock to the buffer.
In this case, a timer handler to the corresponding sync object is called
in five (default) seconds and then the timed-out buffer is unlocked by work
queue handler to avoid lockups and to enforce resources of the buffer.
The below is how to use interfaces for device driver:
1. Allocate and Initialize a sync object:
static void xxx_dmabuf_sync_free(void *priv)
{
struct xxx_context *ctx = priv;
if (!ctx)
return;
