| Message ID | 20191217032034.54897-3-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Implement V4L2_BUF_FLAG_NO_CACHE_* flags | expand |
On 12/17/19 4:20 AM, Sergey Senozhatsky wrote: > Set video buffer cache management flags corresponding to V4L2 cache > flags. > > Both ->prepare() and ->finish() cache management hints should be > passed during this stage (buffer preparation), because there is no > other way for user-space to skip ->finish() cache flush. > > There are two possible alternative approaches: > - The first one is to move cache sync from ->finish() to dqbuf(). > But this breaks some drivers, that need to fix-up buffers before > dequeueing them. > > - The second one is to move ->finish() call from ->done() to dqbuf. Please combine this patch with patch 13/15! > > Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> > --- > .../media/common/videobuf2/videobuf2-v4l2.c | 22 +++++++++++++++++++ > 1 file changed, 22 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index e652f4318284..2fccfe2a57f8 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -337,6 +337,27 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b > return 0; > } > > +static void set_buffer_cache_hints(struct vb2_queue *q, > + struct vb2_buffer *vb, > + struct v4l2_buffer *b) > +{ > + vb->need_cache_sync_on_prepare = 1; > + > + if (q->dma_dir != DMA_TO_DEVICE) What should be done when dma_dir == DMA_BIDIRECTIONAL? > + vb->need_cache_sync_on_finish = 1; > + else > + vb->need_cache_sync_on_finish = 0; > + > + if (!q->allow_cache_hints) > + return; > + > + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE) > + vb->need_cache_sync_on_finish = 0; > + > + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN) > + vb->need_cache_sync_on_prepare = 0; > +} > + > static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, > struct v4l2_buffer *b, bool is_prepare, > struct media_request **p_req) > @@ -381,6 +402,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md > } > > if (!vb->prepared) { > + set_buffer_cache_hints(q, vb, b); > /* Copy relevant information provided by the userspace */ > memset(vbuf->planes, 0, > sizeof(vbuf->planes[0]) * vb->num_planes); > Regards, Hans
Hi, Sorry for the delayed. On (20/01/10 11:24), Hans Verkuil wrote: > On 12/17/19 4:20 AM, Sergey Senozhatsky wrote: > > Set video buffer cache management flags corresponding to V4L2 cache > > flags. > > > > Both ->prepare() and ->finish() cache management hints should be > > passed during this stage (buffer preparation), because there is no > > other way for user-space to skip ->finish() cache flush. > > > > There are two possible alternative approaches: > > - The first one is to move cache sync from ->finish() to dqbuf(). > > But this breaks some drivers, that need to fix-up buffers before > > dequeueing them. > > > > - The second one is to move ->finish() call from ->done() to dqbuf. > > Please combine this patch with patch 13/15! OK. [..] > > } > > > > +static void set_buffer_cache_hints(struct vb2_queue *q, > > + struct vb2_buffer *vb, > > + struct v4l2_buffer *b) > > +{ > > + vb->need_cache_sync_on_prepare = 1; > > + > > + if (q->dma_dir != DMA_TO_DEVICE) > > What should be done when dma_dir == DMA_BIDIRECTIONAL? Well, ->need_cache_sync_on_prepare/->need_cache_sync_on_finish are set, by default. If the queue supports user-space cache hints (driver has set ->allow_cache_hints), then user-space can override cache behavior. We probably cannot enforce any other behavior here. Am I missing something? > > + vb->need_cache_sync_on_finish = 1; > > + else > > + vb->need_cache_sync_on_finish = 0; > > + > > + if (!q->allow_cache_hints) > > + return; > > + > > + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE) > > + vb->need_cache_sync_on_finish = 0; > > + > > + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN) > > + vb->need_cache_sync_on_prepare = 0; > > +} -ss
On (20/01/22 10:39), Sergey Senozhatsky wrote: > [..] > > > } > > > > > > +static void set_buffer_cache_hints(struct vb2_queue *q, > > > + struct vb2_buffer *vb, > > > + struct v4l2_buffer *b) > > > +{ > > > + vb->need_cache_sync_on_prepare = 1; > > > + > > > + if (q->dma_dir != DMA_TO_DEVICE) > > > > What should be done when dma_dir == DMA_BIDIRECTIONAL? > [..] > We probably cannot enforce any other behavior here. Am I missing > something? Never mind. I got your point. -ss
On Wed, Jan 22, 2020 at 11:53 AM Sergey Senozhatsky <senozhatsky@chromium.org> wrote: > > On (20/01/22 10:39), Sergey Senozhatsky wrote: > > [..] > > > > } > > > > > > > > +static void set_buffer_cache_hints(struct vb2_queue *q, > > > > + struct vb2_buffer *vb, > > > > + struct v4l2_buffer *b) > > > > +{ > > > > + vb->need_cache_sync_on_prepare = 1; > > > > + > > > > + if (q->dma_dir != DMA_TO_DEVICE) > > > > > > What should be done when dma_dir == DMA_BIDIRECTIONAL? > > > > [..] > > > We probably cannot enforce any other behavior here. Am I missing > > something? > > Never mind. I got your point. DMA_BIDIRECTIONAL by default needs sync on both prepare and finish. need_cache_sync_on_prepare is initialized to 1. Since DMA_BIDIRECTIONAL != DMA_TO_DEVICE, need_cache_sync_on_finish would also be set to 1. Is anything still missing?
diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index e652f4318284..2fccfe2a57f8 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -337,6 +337,27 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b return 0; } +static void set_buffer_cache_hints(struct vb2_queue *q, + struct vb2_buffer *vb, + struct v4l2_buffer *b) +{ + vb->need_cache_sync_on_prepare = 1; + + if (q->dma_dir != DMA_TO_DEVICE) + vb->need_cache_sync_on_finish = 1; + else + vb->need_cache_sync_on_finish = 0; + + if (!q->allow_cache_hints) + return; + + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_INVALIDATE) + vb->need_cache_sync_on_finish = 0; + + if (b->flags & V4L2_BUF_FLAG_NO_CACHE_CLEAN) + vb->need_cache_sync_on_prepare = 0; +} + static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *mdev, struct v4l2_buffer *b, bool is_prepare, struct media_request **p_req) @@ -381,6 +402,7 @@ static int vb2_queue_or_prepare_buf(struct vb2_queue *q, struct media_device *md } if (!vb->prepared) { + set_buffer_cache_hints(q, vb, b); /* Copy relevant information provided by the userspace */ memset(vbuf->planes, 0, sizeof(vbuf->planes[0]) * vb->num_planes);
Set video buffer cache management flags corresponding to V4L2 cache flags. Both ->prepare() and ->finish() cache management hints should be passed during this stage (buffer preparation), because there is no other way for user-space to skip ->finish() cache flush. There are two possible alternative approaches: - The first one is to move cache sync from ->finish() to dqbuf(). But this breaks some drivers, that need to fix-up buffers before dequeueing them. - The second one is to move ->finish() call from ->done() to dqbuf. Signed-off-by: Sergey Senozhatsky <senozhatsky@chromium.org> --- .../media/common/videobuf2/videobuf2-v4l2.c | 22 +++++++++++++++++++ 1 file changed, 22 insertions(+)