On 09/11/2015 06:26 PM, Hans Verkuil wrote: > On 09/11/2015 01:50 PM, Sakari Ailus wrote: >> The V4L2_BUF_FLAG_NO_CACHE_INVALIDATE and V4L2_BUF_FLAG_NO_CACHE_CLEAN >> buffer flags are currently not used by the kernel. Replace the definitions >> by a single V4L2_BUF_FLAG_NO_CACHE_SYNC flag to be used by further >> patches. >> >> Different cache architectures should not be visible to the user space >> which can make no meaningful use of the differences anyway. In case a >> device can make use of non-coherent memory accesses, the necessary cache >> operations depend on the CPU architecture and the buffer type, not the >> requests of the user. The cache operation itself may be skipped on the >> user's request which was the purpose of the two flags. >> >> On ARM the invalidate and clean are separate operations whereas on >> x86(-64) the two are a single operation (flush). Whether the hardware uses >> the buffer for reading (V4L2_BUF_TYPE_*_OUTPUT*) or writing >> (V4L2_BUF_TYPE_*CAPTURE*) already defines the required cache operation >> (clean and invalidate, respectively). No user input is required. >> >> Signed-off-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx> > > Acked-by: Hans Verkuil <hans.verkuil@xxxxxxxxx> Let me add something here: drivers should be able to override this flag. There are drivers that need to do fixups in buf_finish (a typical example is to fill in some JPEG or MPEG header). Obviously this does require syncing to the cpu. This is actually a bigger problem since trying to do this with dmabuf can fail as well. I made patches for that, but I never got around to finishing it: http://git.linuxtv.org/cgit.cgi/hverkuil/media_tree.git/log/?h=vb2-prep5 Basically this changes drivers so they are explicit when they need cpu access to a buffer. By using these begin/end_cpu_access functions you can still implement fixups correctly, although it does mean that the NO_CACHE_SYNC is effectively (but unavoidably) ignored. I think my patch series should be merged with this patch series The cover letter I wrote for the RFC patch series containing these cpu access functions is here: http://comments.gmane.org/gmane.linux.drivers.video-input-infrastructure/84399 I quote: "The last 5 patches make this more strict by requiring all cpu access to be bracketed by calls to vb2_plane_begin/end_cpu_access() which replaces the old vb2_plane_vaddr() call. Note: two drivers still use the vb2_plane_vaddr() call: coda and exynos4-is/fimc-capture.c. For both drivers I will need some help since I am not sure where to put the begin/end calls. Patch 15 removes the vb2_plane_vaddr call, so obviously those two drivers won't compile after that." So that's the missing part that I never got around to. Of course, there are new drivers as well since the last time I posted it, so there may be more to do now. I also said: "The issues that patches 12-16 address are [...] only an issue when using dmabuf with drivers that need cpu access." But it will *also* be an issue once this new NO_CACHE_SYNC flag is honored. Regards, Hans > >> --- >> Documentation/DocBook/media/v4l/io.xml | 25 +++++++++++-------------- >> include/trace/events/v4l2.h | 3 +-- >> include/uapi/linux/videodev2.h | 7 +++++-- >> 3 files changed, 17 insertions(+), 18 deletions(-) >> >> diff --git a/Documentation/DocBook/media/v4l/io.xml b/Documentation/DocBook/media/v4l/io.xml >> index 7bbc2a4..4facd63 100644 >> --- a/Documentation/DocBook/media/v4l/io.xml >> +++ b/Documentation/DocBook/media/v4l/io.xml >> @@ -1112,21 +1112,18 @@ application. Drivers set or clear this flag when the >> linkend="vidioc-qbuf">VIDIOC_DQBUF</link> ioctl is called.</entry> >> </row> >> <row> >> - <entry><constant>V4L2_BUF_FLAG_NO_CACHE_INVALIDATE</constant></entry> >> + <entry><constant>V4L2_BUF_FLAG_NO_CACHE_SYNC</constant></entry> >> <entry>0x00000800</entry> >> - <entry>Caches do not have to be invalidated for this buffer. >> -Typically applications shall use this flag if the data captured in the buffer >> -is not going to be touched by the CPU, instead the buffer will, probably, be >> -passed on to a DMA-capable hardware unit for further processing or output. >> -</entry> >> - </row> >> - <row> >> - <entry><constant>V4L2_BUF_FLAG_NO_CACHE_CLEAN</constant></entry> >> - <entry>0x00001000</entry> >> - <entry>Caches do not have to be cleaned for this buffer. >> -Typically applications shall use this flag for output buffers if the data >> -in this buffer has not been created by the CPU but by some DMA-capable unit, >> -in which case caches have not been used.</entry> >> + <entry>Do not perform CPU cache synchronisation operations >> + when the buffer is queued or dequeued. The user is >> + responsible for the correct use of this flag. It should be >> + only used when the buffer is not accessed using the CPU, >> + e.g. the buffer is written to by a hardware block and then >> + read by another one, in which case the flag should be set >> + in both <link linkend="vidioc-qbuf">VIDIOC_DQBUF</link> >> + and <link linkend="vidioc-qbuf">VIDIOC_QBUF</link> IOCTLs. >> + The flag has no effect on some devices / architectures. >> + </entry> >> </row> >> <row> >> <entry><constant>V4L2_BUF_FLAG_LAST</constant></entry> >> diff --git a/include/trace/events/v4l2.h b/include/trace/events/v4l2.h >> index dbf017b..4cee91d 100644 >> --- a/include/trace/events/v4l2.h >> +++ b/include/trace/events/v4l2.h >> @@ -78,8 +78,7 @@ SHOW_FIELD >> { V4L2_BUF_FLAG_ERROR, "ERROR" }, \ >> { V4L2_BUF_FLAG_TIMECODE, "TIMECODE" }, \ >> { V4L2_BUF_FLAG_PREPARED, "PREPARED" }, \ >> - { V4L2_BUF_FLAG_NO_CACHE_INVALIDATE, "NO_CACHE_INVALIDATE" }, \ >> - { V4L2_BUF_FLAG_NO_CACHE_CLEAN, "NO_CACHE_CLEAN" }, \ >> + { V4L2_BUF_FLAG_NO_CACHE_SYNC, "NO_CACHE_SYNC" }, \ >> { V4L2_BUF_FLAG_TIMESTAMP_MASK, "TIMESTAMP_MASK" }, \ >> { V4L2_BUF_FLAG_TIMESTAMP_UNKNOWN, "TIMESTAMP_UNKNOWN" }, \ >> { V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC, "TIMESTAMP_MONOTONIC" }, \ >> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h >> index 3228fbe..8d85aac 100644 >> --- a/include/uapi/linux/videodev2.h >> +++ b/include/uapi/linux/videodev2.h >> @@ -875,8 +875,11 @@ struct v4l2_buffer { >> #define V4L2_BUF_FLAG_TIMECODE 0x00000100 >> /* Buffer is prepared for queuing */ >> #define V4L2_BUF_FLAG_PREPARED 0x00000400 >> -/* Cache handling flags */ >> -#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE 0x00000800 >> +/* Cache sync hint */ >> +#define V4L2_BUF_FLAG_NO_CACHE_SYNC 0x00000800 >> +/* DEPRECATED. THIS WILL BE REMOVED IN THE FUTURE! */ >> +#define V4L2_BUF_FLAG_NO_CACHE_INVALIDATE V4L2_BUF_FLAG_NO_CACHE_SYNC >> +/* DEPRECATED. THIS WILL BE REMOVED IN THE FUTURE! */ >> #define V4L2_BUF_FLAG_NO_CACHE_CLEAN 0x00001000 >> /* Timestamp type */ >> #define V4L2_BUF_FLAG_TIMESTAMP_MASK 0x0000e000 >> > > -- > To unsubscribe from this list: send the line "unsubscribe linux-media" in > the body of a message to majordomo@xxxxxxxxxxxxxxx > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
