[RFC v4 03/18] vb2: Move cache synchronisation from buffer done to dqbuf handler

[Tomasz Figa]

Hi Sakari, Hans,

On Tue, May 9, 2017 at 12:05 AM Sakari Ailus
<sakari.ailus at 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 at linux.intel.com>
> Acked-by: Hans Verkuil <hans.verkuil at 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