Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi Jacopo, On Wed, Nov 14, 2018 at 01:25:11AM +0100, jacopo mondi wrote: > On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: > > Hi, > > > > This series adds support for the Intel IPU3 (Image Processing Unit) > > ImgU which is essentially a modern memory-to-memory ISP. It implements > > raw Bayer to YUV image format conversion as well as a large number of > > other pixel processing algorithms for improving the image quality. > > > > Meta data formats are defined for image statistics (3A, i.e. automatic > > white balance, exposure and focus, histogram and local area contrast > > enhancement) as well as for the pixel processing algorithm parameters. > > The documentation for these formats is currently not included in the > > patchset but will be added in a future version of this set. > > > > The algorithm parameters need to be considered specific to a given frame > > and typically a large number of these parameters change on frame to frame > > basis. Additionally, the parameters are highly structured (and not a flat > > space of independent configuration primitives). They also reflect the > > data structures used by the firmware and the hardware. On top of that, > > the algorithms require highly specialized user space to make meaningful > > use of them. For these reasons it has been chosen video buffers to pass > > the parameters to the device. > > > > On individual patches: > > > > The heart of ImgU is the CSS, or Camera Subsystem, which contains the > > image processors and HW accelerators. > > > > The 3A statistics and other firmware parameter computation related > > functions are implemented in patch 11. > > > > All IPU3 pipeline default settings can be found in patch 10. > > > > Seems to me that patch 10 didn't make it to the mailing list, am I > wrong? > > I'm pointing it out as the same happened on your v6. Thanks for pointing this out. I've uploaded the entire set here: https://git.linuxtv.org/sailus/media_tree.git/log/?h=ipu3-v7> including the 10th patch: https://git.linuxtv.org/sailus/media_tree.git/commit/?h=ipu3-v7=41e2f0d114dbc195efed079202d22748ddedbe83> It's too big to get through the list server. :-( Luckily, it's mostly tables so there's not much to review there. Default settings, effectively. -- Regards, Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH 1/9] videodev2.h: add tag support
On 11/13/2018 10:42 AM, Hans Verkuil wrote: > From: Hans Verkuil > > Add support for 'tags' to struct v4l2_buffer. These can be used > by m2m devices so userspace can set a tag for an output buffer and > this value will then be copied to the capture buffer(s). > > This tag can be used to refer to capture buffers, something that > is needed by stateless HW codecs. > > The new V4L2_BUF_CAP_SUPPORTS_TAGS capability indicates whether > or not tags are supported. > > Signed-off-by: Hans Verkuil > --- > include/uapi/linux/videodev2.h | 38 +- > 1 file changed, 37 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c8e8ff810190..ec1fef2a9445 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -879,6 +879,7 @@ struct v4l2_requestbuffers { > #define V4L2_BUF_CAP_SUPPORTS_USERPTR(1 << 1) > #define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2) > #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3) > +#define V4L2_BUF_CAP_SUPPORTS_TAGS (1 << 4) > > /** > * struct v4l2_plane - plane info for multi-planar buffers > @@ -912,6 +913,11 @@ struct v4l2_plane { > __u32 reserved[11]; > }; > > +struct v4l2_buffer_tag { > + __u32 low; > + __u32 high; > +}; > + > /** > * struct v4l2_buffer - video buffer info > * @index: id number of the buffer > @@ -950,7 +956,10 @@ struct v4l2_buffer { > __u32 flags; > __u32 field; > struct timeval timestamp; > - struct v4l2_timecodetimecode; > + union { > + struct v4l2_timecodetimecode; > + struct v4l2_buffer_tag tag; > + }; > __u32 sequence; > > /* memory location */ > @@ -988,6 +997,8 @@ struct v4l2_buffer { > #define V4L2_BUF_FLAG_IN_REQUEST 0x0080 > /* timecode field is valid */ > #define V4L2_BUF_FLAG_TIMECODE 0x0100 > +/* tag field is valid */ > +#define V4L2_BUF_FLAG_TAG0x0200 > /* Buffer is prepared for queuing */ > #define V4L2_BUF_FLAG_PREPARED 0x0400 > /* Cache handling flags */ > @@ -1007,6 +1018,31 @@ struct v4l2_buffer { > /* request_fd is valid */ > #define V4L2_BUF_FLAG_REQUEST_FD 0x0080 > > +static inline void v4l2_buffer_set_tag(struct v4l2_buffer *buf, __u64 tag) > +{ > + buf->tag.high = tag >> 32; > + buf->tag.low = tag & 0xULL; > + buf->flags |= V4L2_BUF_FLAG_TAG; > +} > + > +static inline void v4l2_buffer_set_tag_ptr(struct v4l2_buffer *buf, > +const void *tag) > +{ > + v4l2_buffer_set_tag(buf, (uintptr_t)tag); > +} > + > +static inline __u64 v4l2_buffer_get_tag(const struct v4l2_buffer *buf) > +{ > + if (!(buf->flags & V4L2_BUF_FLAG_TAG)) > + return 0; > + return (((__u64)buf->tag.high) << 32) | (__u64)buf->tag.low; > +} > + > +static inline void *v4l2_buffer_get_tag_ptr(const struct v4l2_buffer *buf) > +{ > + return (void *)(uintptr_t)v4l2_buffer_get_tag(buf); > +} > + I'm reconsidering my decision to use a u64 for the tag. It is too fiddly due to the fact that I have to use a struct v4l2_buffer_tag. I think I'll just use a u32. I'll post a new version where I make this change. Regards, Hans > /** > * struct v4l2_exportbuffer - export of video buffer as DMABUF file > descriptor > * >
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
On 11/14/2018 05:58 AM, Sakari Ailus wrote: > On Tue, Nov 13, 2018 at 07:04:01PM +0800, Bing Bu Cao wrote: >> >> On 11/13/2018 06:31 PM, Sakari Ailus wrote: >>> Hi Bing Bu, >>> >>> On Mon, Nov 12, 2018 at 12:31:16PM +0800, Bing Bu Cao wrote: On 11/09/2018 06:09 PM, Sakari Ailus wrote: > Hi Bing Bu, > > On Wed, Nov 07, 2018 at 12:16:47PM +0800, Bing Bu Cao wrote: >> On 11/01/2018 08:03 PM, Sakari Ailus wrote: >>> Hi Yong, >>> >>> Thanks for the update! >>> >>> On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: Hi, This series adds support for the Intel IPU3 (Image Processing Unit) ImgU which is essentially a modern memory-to-memory ISP. It implements raw Bayer to YUV image format conversion as well as a large number of other pixel processing algorithms for improving the image quality. Meta data formats are defined for image statistics (3A, i.e. automatic white balance, exposure and focus, histogram and local area contrast enhancement) as well as for the pixel processing algorithm parameters. The documentation for these formats is currently not included in the patchset but will be added in a future version of this set. The algorithm parameters need to be considered specific to a given frame and typically a large number of these parameters change on frame to frame basis. Additionally, the parameters are highly structured (and not a flat space of independent configuration primitives). They also reflect the data structures used by the firmware and the hardware. On top of that, the algorithms require highly specialized user space to make meaningful use of them. For these reasons it has been chosen video buffers to pass the parameters to the device. On individual patches: The heart of ImgU is the CSS, or Camera Subsystem, which contains the image processors and HW accelerators. The 3A statistics and other firmware parameter computation related functions are implemented in patch 11. All IPU3 pipeline default settings can be found in patch 10. To access DDR via ImgU's own memory space, IPU3 is also equipped with its own MMU unit, the driver is implemented in patch 6. Patch 7 uses above driver for DMA mapping operation. The communication between IPU3 firmware and driver is implemented with circular queues in patch 8. Patch 9 provide some utility functions and manage IPU3 fw download and install. The firmware which is called ipu3-fw.bin can be downloaded from: git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) Firmware ABI is defined in patches 4 and 5. Patches 12 and 13 are of the same file, the former contains all h/w programming related code, the latter implements interface functions for access fw & hw capabilities. Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: https://patchwork.kernel.org/patch/9976295/> >>> I've pushed the latest set here: >>> >>> https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> >>> >>> You can just say the entire set depends on those going forward; the >>> documentation is needed, too. >>> Patch 15 represents the top level that glues all of the other components together, passing arguments between the components. Patch 16 is a recent effort to extend v6 for advanced camera features like Continuous View Finder (CVF) and Snapshot During Video(SDV) support. Link to user space implementation: git clone https://chromium.googlesource.com/chromiumos/platform/arc-camera ImgU media topology print: # media-ctl -d /dev/media0 -p Media controller API version 4.19.0 Media device information driver ipu3-imgu model ipu3-imgu serial bus infoPCI::00:05.0 hw revision 0x80862015 driver version 4.19.0 Device topology - entity 1: ipu3-imgu 0 (5 pads, 5 links) type V4L2 subdev subtype Unknown flags 0 device node name /dev/v4l-subdev0 pad0: Sink [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown >>> This doesn't seem right. Which formats can be
cron job: media_tree daily build: WARNINGS
This message is generated daily by a cron job that builds media_tree for the kernels and architectures in the list below. Results of the daily build of media_tree: date: Wed Nov 14 05:00:11 CET 2018 media-tree git hash:fbe57dde7126d1b2712ab5ea93fb9d15f89de708 media_build git hash: a8aef9cea0a4a2f3ea86c0b37bd6a1378018c0c1 v4l-utils git hash: 98b4c9f276a18535b5691e5f350f59ffbf5a9aa5 edid-decode git hash: 5eeb151a748788666534d6ea3da07f90400d24c2 gcc version:i686-linux-gcc (GCC) 8.2.0 sparse version: 0.5.2 smatch version: 0.5.1 host hardware: x86_64 host os:4.18.0-2-amd64 linux-git-arm-at91: WARNINGS linux-git-arm-davinci: WARNINGS linux-git-arm-multi: WARNINGS linux-git-arm-pxa: WARNINGS linux-git-arm-stm32: WARNINGS linux-git-arm64: WARNINGS linux-git-i686: WARNINGS linux-git-mips: OK linux-git-powerpc64: WARNINGS linux-git-sh: WARNINGS linux-git-x86_64: WARNINGS Check COMPILE_TEST: OK linux-3.10.108-i686: OK linux-3.10.108-x86_64: OK linux-3.11.10-i686: OK linux-3.11.10-x86_64: OK linux-3.12.74-i686: OK linux-3.12.74-x86_64: OK linux-3.13.11-i686: OK linux-3.13.11-x86_64: OK linux-3.14.79-i686: OK linux-3.14.79-x86_64: OK linux-3.15.10-i686: OK linux-3.15.10-x86_64: OK linux-3.16.57-i686: OK linux-3.16.57-x86_64: OK linux-3.17.8-i686: OK linux-3.17.8-x86_64: OK linux-3.18.123-i686: OK linux-3.18.123-x86_64: OK linux-3.19.8-i686: OK linux-3.19.8-x86_64: OK linux-4.0.9-i686: OK linux-4.0.9-x86_64: OK linux-4.1.52-i686: OK linux-4.1.52-x86_64: OK linux-4.2.8-i686: OK linux-4.2.8-x86_64: OK linux-4.3.6-i686: OK linux-4.3.6-x86_64: OK linux-4.4.159-i686: OK linux-4.4.159-x86_64: OK linux-4.5.7-i686: OK linux-4.5.7-x86_64: OK linux-4.6.7-i686: OK linux-4.6.7-x86_64: OK linux-4.7.10-i686: OK linux-4.7.10-x86_64: OK linux-4.8.17-i686: OK linux-4.8.17-x86_64: OK linux-4.9.131-i686: OK linux-4.9.131-x86_64: OK linux-4.10.17-i686: OK linux-4.10.17-x86_64: OK linux-4.11.12-i686: OK linux-4.11.12-x86_64: OK linux-4.12.14-i686: OK linux-4.12.14-x86_64: OK linux-4.13.16-i686: OK linux-4.13.16-x86_64: OK linux-4.14.74-i686: OK linux-4.14.74-x86_64: OK linux-4.15.18-i686: OK linux-4.15.18-x86_64: OK linux-4.16.18-i686: OK linux-4.16.18-x86_64: OK linux-4.17.19-i686: OK linux-4.17.19-x86_64: OK linux-4.18.12-i686: OK linux-4.18.12-x86_64: OK linux-4.19.1-i686: OK linux-4.19.1-x86_64: OK linux-4.20-rc1-i686: OK linux-4.20-rc1-x86_64: OK apps: OK spec-git: OK sparse: WARNINGS Detailed results are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Wednesday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
On Wed, Nov 14, 2018 at 8:51 AM Nicolas Dufresne wrote: > > Le mercredi 14 novembre 2018 à 00:27 +0200, Sakari Ailus a écrit : > > Hi Philipp, > > > > On Tue, Nov 13, 2018 at 04:06:21PM +0100, Philipp Zabel wrote: > > > From: John Sheu > > > > > > Videobuf2 presently does not allow VIDIOC_REQBUFS to destroy outstanding > > > buffers if the queue is of type V4L2_MEMORY_MMAP, and if the buffers are > > > considered "in use". This is different behavior than for other memory > > > types and prevents us from deallocating buffers in following two cases: > > > > > > 1) There are outstanding mmap()ed views on the buffer. However even if > > >we put the buffer in reqbufs(0), there will be remaining references, > > >due to vma .open/close() adjusting vb2 buffer refcount appropriately. > > >This means that the buffer will be in fact freed only when the last > > >mmap()ed view is unmapped. > > > > > > 2) Buffer has been exported as a DMABUF. Refcount of the vb2 buffer > > >is managed properly by VB2 DMABUF ops, i.e. incremented on DMABUF > > >get and decremented on DMABUF release. This means that the buffer > > >will be alive until all importers release it. > > > > > > Considering both cases above, there does not seem to be any need to > > > prevent reqbufs(0) operation, because buffer lifetime is already > > > properly managed by both mmap() and DMABUF code paths. Let's remove it > > > and allow userspace freeing the queue (and potentially allocating a new > > > one) even though old buffers might be still in processing. > > > > > > To let userspace know that the kernel now supports orphaning buffers > > > that are still in use, add a new V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS > > > to be set by reqbufs and create_bufs. > > > > > > Signed-off-by: John Sheu > > > Reviewed-by: Pawel Osciak > > > Reviewed-by: Tomasz Figa > > > Signed-off-by: Tomasz Figa > > > [p.za...@pengutronix.de: moved __vb2_queue_cancel out of the mmap_lock > > > and added V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS] > > > Signed-off-by: Philipp Zabel > > > > This lets the user to allocate lots of mmap'ed buffers that are pinned in > > physical memory. Considering that we don't really have a proper mechanism > > to limit that anyway, > > It's currently limited to 32 buffers. It's not worst then DRM dumb > buffers which will let you allocate as much as you want. > 32 buffers for one mem2mem instance. One can open many of those and allocate more anyway. I think it's a part of the global problem of DMA memory not being accounted to the process allocating... Best regards, Tomasz > > > > Acked-by: Sakari Ailus > > > > That said, the patch must be accompanied by the documentation change in > > Documentation/media/uapi/v4l/vidioc-reqbufs.rst . > > > > I wonder what Hans thinks. > > > > > --- > > > .../media/common/videobuf2/videobuf2-core.c | 26 +-- > > > .../media/common/videobuf2/videobuf2-v4l2.c | 2 +- > > > include/uapi/linux/videodev2.h| 1 + > > > 3 files changed, 3 insertions(+), 26 deletions(-) > > > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > > > b/drivers/media/common/videobuf2/videobuf2-core.c > > > index 975ff5669f72..608459450c1e 100644 > > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > > @@ -553,20 +553,6 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct > > > vb2_buffer *vb) > > > } > > > EXPORT_SYMBOL(vb2_buffer_in_use); > > > > > > -/* > > > - * __buffers_in_use() - return true if any buffers on the queue are in > > > use and > > > - * the queue cannot be freed (by the means of REQBUFS(0)) call > > > - */ > > > -static bool __buffers_in_use(struct vb2_queue *q) > > > -{ > > > - unsigned int buffer; > > > - for (buffer = 0; buffer < q->num_buffers; ++buffer) { > > > - if (vb2_buffer_in_use(q, q->bufs[buffer])) > > > - return true; > > > - } > > > - return false; > > > -} > > > - > > > void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) > > > { > > > call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); > > > @@ -674,23 +660,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > > > vb2_memory memory, > > > > > > if (*count == 0 || q->num_buffers != 0 || > > > (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > > > - /* > > > -* We already have buffers allocated, so first check if they > > > -* are not in use and can be freed. > > > -*/ > > > - mutex_lock(>mmap_lock); > > > - if (q->memory == VB2_MEMORY_MMAP && __buffers_in_use(q)) { > > > - mutex_unlock(>mmap_lock); > > > - dprintk(1, "memory in use, cannot free\n"); > > > - return -EBUSY; > > > - } > > > - > > > /* > > > * Call queue_cancel to clean up any buffers in the
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: > Hi, > > This series adds support for the Intel IPU3 (Image Processing Unit) > ImgU which is essentially a modern memory-to-memory ISP. It implements > raw Bayer to YUV image format conversion as well as a large number of > other pixel processing algorithms for improving the image quality. > > Meta data formats are defined for image statistics (3A, i.e. automatic > white balance, exposure and focus, histogram and local area contrast > enhancement) as well as for the pixel processing algorithm parameters. > The documentation for these formats is currently not included in the > patchset but will be added in a future version of this set. > > The algorithm parameters need to be considered specific to a given frame > and typically a large number of these parameters change on frame to frame > basis. Additionally, the parameters are highly structured (and not a flat > space of independent configuration primitives). They also reflect the > data structures used by the firmware and the hardware. On top of that, > the algorithms require highly specialized user space to make meaningful > use of them. For these reasons it has been chosen video buffers to pass > the parameters to the device. > > On individual patches: > > The heart of ImgU is the CSS, or Camera Subsystem, which contains the > image processors and HW accelerators. > > The 3A statistics and other firmware parameter computation related > functions are implemented in patch 11. > > All IPU3 pipeline default settings can be found in patch 10. > Seems to me that patch 10 didn't make it to the mailing list, am I wrong? I'm pointing it out as the same happened on your v6. Thanks j > To access DDR via ImgU's own memory space, IPU3 is also equipped with > its own MMU unit, the driver is implemented in patch 6. > > Patch 7 uses above driver for DMA mapping operation. > > The communication between IPU3 firmware and driver is implemented with > circular > queues in patch 8. > > Patch 9 provide some utility functions and manage IPU3 fw download and > install. > > The firmware which is called ipu3-fw.bin can be downloaded from: > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) > > Firmware ABI is defined in patches 4 and 5. > > Patches 12 and 13 are of the same file, the former contains all h/w > programming > related code, the latter implements interface functions for access fw & hw > capabilities. > > Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: > > https://patchwork.kernel.org/patch/9976295/> > > Patch 15 represents the top level that glues all of the other components > together, > passing arguments between the components. > > Patch 16 is a recent effort to extend v6 for advanced camera features like > Continuous View Finder (CVF) and Snapshot During Video(SDV) support. > > Link to user space implementation: > > git clone https://chromium.googlesource.com/chromiumos/platform/arc-camera > > ImgU media topology print: > > # media-ctl -d /dev/media0 -p > Media controller API version 4.19.0 > > Media device information > > driver ipu3-imgu > model ipu3-imgu > serial > bus infoPCI::00:05.0 > hw revision 0x80862015 > driver version 4.19.0 > > Device topology > - entity 1: ipu3-imgu 0 (5 pads, 5 links) > type V4L2 subdev subtype Unknown flags 0 > device node name /dev/v4l-subdev0 > pad0: Sink > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown >crop:(0,0)/1920x1080 >compose:(0,0)/1920x1080] > <- "ipu3-imgu 0 input":0 [] > pad1: Sink > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > <- "ipu3-imgu 0 parameters":0 [] > pad2: Source > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > -> "ipu3-imgu 0 output":0 [] > pad3: Source > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > -> "ipu3-imgu 0 viewfinder":0 [] > pad4: Source > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > -> "ipu3-imgu 0 3a stat":0 [] > > - entity 7: ipu3-imgu 1 (5 pads, 5 links) > type V4L2 subdev subtype Unknown flags 0 > device node name /dev/v4l-subdev1 > pad0: Sink > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown >crop:(0,0)/1920x1080 >compose:(0,0)/1920x1080] > <- "ipu3-imgu 1 input":0 [] > pad1: Sink > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > <- "ipu3-imgu 1 parameters":0 [] > pad2: Source > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown] > -> "ipu3-imgu 1 output":0 [] > pad3: Source >
Re: [PATCH 0/7] media: i2c: small enhancements for camera sensor drivers
Hello Mita-san, thanks for the patches On Tue, Nov 13, 2018 at 01:00:47AM +0900, Akinobu Mita wrote: > This patchset addds relatively small enhancements (log_status ioctl, event > interface, V4L2_CID_TEST_PATTERN control, and V4L2_CID_COLORFX control) for > mt9m111, ov2640, ov5640, ov7670, and ov772x drivers. I have these devices > so these patches are tested with real devices. > For the ov772x part: Acked-by: Jacopo Mondi Thanks j > Akinobu Mita (7): > media: mt9m111: support log_status ioctl and event interface > media: mt9m111: add V4L2_CID_COLORFX control > media: ov2640: add V4L2_CID_TEST_PATTERN control > media: ov2640: support log_status ioctl and event interface > media: ov5640: support log_status ioctl and event interface > media: ov7670: support log_status ioctl and event interface > media: ov772x: support log_status ioctl and event interface > > drivers/media/i2c/mt9m111.c | 44 ++-- > drivers/media/i2c/ov2640.c | 21 +++-- > drivers/media/i2c/ov5640.c | 7 ++- > drivers/media/i2c/ov7670.c | 6 +- > drivers/media/i2c/ov772x.c | 7 ++- > 5 files changed, 78 insertions(+), 7 deletions(-) > > Cc: Steve Longerbeam > Cc: Jonathan Corbet > Cc: Jacopo Mondi > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > -- > 2.7.4 > signature.asc Description: PGP signature
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Le mercredi 14 novembre 2018 à 00:27 +0200, Sakari Ailus a écrit : > Hi Philipp, > > On Tue, Nov 13, 2018 at 04:06:21PM +0100, Philipp Zabel wrote: > > From: John Sheu > > > > Videobuf2 presently does not allow VIDIOC_REQBUFS to destroy outstanding > > buffers if the queue is of type V4L2_MEMORY_MMAP, and if the buffers are > > considered "in use". This is different behavior than for other memory > > types and prevents us from deallocating buffers in following two cases: > > > > 1) There are outstanding mmap()ed views on the buffer. However even if > >we put the buffer in reqbufs(0), there will be remaining references, > >due to vma .open/close() adjusting vb2 buffer refcount appropriately. > >This means that the buffer will be in fact freed only when the last > >mmap()ed view is unmapped. > > > > 2) Buffer has been exported as a DMABUF. Refcount of the vb2 buffer > >is managed properly by VB2 DMABUF ops, i.e. incremented on DMABUF > >get and decremented on DMABUF release. This means that the buffer > >will be alive until all importers release it. > > > > Considering both cases above, there does not seem to be any need to > > prevent reqbufs(0) operation, because buffer lifetime is already > > properly managed by both mmap() and DMABUF code paths. Let's remove it > > and allow userspace freeing the queue (and potentially allocating a new > > one) even though old buffers might be still in processing. > > > > To let userspace know that the kernel now supports orphaning buffers > > that are still in use, add a new V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS > > to be set by reqbufs and create_bufs. > > > > Signed-off-by: John Sheu > > Reviewed-by: Pawel Osciak > > Reviewed-by: Tomasz Figa > > Signed-off-by: Tomasz Figa > > [p.za...@pengutronix.de: moved __vb2_queue_cancel out of the mmap_lock > > and added V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS] > > Signed-off-by: Philipp Zabel > > This lets the user to allocate lots of mmap'ed buffers that are pinned in > physical memory. Considering that we don't really have a proper mechanism > to limit that anyway, It's currently limited to 32 buffers. It's not worst then DRM dumb buffers which will let you allocate as much as you want. > > Acked-by: Sakari Ailus > > That said, the patch must be accompanied by the documentation change in > Documentation/media/uapi/v4l/vidioc-reqbufs.rst . > > I wonder what Hans thinks. > > > --- > > .../media/common/videobuf2/videobuf2-core.c | 26 +-- > > .../media/common/videobuf2/videobuf2-v4l2.c | 2 +- > > include/uapi/linux/videodev2.h| 1 + > > 3 files changed, 3 insertions(+), 26 deletions(-) > > > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > > b/drivers/media/common/videobuf2/videobuf2-core.c > > index 975ff5669f72..608459450c1e 100644 > > --- a/drivers/media/common/videobuf2/videobuf2-core.c > > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > > @@ -553,20 +553,6 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct > > vb2_buffer *vb) > > } > > EXPORT_SYMBOL(vb2_buffer_in_use); > > > > -/* > > - * __buffers_in_use() - return true if any buffers on the queue are in use > > and > > - * the queue cannot be freed (by the means of REQBUFS(0)) call > > - */ > > -static bool __buffers_in_use(struct vb2_queue *q) > > -{ > > - unsigned int buffer; > > - for (buffer = 0; buffer < q->num_buffers; ++buffer) { > > - if (vb2_buffer_in_use(q, q->bufs[buffer])) > > - return true; > > - } > > - return false; > > -} > > - > > void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) > > { > > call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); > > @@ -674,23 +660,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > > vb2_memory memory, > > > > if (*count == 0 || q->num_buffers != 0 || > > (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > > - /* > > -* We already have buffers allocated, so first check if they > > -* are not in use and can be freed. > > -*/ > > - mutex_lock(>mmap_lock); > > - if (q->memory == VB2_MEMORY_MMAP && __buffers_in_use(q)) { > > - mutex_unlock(>mmap_lock); > > - dprintk(1, "memory in use, cannot free\n"); > > - return -EBUSY; > > - } > > - > > /* > > * Call queue_cancel to clean up any buffers in the > > * QUEUED state which is possible if buffers were prepared or > > * queued without ever calling STREAMON. > > */ > > __vb2_queue_cancel(q); > > + mutex_lock(>mmap_lock); > > ret = __vb2_queue_free(q, q->num_buffers); > > mutex_unlock(>mmap_lock); > > if (ret) > > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c > >
Re: [PATCH 4/4] SoC camera: Tidy the header
Hi Hans, Mauro, On Wed, Oct 31, 2018 at 07:54:21AM -0300, Mauro Carvalho Chehab wrote: > Em Wed, 31 Oct 2018 11:00:22 +0100 > Hans Verkuil escreveu: > > > On 10/31/2018 10:40 AM, Mauro Carvalho Chehab wrote: > > > Em Wed, 31 Oct 2018 11:29:45 +0200 > > > Sakari Ailus escreveu: > > > > > >> Hi Mauro, > > >> > > >> On Tue, Oct 30, 2018 at 09:06:18AM -0300, Mauro Carvalho Chehab wrote: > > >>> Em Tue, 30 Oct 2018 01:00:29 +0200 > > >>> Sakari Ailus escreveu: > > >>> > > Clean up the SoC camera framework header. It only exists now to keep > > board > > code compiling. The header can be removed once the board code > > dependencies > > to it has been removed. > > > > Signed-off-by: Sakari Ailus > > --- > > include/media/soc_camera.h | 335 > > - > > 1 file changed, 335 deletions(-) > > > > diff --git a/include/media/soc_camera.h b/include/media/soc_camera.h > > index b7e42a1b0910..14d19da6052a 100644 > > --- a/include/media/soc_camera.h > > +++ b/include/media/soc_camera.h > > @@ -22,172 +22,6 @@ > > #include > > #include > > >>> > > >>> That doesn't make any sense. soc_camera.h should have the same fate > > >>> as the entire soc_camera infrastructure: either be removed or moved > > >>> to staging, and everything else that doesn't have the same fate > > >>> should get rid of this header. > > >> > > >> We can't just remove this; there is board code that depends on it. > > >> > > >> The intent is to remove the board code as well but presuming that the > > >> board > > >> code would be merged through a different tree, it'd be less hassle to > > >> wait > > >> a bit; hence the patch removing any unnecessary stuff here. > > > > > > Then we need *first* to remove board code, wait for those changes to be > > > applied and *then* remove soc_camera (and not the opposite). > > > > Please note that the camera support for all the remaining boards using > > soc_camera has been dead for years. The soc_camera drivers that they > > depended > > on have been removed a long time ago. > > > > So all they depend on are the header. We can safely remove soc_camera > > without > > loss of functionality (and thus prevent others from basing new drivers on > > soc_camera), while we work to update the board files so we can remove this > > last > > header. > > > > I have modified some board files here: > > > > https://git.linuxtv.org/hverkuil/media_tree.git/commit/?h=rm-soc-camera=d7ae2fcf6e447022f0780bb86a2b63d5c7cf4d35 > > Good! the arch-specific mach-* changes should likely be on separate > patches, though. > > > Only arch/arm/mach-imx/mach-imx27_visstrim_m10.c hasn't been fixed yet in > > that > > patch (ENOTIME). > > So, the code we don't manage seems to be just 3 archs, right (mach-omap1, > mach-pxa and mach-imx)? > > Btw, the soc_camera dependent code at mach-imx27_visstrim_m10.c is: > > static struct soc_camera_link iclink_tvp5150 = { > .bus_id = 0, > .board_info = _i2c_camera, > .i2c_adapter_id = 0, > .power = visstrim_camera_power, > .reset = visstrim_camera_reset, > }; > > ... > platform_device_register_resndata(NULL, "soc-camera-pdrv", 0, NULL, 0, > _tvp5150, > sizeof(iclink_tvp5150)); > > > As tvp5150 is not actually a soc_camera driver, I suspect that > the conversion here would be to make it to use the tvp5150 driver > directly, instead of doing it via soc_camera. Using board files? The IMX27 SoC has plenty of dts files around which suggests that it's just this particular board that has not been converted. I don't think this is a good excuse to keep the board file stuff around. If someone is willing to add dts files for the board, all the better. But I'm not going to do that. > > > The problem is just lack of time to clean this up and figure out who should > > take these board patches. > > No need to rush it. > > > So I think it is a nice solution to just replace the header with a dummy > > version > > so the board files still compile, and then we can delete the dead soc_camera > > driver. It's probably easier as well to push through the board file changes > > once > > soc_camera is removed since you can just point out that the framework it > > depended > > on is gone. > > I still think that reverting the order and rushing the removal is not the > way to go. I think no-one can blame us for rushing the removal of this: it's been broken for years. I propose removing the dead code with the least effort while minimising collateral damage as well as additional maintenance work done to support the to-be-removed code. As far as I can see, that involves: - Removing the SoC camera framework (as my set does; there'll be v2 to fix a few minor matters), - Removing the board files (I can
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Hi Philipp, On Tue, Nov 13, 2018 at 04:06:21PM +0100, Philipp Zabel wrote: > From: John Sheu > > Videobuf2 presently does not allow VIDIOC_REQBUFS to destroy outstanding > buffers if the queue is of type V4L2_MEMORY_MMAP, and if the buffers are > considered "in use". This is different behavior than for other memory > types and prevents us from deallocating buffers in following two cases: > > 1) There are outstanding mmap()ed views on the buffer. However even if >we put the buffer in reqbufs(0), there will be remaining references, >due to vma .open/close() adjusting vb2 buffer refcount appropriately. >This means that the buffer will be in fact freed only when the last >mmap()ed view is unmapped. > > 2) Buffer has been exported as a DMABUF. Refcount of the vb2 buffer >is managed properly by VB2 DMABUF ops, i.e. incremented on DMABUF >get and decremented on DMABUF release. This means that the buffer >will be alive until all importers release it. > > Considering both cases above, there does not seem to be any need to > prevent reqbufs(0) operation, because buffer lifetime is already > properly managed by both mmap() and DMABUF code paths. Let's remove it > and allow userspace freeing the queue (and potentially allocating a new > one) even though old buffers might be still in processing. > > To let userspace know that the kernel now supports orphaning buffers > that are still in use, add a new V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS > to be set by reqbufs and create_bufs. > > Signed-off-by: John Sheu > Reviewed-by: Pawel Osciak > Reviewed-by: Tomasz Figa > Signed-off-by: Tomasz Figa > [p.za...@pengutronix.de: moved __vb2_queue_cancel out of the mmap_lock > and added V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS] > Signed-off-by: Philipp Zabel This lets the user to allocate lots of mmap'ed buffers that are pinned in physical memory. Considering that we don't really have a proper mechanism to limit that anyway, Acked-by: Sakari Ailus That said, the patch must be accompanied by the documentation change in Documentation/media/uapi/v4l/vidioc-reqbufs.rst . I wonder what Hans thinks. > --- > .../media/common/videobuf2/videobuf2-core.c | 26 +-- > .../media/common/videobuf2/videobuf2-v4l2.c | 2 +- > include/uapi/linux/videodev2.h| 1 + > 3 files changed, 3 insertions(+), 26 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > b/drivers/media/common/videobuf2/videobuf2-core.c > index 975ff5669f72..608459450c1e 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -553,20 +553,6 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct > vb2_buffer *vb) > } > EXPORT_SYMBOL(vb2_buffer_in_use); > > -/* > - * __buffers_in_use() - return true if any buffers on the queue are in use > and > - * the queue cannot be freed (by the means of REQBUFS(0)) call > - */ > -static bool __buffers_in_use(struct vb2_queue *q) > -{ > - unsigned int buffer; > - for (buffer = 0; buffer < q->num_buffers; ++buffer) { > - if (vb2_buffer_in_use(q, q->bufs[buffer])) > - return true; > - } > - return false; > -} > - > void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) > { > call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); > @@ -674,23 +660,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > vb2_memory memory, > > if (*count == 0 || q->num_buffers != 0 || > (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { > - /* > - * We already have buffers allocated, so first check if they > - * are not in use and can be freed. > - */ > - mutex_lock(>mmap_lock); > - if (q->memory == VB2_MEMORY_MMAP && __buffers_in_use(q)) { > - mutex_unlock(>mmap_lock); > - dprintk(1, "memory in use, cannot free\n"); > - return -EBUSY; > - } > - > /* >* Call queue_cancel to clean up any buffers in the >* QUEUED state which is possible if buffers were prepared or >* queued without ever calling STREAMON. >*/ > __vb2_queue_cancel(q); > + mutex_lock(>mmap_lock); > ret = __vb2_queue_free(q, q->num_buffers); > mutex_unlock(>mmap_lock); > if (ret) > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c > b/drivers/media/common/videobuf2/videobuf2-v4l2.c > index a17033ab2c22..f02d452ceeb9 100644 > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c > @@ -624,7 +624,7 @@ EXPORT_SYMBOL(vb2_querybuf); > > static void fill_buf_caps(struct vb2_queue *q, u32 *caps) > { > - *caps = 0; > + *caps = V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS; >
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
On Tue, Nov 13, 2018 at 07:04:01PM +0800, Bing Bu Cao wrote: > > > On 11/13/2018 06:31 PM, Sakari Ailus wrote: > > Hi Bing Bu, > > > > On Mon, Nov 12, 2018 at 12:31:16PM +0800, Bing Bu Cao wrote: > >> > >> On 11/09/2018 06:09 PM, Sakari Ailus wrote: > >>> Hi Bing Bu, > >>> > >>> On Wed, Nov 07, 2018 at 12:16:47PM +0800, Bing Bu Cao wrote: > On 11/01/2018 08:03 PM, Sakari Ailus wrote: > > Hi Yong, > > > > Thanks for the update! > > > > On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: > >> Hi, > >> > >> This series adds support for the Intel IPU3 (Image Processing Unit) > >> ImgU which is essentially a modern memory-to-memory ISP. It implements > >> raw Bayer to YUV image format conversion as well as a large number of > >> other pixel processing algorithms for improving the image quality. > >> > >> Meta data formats are defined for image statistics (3A, i.e. automatic > >> white balance, exposure and focus, histogram and local area contrast > >> enhancement) as well as for the pixel processing algorithm parameters. > >> The documentation for these formats is currently not included in the > >> patchset but will be added in a future version of this set. > >> > >> The algorithm parameters need to be considered specific to a given > >> frame > >> and typically a large number of these parameters change on frame to > >> frame > >> basis. Additionally, the parameters are highly structured (and not a > >> flat > >> space of independent configuration primitives). They also reflect the > >> data structures used by the firmware and the hardware. On top of that, > >> the algorithms require highly specialized user space to make meaningful > >> use of them. For these reasons it has been chosen video buffers to pass > >> the parameters to the device. > >> > >> On individual patches: > >> > >> The heart of ImgU is the CSS, or Camera Subsystem, which contains the > >> image processors and HW accelerators. > >> > >> The 3A statistics and other firmware parameter computation related > >> functions are implemented in patch 11. > >> > >> All IPU3 pipeline default settings can be found in patch 10. > >> > >> To access DDR via ImgU's own memory space, IPU3 is also equipped with > >> its own MMU unit, the driver is implemented in patch 6. > >> > >> Patch 7 uses above driver for DMA mapping operation. > >> > >> The communication between IPU3 firmware and driver is implemented with > >> circular > >> queues in patch 8. > >> > >> Patch 9 provide some utility functions and manage IPU3 fw download and > >> install. > >> > >> The firmware which is called ipu3-fw.bin can be downloaded from: > >> > >> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > >> (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) > >> > >> Firmware ABI is defined in patches 4 and 5. > >> > >> Patches 12 and 13 are of the same file, the former contains all h/w > >> programming > >> related code, the latter implements interface functions for access fw > >> & hw > >> capabilities. > >> > >> Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: > >> > >> https://patchwork.kernel.org/patch/9976295/> > > I've pushed the latest set here: > > > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> > > > > You can just say the entire set depends on those going forward; the > > documentation is needed, too. > > > >> Patch 15 represents the top level that glues all of the other > >> components together, > >> passing arguments between the components. > >> > >> Patch 16 is a recent effort to extend v6 for advanced camera features > >> like > >> Continuous View Finder (CVF) and Snapshot During Video(SDV) support. > >> > >> Link to user space implementation: > >> > >> git clone > >> https://chromium.googlesource.com/chromiumos/platform/arc-camera > >> > >> ImgU media topology print: > >> > >> # media-ctl -d /dev/media0 -p > >> Media controller API version 4.19.0 > >> > >> Media device information > >> > >> driver ipu3-imgu > >> model ipu3-imgu > >> serial > >> bus infoPCI::00:05.0 > >> hw revision 0x80862015 > >> driver version 4.19.0 > >> > >> Device topology > >> - entity 1: ipu3-imgu 0 (5 pads, 5 links) > >> type V4L2 subdev subtype Unknown flags 0 > >> device node name /dev/v4l-subdev0 > >>pad0: Sink > >>[fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown > > This doesn't seem right. Which formats can be enumerated from the pad? > >>> Looking at the
Re: [PATCH 3/7] media: ov2640: add V4L2_CID_TEST_PATTERN control
On Tue, Nov 13, 2018 at 10:55:46PM +0900, Akinobu Mita wrote: > 2018年11月13日(火) 19:37 Sakari Ailus : > > > > On Tue, Nov 13, 2018 at 01:00:50AM +0900, Akinobu Mita wrote: > > > The ov2640 has the test pattern generator features. This makes use of > > > it through V4L2_CID_TEST_PATTERN control. > > > > > > Cc: Sakari Ailus > > > Cc: Mauro Carvalho Chehab > > > Signed-off-by: Akinobu Mita > > > --- > > > drivers/media/i2c/ov2640.c | 14 +- > > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c > > > index 20a8853..4992d77 100644 > > > --- a/drivers/media/i2c/ov2640.c > > > +++ b/drivers/media/i2c/ov2640.c > > > @@ -705,6 +705,11 @@ static int ov2640_reset(struct i2c_client *client) > > > return ret; > > > } > > > > > > +static const char * const ov2640_test_pattern_menu[] = { > > > + "Disabled", > > > + "Color bar", > > > > s/b/B/ > > > > The smiapp driver uses "Eight Vertical Colour Bars", not sure if that'd fit > > here or not. FYI. > > This test pattern shows eight vertical color bars with blending actual > sensor image, although the datasheet tells just 'Color bar'. > > So either is fine for me. I'll use what the smiapp driver does. Thanks. -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
> On Sat, 2018-11-03 at 07:55 -0700, Derek Kelly wrote: > > The following patch adds event codes for common buttons found on > > various > > provider and universal remote controls. They represent functions not > > covered by existing event codes. Once added, rc_keymaps can be > > updated > > accordingly where applicable. > > Would be great to have more than "those are used", such as knowing how > they are labeled, both with text and/or icons, and an explanation as to > why a particular existing key isn't usable. Hi Bastien, Text & icons may vary from remote to remote but the purpose/function of those buttons is basically the same. As explained, the defines in this patch represent functions not already addressed by other defines. See below for more detail. The one thing I will add that I probably should've mentioned is that these defines focus on media/htpc/stb. If you're not aware, Linux has become a common choice for these types of systems thanks to the popularity of software like Plex, Kodi, Mythtv, VDR, etc. Lastly, all these represent *common* functions in this area. Please keep this in mind as you read further. > > +/* Remote control buttons found across provider & universal remotes */ > > +#define KEY_LIVE_TV 0x2e8 /* Jump to live tv viewing */ > > KEY_TV? KEY_TV selects TV as a *input source* the same as KEY_VCR, KEY_SAT, KEY_CD, etc. whereas KEY_LIVE_TV jumps directly to live tv as opposed to local/networked media playback, dvr playback, etc. > > +#define KEY_OPTIONS 0x2e9 /* Jump to options */ > > KEY_OPTION? Software vs. media playback options. > > +#define KEY_INTERACTIVE 0x2ea /* Jump to > > interactive system/menu/item */ > > +#define KEY_MIC_INPUT0x2eb /* Trigger MIC > > input/listen mode */ > > KEY_MICMUTE? This button doesn't mute the mic, in fact it does the opposite. The mic is off until you press this button, thus triggering MIC input/listen mode and allowing the user to speak his commands. It automatically shuts off after X seconds of silence. > > +#define KEY_SCREEN_INPUT 0x2ec /* Open on-screen input > > system */ > > KEY_SWITCHVIDEOMODE? KEY_SWITCHVIDEOMODE is used for "Cycle between available video outputs (Monitor/LCD/TV-out/etc) ". This is poorly labeled in my opinion and should've been called KEY_SWITCHVIDEOOUTPUT or something similar. "Video mode" typically refers to something entirely different - how video is presented on the display, not what physical display you're using. KEY_SCREEN_INPUT is used to bring up things like an on-screen keyboard or other on-onscreen user input method. > > +#define KEY_SYSTEM_MENU 0x2ed /* Open systems > > menu/display */ > > KEY_MENU? Systems menus as pertains to DVB. KEY_MENU is generic and having only one `menu` option is problematic when you have different types of menus which aren't accessible from each other. > > +#define KEY_SERVICES 0x2ee /* Access services */ > > +#define KEY_DISPLAY_FORMAT 0x2ef /* Cycle display formats */ > > KEY_CONTEXT_MENU? KEY_DISPLAY_FORMAT doesn't open any menus and is used to cycle through how video is displayed on-screen to the user; full, zoomed, letterboxed, stretched, etc. KEY_CONTEXT_MENU would be for something like bringing up a playback menu where you'd set things like upscaling, deinterlacing, audio mixdown/mixup, etc. > > +#define KEY_PIP 0x2f0 /* Toggle > > Picture-in-Picture on/off */ > > +#define KEY_PIP_SWAP 0x2f1 /* Swap contents between main > > view and PIP window */ > > +#define KEY_PIP_POSITION 0x2f2 /* Cycle PIP window position > > */ > > + > > /* We avoid low common keys in module aliases so they don't get huge. */ > > #define KEY_MIN_INTERESTING KEY_MUTE > > #define KEY_MAX 0x2ff > Hopefully that makes things more clear. This patch helps users map common (media/htpc/stb) remote control buttons directly to their real functions as opposed to mapping them to some random unrelated & unused event, which can be both confusing and problematic on systems where both remote controls and say bluetooth keyboards are used. Best regards, Derek
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Hi Philipp, On 11/13/18 16:06, Philipp Zabel wrote: > From: John Sheu > > Videobuf2 presently does not allow VIDIOC_REQBUFS to destroy outstanding > buffers if the queue is of type V4L2_MEMORY_MMAP, and if the buffers are > considered "in use". This is different behavior than for other memory > types and prevents us from deallocating buffers in following two cases: > > 1) There are outstanding mmap()ed views on the buffer. However even if >we put the buffer in reqbufs(0), there will be remaining references, >due to vma .open/close() adjusting vb2 buffer refcount appropriately. >This means that the buffer will be in fact freed only when the last >mmap()ed view is unmapped. > > 2) Buffer has been exported as a DMABUF. Refcount of the vb2 buffer >is managed properly by VB2 DMABUF ops, i.e. incremented on DMABUF >get and decremented on DMABUF release. This means that the buffer >will be alive until all importers release it. > > Considering both cases above, there does not seem to be any need to > prevent reqbufs(0) operation, because buffer lifetime is already > properly managed by both mmap() and DMABUF code paths. Let's remove it > and allow userspace freeing the queue (and potentially allocating a new > one) even though old buffers might be still in processing. > > To let userspace know that the kernel now supports orphaning buffers > that are still in use, add a new V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS > to be set by reqbufs and create_bufs. Looks good, but I have some questions: 1) does v4l2-compliance together with vivid (easiest to test) still work? I don't think I have a proper test for this in v4l2-compliance, but I'm not 100% certain. If it fails with this patch, then please provide a fix for v4l2-compliance as well. 2) I would like to see a new test in v4l2-compliance for this: i.e. if the capability is set, then check that you can call REQBUFS(0) before unmapping all buffers. Ditto with dmabuffers. I said during the media summit that I wanted to be more strict about requiring compliance tests before adding new features, so you're the unlucky victim of that :-) Look for munmap_bufs in v4l2-test-buffers.cpp (the MMAP case). The dmabuf tests are a bit trickier since I noticed that I never actually close the dmabuf fds in v4l2-compliance. This will fix that: diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp b/utils/v4l2-compliance/v4l2-test-buffers.cpp index c59a56d9..03639301 100644 --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp @@ -1201,6 +1201,7 @@ int testDmaBuf(struct node *expbuf_node, struct node *node, unsigned frame_count fail_on_test(captureBufs(node, q, m2m_q, frame_count, true)); fail_on_test(node->streamoff(q.g_type())); fail_on_test(node->streamoff(q.g_type())); + exp_q.close_exported_fds(); } return 0; } What is also missing in testDmaBuf is calling q.reqbufs(node, 0) to free all buffers, and I never munmap the buffers by calling q.munmap_bufs(node); In other words, clearly I never wrote proper tests for the behavior of mmap()/dmabuf and REQBUFS(0). Regards, Hans > > Signed-off-by: John Sheu > Reviewed-by: Pawel Osciak > Reviewed-by: Tomasz Figa > Signed-off-by: Tomasz Figa > [p.za...@pengutronix.de: moved __vb2_queue_cancel out of the mmap_lock > and added V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS] > Signed-off-by: Philipp Zabel > --- > .../media/common/videobuf2/videobuf2-core.c | 26 +-- > .../media/common/videobuf2/videobuf2-v4l2.c | 2 +- > include/uapi/linux/videodev2.h| 1 + > 3 files changed, 3 insertions(+), 26 deletions(-) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c > b/drivers/media/common/videobuf2/videobuf2-core.c > index 975ff5669f72..608459450c1e 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -553,20 +553,6 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct > vb2_buffer *vb) > } > EXPORT_SYMBOL(vb2_buffer_in_use); > > -/* > - * __buffers_in_use() - return true if any buffers on the queue are in use > and > - * the queue cannot be freed (by the means of REQBUFS(0)) call > - */ > -static bool __buffers_in_use(struct vb2_queue *q) > -{ > - unsigned int buffer; > - for (buffer = 0; buffer < q->num_buffers; ++buffer) { > - if (vb2_buffer_in_use(q, q->bufs[buffer])) > - return true; > - } > - return false; > -} > - > void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) > { > call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); > @@ -674,23 +660,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum > vb2_memory memory, > > if (*count == 0 || q->num_buffers != 0 || > (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { >
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Sorry, that should have said [PATCH v2]. regards Philipp
[PATCH 1/2] vb2: add waiting_in_dqbuf flag
Calling VIDIOC_DQBUF can release the core serialization lock pointed to by vb2_queue->lock if it has to wait for a new buffer to arrive. However, if userspace dup()ped the video device filehandle, then it is possible to read or call DQBUF from two filehandles at the same time. It is also possible to call REQBUFS from one filehandle while the other is waiting for a buffer. This will remove all the buffers and reallocate new ones. Removing all the buffers isn't the problem here (that's already handled correctly by DQBUF), but the reallocating part is: DQBUF isn't aware that the buffers have changed. This is fixed by setting a flag whenever the lock is released while waiting for a buffer to arrive. And checking the flag where needed so we can return -EBUSY. Signed-off-by: Hans Verkuil --- .../media/common/videobuf2/videobuf2-core.c| 18 ++ include/media/videobuf2-core.h | 1 + 2 files changed, 19 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 03954c13024c..138223af701f 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -672,6 +672,11 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, return -EBUSY; } + if (q->waiting_in_dqbuf && *count) { + dprintk(1, "another dup()ped fd is waiting for a buffer\n"); + return -EBUSY; + } + if (*count == 0 || q->num_buffers != 0 || (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { /* @@ -1624,6 +1629,11 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) for (;;) { int ret; + if (q->waiting_in_dqbuf) { + dprintk(1, "another dup()ped fd is waiting for a buffer\n"); + return -EBUSY; + } + if (!q->streaming) { dprintk(1, "streaming off, will not wait for buffers\n"); return -EINVAL; @@ -1651,6 +1661,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) return -EAGAIN; } + q->waiting_in_dqbuf = 1; /* * We are streaming and blocking, wait for another buffer to * become ready or for streamoff. Driver's lock is released to @@ -1671,6 +1682,7 @@ static int __vb2_wait_for_done_vb(struct vb2_queue *q, int nonblocking) * the locks or return an error if one occurred. */ call_void_qop(q, wait_finish, q); + q->waiting_in_dqbuf = 0; if (ret) { dprintk(1, "sleep was interrupted\n"); return ret; @@ -2547,6 +2559,12 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ if (!data) return -EINVAL; + if (q->waiting_in_dqbuf) { + dprintk(3, "another dup()ped fd is %s\n", + read ? "reading" : "writing"); + return -EBUSY; + } + /* * Initialize emulator on first call. */ diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index e86981d615ae..613f22910174 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -584,6 +584,7 @@ struct vb2_queue { unsigned intstart_streaming_called:1; unsigned interror:1; unsigned intwaiting_for_buffers:1; + unsigned intwaiting_in_dqbuf:1; unsigned intis_multiplanar:1; unsigned intis_output:1; unsigned intcopy_timestamp:1; -- 2.19.1
[PATCH 0/2] vb2: fix syzkaller race conditions
From: Hans Verkuil These two patches fix syzkaller race conditions. The basic problem is the same for both: in some specific cases an ioctl can release the core serialization mutex, thus allowing another thread to access the same vb2 queue through a dup()ped filehandle. This can happen in VIDIOC_DQBUF and read()/write() (this calls dqbuf internally, so it is really the same problem): if no new buffer is available the DQBUF ioctl will block and wait for a new buffer to arrive. Before it waits it releases the serialization lock, and afterwards it reacquires it. This is intentional, since you do not want to block other ioctls while waiting for a buffer. But this means that you need to flag that you are waiting for a buffer and check the flag in the appropriate places. The same can happen in the stop_streaming operation: the driver may have to release the serialization lock while waiting for streaming to stop (vivid does this). The same approach is used to prevent new read()s/write()s or QBUF ioctls while it is in stop_streaming. These flags are always checked/set with the serialization mutex locked. Regards, Hans Hans Verkuil (2): vb2: add waiting_in_dqbuf flag vb2: don't allow queueing buffers when canceling queue .../media/common/videobuf2/videobuf2-core.c | 32 ++- include/media/videobuf2-core.h| 2 ++ 2 files changed, 33 insertions(+), 1 deletion(-) -- 2.19.1
[PATCH 2/2] vb2: don't allow queueing buffers when canceling queue
Calling the stop_streaming op can release the core serialization lock pointed to by vb2_queue->lock if it has to wait for buffers to finish. An example of that behavior is the vivid driver. However, if userspace dup()ped the video device filehandle, then it is possible to stop streaming on one filehandle and call read/write or VIDIOC_QBUF from the other. This is fixed by setting a flag whenever stop_streaming is called and checking the flag where needed so we can return -EBUSY. Signed-off-by: Hans Verkuil Reported-by: syzbot+736c3aae4af7b50d9...@syzkaller.appspotmail.com --- drivers/media/common/videobuf2/videobuf2-core.c | 14 +- include/media/videobuf2-core.h | 1 + 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 138223af701f..560577321fe7 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -1503,6 +1503,10 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb, dprintk(1, "fatal error occurred on queue\n"); return -EIO; } + if (q->in_stop_streaming) { + dprintk(1, "stop_streaming is called\n"); + return -EBUSY; + } vb = q->bufs[index]; @@ -1834,8 +1838,11 @@ static void __vb2_queue_cancel(struct vb2_queue *q) * Tell driver to stop all transactions and release all queued * buffers. */ - if (q->start_streaming_called) + if (q->start_streaming_called) { + q->in_stop_streaming = 1; call_void_qop(q, stop_streaming, q); + q->in_stop_streaming = 0; + } /* * If you see this warning, then the driver isn't cleaning up properly @@ -2565,6 +2572,11 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ return -EBUSY; } + if (q->in_stop_streaming) { + dprintk(3, "stop_streaming is called\n"); + return -EBUSY; + } + /* * Initialize emulator on first call. */ diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index 613f22910174..5a3d3ada5940 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -585,6 +585,7 @@ struct vb2_queue { unsigned interror:1; unsigned intwaiting_for_buffers:1; unsigned intwaiting_in_dqbuf:1; + unsigned intin_stop_streaming:1; unsigned intis_multiplanar:1; unsigned intis_output:1; unsigned intcopy_timestamp:1; -- 2.19.1
[PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
From: John Sheu Videobuf2 presently does not allow VIDIOC_REQBUFS to destroy outstanding buffers if the queue is of type V4L2_MEMORY_MMAP, and if the buffers are considered "in use". This is different behavior than for other memory types and prevents us from deallocating buffers in following two cases: 1) There are outstanding mmap()ed views on the buffer. However even if we put the buffer in reqbufs(0), there will be remaining references, due to vma .open/close() adjusting vb2 buffer refcount appropriately. This means that the buffer will be in fact freed only when the last mmap()ed view is unmapped. 2) Buffer has been exported as a DMABUF. Refcount of the vb2 buffer is managed properly by VB2 DMABUF ops, i.e. incremented on DMABUF get and decremented on DMABUF release. This means that the buffer will be alive until all importers release it. Considering both cases above, there does not seem to be any need to prevent reqbufs(0) operation, because buffer lifetime is already properly managed by both mmap() and DMABUF code paths. Let's remove it and allow userspace freeing the queue (and potentially allocating a new one) even though old buffers might be still in processing. To let userspace know that the kernel now supports orphaning buffers that are still in use, add a new V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS to be set by reqbufs and create_bufs. Signed-off-by: John Sheu Reviewed-by: Pawel Osciak Reviewed-by: Tomasz Figa Signed-off-by: Tomasz Figa [p.za...@pengutronix.de: moved __vb2_queue_cancel out of the mmap_lock and added V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS] Signed-off-by: Philipp Zabel --- .../media/common/videobuf2/videobuf2-core.c | 26 +-- .../media/common/videobuf2/videobuf2-v4l2.c | 2 +- include/uapi/linux/videodev2.h| 1 + 3 files changed, 3 insertions(+), 26 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 975ff5669f72..608459450c1e 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -553,20 +553,6 @@ bool vb2_buffer_in_use(struct vb2_queue *q, struct vb2_buffer *vb) } EXPORT_SYMBOL(vb2_buffer_in_use); -/* - * __buffers_in_use() - return true if any buffers on the queue are in use and - * the queue cannot be freed (by the means of REQBUFS(0)) call - */ -static bool __buffers_in_use(struct vb2_queue *q) -{ - unsigned int buffer; - for (buffer = 0; buffer < q->num_buffers; ++buffer) { - if (vb2_buffer_in_use(q, q->bufs[buffer])) - return true; - } - return false; -} - void vb2_core_querybuf(struct vb2_queue *q, unsigned int index, void *pb) { call_void_bufop(q, fill_user_buffer, q->bufs[index], pb); @@ -674,23 +660,13 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory memory, if (*count == 0 || q->num_buffers != 0 || (q->memory != VB2_MEMORY_UNKNOWN && q->memory != memory)) { - /* -* We already have buffers allocated, so first check if they -* are not in use and can be freed. -*/ - mutex_lock(>mmap_lock); - if (q->memory == VB2_MEMORY_MMAP && __buffers_in_use(q)) { - mutex_unlock(>mmap_lock); - dprintk(1, "memory in use, cannot free\n"); - return -EBUSY; - } - /* * Call queue_cancel to clean up any buffers in the * QUEUED state which is possible if buffers were prepared or * queued without ever calling STREAMON. */ __vb2_queue_cancel(q); + mutex_lock(>mmap_lock); ret = __vb2_queue_free(q, q->num_buffers); mutex_unlock(>mmap_lock); if (ret) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index a17033ab2c22..f02d452ceeb9 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -624,7 +624,7 @@ EXPORT_SYMBOL(vb2_querybuf); static void fill_buf_caps(struct vb2_queue *q, u32 *caps) { - *caps = 0; + *caps = V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS; if (q->io_modes & VB2_MMAP) *caps |= V4L2_BUF_CAP_SUPPORTS_MMAP; if (q->io_modes & VB2_USERPTR) diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c8e8ff810190..2a223835214c 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -879,6 +879,7 @@ struct v4l2_requestbuffers { #define V4L2_BUF_CAP_SUPPORTS_USERPTR (1 << 1) #define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2) #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3) +#define V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS (1 << 4) /** *
[PATCH] vb2: vb2_mmap: move lock up
If a filehandle is dup()ped, then it is possible to close it from one fd and call mmap from the other. This creates a race condition in vb2_mmap where it is using queue data that __vb2_queue_free (called from close()) is in the process of releasing. By moving up the mutex_lock(mmap_lock) in vb2_mmap this race is avoided since __vb2_queue_free is called with the same mutex locked. So vb2_mmap now reads consistent buffer data. Signed-off-by: Hans Verkuil Reported-by: syzbot+be93025dd45dccd89...@syzkaller.appspotmail.com --- drivers/media/common/videobuf2/videobuf2-core.c | 11 --- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index c49c67473408..03954c13024c 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -2120,9 +2120,13 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) return -EINVAL; } } + + mutex_lock(>mmap_lock); + if (vb2_fileio_is_active(q)) { dprintk(1, "mmap: file io in progress\n"); - return -EBUSY; + ret = -EBUSY; + goto unlock; } /* @@ -2130,7 +2134,7 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) */ ret = __find_plane_by_offset(q, off, , ); if (ret) - return ret; + goto unlock; vb = q->bufs[buffer]; @@ -2146,8 +2150,9 @@ int vb2_mmap(struct vb2_queue *q, struct vm_area_struct *vma) return -EINVAL; } - mutex_lock(>mmap_lock); ret = call_memop(vb, mmap, vb->planes[plane].mem_priv, vma); + +unlock: mutex_unlock(>mmap_lock); if (ret) return ret; -- 2.19.1
Re: [PATCH 3/7] media: ov2640: add V4L2_CID_TEST_PATTERN control
2018年11月13日(火) 19:37 Sakari Ailus : > > On Tue, Nov 13, 2018 at 01:00:50AM +0900, Akinobu Mita wrote: > > The ov2640 has the test pattern generator features. This makes use of > > it through V4L2_CID_TEST_PATTERN control. > > > > Cc: Sakari Ailus > > Cc: Mauro Carvalho Chehab > > Signed-off-by: Akinobu Mita > > --- > > drivers/media/i2c/ov2640.c | 14 +- > > 1 file changed, 13 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c > > index 20a8853..4992d77 100644 > > --- a/drivers/media/i2c/ov2640.c > > +++ b/drivers/media/i2c/ov2640.c > > @@ -705,6 +705,11 @@ static int ov2640_reset(struct i2c_client *client) > > return ret; > > } > > > > +static const char * const ov2640_test_pattern_menu[] = { > > + "Disabled", > > + "Color bar", > > s/b/B/ > > The smiapp driver uses "Eight Vertical Colour Bars", not sure if that'd fit > here or not. FYI. This test pattern shows eight vertical color bars with blending actual sensor image, although the datasheet tells just 'Color bar'. So either is fine for me.
Re: [PATCH v2] Input: Add missing event codes for common IR remote buttons
On Sat, 2018-11-03 at 07:55 -0700, Derek Kelly wrote: > The following patch adds event codes for common buttons found on > various > provider and universal remote controls. They represent functions not > covered by existing event codes. Once added, rc_keymaps can be > updated > accordingly where applicable. Would be great to have more than "those are used", such as knowing how they are labeled, both with text and/or icons, and an explanation as to why a particular existing key isn't usable. > v2 changes: > Renamed KEY_SYSTEM to KEY_SYSTEM_MENU to avoid conflict with powerpc > KEY_SYSTEM define. > > Signed-off-by: Derek Kelly > --- > include/uapi/linux/input-event-codes.h | 13 + > 1 file changed, 13 insertions(+) > > diff --git a/include/uapi/linux/input-event-codes.h > b/include/uapi/linux/input-event-codes.h > index 53fbae27b280..a15fd3c944d2 100644 > --- a/include/uapi/linux/input-event-codes.h > +++ b/include/uapi/linux/input-event-codes.h > @@ -689,6 +689,19 @@ > #define BTN_TRIGGER_HAPPY39 0x2e6 > #define BTN_TRIGGER_HAPPY40 0x2e7 > > +/* Remote control buttons found across provider & universal remotes */ > +#define KEY_LIVE_TV 0x2e8 /* Jump to live tv viewing */ KEY_TV? > +#define KEY_OPTIONS 0x2e9 /* Jump to options */ KEY_OPTION? > +#define KEY_INTERACTIVE 0x2ea /* Jump to interactive > system/menu/item */ > +#define KEY_MIC_INPUT0x2eb /* Trigger MIC > input/listen mode */ KEY_MICMUTE? > +#define KEY_SCREEN_INPUT 0x2ec /* Open on-screen input system > */ KEY_SWITCHVIDEOMODE? > +#define KEY_SYSTEM_MENU 0x2ed /* Open systems > menu/display */ KEY_MENU? > +#define KEY_SERVICES 0x2ee /* Access services */ > +#define KEY_DISPLAY_FORMAT 0x2ef /* Cycle display formats */ KEY_CONTEXT_MENU? > +#define KEY_PIP 0x2f0 /* Toggle > Picture-in-Picture on/off */ > +#define KEY_PIP_SWAP 0x2f1 /* Swap contents between main > view and PIP window */ > +#define KEY_PIP_POSITION 0x2f2 /* Cycle PIP window position */ > + > /* We avoid low common keys in module aliases so they don't get huge. */ > #define KEY_MIN_INTERESTING KEY_MUTE > #define KEY_MAX 0x2ff
[PATCH v5 10/11] media: ov5640: Add 60 fps support
Now that we have everything in place to compute the clock rate at runtime, we can enable the 60fps framerate for the mode we tested it with. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index dd6a07a8a4e5..7fa508f61dc6 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -110,6 +110,7 @@ enum ov5640_mode_id { enum ov5640_frame_rate { OV5640_15_FPS = 0, OV5640_30_FPS, + OV5640_60_FPS, OV5640_NUM_FRAMERATES, }; @@ -138,6 +139,7 @@ MODULE_PARM_DESC(virtual_channel, static const int ov5640_framerates[] = { [OV5640_15_FPS] = 15, [OV5640_30_FPS] = 30, + [OV5640_60_FPS] = 60, }; /* regulator supplies */ @@ -1562,6 +1564,11 @@ ov5640_find_mode(struct ov5640_dev *sensor, enum ov5640_frame_rate fr, (!nearest && (mode->hact != width || mode->vact != height))) return NULL; + /* Only 640x480 can operate at 60fps (for now) */ + if (fr == OV5640_60_FPS && + !(mode->hact == 640 && mode->vact == 480)) + return NULL; + return mode; } @@ -2057,12 +2064,13 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor, int i; minfps = ov5640_framerates[OV5640_15_FPS]; - maxfps = ov5640_framerates[OV5640_30_FPS]; + maxfps = ov5640_framerates[OV5640_60_FPS]; if (fi->numerator == 0) { fi->denominator = maxfps; fi->numerator = 1; - return OV5640_30_FPS; + rate = OV5640_60_FPS; + goto find_mode; } fps = clamp_val(DIV_ROUND_CLOSEST(fi->denominator, fi->numerator), @@ -2081,6 +2089,7 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor, fi->numerator = 1; fi->denominator = best_fps; +find_mode: mode = ov5640_find_mode(sensor, rate, width, height, false); return mode ? rate : -EINVAL; } @@ -2700,8 +2709,11 @@ static int ov5640_s_frame_interval(struct v4l2_subdev *sd, frame_rate = ov5640_try_frame_interval(sensor, >interval, mode->hact, mode->vact); - if (frame_rate < 0) - frame_rate = OV5640_15_FPS; + if (frame_rate < 0) { + /* Always return a valid frame interval value */ + fi->interval = sensor->frame_interval; + goto out; + } mode = ov5640_find_mode(sensor, frame_rate, mode->hact, mode->vact, true); -- 2.19.1
[PATCH v5 05/11] media: ov5640: Compute the clock rate at runtime
The clock rate, while hardcoded until now, is actually a function of the resolution, framerate and bytes per pixel. Now that we have an algorithm to adjust our clock rate, we can select it dynamically when we change the mode. This changes a bit the clock rate being used, with the following effect: +--+--+--+--+-+-++---+ | Hact | Vact | Htot | Vtot | FPS | Hardcoded clock | Computed clock | Deviation | +--+--+--+--+-+-++---+ | 640 | 480 | 1896 | 1080 | 15 |5600 | 61430400 | 8.84 % | | 640 | 480 | 1896 | 1080 | 30 | 11200 | 122860800 | 8.84 % | | 1024 | 768 | 1896 | 1080 | 15 |5600 | 61430400 | 8.84 % | | 1024 | 768 | 1896 | 1080 | 30 | 11200 | 122860800 | 8.84 % | | 320 | 240 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 % | | 320 | 240 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 % | | 176 | 144 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 % | | 176 | 144 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 % | | 720 | 480 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 % | | 720 | 480 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 % | | 720 | 576 | 1896 | 984 | 15 |5600 | 55969920 | 0.05 % | | 720 | 576 | 1896 | 984 | 30 | 11200 | 111939840 | 0.05 % | | 1280 | 720 | 1892 | 740 | 15 |4200 | 42002400 | 0.01 % | | 1280 | 720 | 1892 | 740 | 30 |8400 | 84004800 | 0.01 % | | 1920 | 1080 | 2500 | 1120 | 15 |8400 | 8400 | 0.00 % | | 1920 | 1080 | 2500 | 1120 | 30 | 16800 | 16800 | 0.00 % | | 2592 | 1944 | 2844 | 1944 | 15 |8400 | 165862080 | 49.36 % | +--+--+--+--+-+-++---+ Only the 640x480, 1024x768 and 2592x1944 modes are significantly affected by the new formula. In this case, 640x480 and 1024x768 are actually fixed by this change. Indeed, the sensor was sending data at, for example, 27.33fps instead of 30fps. This is -9%, which is roughly what we're seeing in the array. Testing these modes with the new clock setup actually fix that error, and data are now sent at around 30fps. 2592x1944, on the other hand, is probably due to the fact that this mode can only be used using MIPI-CSI2, in a two lane mode, and never really tested with a DVP bus. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 25613ecf83c5..bcfb2b25a450 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -1992,7 +1992,8 @@ static int ov5640_set_mode(struct ov5640_dev *sensor) * All the formats we support have 16 bits per pixel, seems to require * the same rate than YUV, so we can just use 16 bpp all the time. */ - rate = mode->pixel_clock * 16; + rate = mode->vtot * mode->htot * 16; + rate *= ov5640_framerates[sensor->current_fr]; if (sensor->ep.bus_type == V4L2_MBUS_CSI2_DPHY) { rate = rate / sensor->ep.bus.mipi_csi2.num_data_lanes; ret = ov5640_set_mipi_pclk(sensor, rate); -- 2.19.1
[PATCH v5 06/11] media: ov5640: Remove pixel clock rates
The pixel clock rates were introduced to report the initially static clock rate. Since this is now handled dynamically, we can remove them entirely. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 21 + 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index bcfb2b25a450..e96063c9e352 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -172,7 +172,6 @@ struct ov5640_mode_info { u32 htot; u32 vact; u32 vtot; - u32 pixel_clock; const struct reg_value *reg_data; u32 reg_data_size; }; @@ -696,7 +695,6 @@ static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = { /* power-on sensor init reg table */ static const struct ov5640_mode_info ov5640_mode_init_data = { 0, SUBSAMPLING, 640, 1896, 480, 984, - 5600, ov5640_init_setting_30fps_VGA, ARRAY_SIZE(ov5640_init_setting_30fps_VGA), }; @@ -706,91 +704,74 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = { { {OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 1896, 144, 984, -2800, ov5640_setting_15fps_QCIF_176_144, ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)}, {OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320, 1896, 240, 984, -2800, ov5640_setting_15fps_QVGA_320_240, ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)}, {OV5640_MODE_VGA_640_480, SUBSAMPLING, 640, 1896, 480, 1080, -2800, ov5640_setting_15fps_VGA_640_480, ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)}, {OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 1896, 480, 984, -2800, ov5640_setting_15fps_NTSC_720_480, ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)}, {OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 1896, 576, 984, -2800, ov5640_setting_15fps_PAL_720_576, ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)}, {OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 1896, 768, 1080, -2800, ov5640_setting_15fps_XGA_1024_768, ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)}, {OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 1892, 720, 740, -2100, ov5640_setting_15fps_720P_1280_720, ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)}, {OV5640_MODE_1080P_1920_1080, SCALING, 1920, 2500, 1080, 1120, -4200, ov5640_setting_15fps_1080P_1920_1080, ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)}, {OV5640_MODE_QSXGA_2592_1944, SCALING, 2592, 2844, 1944, 1968, -8400, ov5640_setting_15fps_QSXGA_2592_1944, ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)}, }, { {OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 1896, 144, 984, -5600, ov5640_setting_30fps_QCIF_176_144, ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)}, {OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320, 1896, 240, 984, -5600, ov5640_setting_30fps_QVGA_320_240, ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)}, {OV5640_MODE_VGA_640_480, SUBSAMPLING, 640, 1896, 480, 1080, -5600, ov5640_setting_30fps_VGA_640_480, ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)}, {OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 1896, 480, 984, -5600, ov5640_setting_30fps_NTSC_720_480, ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)}, {OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 1896, 576, 984, -5600, ov5640_setting_30fps_PAL_720_576, ARRAY_SIZE(ov5640_setting_30fps_PAL_720_576)}, {OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 1896, 768, 1080, -5600, ov5640_setting_30fps_XGA_1024_768, ARRAY_SIZE(ov5640_setting_30fps_XGA_1024_768)}, {OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 1892, 720, 740, -4200, ov5640_setting_30fps_720P_1280_720, ARRAY_SIZE(ov5640_setting_30fps_720P_1280_720)}, {OV5640_MODE_1080P_1920_1080,
[PATCH v5 04/11] media: ov5640: Remove redundant register setup
The MIPI divider is also cleared as part of the clock setup sequence, so we can remove that code. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 1b295d07aa15..25613ecf83c5 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -1403,16 +1403,6 @@ static int ov5640_set_stream_dvp(struct ov5640_dev *sensor, bool on) */ if (on) { - /* -* reset MIPI PCLK/SERCLK divider -* -* SC PLL CONTRL1 0 -* - [3..0]:MIPI PCLK/SERCLK divider -*/ - ret = ov5640_mod_reg(sensor, OV5640_REG_SC_PLL_CTRL1, 0x0f, 0); - if (ret) - return ret; - /* * configure parallel port control lines polarity * -- 2.19.1
[PATCH v5 03/11] media: ov5640: Remove redundant defines
The OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT and OV5640_SCLK_ROOT_DIVIDER_DEFAULT defines represent exactly the same setup, and are at the same value, than the more consistent with the rest of the driver OV5640_SCLK2X_ROOT_DIV and OV5640_SCLK_ROOT_DIV. Remove them. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 7 ++- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 584e01ea765b..1b295d07aa15 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -94,9 +94,6 @@ #define OV5640_REG_SDE_CTRL5 0x5585 #define OV5640_REG_AVG_READOUT 0x56a1 -#define OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT 1 -#define OV5640_SCLK_ROOT_DIVIDER_DEFAULT 2 - enum ov5640_mode_id { OV5640_MODE_QCIF_176_144 = 0, OV5640_MODE_QVGA_320_240, @@ -2086,8 +2083,8 @@ static int ov5640_restore_mode(struct ov5640_dev *sensor) sensor->last_mode = _mode_init_data; ret = ov5640_mod_reg(sensor, OV5640_REG_SYS_ROOT_DIVIDER, 0x3f, -(ilog2(OV5640_SCLK2X_ROOT_DIVIDER_DEFAULT) << 2) | -ilog2(OV5640_SCLK_ROOT_DIVIDER_DEFAULT)); +(ilog2(OV5640_SCLK2X_ROOT_DIV) << 2) | +ilog2(OV5640_SCLK_ROOT_DIV)); if (ret) return ret; -- 2.19.1
[PATCH v5 09/11] media: ov5640: Make the FPS clamping / rounding more extendable
The current code uses an algorithm to clamp the FPS values and round them to the closest supported one that isn't really allows to be extended to more than two values. Rework it a bit to make it much easier to extend the amount of FPS options we support. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 27 +++ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index fc2e03193da6..dd6a07a8a4e5 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -2053,7 +2053,8 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor, { const struct ov5640_mode_info *mode; enum ov5640_frame_rate rate = OV5640_30_FPS; - u32 minfps, maxfps, fps; + int minfps, maxfps, best_fps, fps; + int i; minfps = ov5640_framerates[OV5640_15_FPS]; maxfps = ov5640_framerates[OV5640_30_FPS]; @@ -2064,19 +2065,21 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor, return OV5640_30_FPS; } - fps = DIV_ROUND_CLOSEST(fi->denominator, fi->numerator); + fps = clamp_val(DIV_ROUND_CLOSEST(fi->denominator, fi->numerator), + minfps, maxfps); + + best_fps = minfps; + for (i = 0; i < ARRAY_SIZE(ov5640_framerates); i++) { + int curr_fps = ov5640_framerates[i]; + + if (abs(curr_fps - fps) < abs(best_fps - fps)) { + best_fps = curr_fps; + rate = i; + } + } fi->numerator = 1; - if (fps > maxfps) - fi->denominator = maxfps; - else if (fps < minfps) - fi->denominator = minfps; - else if (2 * fps >= 2 * minfps + (maxfps - minfps)) - fi->denominator = maxfps; - else - fi->denominator = minfps; - - rate = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS; + fi->denominator = best_fps; mode = ov5640_find_mode(sensor, rate, width, height, false); return mode ? rate : -EINVAL; -- 2.19.1
[PATCH v5 07/11] media: ov5640: Enhance FPS handling
Now that we have moved the clock generation logic out of the bytes array, these arrays are identical between the 15fps and 30fps variants. Remove the duplicate entries, and convert the code accordingly. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 306 +++-- 1 file changed, 51 insertions(+), 255 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index e96063c9e352..be047dd7fbfc 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -340,64 +340,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = { {0x3a1f, 0x14, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3c00, 0x04, 0, 300}, }; -static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = { - {0x3c07, 0x08, 0, 0}, - {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, - {0x3814, 0x31, 0, 0}, - {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, - {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0}, - {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0}, - {0x3810, 0x00, 0, 0}, - {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0}, - {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0}, - {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0}, - {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0}, - {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0}, - {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0}, - {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0}, - {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0}, - {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0}, -}; - -static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = { - {0x3c07, 0x08, 0, 0}, - {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, - {0x3814, 0x31, 0, 0}, - {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, - {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0}, - {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0}, - {0x3810, 0x00, 0, 0}, - {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0}, - {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0}, - {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0}, - {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x27, 0, 0}, - {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0}, - {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0}, - {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0}, - {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0}, - {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, -}; - -static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = { - {0x3c07, 0x08, 0, 0}, - {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, - {0x3814, 0x31, 0, 0}, - {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, - {0x3802, 0x00, 0, 0}, {0x3803, 0x04, 0, 0}, {0x3804, 0x0a, 0, 0}, - {0x3805, 0x3f, 0, 0}, {0x3806, 0x07, 0, 0}, {0x3807, 0x9b, 0, 0}, - {0x3810, 0x00, 0, 0}, - {0x3811, 0x10, 0, 0}, {0x3812, 0x00, 0, 0}, {0x3813, 0x06, 0, 0}, - {0x3618, 0x00, 0, 0}, {0x3612, 0x29, 0, 0}, {0x3708, 0x64, 0, 0}, - {0x3709, 0x52, 0, 0}, {0x370c, 0x03, 0, 0}, {0x3a02, 0x03, 0, 0}, - {0x3a03, 0xd8, 0, 0}, {0x3a08, 0x01, 0, 0}, {0x3a09, 0x0e, 0, 0}, - {0x3a0a, 0x00, 0, 0}, {0x3a0b, 0xf6, 0, 0}, {0x3a0e, 0x03, 0, 0}, - {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x03, 0, 0}, {0x3a15, 0xd8, 0, 0}, - {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0}, - {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0}, - {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0}, -}; - -static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = { +static const struct reg_value ov5640_setting_VGA_640_480[] = { {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, @@ -416,7 +359,7 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = { {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, }; -static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = { +static const struct reg_value ov5640_setting_XGA_1024_768[] = { {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, @@ -435,7 +378,7 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = { {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, }; -static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = { +static const
[PATCH v5 02/11] media: ov5640: Remove the clocks registers initialization
Part of the hardcoded initialization sequence is to set up the proper clock dividers. However, this is now done dynamically through proper code and as such, the static one is now redundant. Let's remove it. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 46 ++ 1 file changed, 22 insertions(+), 24 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 8476f85bb8e7..584e01ea765b 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -262,8 +262,7 @@ static inline struct v4l2_subdev *ctrl_to_sd(struct v4l2_ctrl *ctrl) static const struct reg_value ov5640_init_setting_30fps_VGA[] = { {0x3103, 0x11, 0, 0}, {0x3008, 0x82, 0, 5}, {0x3008, 0x42, 0, 0}, {0x3103, 0x03, 0, 0}, {0x3017, 0x00, 0, 0}, {0x3018, 0x00, 0, 0}, - {0x3034, 0x18, 0, 0}, {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, - {0x3037, 0x13, 0, 0}, {0x3630, 0x36, 0, 0}, + {0x3630, 0x36, 0, 0}, {0x3631, 0x0e, 0, 0}, {0x3632, 0xe2, 0, 0}, {0x3633, 0x12, 0, 0}, {0x3621, 0xe0, 0, 0}, {0x3704, 0xa0, 0, 0}, {0x3703, 0x5a, 0, 0}, {0x3715, 0x78, 0, 0}, {0x3717, 0x01, 0, 0}, {0x370b, 0x60, 0, 0}, @@ -346,7 +345,7 @@ static const struct reg_value ov5640_init_setting_30fps_VGA[] = { }; static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = { - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -365,7 +364,7 @@ static const struct reg_value ov5640_setting_30fps_VGA_640_480[] = { }; static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = { - {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -384,7 +383,7 @@ static const struct reg_value ov5640_setting_15fps_VGA_640_480[] = { }; static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = { - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -400,11 +399,10 @@ static const struct reg_value ov5640_setting_30fps_XGA_1024_768[] = { {0x4001, 0x02, 0, 0}, {0x4004, 0x02, 0, 0}, {0x4713, 0x03, 0, 0}, {0x4407, 0x04, 0, 0}, {0x460b, 0x35, 0, 0}, {0x460c, 0x22, 0, 0}, {0x3824, 0x02, 0, 0}, {0x5001, 0xa3, 0, 0}, {0x3503, 0x00, 0, 0}, - {0x3035, 0x12, 0, 0}, }; static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = { - {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -423,7 +421,7 @@ static const struct reg_value ov5640_setting_15fps_XGA_1024_768[] = { }; static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = { - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -442,7 +440,7 @@ static const struct reg_value ov5640_setting_30fps_QVGA_320_240[] = { }; static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = { - {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -461,7 +459,7 @@ static const struct reg_value ov5640_setting_15fps_QVGA_320_240[] = { }; static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = { - {0x3035, 0x14, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0}, {0x3815, 0x31, 0, 0}, {0x3800, 0x00, 0, 0}, {0x3801, 0x00, 0, 0}, @@ -480,7 +478,7 @@ static const struct reg_value ov5640_setting_30fps_QCIF_176_144[] = { }; static const struct reg_value ov5640_setting_15fps_QCIF_176_144[] = { - {0x3035, 0x22, 0, 0}, {0x3036, 0x38, 0, 0}, {0x3c07, 0x08, 0, 0}, + {0x3c07, 0x08, 0, 0}, {0x3c09, 0x1c, 0, 0}, {0x3c0a, 0x9c, 0, 0}, {0x3c0b, 0x40, 0, 0}, {0x3814, 0x31, 0, 0},
[PATCH v5 08/11] media: ov5640: Make the return rate type more explicit
In the ov5640_try_frame_interval function, the ret variable actually holds the frame rate index to use, which is represented by the enum ov5640_frame_rate in the driver. Make it more obvious. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index be047dd7fbfc..fc2e03193da6 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -2052,8 +2052,8 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor, u32 width, u32 height) { const struct ov5640_mode_info *mode; + enum ov5640_frame_rate rate = OV5640_30_FPS; u32 minfps, maxfps, fps; - int ret; minfps = ov5640_framerates[OV5640_15_FPS]; maxfps = ov5640_framerates[OV5640_30_FPS]; @@ -2076,10 +2076,10 @@ static int ov5640_try_frame_interval(struct ov5640_dev *sensor, else fi->denominator = minfps; - ret = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS; + rate = (fi->denominator == minfps) ? OV5640_15_FPS : OV5640_30_FPS; - mode = ov5640_find_mode(sensor, ret, width, height, false); - return mode ? ret : -EINVAL; + mode = ov5640_find_mode(sensor, rate, width, height, false); + return mode ? rate : -EINVAL; } static int ov5640_get_fmt(struct v4l2_subdev *sd, -- 2.19.1
[PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate
The clock structure for the PCLK is quite obscure in the documentation, and was hardcoded through the bytes array of each and every mode. This is troublesome, since we cannot adjust it at runtime based on other parameters (such as the number of bytes per pixel), and we can't support either framerates that have not been used by the various vendors, since we don't have the needed initialization sequence. We can however understand how the clock tree works, and then implement some functions to derive the various parameters from a given rate. And now that those parameters are calculated at runtime, we can remove them from the initialization sequence. The modes also gained a new parameter which is the clock that they are running at, from the register writes they were doing, so for now the switch to the new algorithm should be transparent. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 366 - 1 file changed, 365 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index eaefdb58653b..8476f85bb8e7 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -175,6 +175,7 @@ struct ov5640_mode_info { u32 htot; u32 vact; u32 vtot; + u32 pixel_clock; const struct reg_value *reg_data; u32 reg_data_size; }; @@ -700,6 +701,7 @@ static const struct reg_value ov5640_setting_15fps_QSXGA_2592_1944[] = { /* power-on sensor init reg table */ static const struct ov5640_mode_info ov5640_mode_init_data = { 0, SUBSAMPLING, 640, 1896, 480, 984, + 5600, ov5640_init_setting_30fps_VGA, ARRAY_SIZE(ov5640_init_setting_30fps_VGA), }; @@ -709,74 +711,91 @@ ov5640_mode_data[OV5640_NUM_FRAMERATES][OV5640_NUM_MODES] = { { {OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 1896, 144, 984, +2800, ov5640_setting_15fps_QCIF_176_144, ARRAY_SIZE(ov5640_setting_15fps_QCIF_176_144)}, {OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320, 1896, 240, 984, +2800, ov5640_setting_15fps_QVGA_320_240, ARRAY_SIZE(ov5640_setting_15fps_QVGA_320_240)}, {OV5640_MODE_VGA_640_480, SUBSAMPLING, 640, 1896, 480, 1080, +2800, ov5640_setting_15fps_VGA_640_480, ARRAY_SIZE(ov5640_setting_15fps_VGA_640_480)}, {OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 1896, 480, 984, +2800, ov5640_setting_15fps_NTSC_720_480, ARRAY_SIZE(ov5640_setting_15fps_NTSC_720_480)}, {OV5640_MODE_PAL_720_576, SUBSAMPLING, 720, 1896, 576, 984, +2800, ov5640_setting_15fps_PAL_720_576, ARRAY_SIZE(ov5640_setting_15fps_PAL_720_576)}, {OV5640_MODE_XGA_1024_768, SUBSAMPLING, 1024, 1896, 768, 1080, +2800, ov5640_setting_15fps_XGA_1024_768, ARRAY_SIZE(ov5640_setting_15fps_XGA_1024_768)}, {OV5640_MODE_720P_1280_720, SUBSAMPLING, 1280, 1892, 720, 740, +2100, ov5640_setting_15fps_720P_1280_720, ARRAY_SIZE(ov5640_setting_15fps_720P_1280_720)}, {OV5640_MODE_1080P_1920_1080, SCALING, 1920, 2500, 1080, 1120, +4200, ov5640_setting_15fps_1080P_1920_1080, ARRAY_SIZE(ov5640_setting_15fps_1080P_1920_1080)}, {OV5640_MODE_QSXGA_2592_1944, SCALING, 2592, 2844, 1944, 1968, +8400, ov5640_setting_15fps_QSXGA_2592_1944, ARRAY_SIZE(ov5640_setting_15fps_QSXGA_2592_1944)}, }, { {OV5640_MODE_QCIF_176_144, SUBSAMPLING, 176, 1896, 144, 984, +5600, ov5640_setting_30fps_QCIF_176_144, ARRAY_SIZE(ov5640_setting_30fps_QCIF_176_144)}, {OV5640_MODE_QVGA_320_240, SUBSAMPLING, 320, 1896, 240, 984, +5600, ov5640_setting_30fps_QVGA_320_240, ARRAY_SIZE(ov5640_setting_30fps_QVGA_320_240)}, {OV5640_MODE_VGA_640_480, SUBSAMPLING, 640, 1896, 480, 1080, +5600, ov5640_setting_30fps_VGA_640_480, ARRAY_SIZE(ov5640_setting_30fps_VGA_640_480)}, {OV5640_MODE_NTSC_720_480, SUBSAMPLING, 720, 1896, 480, 984, +5600, ov5640_setting_30fps_NTSC_720_480, ARRAY_SIZE(ov5640_setting_30fps_NTSC_720_480)}, {OV5640_MODE_PAL_720_576,
[PATCH v5 00/11] media: ov5640: Misc cleanup and improvements
Hi, Here is a "small" series that mostly cleans up the ov5640 driver code, slowly getting rid of the big data array for more understandable code (hopefully). The biggest addition would be the clock rate computation at runtime, instead of relying on those arrays to setup the clock tree properly. As a side effect, it fixes the framerate that was off by around 10% on the smaller resolutions, and we now support 60fps. This also introduces a bunch of new features. Let me know what you think, Maxime Changes from v4: - Squashed Jacopo patches fixing the MIPI-CSI case - Prefer clock rates superior to the ideal clock rate, even if it means having a less precise one. - Fix the JPEG case according to Hugues suggestions - Rebased on 4.20 Changes from v3: - Rebased on current Sakari tree - Fixed an error when changing only the framerate Changes from v2: - Rebased on latest Sakari PR - Fixed the issues reported by Hugues: improper FPS returned for formats, improper rounding of the FPS, some with his suggestions, some by simplifying the logic. - Expanded the clock tree comments based on the feedback from Samuel Bobrowicz and Loic Poulain - Merged some of the changes made by Samuel Bobrowicz to fix the MIPI rate computation, fix the call sites of the ov5640_set_timings function, the auto-exposure calculation call, etc. - Split the patches into smaller ones in order to make it more readable (hopefully) Changes from v1: - Integrated Hugues' suggestions to fix v4l2-compliance - Fixed the bus width with JPEG - Dropped the clock rate calculation loops for something simpler as suggested by Sakari - Cache the exposure value instead of using the control value - Rebased on top of 4.17 Maxime Ripard (11): media: ov5640: Adjust the clock based on the expected rate media: ov5640: Remove the clocks registers initialization media: ov5640: Remove redundant defines media: ov5640: Remove redundant register setup media: ov5640: Compute the clock rate at runtime media: ov5640: Remove pixel clock rates media: ov5640: Enhance FPS handling media: ov5640: Make the return rate type more explicit media: ov5640: Make the FPS clamping / rounding more extendable media: ov5640: Add 60 fps support media: ov5640: Remove duplicate auto-exposure setup drivers/media/i2c/ov5640.c | 748 ++--- 1 file changed, 445 insertions(+), 303 deletions(-) -- 2.19.1
[PATCH v5 11/11] media: ov5640: Remove duplicate auto-exposure setup
The autoexposure setup in the 1080p init array is redundant with the default value of the sensor. Remove it. Signed-off-by: Maxime Ripard --- drivers/media/i2c/ov5640.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/media/i2c/ov5640.c b/drivers/media/i2c/ov5640.c index 7fa508f61dc6..0bb5f78571fe 100644 --- a/drivers/media/i2c/ov5640.c +++ b/drivers/media/i2c/ov5640.c @@ -504,7 +504,7 @@ static const struct reg_value ov5640_setting_1080P_1920_1080[] = { {0x3a0e, 0x03, 0, 0}, {0x3a0d, 0x04, 0, 0}, {0x3a14, 0x04, 0, 0}, {0x3a15, 0x60, 0, 0}, {0x4713, 0x02, 0, 0}, {0x4407, 0x04, 0, 0}, {0x460b, 0x37, 0, 0}, {0x460c, 0x20, 0, 0}, {0x3824, 0x04, 0, 0}, - {0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, {0x3503, 0, 0, 0}, + {0x4005, 0x1a, 0, 0}, {0x3008, 0x02, 0, 0}, }; static const struct reg_value ov5640_setting_QSXGA_2592_1944[] = { -- 2.19.1
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
On 11/13/2018 06:31 PM, Sakari Ailus wrote: > Hi Bing Bu, > > On Mon, Nov 12, 2018 at 12:31:16PM +0800, Bing Bu Cao wrote: >> >> On 11/09/2018 06:09 PM, Sakari Ailus wrote: >>> Hi Bing Bu, >>> >>> On Wed, Nov 07, 2018 at 12:16:47PM +0800, Bing Bu Cao wrote: On 11/01/2018 08:03 PM, Sakari Ailus wrote: > Hi Yong, > > Thanks for the update! > > On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: >> Hi, >> >> This series adds support for the Intel IPU3 (Image Processing Unit) >> ImgU which is essentially a modern memory-to-memory ISP. It implements >> raw Bayer to YUV image format conversion as well as a large number of >> other pixel processing algorithms for improving the image quality. >> >> Meta data formats are defined for image statistics (3A, i.e. automatic >> white balance, exposure and focus, histogram and local area contrast >> enhancement) as well as for the pixel processing algorithm parameters. >> The documentation for these formats is currently not included in the >> patchset but will be added in a future version of this set. >> >> The algorithm parameters need to be considered specific to a given frame >> and typically a large number of these parameters change on frame to frame >> basis. Additionally, the parameters are highly structured (and not a flat >> space of independent configuration primitives). They also reflect the >> data structures used by the firmware and the hardware. On top of that, >> the algorithms require highly specialized user space to make meaningful >> use of them. For these reasons it has been chosen video buffers to pass >> the parameters to the device. >> >> On individual patches: >> >> The heart of ImgU is the CSS, or Camera Subsystem, which contains the >> image processors and HW accelerators. >> >> The 3A statistics and other firmware parameter computation related >> functions are implemented in patch 11. >> >> All IPU3 pipeline default settings can be found in patch 10. >> >> To access DDR via ImgU's own memory space, IPU3 is also equipped with >> its own MMU unit, the driver is implemented in patch 6. >> >> Patch 7 uses above driver for DMA mapping operation. >> >> The communication between IPU3 firmware and driver is implemented with >> circular >> queues in patch 8. >> >> Patch 9 provide some utility functions and manage IPU3 fw download and >> install. >> >> The firmware which is called ipu3-fw.bin can be downloaded from: >> >> git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git >> (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) >> >> Firmware ABI is defined in patches 4 and 5. >> >> Patches 12 and 13 are of the same file, the former contains all h/w >> programming >> related code, the latter implements interface functions for access fw & >> hw >> capabilities. >> >> Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: >> >> https://patchwork.kernel.org/patch/9976295/> > I've pushed the latest set here: > > https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> > > You can just say the entire set depends on those going forward; the > documentation is needed, too. > >> Patch 15 represents the top level that glues all of the other components >> together, >> passing arguments between the components. >> >> Patch 16 is a recent effort to extend v6 for advanced camera features >> like >> Continuous View Finder (CVF) and Snapshot During Video(SDV) support. >> >> Link to user space implementation: >> >> git clone >> https://chromium.googlesource.com/chromiumos/platform/arc-camera >> >> ImgU media topology print: >> >> # media-ctl -d /dev/media0 -p >> Media controller API version 4.19.0 >> >> Media device information >> >> driver ipu3-imgu >> model ipu3-imgu >> serial >> bus infoPCI::00:05.0 >> hw revision 0x80862015 >> driver version 4.19.0 >> >> Device topology >> - entity 1: ipu3-imgu 0 (5 pads, 5 links) >> type V4L2 subdev subtype Unknown flags 0 >> device node name /dev/v4l-subdev0 >> pad0: Sink >> [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown > This doesn't seem right. Which formats can be enumerated from the pad? >>> Looking at the code, the OUTPUT video nodes have 10-bit GRBG (or a variant) >>> format whereas the CAPTURE video nodes always have NV12. Can you confirm? >> Hi, Sakari, >> Yes, I think the pad_fmt should also be changed. >> Yong, could you add some extra code for this and test? like: >> >> static int ipu3_v4l2_node_setup(struct imgu_device *imgu,
Re: [PATCH 3/7] media: ov2640: add V4L2_CID_TEST_PATTERN control
On Tue, Nov 13, 2018 at 01:00:50AM +0900, Akinobu Mita wrote: > The ov2640 has the test pattern generator features. This makes use of > it through V4L2_CID_TEST_PATTERN control. > > Cc: Sakari Ailus > Cc: Mauro Carvalho Chehab > Signed-off-by: Akinobu Mita > --- > drivers/media/i2c/ov2640.c | 14 +- > 1 file changed, 13 insertions(+), 1 deletion(-) > > diff --git a/drivers/media/i2c/ov2640.c b/drivers/media/i2c/ov2640.c > index 20a8853..4992d77 100644 > --- a/drivers/media/i2c/ov2640.c > +++ b/drivers/media/i2c/ov2640.c > @@ -705,6 +705,11 @@ static int ov2640_reset(struct i2c_client *client) > return ret; > } > > +static const char * const ov2640_test_pattern_menu[] = { > + "Disabled", > + "Color bar", s/b/B/ The smiapp driver uses "Eight Vertical Colour Bars", not sure if that'd fit here or not. FYI. > +}; > + > /* > * functions > */ > @@ -740,6 +745,9 @@ static int ov2640_s_ctrl(struct v4l2_ctrl *ctrl) > case V4L2_CID_HFLIP: > val = ctrl->val ? REG04_HFLIP_IMG : 0x00; > return ov2640_mask_set(client, REG04, REG04_HFLIP_IMG, val); > + case V4L2_CID_TEST_PATTERN: > + val = ctrl->val ? COM7_COLOR_BAR_TEST : 0x00; > + return ov2640_mask_set(client, COM7, COM7_COLOR_BAR_TEST, val); > } > > return -EINVAL; > @@ -1184,12 +1192,16 @@ static int ov2640_probe(struct i2c_client *client, > v4l2_i2c_subdev_init(>subdev, client, _subdev_ops); > priv->subdev.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE; > mutex_init(>lock); > - v4l2_ctrl_handler_init(>hdl, 2); > + v4l2_ctrl_handler_init(>hdl, 3); > priv->hdl.lock = >lock; > v4l2_ctrl_new_std(>hdl, _ctrl_ops, > V4L2_CID_VFLIP, 0, 1, 1, 0); > v4l2_ctrl_new_std(>hdl, _ctrl_ops, > V4L2_CID_HFLIP, 0, 1, 1, 0); > + v4l2_ctrl_new_std_menu_items(>hdl, _ctrl_ops, > + V4L2_CID_TEST_PATTERN, > + ARRAY_SIZE(ov2640_test_pattern_menu) - 1, 0, 0, > + ov2640_test_pattern_menu); > priv->subdev.ctrl_handler = >hdl; > if (priv->hdl.error) { > ret = priv->hdl.error; > -- > 2.7.4 > -- Sakari Ailus sakari.ai...@linux.intel.com
Re: [PATCH v7 00/16] Intel IPU3 ImgU patchset
Hi Bing Bu, On Mon, Nov 12, 2018 at 12:31:16PM +0800, Bing Bu Cao wrote: > > > On 11/09/2018 06:09 PM, Sakari Ailus wrote: > > Hi Bing Bu, > > > > On Wed, Nov 07, 2018 at 12:16:47PM +0800, Bing Bu Cao wrote: > >> On 11/01/2018 08:03 PM, Sakari Ailus wrote: > >>> Hi Yong, > >>> > >>> Thanks for the update! > >>> > >>> On Mon, Oct 29, 2018 at 03:22:54PM -0700, Yong Zhi wrote: > Hi, > > This series adds support for the Intel IPU3 (Image Processing Unit) > ImgU which is essentially a modern memory-to-memory ISP. It implements > raw Bayer to YUV image format conversion as well as a large number of > other pixel processing algorithms for improving the image quality. > > Meta data formats are defined for image statistics (3A, i.e. automatic > white balance, exposure and focus, histogram and local area contrast > enhancement) as well as for the pixel processing algorithm parameters. > The documentation for these formats is currently not included in the > patchset but will be added in a future version of this set. > > The algorithm parameters need to be considered specific to a given frame > and typically a large number of these parameters change on frame to frame > basis. Additionally, the parameters are highly structured (and not a flat > space of independent configuration primitives). They also reflect the > data structures used by the firmware and the hardware. On top of that, > the algorithms require highly specialized user space to make meaningful > use of them. For these reasons it has been chosen video buffers to pass > the parameters to the device. > > On individual patches: > > The heart of ImgU is the CSS, or Camera Subsystem, which contains the > image processors and HW accelerators. > > The 3A statistics and other firmware parameter computation related > functions are implemented in patch 11. > > All IPU3 pipeline default settings can be found in patch 10. > > To access DDR via ImgU's own memory space, IPU3 is also equipped with > its own MMU unit, the driver is implemented in patch 6. > > Patch 7 uses above driver for DMA mapping operation. > > The communication between IPU3 firmware and driver is implemented with > circular > queues in patch 8. > > Patch 9 provide some utility functions and manage IPU3 fw download and > install. > > The firmware which is called ipu3-fw.bin can be downloaded from: > > git://git.kernel.org/pub/scm/linux/kernel/git/firmware/linux-firmware.git > (commit 2c27b0cb02f18c022d8378e0e1abaf8b7ae8188f) > > Firmware ABI is defined in patches 4 and 5. > > Patches 12 and 13 are of the same file, the former contains all h/w > programming > related code, the latter implements interface functions for access fw & > hw > capabilities. > > Patch 14 has a dependency on Sakari's V4L2_BUF_TYPE_META_OUTPUT work: > > https://patchwork.kernel.org/patch/9976295/> > >>> I've pushed the latest set here: > >>> > >>> https://git.linuxtv.org/sailus/media_tree.git/log/?h=meta-output> > >>> > >>> You can just say the entire set depends on those going forward; the > >>> documentation is needed, too. > >>> > Patch 15 represents the top level that glues all of the other components > together, > passing arguments between the components. > > Patch 16 is a recent effort to extend v6 for advanced camera features > like > Continuous View Finder (CVF) and Snapshot During Video(SDV) support. > > Link to user space implementation: > > git clone > https://chromium.googlesource.com/chromiumos/platform/arc-camera > > ImgU media topology print: > > # media-ctl -d /dev/media0 -p > Media controller API version 4.19.0 > > Media device information > > driver ipu3-imgu > model ipu3-imgu > serial > bus infoPCI::00:05.0 > hw revision 0x80862015 > driver version 4.19.0 > > Device topology > - entity 1: ipu3-imgu 0 (5 pads, 5 links) > type V4L2 subdev subtype Unknown flags 0 > device node name /dev/v4l-subdev0 > pad0: Sink > [fmt:UYVY8_2X8/1920x1080 field:none colorspace:unknown > >>> This doesn't seem right. Which formats can be enumerated from the pad? > > Looking at the code, the OUTPUT video nodes have 10-bit GRBG (or a variant) > > format whereas the CAPTURE video nodes always have NV12. Can you confirm? > Hi, Sakari, > Yes, I think the pad_fmt should also be changed. > Yong, could you add some extra code for this and test? like: > > static int ipu3_v4l2_node_setup(struct imgu_device *imgu, unsigned int pipe, > ... >
[PATCH 8/9] vicodec: add tag support
From: Hans Verkuil Copy tags in vicodec. Signed-off-by: Hans Verkuil --- drivers/media/platform/vicodec/vicodec-core.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/vicodec/vicodec-core.c b/drivers/media/platform/vicodec/vicodec-core.c index b292cff26c86..72245183b077 100644 --- a/drivers/media/platform/vicodec/vicodec-core.c +++ b/drivers/media/platform/vicodec/vicodec-core.c @@ -190,18 +190,8 @@ static int device_process(struct vicodec_ctx *ctx, } out_vb->sequence = q_cap->sequence++; - out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp; - - if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE) - out_vb->timecode = in_vb->timecode; - out_vb->field = in_vb->field; out_vb->flags &= ~V4L2_BUF_FLAG_LAST; - out_vb->flags |= in_vb->flags & - (V4L2_BUF_FLAG_TIMECODE | -V4L2_BUF_FLAG_KEYFRAME | -V4L2_BUF_FLAG_PFRAME | -V4L2_BUF_FLAG_BFRAME | -V4L2_BUF_FLAG_TSTAMP_SRC_MASK); + v4l2_m2m_buf_copy_data(in_vb, out_vb, !ctx->is_enc); return 0; } @@ -1066,6 +1056,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; src_vq->lock = ctx->is_enc ? >dev->enc_mutex : >dev->dec_mutex; + src_vq->supports_tags = true; ret = vb2_queue_init(src_vq); if (ret) @@ -1081,6 +1072,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, dst_vq->mem_ops = _vmalloc_memops; dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; dst_vq->lock = src_vq->lock; + dst_vq->supports_tags = true; return vb2_queue_init(dst_vq); } -- 2.19.1
[PATCH 2/9] vb2: add tag support
From: Hans Verkuil Add support for tags to vb2. Besides just storing and setting the tag this patch also adds the vb2_find_tag() function that can be used to find a buffer with the given tag. This function will only look at DEQUEUED and DONE buffers, i.e. buffers that are already processed. Signed-off-by: Hans Verkuil --- .../media/common/videobuf2/videobuf2-v4l2.c | 43 --- include/media/videobuf2-v4l2.h| 21 - 2 files changed, 58 insertions(+), 6 deletions(-) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index a17033ab2c22..36781d2367a9 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -50,7 +50,8 @@ module_param(debug, int, 0644); V4L2_BUF_FLAG_TIMESTAMP_MASK) /* Output buffer flags that should be passed on to the driver */ #define V4L2_BUFFER_OUT_FLAGS (V4L2_BUF_FLAG_PFRAME | V4L2_BUF_FLAG_BFRAME | \ -V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_TIMECODE) +V4L2_BUF_FLAG_KEYFRAME | \ +V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG) /* * __verify_planes_array() - verify that the planes array passed in struct @@ -144,8 +145,11 @@ static void __copy_timestamp(struct vb2_buffer *vb, const void *pb) */ if (q->copy_timestamp) vb->timestamp = timeval_to_ns(>timestamp); - vbuf->flags |= b->flags & V4L2_BUF_FLAG_TIMECODE; - if (b->flags & V4L2_BUF_FLAG_TIMECODE) + vbuf->flags |= b->flags & + (V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG); + if (b->flags & V4L2_BUF_FLAG_TAG) + vbuf->tag = v4l2_buffer_get_tag(b); + else if (b->flags & V4L2_BUF_FLAG_TIMECODE) vbuf->timecode = b->timecode; } }; @@ -195,6 +199,7 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b } vbuf->sequence = 0; vbuf->request_fd = -1; + vbuf->tag = 0; if (V4L2_TYPE_IS_MULTIPLANAR(b->type)) { switch (b->memory) { @@ -314,13 +319,19 @@ static int vb2_fill_vb2_v4l2_buffer(struct vb2_buffer *vb, struct v4l2_buffer *b } if (V4L2_TYPE_IS_OUTPUT(b->type)) { + if ((b->flags & V4L2_BUF_FLAG_TIMECODE) && + (b->flags & V4L2_BUF_FLAG_TAG)) { + dprintk(1, "buffer flag TIMECODE cannot be combined with flag TAG\n"); + return -EINVAL; + } + /* * For output buffers mask out the timecode flag: * this will be handled later in vb2_qbuf(). * The 'field' is valid metadata for this output buffer * and so that needs to be copied here. */ - vbuf->flags &= ~V4L2_BUF_FLAG_TIMECODE; + vbuf->flags &= ~(V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG); vbuf->field = b->field; } else { /* Zero any output buffer flags as this is a capture buffer */ @@ -461,7 +472,10 @@ static void __fill_v4l2_buffer(struct vb2_buffer *vb, void *pb) b->flags = vbuf->flags; b->field = vbuf->field; b->timestamp = ns_to_timeval(vb->timestamp); - b->timecode = vbuf->timecode; + if (b->flags & V4L2_BUF_FLAG_TAG) + v4l2_buffer_set_tag(b, vbuf->tag); + else + b->timecode = vbuf->timecode; b->sequence = vbuf->sequence; b->reserved2 = 0; b->request_fd = 0; @@ -587,6 +601,25 @@ static const struct vb2_buf_ops v4l2_buf_ops = { .copy_timestamp = __copy_timestamp, }; +int vb2_find_tag(const struct vb2_queue *q, u64 tag, +unsigned int start_idx) +{ + unsigned int i; + + for (i = start_idx; i < q->num_buffers; i++) { + struct vb2_buffer *vb = q->bufs[i]; + struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb); + + if ((vb->state == VB2_BUF_STATE_DEQUEUED || +vb->state == VB2_BUF_STATE_DONE) && + (vbuf->flags & V4L2_BUF_FLAG_TAG) && + vbuf->tag == tag) + return i; + } + return -1; +} +EXPORT_SYMBOL_GPL(vb2_find_tag); + /* * vb2_querybuf() - query video buffer information * @q: videobuf queue diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h index 727855463838..60ff7348a2b6 100644 --- a/include/media/videobuf2-v4l2.h +++ b/include/media/videobuf2-v4l2.h @@ -31,8 +31,9 @@ * @field: field order of the image in the buffer, as defined by * v4l2_field. * @timecode: frame timecode. + * @tag: user specified buffer tag
[PATCH 4/9] buffer.rst: document the new buffer tag feature.
Document V4L2_BUF_FLAG_TAG and struct v4l2_buffer_tag. Signed-off-by: Hans Verkuil --- Documentation/media/uapi/v4l/buffer.rst | 61 --- .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++ 2 files changed, 57 insertions(+), 8 deletions(-) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index 2e266d32470a..8160631bb9d9 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -220,17 +220,29 @@ struct v4l2_buffer use ``V4L2_BUF_FLAG_TIMESTAMP_COPY`` the application has to fill in the timestamp which will be copied by the driver to the capture stream. -* - struct :c:type:`v4l2_timecode` +* - union +* - + - struct :c:type:`v4l2_timecode` - ``timecode`` - - - - When ``type`` is ``V4L2_BUF_TYPE_VIDEO_CAPTURE`` and the - ``V4L2_BUF_FLAG_TIMECODE`` flag is set in ``flags``, this + - When the ``V4L2_BUF_FLAG_TIMECODE`` flag is set in ``flags``, this structure contains a frame timecode. In :c:type:`V4L2_FIELD_ALTERNATE ` mode the top and bottom field contain the same timecode. Timecodes are intended to help video editing and are typically recorded on video tapes, but also embedded in compressed formats like MPEG. This field is independent of the ``timestamp`` and ``sequence`` fields. +* - + - struct :c:type:`v4l2_buffer_tag` + - ``tag`` + - When the ``V4L2_BUF_FLAG_TAG`` flag is set in ``flags``, this + structure contains a user-specified 64-bit tag value. It can be + set with the helper functions :c:func:`v4l2_buffer_set_tag()` or + :c:func:`v4l2_buffer_set_tag_ptr()`, and it can be retrieved with + the helper functions :c:func:`v4l2_buffer_get_tag()` or +:c:func:`v4l2_buffer_get_tag_ptr()`. + + It is used by stateless codecs where this tag can be used to + refer to buffers that contain reference frames. * - __u32 - ``sequence`` - @@ -567,6 +579,14 @@ Buffer Flags when the ``VIDIOC_DQBUF`` ioctl is called. Applications can set this bit and the corresponding ``timecode`` structure when ``type`` refers to an output stream. +* .. _`V4L2-BUF-FLAG-TAG`: + + - ``V4L2_BUF_FLAG_TAG`` + - 0x0200 + - The ``tag`` field is valid. Applications can set + this bit and the corresponding ``tag`` structure. If tags are + supported then the ``V4L2_BUF_CAP_SUPPORTS_TAGS`` capability + is also set. * .. _`V4L2-BUF-FLAG-PREPARED`: - ``V4L2_BUF_FLAG_PREPARED`` @@ -704,10 +724,10 @@ enum v4l2_memory Timecodes = -The struct :c:type:`v4l2_timecode` structure is designed to hold a -:ref:`smpte12m` or similar timecode. (struct -struct :c:type:`timeval` timestamps are stored in struct -:c:type:`v4l2_buffer` field ``timestamp``.) +The :c:type:`v4l2_buffer_tag` structure is designed to hold a +:ref:`smpte12m` or similar timecode. +(struct :c:type:`timeval` timestamps are stored in the struct +:c:type:`v4l2_buffer` ``timestamp`` field.) .. c:type:: v4l2_timecode @@ -807,3 +827,28 @@ Timecode Flags * - ``V4L2_TC_USERBITS_8BITCHARS`` - 0x0008 - 8-bit ISO characters. + +Tags + + +The :c:type:`v4l2_buffer_tag` structure is designed to hold a +64 bit tag. + +.. c:type:: v4l2_buffer_tag + +struct v4l2_buffer_tag +-- + +.. tabularcolumns:: |p{4.4cm}|p{4.4cm}|p{8.7cm}| + +.. flat-table:: +:header-rows: 0 +:stub-columns: 0 +:widths: 1 1 2 + +* - __u32 + - ``low`` + - Low 32 bits of the 64 bit tag value. +* - __u32 + - ``high`` + - High 32 bits of the 64 bit tag value. diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst index d40c60e8..5090a62f324c 100644 --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst @@ -112,6 +112,7 @@ any DMA in progress, an implicit .. _V4L2-BUF-CAP-SUPPORTS-USERPTR: .. _V4L2-BUF-CAP-SUPPORTS-DMABUF: .. _V4L2-BUF-CAP-SUPPORTS-REQUESTS: +.. _V4L2-BUF-CAP-SUPPORTS-TAGS: .. cssclass:: longtable @@ -132,6 +133,9 @@ any DMA in progress, an implicit * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS`` - 0x0008 - This buffer type supports :ref:`requests `. +* - ``V4L2_BUF_CAP_SUPPORTS_TAGS`` + - 0x0010 + - This buffer type supports ``V4L2_BUF_FLAG_TAG``. Return Value -- 2.19.1
[PATCH 5/9] v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function
Memory-to-memory devices should copy various parts of struct v4l2_buffer from the output buffer to the capture buffer. Add a helper function that does that to simplify the driver code. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-mem2mem.c | 23 +++ include/media/v4l2-mem2mem.h | 21 + 2 files changed, 44 insertions(+) diff --git a/drivers/media/v4l2-core/v4l2-mem2mem.c b/drivers/media/v4l2-core/v4l2-mem2mem.c index 1ed2465972ac..bfd5321fda1c 100644 --- a/drivers/media/v4l2-core/v4l2-mem2mem.c +++ b/drivers/media/v4l2-core/v4l2-mem2mem.c @@ -953,6 +953,29 @@ void v4l2_m2m_buf_queue(struct v4l2_m2m_ctx *m2m_ctx, } EXPORT_SYMBOL_GPL(v4l2_m2m_buf_queue); +void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb, + struct vb2_v4l2_buffer *cap_vb, + bool copy_frame_flags) +{ + u32 mask = V4L2_BUF_FLAG_TIMECODE | V4L2_BUF_FLAG_TAG | + V4L2_BUF_FLAG_TSTAMP_SRC_MASK; + + if (copy_frame_flags) + mask |= V4L2_BUF_FLAG_KEYFRAME | V4L2_BUF_FLAG_PFRAME | + V4L2_BUF_FLAG_BFRAME; + + cap_vb->vb2_buf.timestamp = out_vb->vb2_buf.timestamp; + + if (out_vb->flags & V4L2_BUF_FLAG_TAG) + cap_vb->tag = out_vb->tag; + if (out_vb->flags & V4L2_BUF_FLAG_TIMECODE) + cap_vb->timecode = out_vb->timecode; + cap_vb->field = out_vb->field; + cap_vb->flags &= ~mask; + cap_vb->flags |= out_vb->flags & mask; +} +EXPORT_SYMBOL_GPL(v4l2_m2m_buf_copy_data); + void v4l2_m2m_request_queue(struct media_request *req) { struct media_request_object *obj, *obj_safe; diff --git a/include/media/v4l2-mem2mem.h b/include/media/v4l2-mem2mem.h index 5467264771ec..35716af7ccbc 100644 --- a/include/media/v4l2-mem2mem.h +++ b/include/media/v4l2-mem2mem.h @@ -622,6 +622,27 @@ v4l2_m2m_dst_buf_remove_by_idx(struct v4l2_m2m_ctx *m2m_ctx, unsigned int idx) return v4l2_m2m_buf_remove_by_idx(_ctx->cap_q_ctx, idx); } +/** + * v4l2_m2m_buf_copy_data() - copy buffer data from the output buffer to the + * capture buffer + * + * @out_vb: the output buffer that is the source of the data. + * @cap_vb: the capture buffer that will receive the data. + * @copy_frame_flags: copy the KEY/B/PFRAME flags as well. + * + * This helper function copies the timestamp, timecode (if the TIMECODE + * buffer flag was set), tag (if the TAG buffer flag was set), field + * and the TIMECODE, TAG, KEYFRAME, BFRAME, PFRAME and TSTAMP_SRC_MASK + * flags from @out_vb to @cap_vb. + * + * If @copy_frame_flags is false, then the KEYFRAME, BFRAME and PFRAME + * flags are not copied. This is typically needed for encoders that + * set this bits explicitly. + */ +void v4l2_m2m_buf_copy_data(const struct vb2_v4l2_buffer *out_vb, + struct vb2_v4l2_buffer *cap_vbi, + bool copy_frame_flags); + /* v4l2 request helper */ void v4l2_m2m_request_queue(struct media_request *req); -- 2.19.1
[PATCH 0/9] vb2/cedrus: add tag support
As was discussed here (among other places): https://lkml.org/lkml/2018/10/19/440 using capture queue buffer indices to refer to reference frames is not a good idea. A better idea is to use a 'tag' where the application can assign a u64 tag to an output buffer, which is then copied to the capture buffer(s) derived from the output buffer. A u64 is chosen since this allows userspace to also use pointers to internal structures as 'tag'. The first three patches add core tag support, the next patch document the tag support, then a new helper function is added to v4l2-mem2mem.c to easily copy data from a source to a destination buffer that drivers can use. Next a new supports_tags vb2_queue flag is added to indicate that the driver supports tags. Ideally this should not be necessary, but that would require that all m2m drivers are converted to using the new helper function introduced in the previous patch. That takes more time then I have now since we want to get this in for 4.20. Finally the vim2m, vicodec and cedrus drivers are converted to support tags. I also removed the 'pad' fields from the mpeg2 control structs (it should never been added in the first place) and aligned the structs to a u32 boundary (u64 for the tag values). Note that this might change further (Paul suggested using bitfields). Also note that the cedrus code doesn't set the sequence counter, that's something that should still be added before this driver can be moved out of staging. Note: if no buffer is found for a certain tag, then the dma address is just set to 0. That happened before as well with invalid buffer indices. This should be checked in the driver! The previous RFC series was tested successfully with the cedrus driver. Regards, Hans Main changes since the RFC: - Added new buffer capability flag - Added m2m helper to copy data between buffers - Added documentation - Added tag logging in v4l2-ioctl.c Hans Verkuil (9): videodev2.h: add tag support vb2: add tag support v4l2-ioctl.c: log v4l2_buffer tag buffer.rst: document the new buffer tag feature. v4l2-mem2mem: add v4l2_m2m_buf_copy_data helper function vb2: add new supports_tags queue flag vim2m: add tag support vicodec: add tag support cedrus: add tag support Documentation/media/uapi/v4l/buffer.rst | 61 --- .../media/uapi/v4l/vidioc-reqbufs.rst | 4 ++ .../media/common/videobuf2/videobuf2-v4l2.c | 45 -- drivers/media/platform/vicodec/vicodec-core.c | 14 + drivers/media/platform/vim2m.c| 14 + drivers/media/v4l2-core/v4l2-ctrls.c | 9 --- drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++- drivers/media/v4l2-core/v4l2-mem2mem.c| 23 +++ drivers/staging/media/sunxi/cedrus/cedrus.h | 9 ++- .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 + .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 +++ .../staging/media/sunxi/cedrus/cedrus_video.c | 2 + include/media/v4l2-mem2mem.h | 21 +++ include/media/videobuf2-core.h| 2 + include/media/videobuf2-v4l2.h| 21 ++- include/uapi/linux/v4l2-controls.h| 14 ++--- include/uapi/linux/videodev2.h| 38 +++- 17 files changed, 236 insertions(+), 73 deletions(-) -- 2.19.1
[PATCH 7/9] vim2m: add tag support
From: Hans Verkuil Copy tags in vim2m. Signed-off-by: Hans Verkuil --- drivers/media/platform/vim2m.c | 14 +++--- 1 file changed, 3 insertions(+), 11 deletions(-) diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c index d82db738f174..9d1222f489b8 100644 --- a/drivers/media/platform/vim2m.c +++ b/drivers/media/platform/vim2m.c @@ -241,17 +241,7 @@ static int device_process(struct vim2m_ctx *ctx, out_vb->sequence = get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE)->sequence++; in_vb->sequence = q_data->sequence++; - out_vb->vb2_buf.timestamp = in_vb->vb2_buf.timestamp; - - if (in_vb->flags & V4L2_BUF_FLAG_TIMECODE) - out_vb->timecode = in_vb->timecode; - out_vb->field = in_vb->field; - out_vb->flags = in_vb->flags & - (V4L2_BUF_FLAG_TIMECODE | -V4L2_BUF_FLAG_KEYFRAME | -V4L2_BUF_FLAG_PFRAME | -V4L2_BUF_FLAG_BFRAME | -V4L2_BUF_FLAG_TSTAMP_SRC_MASK); + v4l2_m2m_buf_copy_data(out_vb, in_vb, true); switch (ctx->mode) { case MEM2MEM_HFLIP | MEM2MEM_VFLIP: @@ -856,6 +846,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds src_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; src_vq->lock = >dev->dev_mutex; src_vq->supports_requests = true; + src_vq->supports_tags = true; ret = vb2_queue_init(src_vq); if (ret) @@ -869,6 +860,7 @@ static int queue_init(void *priv, struct vb2_queue *src_vq, struct vb2_queue *ds dst_vq->mem_ops = _vmalloc_memops; dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY; dst_vq->lock = >dev->dev_mutex; + dst_vq->supports_tags = true; return vb2_queue_init(dst_vq); } -- 2.19.1
[PATCH 9/9] cedrus: add tag support
Replace old reference frame indices by new tag method. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ctrls.c | 9 drivers/staging/media/sunxi/cedrus/cedrus.h | 9 +--- .../staging/media/sunxi/cedrus/cedrus_dec.c | 2 ++ .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 21 --- .../staging/media/sunxi/cedrus/cedrus_video.c | 2 ++ include/uapi/linux/v4l2-controls.h| 14 + 6 files changed, 24 insertions(+), 33 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ctrls.c b/drivers/media/v4l2-core/v4l2-ctrls.c index 5f2b033a7a42..b854cceb19dc 100644 --- a/drivers/media/v4l2-core/v4l2-ctrls.c +++ b/drivers/media/v4l2-core/v4l2-ctrls.c @@ -1660,15 +1660,6 @@ static int std_validate(const struct v4l2_ctrl *ctrl, u32 idx, return -EINVAL; } - if (p_mpeg2_slice_params->backward_ref_index >= VIDEO_MAX_FRAME || - p_mpeg2_slice_params->forward_ref_index >= VIDEO_MAX_FRAME) - return -EINVAL; - - if (p_mpeg2_slice_params->pad || - p_mpeg2_slice_params->picture.pad || - p_mpeg2_slice_params->sequence.pad) - return -EINVAL; - return 0; case V4L2_CTRL_TYPE_MPEG2_QUANTIZATION: diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h index 3f61248c57ac..781676b55a1b 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus.h +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h @@ -142,11 +142,14 @@ static inline dma_addr_t cedrus_buf_addr(struct vb2_buffer *buf, } static inline dma_addr_t cedrus_dst_buf_addr(struct cedrus_ctx *ctx, -unsigned int index, -unsigned int plane) +int index, unsigned int plane) { - struct vb2_buffer *buf = ctx->dst_bufs[index]; + struct vb2_buffer *buf; + if (index < 0) + return 0; + + buf = ctx->dst_bufs[index]; return buf ? cedrus_buf_addr(buf, >dst_fmt, plane) : 0; } diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c index e40180a33951..097d36e24e27 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_dec.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_dec.c @@ -53,6 +53,8 @@ void cedrus_device_run(void *priv) break; } + v4l2_m2m_buf_copy_data(run.src, run.dst); + dev->dec_ops[ctx->current_codec]->setup(ctx, ); spin_unlock_irqrestore(>dev->irq_lock, flags); diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index 9abd39cae38c..fdde9a099153 100644 --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c @@ -82,7 +82,10 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run) dma_addr_t fwd_luma_addr, fwd_chroma_addr; dma_addr_t bwd_luma_addr, bwd_chroma_addr; struct cedrus_dev *dev = ctx->dev; + struct vb2_queue *cap_q = >fh.m2m_ctx->cap_q_ctx.q; const u8 *matrix; + int forward_idx; + int backward_idx; unsigned int i; u32 reg; @@ -156,23 +159,17 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run) cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg); /* Forward and backward prediction reference buffers. */ + forward_idx = vb2_find_tag(cap_q, slice_params->forward_ref_tag, 0); - fwd_luma_addr = cedrus_dst_buf_addr(ctx, - slice_params->forward_ref_index, - 0); - fwd_chroma_addr = cedrus_dst_buf_addr(ctx, - slice_params->forward_ref_index, - 1); + fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0); + fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1); cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr); cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr); - bwd_luma_addr = cedrus_dst_buf_addr(ctx, - slice_params->backward_ref_index, - 0); - bwd_chroma_addr = cedrus_dst_buf_addr(ctx, - slice_params->backward_ref_index, - 1); + backward_idx = vb2_find_tag(cap_q, slice_params->backward_ref_tag, 0); + bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0); + bwd_chroma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 1); cedrus_write(dev,
[PATCH 3/9] v4l2-ioctl.c: log v4l2_buffer tag
When debugging is on, log the new tag field of struct v4l2_buffer as well. Signed-off-by: Hans Verkuil --- drivers/media/v4l2-core/v4l2-ioctl.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c index c63746968fa3..81b9b3ccd7c3 100644 --- a/drivers/media/v4l2-core/v4l2-ioctl.c +++ b/drivers/media/v4l2-core/v4l2-ioctl.c @@ -498,9 +498,12 @@ static void v4l_print_buffer(const void *arg, bool write_only) p->bytesused, p->m.userptr, p->length); } - printk(KERN_DEBUG "timecode=%02d:%02d:%02d type=%d, flags=0x%08x, frames=%d, userbits=0x%08x\n", - tc->hours, tc->minutes, tc->seconds, - tc->type, tc->flags, tc->frames, *(__u32 *)tc->userbits); + if (p->flags & V4L2_BUF_FLAG_TAG) + printk(KERN_DEBUG "tag=%llx\n", v4l2_buffer_get_tag(p)); + else if (p->flags & V4L2_BUF_FLAG_TIMECODE) + printk(KERN_DEBUG "timecode=%02d:%02d:%02d type=%d, flags=0x%08x, frames=%d, userbits=0x%08x\n", + tc->hours, tc->minutes, tc->seconds, + tc->type, tc->flags, tc->frames, *(__u32 *)tc->userbits); } static void v4l_print_exportbuffer(const void *arg, bool write_only) -- 2.19.1
[PATCH 1/9] videodev2.h: add tag support
From: Hans Verkuil Add support for 'tags' to struct v4l2_buffer. These can be used by m2m devices so userspace can set a tag for an output buffer and this value will then be copied to the capture buffer(s). This tag can be used to refer to capture buffers, something that is needed by stateless HW codecs. The new V4L2_BUF_CAP_SUPPORTS_TAGS capability indicates whether or not tags are supported. Signed-off-by: Hans Verkuil --- include/uapi/linux/videodev2.h | 38 +- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h index c8e8ff810190..ec1fef2a9445 100644 --- a/include/uapi/linux/videodev2.h +++ b/include/uapi/linux/videodev2.h @@ -879,6 +879,7 @@ struct v4l2_requestbuffers { #define V4L2_BUF_CAP_SUPPORTS_USERPTR (1 << 1) #define V4L2_BUF_CAP_SUPPORTS_DMABUF (1 << 2) #define V4L2_BUF_CAP_SUPPORTS_REQUESTS (1 << 3) +#define V4L2_BUF_CAP_SUPPORTS_TAGS (1 << 4) /** * struct v4l2_plane - plane info for multi-planar buffers @@ -912,6 +913,11 @@ struct v4l2_plane { __u32 reserved[11]; }; +struct v4l2_buffer_tag { + __u32 low; + __u32 high; +}; + /** * struct v4l2_buffer - video buffer info * @index: id number of the buffer @@ -950,7 +956,10 @@ struct v4l2_buffer { __u32 flags; __u32 field; struct timeval timestamp; - struct v4l2_timecodetimecode; + union { + struct v4l2_timecodetimecode; + struct v4l2_buffer_tag tag; + }; __u32 sequence; /* memory location */ @@ -988,6 +997,8 @@ struct v4l2_buffer { #define V4L2_BUF_FLAG_IN_REQUEST 0x0080 /* timecode field is valid */ #define V4L2_BUF_FLAG_TIMECODE 0x0100 +/* tag field is valid */ +#define V4L2_BUF_FLAG_TAG 0x0200 /* Buffer is prepared for queuing */ #define V4L2_BUF_FLAG_PREPARED 0x0400 /* Cache handling flags */ @@ -1007,6 +1018,31 @@ struct v4l2_buffer { /* request_fd is valid */ #define V4L2_BUF_FLAG_REQUEST_FD 0x0080 +static inline void v4l2_buffer_set_tag(struct v4l2_buffer *buf, __u64 tag) +{ + buf->tag.high = tag >> 32; + buf->tag.low = tag & 0xULL; + buf->flags |= V4L2_BUF_FLAG_TAG; +} + +static inline void v4l2_buffer_set_tag_ptr(struct v4l2_buffer *buf, + const void *tag) +{ + v4l2_buffer_set_tag(buf, (uintptr_t)tag); +} + +static inline __u64 v4l2_buffer_get_tag(const struct v4l2_buffer *buf) +{ + if (!(buf->flags & V4L2_BUF_FLAG_TAG)) + return 0; + return (((__u64)buf->tag.high) << 32) | (__u64)buf->tag.low; +} + +static inline void *v4l2_buffer_get_tag_ptr(const struct v4l2_buffer *buf) +{ + return (void *)(uintptr_t)v4l2_buffer_get_tag(buf); +} + /** * struct v4l2_exportbuffer - export of video buffer as DMABUF file descriptor * -- 2.19.1
[PATCH 6/9] vb2: add new supports_tags queue flag
Add new flag to indicate that buffer tags are supported. Signed-off-by: Hans Verkuil --- drivers/media/common/videobuf2/videobuf2-v4l2.c | 2 ++ include/media/videobuf2-core.h | 2 ++ 2 files changed, 4 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c index 36781d2367a9..cb46e37b2629 100644 --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c @@ -666,6 +666,8 @@ static void fill_buf_caps(struct vb2_queue *q, u32 *caps) *caps |= V4L2_BUF_CAP_SUPPORTS_DMABUF; if (q->supports_requests) *caps |= V4L2_BUF_CAP_SUPPORTS_REQUESTS; + if (q->supports_tags) + *caps |= V4L2_BUF_CAP_SUPPORTS_TAGS; } int vb2_reqbufs(struct vb2_queue *q, struct v4l2_requestbuffers *req) diff --git a/include/media/videobuf2-core.h b/include/media/videobuf2-core.h index e86981d615ae..81f2dbfd0094 100644 --- a/include/media/videobuf2-core.h +++ b/include/media/videobuf2-core.h @@ -473,6 +473,7 @@ struct vb2_buf_ops { * has not been called. This is a vb1 idiom that has been adopted * also by vb2. * @supports_requests: this queue supports the Request API. + * @supports_tags: this queue supports tags in struct v4l2_buffer. * @uses_qbuf: qbuf was used directly for this queue. Set to 1 the first * time this is called. Set to 0 when the queue is canceled. * If this is 1, then you cannot queue buffers from a request. @@ -547,6 +548,7 @@ struct vb2_queue { unsignedallow_zero_bytesused:1; unsigned quirk_poll_must_check_waiting_for_buffers:1; unsignedsupports_requests:1; + unsignedsupports_tags:1; unsigneduses_qbuf:1; unsigneduses_requests:1; -- 2.19.1
Re: [PATCH v5 0/5] Make sure .device_run is always called in non-atomic context
Hi, On Mon, 2018-11-12 at 18:05 -0300, Ezequiel Garcia wrote: > On Mon, 12 Nov 2018 at 13:52, Paul Kocialkowski > wrote: > > Hi, > > > > On Sun, 2018-11-11 at 18:26 -0300, Ezequiel Garcia wrote: > > > On Thu, 2018-10-18 at 15:02 -0300, Ezequiel Garcia wrote: > > > > This series goal is to avoid drivers from having ad-hoc code > > > > to call .device_run in non-atomic context. Currently, .device_run > > > > can be called via v4l2_m2m_job_finish(), not only running > > > > in interrupt context, but also creating a nasty re-entrant > > > > path into mem2mem drivers. > > > > > > > > The proposed solution is to add a per-device worker that is scheduled > > > > by v4l2_m2m_job_finish, which replaces drivers having a threaded > > > > interrupt > > > > or similar. > > > > > > > > This change allows v4l2_m2m_job_finish() to be called in interrupt > > > > context, separating .device_run and v4l2_m2m_job_finish() contexts. > > > > > > > > It's worth mentioning that v4l2_m2m_cancel_job() doesn't need > > > > to flush or cancel the new worker, because the job_spinlock > > > > synchronizes both and also because the core prevents simultaneous > > > > jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the > > > > worker will be unable to run a new job. > > > > > > > > Patches apply on top of Request API and the Cedrus VPU > > > > driver. > > > > > > > > Tested with cedrus driver using v4l2-request-test and > > > > vicodec driver using gstreamer. > > > > > > > > Ezequiel Garcia (4): > > > > mem2mem: Require capture and output mutexes to match > > > > v4l2-ioctl.c: Simplify locking for m2m devices > > > > v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish > > > > media: cedrus: Get rid of interrupt bottom-half > > > > > > > > Sakari Ailus (1): > > > > v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule > > > > > > > > drivers/media/v4l2-core/v4l2-ioctl.c | 47 + > > > > drivers/media/v4l2-core/v4l2-mem2mem.c| 66 --- > > > > .../staging/media/sunxi/cedrus/cedrus_hw.c| 26 ++-- > > > > 3 files changed, 51 insertions(+), 88 deletions(-) > > > > > > > > > > Hans, Maxime: > > > > > > Any feedback for this? > > > > I just tested the whole series with the cedrus driver and everything > > looks good! > > > > Good! So this means we can add a Tested-by for the entire series? Definitely, I just sent the tested-by line on the cover letter! > > Removing the interrupt bottom-half in favor of a workqueue in the core > > seems like a good way to simplify m2m driver development by avoiding > > per-driver workqueues or threaded irqs. > > > > Thanks for the test! Cheers, Paul -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: This is a digitally signed message part
Re: [PATCH v5 0/5] Make sure .device_run is always called in non-atomic context
Hi, On Thu, 2018-10-18 at 15:02 -0300, Ezequiel Garcia wrote: > This series goal is to avoid drivers from having ad-hoc code > to call .device_run in non-atomic context. Currently, .device_run > can be called via v4l2_m2m_job_finish(), not only running > in interrupt context, but also creating a nasty re-entrant > path into mem2mem drivers. > > The proposed solution is to add a per-device worker that is scheduled > by v4l2_m2m_job_finish, which replaces drivers having a threaded interrupt > or similar. > > This change allows v4l2_m2m_job_finish() to be called in interrupt > context, separating .device_run and v4l2_m2m_job_finish() contexts. > > It's worth mentioning that v4l2_m2m_cancel_job() doesn't need > to flush or cancel the new worker, because the job_spinlock > synchronizes both and also because the core prevents simultaneous > jobs. Either v4l2_m2m_cancel_job() will wait for the worker, or the > worker will be unable to run a new job. > > Patches apply on top of Request API and the Cedrus VPU > driver. > > Tested with cedrus driver using v4l2-request-test and > vicodec driver using gstreamer. For the entire series, tested with the cedrus driver: Tested-by: Paul Kocialkowski Cheers, Paul > Ezequiel Garcia (4): > mem2mem: Require capture and output mutexes to match > v4l2-ioctl.c: Simplify locking for m2m devices > v4l2-mem2mem: Avoid calling .device_run in v4l2_m2m_job_finish > media: cedrus: Get rid of interrupt bottom-half > > Sakari Ailus (1): > v4l2-mem2mem: Simplify exiting the function in __v4l2_m2m_try_schedule > > drivers/media/v4l2-core/v4l2-ioctl.c | 47 + > drivers/media/v4l2-core/v4l2-mem2mem.c| 66 --- > .../staging/media/sunxi/cedrus/cedrus_hw.c| 26 ++-- > 3 files changed, 51 insertions(+), 88 deletions(-) > -- Paul Kocialkowski, Bootlin (formerly Free Electrons) Embedded Linux and kernel engineering https://bootlin.com signature.asc Description: This is a digitally signed message part