| Message ID | 20200204025641.218376-1-senozhatsky@chromium.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Implement V4L2_BUF_FLAG_NO_CACHE_* flags | expand |
On 2/4/20 3:56 AM, Sergey Senozhatsky wrote: > Hello, > > V2 of the patchset, reshuffled, added more documentation, > addressed some of the feedback ;) Still in RFC, tho > > v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/ > > --- > This is a reworked version of the vb2 cache hints > (V4L2_BUF_FLAG_NO_CACHE_INVALIDATE / V4L2_BUF_FLAG_NO_CACHE_CLEAN) > support patch series which previsouly was developed by Sakari and > Laurent [0]. > > The patch set attempts to preserve the existing behvaiour - cache > sync is performed in ->prepare() and ->finish() (unless the buffer > is DMA exported). User space can request “default behavior” override > with cache management hints, which are handled on a per-buffer basis > and should be supplied with v4l2_buffer ->flags during buffer > preparation. There are two possible hints: > > - V4L2_BUF_FLAG_NO_CACHE_INVALIDATE > No cache sync on ->finish() > > - V4L2_BUF_FLAG_NO_CACHE_CLEAN > No cache sync on ->prepare() > > In order to keep things on the safe side, we also require driver > to explicitly state which of its queues (if any) support user space > cache management hints (such queues should have ->allow_cache_hints > bit set). > > The patch set also (to some extent) simplifies allocators' ->prepare() > and ->finish() callbacks. Namely, we move cache management decision > making to the upper - core - layer. For example, if, previously, we > would have something like this > > vb2_buffer_done() > vb2_dc_finish() > if (buf->db_attach) > return; > > where each allocators' ->finish() callback would either bail > out (DMA exported buffer, for instance) or sync, now that "bail > out or sync" decision is made before we call into the allocator. > > Along with cache management hints, user space is also able to > adjust queue's memory consistency attributes. Memory consistency > attribute (dma_attrs) is per-queue, yet it plays its role on the > allocator level, when we allocate buffers’ private memory (planes). > For the time being, only one consistency attribute is supported: > DMA_ATTR_NON_CONSISTENT. This is starting to look pretty good. I think you can drop the RFC for the next time you post this. One thing I would like to see as well is test code in v4l2-compliance, specifically for testing the various flags and capabilities. I.e., if V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS, then it should test that it can set the cache flags and the NON_CONSISTENT flag. If it is not set, then those flags should be cleared when attempting to set them. Also code in v4l2-ctl and common/v4l2-info.cpp to support the new flags, both reporting them, but also setting them. Regards, Hans > > [0] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html > > Sergey Senozhatsky (12): > videobuf2: add cache management members > videobuf2: handle V4L2 buffer cache flags > videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag > videobuf2: add queue memory consistency parameter > videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag > videobuf2: factor out planes prepare/finish functions > videobuf2: do not sync caches when we are allowed not to > videobuf2: check ->synced flag in prepare() and finish() > videobuf2: let user-space know if driver supports cache hints > videobuf2: add begin/end cpu_access callbacks to dma-contig > videobuf2: add begin/end cpu_access callbacks to dma-sg > videobuf2: don't test db_attach in dma-contig prepare and finish > > Documentation/media/uapi/v4l/buffer.rst | 27 +++++ > .../media/uapi/v4l/vidioc-create-bufs.rst | 9 +- > .../media/uapi/v4l/vidioc-reqbufs.rst | 22 +++- > .../media/common/videobuf2/videobuf2-core.c | 110 +++++++++++++----- > .../common/videobuf2/videobuf2-dma-contig.c | 39 ++++++- > .../media/common/videobuf2/videobuf2-dma-sg.c | 30 +++-- > .../media/common/videobuf2/videobuf2-v4l2.c | 59 +++++++++- > drivers/media/dvb-core/dvb_vb2.c | 2 +- > drivers/media/v4l2-core/v4l2-ioctl.c | 5 +- > include/media/videobuf2-core.h | 17 ++- > include/uapi/linux/videodev2.h | 11 +- > 11 files changed, 273 insertions(+), 58 deletions(-) >
On (20/02/19 09:53), Hans Verkuil wrote: [..] > This is starting to look pretty good. I think you can drop the RFC > for the next time you post this. OK! > One thing I would like to see as well is test code in v4l2-compliance, > specifically for testing the various flags and capabilities. I.e., > if V4L2_BUF_CAP_SUPPORTS_CACHE_HINTS, then it should test that it can > set the cache flags and the NON_CONSISTENT flag. If it is not set, > then those flags should be cleared when attempting to set them. > > Also code in v4l2-ctl and common/v4l2-info.cpp to support the new > flags, both reporting them, but also setting them. I do have two trivial patches - "sync videobuf kernel header" and "add new capability to common/v4l2-info.cpp". But v4l2-ctl and v4l2-compliance will require much more time, I think. So I, hopefully, will do it sometime later. -ss
Hello, V2 of the patchset, reshuffled, added more documentation, addressed some of the feedback ;) Still in RFC, tho v1 link: https://lore.kernel.org/lkml/20191217032034.54897-1-senozhatsky@chromium.org/ --- This is a reworked version of the vb2 cache hints (V4L2_BUF_FLAG_NO_CACHE_INVALIDATE / V4L2_BUF_FLAG_NO_CACHE_CLEAN) support patch series which previsouly was developed by Sakari and Laurent [0]. The patch set attempts to preserve the existing behvaiour - cache sync is performed in ->prepare() and ->finish() (unless the buffer is DMA exported). User space can request “default behavior” override with cache management hints, which are handled on a per-buffer basis and should be supplied with v4l2_buffer ->flags during buffer preparation. There are two possible hints: - V4L2_BUF_FLAG_NO_CACHE_INVALIDATE No cache sync on ->finish() - V4L2_BUF_FLAG_NO_CACHE_CLEAN No cache sync on ->prepare() In order to keep things on the safe side, we also require driver to explicitly state which of its queues (if any) support user space cache management hints (such queues should have ->allow_cache_hints bit set). The patch set also (to some extent) simplifies allocators' ->prepare() and ->finish() callbacks. Namely, we move cache management decision making to the upper - core - layer. For example, if, previously, we would have something like this vb2_buffer_done() vb2_dc_finish() if (buf->db_attach) return; where each allocators' ->finish() callback would either bail out (DMA exported buffer, for instance) or sync, now that "bail out or sync" decision is made before we call into the allocator. Along with cache management hints, user space is also able to adjust queue's memory consistency attributes. Memory consistency attribute (dma_attrs) is per-queue, yet it plays its role on the allocator level, when we allocate buffers’ private memory (planes). For the time being, only one consistency attribute is supported: DMA_ATTR_NON_CONSISTENT. [0] https://www.mail-archive.com/linux-media@vger.kernel.org/msg112459.html Sergey Senozhatsky (12): videobuf2: add cache management members videobuf2: handle V4L2 buffer cache flags videobuf2: add V4L2_FLAG_MEMORY_NON_CONSISTENT flag videobuf2: add queue memory consistency parameter videobuf2: handle V4L2_FLAG_MEMORY_NON_CONSISTENT flag videobuf2: factor out planes prepare/finish functions videobuf2: do not sync caches when we are allowed not to videobuf2: check ->synced flag in prepare() and finish() videobuf2: let user-space know if driver supports cache hints videobuf2: add begin/end cpu_access callbacks to dma-contig videobuf2: add begin/end cpu_access callbacks to dma-sg videobuf2: don't test db_attach in dma-contig prepare and finish Documentation/media/uapi/v4l/buffer.rst | 27 +++++ .../media/uapi/v4l/vidioc-create-bufs.rst | 9 +- .../media/uapi/v4l/vidioc-reqbufs.rst | 22 +++- .../media/common/videobuf2/videobuf2-core.c | 110 +++++++++++++----- .../common/videobuf2/videobuf2-dma-contig.c | 39 ++++++- .../media/common/videobuf2/videobuf2-dma-sg.c | 30 +++-- .../media/common/videobuf2/videobuf2-v4l2.c | 59 +++++++++- drivers/media/dvb-core/dvb_vb2.c | 2 +- drivers/media/v4l2-core/v4l2-ioctl.c | 5 +- include/media/videobuf2-core.h | 17 ++- include/uapi/linux/videodev2.h | 11 +- 11 files changed, 273 insertions(+), 58 deletions(-)