[RFC,v4,03/18] vb2: Move cache synchronisation from buffer done to dqbuf handler
Commit Message
The cache synchronisation may be a time consuming operation and thus not
best performed in an interrupt which is a typical context for
vb2_buffer_done() calls. This may consume up to tens of ms on some
machines, depending on the buffer size.
Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
---
drivers/media/v4l2-core/videobuf2-core.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
Comments
Hi Sakari, Hans,
On Tue, May 9, 2017 at 12:05 AM Sakari Ailus
<sakari.ailus@linux.intel.com> wrote:
>
> The cache synchronisation may be a time consuming operation and thus not
> best performed in an interrupt which is a typical context for
> vb2_buffer_done() calls. This may consume up to tens of ms on some
> machines, depending on the buffer size.
>
> Signed-off-by: Sakari Ailus <sakari.ailus@linux.intel.com>
> Acked-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
> drivers/media/v4l2-core/videobuf2-core.c | 9 ++++-----
> 1 file changed, 4 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/media/v4l2-core/videobuf2-core.c b/drivers/media/v4l2-core/videobuf2-core.c
> index 8bf3369..e866115 100644
> --- a/drivers/media/v4l2-core/videobuf2-core.c
> +++ b/drivers/media/v4l2-core/videobuf2-core.c
> @@ -889,7 +889,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
> {
> struct vb2_queue *q = vb->vb2_queue;
> unsigned long flags;
> - unsigned int plane;
>
> if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
> return;
> @@ -910,10 +909,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
> dprintk(4, "done processing on buffer %d, state: %d\n",
> vb->index, state);
>
> - /* sync buffers */
> - for (plane = 0; plane < vb->num_planes; ++plane)
> - call_void_memop(vb, finish, vb->planes[plane].mem_priv);
> -
> spin_lock_irqsave(&q->done_lock, flags);
> if (state == VB2_BUF_STATE_QUEUED ||
> state == VB2_BUF_STATE_REQUEUEING) {
> @@ -1573,6 +1568,10 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
>
> vb->state = VB2_BUF_STATE_DEQUEUED;
>
> + /* sync buffers */
> + for (i = 0; i < vb->num_planes; ++i)
> + call_void_memop(vb, finish, vb->planes[i].mem_priv);
> +
Sorry for digging up this old patch. Posting for reference, in case
someone decides to use or take over this patch.
This piece of code seems to be executed after queue's .buf_finish()
callback. The latter is allowed to access the buffer contents, so it
looks like we're breaking it, because it would now access the buffer
before the cache is synchronized.
Best regards,
Tomasz
@@ -889,7 +889,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
{
struct vb2_queue *q = vb->vb2_queue;
unsigned long flags;
- unsigned int plane;
if (WARN_ON(vb->state != VB2_BUF_STATE_ACTIVE))
return;
@@ -910,10 +909,6 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
dprintk(4, "done processing on buffer %d, state: %d\n",
vb->index, state);
- /* sync buffers */
- for (plane = 0; plane < vb->num_planes; ++plane)
- call_void_memop(vb, finish, vb->planes[plane].mem_priv);
-
spin_lock_irqsave(&q->done_lock, flags);
if (state == VB2_BUF_STATE_QUEUED ||
state == VB2_BUF_STATE_REQUEUEING) {
@@ -1573,6 +1568,10 @@ static void __vb2_dqbuf(struct vb2_buffer *vb)
vb->state = VB2_BUF_STATE_DEQUEUED;
+ /* sync buffers */
+ for (i = 0; i < vb->num_planes; ++i)
+ call_void_memop(vb, finish, vb->planes[i].mem_priv);
+
/* unmap DMABUF buffer */
if (q->memory == VB2_MEMORY_DMABUF)
for (i = 0; i < vb->num_planes; ++i) {