cron job: media_tree daily build: WARNINGS

2018-11-15 Thread Hans Verkuil 
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:   Fri Nov 16 05:00:11 CET 2018
media-tree git hash:fbe57dde7126d1b2712ab5ea93fb9d15f89de708
media_build git hash:   a8aef9cea0a4a2f3ea86c0b37bd6a1378018c0c1
v4l-utils git hash: f61132e81d79880028773b235647892a8340abec
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: OK
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/Friday.log

Full logs are available here:

http://www.xs4all.nl/~hverkuil/logs/Friday.tar.bz2

The Media Infrastructure API from this daily build is here:

http://www.xs4all.nl/~hverkuil/spec/index.html

Re: TechnoTrend CT2-4500 remote not working

2018-11-15 Thread martin.kono...@mknetz.de 
Sean,

Am 15.11.2018 um 00:07 schrieb Sean Young:

>>
>> I turned on dynamic debug and got the following messages in the kernel log:
>>
>> [  837.160992] rc rc0: get_key_fusionhdtv: ff ff ff ff
>> [  837.263927] rc rc0: ir_key_poll
>> [  837.264528] rc rc0: get_key_fusionhdtv: ff ff ff ff
>> [  837.367840] rc rc0: ir_key_poll
>> [  837.368441] rc rc0: get_key_fusionhdtv: ff ff ff ff
>>
>> Pressing a key on the remote did not change the pattern. I rechecked the
>> connection of the IR receiver to the card but it was firmly plugged in.
> 
> Hmm, either the IR signal is not getting to the device, or this is not
> where the IR is reported. I guess also the firmware could be incorrect
> or out of date.

I have obtained the latest firmware from 
http://www.dvbsky.net/Support_linux.html

si2168 6-0064: downloading firmware from file 'dvb-demod-si2168-b40-01.fw'
si2168 6-0064: firmware version: B 4.0.25

The firmware is now newer than before (4.0.11), but I still get no output with 
dynamic debugging.

> 
> Certainly a logic analyser would help here to see if the signal is arriving,
> and where it goes (e.g. directly to a gpio pin).

Currently, I do not have a logic analyser at hand.

> 
> What's the output of the lspci -vvv? Maybe it's reported via gpio and not
> i2c.

The output of lspci -vvv:

17:00.0 Multimedia video controller: Conexant Systems, Inc. CX23885 PCI Video 
and Audio Decoder (rev 04)
Subsystem: Technotrend Systemtechnik GmbH TT-budget CT2-4500 CI
Control: I/O- Mem+ BusMaster+ SpecCycle- MemWINV- VGASnoop- ParErr- 
Stepping- SERR- FastB2B- DisINTx-
Status: Cap+ 66MHz- UDF- FastB2B- ParErr- DEVSEL=fast >TAbort- SERR- 
VC0:Caps:   PATOffset=00 MaxTimeSlots=1 RejSnoopTrans-
Arb:Fixed- WRR32- WRR64- WRR128- TWRR128- WRR256-
Ctrl:   Enable+ ID=0 ArbSelect=Fixed TC/VC=01
Status: NegoPending- InProgress-
Kernel driver in use: cx23885
Kernel modules: cx23885


Regards,

Martin

RE: [PATCH v7 14/16] intel-ipu3: Add v4l2 driver based on media framework

2018-11-15 Thread Zhi, Yong 
Hi, Hans,

Thanks for the review.

> -Original Message-
> From: Hans Verkuil [mailto:hverk...@xs4all.nl]
> Sent: Thursday, November 15, 2018 6:51 AM
> To: Zhi, Yong ; linux-media@vger.kernel.org;
> sakari.ai...@linux.intel.com
> Cc: tf...@chromium.org; mche...@kernel.org; hans.verk...@cisco.com;
> laurent.pinch...@ideasonboard.com; Mani, Rajmohan
> ; Zheng, Jian Xu ; Hu,
> Jerry W ; Toivonen, Tuukka
> ; Qiu, Tian Shu ; Cao,
> Bingbu 
> Subject: Re: [PATCH v7 14/16] intel-ipu3: Add v4l2 driver based on media
> framework
> 
> On 10/29/18 23:23, Yong Zhi wrote:
> > Implement video driver that utilizes v4l2, vb2 queue support and media
> > controller APIs. The driver exposes single subdevice and six nodes.
> >
> > Signed-off-by: Yong Zhi 
> > ---
> >  drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1091
> > ++
> >  1 file changed, 1091 insertions(+)
> >  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> >
> > diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> > b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> 
> 
> 
> > +int ipu3_v4l2_register(struct imgu_device *imgu) {
> 
> 
> 
> > +   /* Initialize vbq */
> > +   vbq->type = node->vdev_fmt.type;
> > +   vbq->io_modes = VB2_USERPTR | VB2_MMAP |
> VB2_DMABUF;
> 
> Are you sure USERPTR works? If you have alignment requirements that the
> buffer starts at a multiple of more than (I think) 8 bytes, then USERPTR won't
> work.

USRPTR was used at the beginning of project, we then switched to dma buffer 
mainly for performance reason:
https://chromium-review.googlesource.com/c/chromiumos/platform/arc-camera/+/620252

> 
> > +   vbq->ops = _vb2_ops;
> > +   vbq->mem_ops = _dma_sg_memops;
> > +   if (imgu->buf_struct_size <= 0)
> > +   imgu->buf_struct_size = sizeof(struct
> ipu3_vb2_buffer);
> > +   vbq->buf_struct_size = imgu->buf_struct_size;
> > +   vbq->timestamp_flags =
> V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> > +   vbq->min_buffers_needed = 0;/* Can streamon w/o buffers
> */
> > +   vbq->drv_priv = imgu;
> > +   vbq->lock = >lock;
> > +   r = vb2_queue_init(vbq);
> > +   if (r) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to initialize video queue (%d)\n", r);
> > +   goto fail_vdev;
> > +   }
> > +
> > +   /* Initialize vdev */
> > +   snprintf(vdev->name, sizeof(vdev->name), "%s %s",
> > +IMGU_NAME, node->name);
> > +   vdev->release = video_device_release_empty;
> > +   vdev->fops = _v4l2_fops;
> > +   vdev->lock = >lock;
> > +   vdev->v4l2_dev = >v4l2_dev;
> > +   vdev->queue = >vbq;
> > +   vdev->vfl_dir = node->output ? VFL_DIR_TX : VFL_DIR_RX;
> > +   video_set_drvdata(vdev, imgu);
> > +   r = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> > +   if (r) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to register video device (%d)\n", r);
> > +   goto fail_vdev;
> > +   }
> > +
> > +   /* Create link between video node and the subdev pad */
> > +   flags = 0;
> > +   if (node->enabled)
> > +   flags |= MEDIA_LNK_FL_ENABLED;
> > +   if (node->immutable)
> > +   flags |= MEDIA_LNK_FL_IMMUTABLE;
> > +   if (node->output) {
> > +   r = media_create_pad_link(>entity, 0,
> > + >subdev.entity,
> > +i, flags);
> > +   } else {
> > +   r = media_create_pad_link(>subdev.entity,
> > + i, >entity, 0, flags);
> > +   }
> > +   if (r)
> > +   goto fail_link;
> > +   }
> > +
> > +   r = media_device_register(>media_dev);
> > +   if (r) {
> > +   dev_err(>pci_dev->dev,
> > +   "failed to register media device (%d)\n", r);
> > +   i--;
> > +   goto fail_link;
> > +   }
> > +
> > +   return 0;
> > +
> > +   for (; i >= 0; i--) {
> > +fail_link:
> > +   video_unregister_device(>nodes[i].vdev);
> > +fail_vdev:
> > +   media_entity_cleanup(>nodes[i].vdev.entity);
> > +fail_vdev_media_entity:
> > +   mutex_destroy(>nodes[i].lock);
> > +   }
> > +fail_subdevs:
> > +   v4l2_device_unregister_subdev(>subdev);
> > +fail_subdev:
> > +   media_entity_cleanup(>subdev.entity);
> > +fail_media_entity:
> > +   kfree(imgu->subdev_pads);
> > +fail_subdev_pads:
> > +   v4l2_device_unregister(>v4l2_dev);
> > +fail_v4l2_dev:
> > +   media_device_cleanup(>media_dev);
> > +
> > +   return r;
> > +}
> > +EXPORT_SYMBOL_GPL(ipu3_v4l2_register);
> > +
> > +int ipu3_v4l2_unregister(struct imgu_device *imgu) {
> > +   unsigned int i;
> > +
> > +

[PATCH v4] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers

2018-11-15 Thread Philipp Zabel 
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: added V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS,
 updated documentation, and added back debug message]
Signed-off-by: Philipp Zabel 
Acked-by: Sakari Ailus 
---
Changes since v3:
 - Rephrased documentation
 - Added debug message
---
 Documentation/media/uapi/v4l/vidioc-reqbufs.rst | 16 +---
 drivers/media/common/videobuf2/videobuf2-core.c |  8 +++-
 drivers/media/common/videobuf2/videobuf2-v4l2.c |  2 +-
 include/uapi/linux/videodev2.h  |  1 +
 4 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst 
b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
index d40c60e8..fb1e643fda5f 100644
--- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
+++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
@@ -59,9 +59,14 @@ When the I/O method is not supported the ioctl returns an 
``EINVAL`` error
 code.
 
 Applications can call :ref:`VIDIOC_REQBUFS` again to change the number of
-buffers, however this cannot succeed when any buffers are still mapped.
-A ``count`` value of zero frees all buffers, after aborting or finishing
-any DMA in progress, an implicit
+buffers. Note that if any buffers are still mapped or exported via DMABUF,
+then :ref:`VIDIOC_REQBUFS` can only succeed if the
+``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS`` capability is set. Otherwise
+:ref:`VIDIOC_REQBUFS` will return the ``EBUSY`` error code.
+If ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS`` is set, then these buffers are
+orphaned and will be freed when they are unmapped or when the exported DMABUF
+fds are closed. A ``count`` value of zero frees or orphans all buffers, after
+aborting or finishing any DMA in progress, an implicit
 :ref:`VIDIOC_STREAMOFF `.
 
 
@@ -132,6 +137,11 @@ any DMA in progress, an implicit
 * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
   - 0x0008
   - This buffer type supports :ref:`requests `.
+* - ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS``
+  - 0x0010
+  - The kernel allows calling :ref:`VIDIOC_REQBUFS` while buffers are still
+mapped or exported via DMABUF. These orphaned buffers will be freed
+when they are unmapped or when the exported DMABUF fds are closed.
 
 Return Value
 
diff --git a/drivers/media/common/videobuf2/videobuf2-core.c 
b/drivers/media/common/videobuf2/videobuf2-core.c
index 975ff5669f72..7329cafc080a 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -679,11 +679,9 @@ int vb2_core_reqbufs(struct vb2_queue *q, enum vb2_memory 
memory,
 * 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;
-   }
+   if (debug && q->memory == VB2_MEMORY_MMAP &&
+   __buffers_in_use(q))
+   dprintk(1, "memory in use, orphaning buffers\n");
 
/*
 * Call queue_cancel to clean up any buffers in the
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
@@

[PATCH v2] v4l2-compliance: test orphaned buffer support

2018-11-15 Thread Philipp Zabel 
Test that V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS is reported equally for
both MMAP and DMABUF memory types. If supported, try to orphan buffers
by calling reqbufs(0) before unmapping or closing DMABUF fds.

Also close exported DMABUF fds and free buffers in testDmaBuf if
orphaned buffers are not supported.

Signed-off-by: Philipp Zabel 
---
Changes since v1:
 - Rename has_orphaned_bufs to supports_orphaned_bufs
 - Check that capabilities are independent of memory type
 - Check that orphaned buffer support is independent of queue for M2M
 - Check that reqbufs(0) returns -EBUSY without orphaned buffer support
---
 contrib/freebsd/include/linux/videodev2.h   |  1 +
 include/linux/videodev2.h   |  1 +
 utils/common/v4l2-info.cpp  |  1 +
 utils/v4l2-compliance/v4l2-compliance.h |  1 +
 utils/v4l2-compliance/v4l2-test-buffers.cpp | 51 ++---
 5 files changed, 50 insertions(+), 5 deletions(-)

diff --git a/contrib/freebsd/include/linux/videodev2.h 
b/contrib/freebsd/include/linux/videodev2.h
index 9928c00e4b68..33153b53c175 100644
--- a/contrib/freebsd/include/linux/videodev2.h
+++ b/contrib/freebsd/include/linux/videodev2.h
@@ -907,6 +907,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)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
index 79418cd39480..a39300cacb6a 100644
--- a/include/linux/videodev2.h
+++ b/include/linux/videodev2.h
@@ -873,6 +873,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)
 
 /**
  * struct v4l2_plane - plane info for multi-planar buffers
diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp
index 258e5446f030..3699c35cb9d6 100644
--- a/utils/common/v4l2-info.cpp
+++ b/utils/common/v4l2-info.cpp
@@ -200,6 +200,7 @@ static const flag_def bufcap_def[] = {
{ V4L2_BUF_CAP_SUPPORTS_USERPTR, "userptr" },
{ V4L2_BUF_CAP_SUPPORTS_DMABUF, "dmabuf" },
{ V4L2_BUF_CAP_SUPPORTS_REQUESTS, "requests" },
+   { V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS, "orphaned-bufs" },
{ 0, NULL }
 };
 
diff --git a/utils/v4l2-compliance/v4l2-compliance.h 
b/utils/v4l2-compliance/v4l2-compliance.h
index def185f17261..02d616f0b47c 100644
--- a/utils/v4l2-compliance/v4l2-compliance.h
+++ b/utils/v4l2-compliance/v4l2-compliance.h
@@ -119,6 +119,7 @@ struct base_node {
__u32 valid_buftypes;
__u32 valid_buftype;
__u32 valid_memorytype;
+   bool supports_orphaned_bufs;
 };
 
 struct node : public base_node, public cv4l_fd {
diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp 
b/utils/v4l2-compliance/v4l2-test-buffers.cpp
index a84be0ab799a..42e743fef43b 100644
--- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
+++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
@@ -400,14 +400,18 @@ int testReqBufs(struct node *node)
mmap_valid = !ret;
if (mmap_valid)
caps = q.g_capabilities();
-   if (caps)
+   if (caps) {
fail_on_test(mmap_valid ^ !!(caps & 
V4L2_BUF_CAP_SUPPORTS_MMAP));
+   if (caps & V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS)
+   node->supports_orphaned_bufs = true;
+   }
 
q.init(i, V4L2_MEMORY_USERPTR);
ret = q.reqbufs(node, 0);
fail_on_test(ret && ret != EINVAL);
userptr_valid = !ret;
fail_on_test(!mmap_valid && userptr_valid);
+   fail_on_test(userptr_valid && (caps != q.g_capabilities()));
if (caps)
fail_on_test(userptr_valid ^ !!(caps & 
V4L2_BUF_CAP_SUPPORTS_USERPTR));
 
@@ -416,6 +420,7 @@ int testReqBufs(struct node *node)
fail_on_test(ret && ret != EINVAL);
dmabuf_valid = !ret;
fail_on_test(!mmap_valid && dmabuf_valid);
+   fail_on_test(dmabuf_valid && (caps != q.g_capabilities()));
if (caps)
fail_on_test(dmabuf_valid ^ !!(caps & 
V4L2_BUF_CAP_SUPPORTS_DMABUF));
 
@@ -754,9 +759,13 @@ static int captureBufs(struct node *node, const cv4l_queue 
,
 
 static int setupM2M(struct node *node, cv4l_queue )
 {
+   __u32 caps;
+
last_m2m_seq.init();
 
fail_on_test(q.reqbufs(node, 2));
+   caps = q.g_capabilities();
+   fail_on_test(node->supports_orphaned_bufs ^ !!(caps & 
V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS));
for (unsigned i = 0; i < q.g_buffers(); i++) {
buffer buf(q);
 
@@ -965,12 +974,32 @@ int

Re: [PATCH v4l-utils] v4l2-compliance: test orphaned buffer support

2018-11-15 Thread Hans Verkuil 
On 11/15/18 13:52, Philipp Zabel wrote:
> On Thu, 2018-11-15 at 11:21 +0100, Hans Verkuil wrote:
>> On 11/14/18 15:38, Philipp Zabel wrote:
>>> Test that V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS is reported equally for
>>> both MMAP and DMABUF memory types. If supported, try to orphan buffers
>>> by calling reqbufs(0) before unmapping or closing DMABUF fds.
>>>
>>> Also close exported DMABUF fds and free buffers in testDmaBuf if
>>> orphaned buffers are not supported.
>>>
>>> Signed-off-by: Philipp Zabel 
>>> ---
>>>  contrib/freebsd/include/linux/videodev2.h   |  1 +
>>>  include/linux/videodev2.h   |  1 +
>>>  utils/common/v4l2-info.cpp  |  1 +
>>>  utils/v4l2-compliance/v4l2-compliance.h |  1 +
>>>  utils/v4l2-compliance/v4l2-test-buffers.cpp | 35 +
>>>  5 files changed, 33 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/contrib/freebsd/include/linux/videodev2.h 
>>> b/contrib/freebsd/include/linux/videodev2.h
>>> index 9928c00e4b68..33153b53c175 100644
>>> --- a/contrib/freebsd/include/linux/videodev2.h
>>> +++ b/contrib/freebsd/include/linux/videodev2.h
>>> @@ -907,6 +907,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)
>>>  
>>>  /**
>>>   * struct v4l2_plane - plane info for multi-planar buffers
>>> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
>>> index 79418cd39480..a39300cacb6a 100644
>>> --- a/include/linux/videodev2.h
>>> +++ b/include/linux/videodev2.h
>>> @@ -873,6 +873,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)
>>>  
>>>  /**
>>>   * struct v4l2_plane - plane info for multi-planar buffers
>>> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp
>>> index 258e5446f030..3699c35cb9d6 100644
>>> --- a/utils/common/v4l2-info.cpp
>>> +++ b/utils/common/v4l2-info.cpp
>>> @@ -200,6 +200,7 @@ static const flag_def bufcap_def[] = {
>>> { V4L2_BUF_CAP_SUPPORTS_USERPTR, "userptr" },
>>> { V4L2_BUF_CAP_SUPPORTS_DMABUF, "dmabuf" },
>>> { V4L2_BUF_CAP_SUPPORTS_REQUESTS, "requests" },
>>> +   { V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS, "orphaned-bufs" },
>>> { 0, NULL }
>>>  };
>>>  
>>> diff --git a/utils/v4l2-compliance/v4l2-compliance.h 
>>> b/utils/v4l2-compliance/v4l2-compliance.h
>>> index def185f17261..88ec260a9bcc 100644
>>> --- a/utils/v4l2-compliance/v4l2-compliance.h
>>> +++ b/utils/v4l2-compliance/v4l2-compliance.h
>>> @@ -119,6 +119,7 @@ struct base_node {
>>> __u32 valid_buftypes;
>>> __u32 valid_buftype;
>>> __u32 valid_memorytype;
>>> +   bool has_orphaned_bufs;
>>
>> I'd rename that to supports_orphaned_bufs.
> 
> Ok.
> 
>>>  };
>>>  
>>>  struct node : public base_node, public cv4l_fd {
>>> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp 
>>> b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> index c59a56d9ced7..6174015cb4e7 100644
>>> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
>>> @@ -400,8 +400,11 @@ int testReqBufs(struct node *node)
>>> mmap_valid = !ret;
>>> if (mmap_valid)
>>> caps = q.g_capabilities();
>>> -   if (caps)
>>> +   if (caps) {
>>> fail_on_test(mmap_valid ^ !!(caps & 
>>> V4L2_BUF_CAP_SUPPORTS_MMAP));
>>> +   if (caps & V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS)
>>> +   node->has_orphaned_bufs = true;
>>> +   }
>>>  
>>> q.init(i, V4L2_MEMORY_USERPTR);
>>> ret = q.reqbufs(node, 0);
>>> @@ -418,8 +421,11 @@ int testReqBufs(struct node *node)
>>> fail_on_test(!mmap_valid && dmabuf_valid);
>>> // Note: dmabuf is only supported with vb2, so we can assume a
>>> // non-0 caps value if dmabuf is supported.
>>> -   if (caps || dmabuf_valid)
>>> +   if (caps || dmabuf_valid) {
>>> fail_on_test(dmabuf_valid ^ !!(caps & 
>>> V4L2_BUF_CAP_SUPPORTS_DMABUF));
>>> +   if (node->has_orphaned_bufs)
>>> +   fail_on_test(userptr_valid ^ !!(caps & 
>>> V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS));
>>
>> Huh? I'm not sure what you are testing here.
> 
> I intended to test that if MMAP supports orphaned buffers, DMABUF should
> as well, but stopped halfway. Otherwise we'd have to keep separate flags
> for MMAP and DMABUF.

It's shared. So the capability field as returned by REQBUFS and CREATE_BUFS
is valid for both MMAP and DMABUF. Or to put it in another way: the returned
capabilities are independent of the memory field.

What would be a

Re: [PATCH v4l-utils] v4l2-compliance: test orphaned buffer support

2018-11-15 Thread Philipp Zabel 
On Thu, 2018-11-15 at 11:21 +0100, Hans Verkuil wrote:
> On 11/14/18 15:38, Philipp Zabel wrote:
> > Test that V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS is reported equally for
> > both MMAP and DMABUF memory types. If supported, try to orphan buffers
> > by calling reqbufs(0) before unmapping or closing DMABUF fds.
> > 
> > Also close exported DMABUF fds and free buffers in testDmaBuf if
> > orphaned buffers are not supported.
> > 
> > Signed-off-by: Philipp Zabel 
> > ---
> >  contrib/freebsd/include/linux/videodev2.h   |  1 +
> >  include/linux/videodev2.h   |  1 +
> >  utils/common/v4l2-info.cpp  |  1 +
> >  utils/v4l2-compliance/v4l2-compliance.h |  1 +
> >  utils/v4l2-compliance/v4l2-test-buffers.cpp | 35 +
> >  5 files changed, 33 insertions(+), 6 deletions(-)
> > 
> > diff --git a/contrib/freebsd/include/linux/videodev2.h 
> > b/contrib/freebsd/include/linux/videodev2.h
> > index 9928c00e4b68..33153b53c175 100644
> > --- a/contrib/freebsd/include/linux/videodev2.h
> > +++ b/contrib/freebsd/include/linux/videodev2.h
> > @@ -907,6 +907,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)
> >  
> >  /**
> >   * struct v4l2_plane - plane info for multi-planar buffers
> > diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> > index 79418cd39480..a39300cacb6a 100644
> > --- a/include/linux/videodev2.h
> > +++ b/include/linux/videodev2.h
> > @@ -873,6 +873,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)
> >  
> >  /**
> >   * struct v4l2_plane - plane info for multi-planar buffers
> > diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp
> > index 258e5446f030..3699c35cb9d6 100644
> > --- a/utils/common/v4l2-info.cpp
> > +++ b/utils/common/v4l2-info.cpp
> > @@ -200,6 +200,7 @@ static const flag_def bufcap_def[] = {
> > { V4L2_BUF_CAP_SUPPORTS_USERPTR, "userptr" },
> > { V4L2_BUF_CAP_SUPPORTS_DMABUF, "dmabuf" },
> > { V4L2_BUF_CAP_SUPPORTS_REQUESTS, "requests" },
> > +   { V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS, "orphaned-bufs" },
> > { 0, NULL }
> >  };
> >  
> > diff --git a/utils/v4l2-compliance/v4l2-compliance.h 
> > b/utils/v4l2-compliance/v4l2-compliance.h
> > index def185f17261..88ec260a9bcc 100644
> > --- a/utils/v4l2-compliance/v4l2-compliance.h
> > +++ b/utils/v4l2-compliance/v4l2-compliance.h
> > @@ -119,6 +119,7 @@ struct base_node {
> > __u32 valid_buftypes;
> > __u32 valid_buftype;
> > __u32 valid_memorytype;
> > +   bool has_orphaned_bufs;
> 
> I'd rename that to supports_orphaned_bufs.

Ok.

> >  };
> >  
> >  struct node : public base_node, public cv4l_fd {
> > diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp 
> > b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> > index c59a56d9ced7..6174015cb4e7 100644
> > --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> > +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> > @@ -400,8 +400,11 @@ int testReqBufs(struct node *node)
> > mmap_valid = !ret;
> > if (mmap_valid)
> > caps = q.g_capabilities();
> > -   if (caps)
> > +   if (caps) {
> > fail_on_test(mmap_valid ^ !!(caps & 
> > V4L2_BUF_CAP_SUPPORTS_MMAP));
> > +   if (caps & V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS)
> > +   node->has_orphaned_bufs = true;
> > +   }
> >  
> > q.init(i, V4L2_MEMORY_USERPTR);
> > ret = q.reqbufs(node, 0);
> > @@ -418,8 +421,11 @@ int testReqBufs(struct node *node)
> > fail_on_test(!mmap_valid && dmabuf_valid);
> > // Note: dmabuf is only supported with vb2, so we can assume a
> > // non-0 caps value if dmabuf is supported.
> > -   if (caps || dmabuf_valid)
> > +   if (caps || dmabuf_valid) {
> > fail_on_test(dmabuf_valid ^ !!(caps & 
> > V4L2_BUF_CAP_SUPPORTS_DMABUF));
> > +   if (node->has_orphaned_bufs)
> > +   fail_on_test(userptr_valid ^ !!(caps & 
> > V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS));
> 
> Huh? I'm not sure what you are testing here.

I intended to test that if MMAP supports orphaned buffers, DMABUF should
as well, but stopped halfway. Otherwise we'd have to keep separate flags
for MMAP and DMABUF.

> > +   }
> >  
> > fail_on_test((can_stream && !is_overlay) && !mmap_valid && 
> > !userptr_valid && !dmabuf_valid);
> > fail_on_test((!can_stream || is_overlay) && (mmap_valid || 
> > userptr_valid || dmabuf_valid));
> > @@ -967,12 +973,22 @@

Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-15 Thread Hans Verkuil 
On 10/29/18 23:22, Yong Zhi wrote:
> These meta formats are used on Intel IPU3 ImgU video queues
> to carry 3A statistics and ISP pipeline parameters.
> 
> V4L2_META_FMT_IPU3_3A
> V4L2_META_FMT_IPU3_PARAMS
> 
> Signed-off-by: Yong Zhi 
> Signed-off-by: Chao C Li 
> Signed-off-by: Rajmohan Mani 
> ---
>  Documentation/media/uapi/v4l/meta-formats.rst  |1 +
>  .../media/uapi/v4l/pixfmt-meta-intel-ipu3.rst  |  181 ++
>  include/uapi/linux/intel-ipu3.h| 2819 
> 
>  3 files changed, 3001 insertions(+)
>  create mode 100644 Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
>  create mode 100644 include/uapi/linux/intel-ipu3.h
> 
> diff --git a/Documentation/media/uapi/v4l/meta-formats.rst 
> b/Documentation/media/uapi/v4l/meta-formats.rst
> index cf971d5..eafc534 100644
> --- a/Documentation/media/uapi/v4l/meta-formats.rst
> +++ b/Documentation/media/uapi/v4l/meta-formats.rst
> @@ -12,6 +12,7 @@ These formats are used for the :ref:`metadata` interface 
> only.
>  .. toctree::
>  :maxdepth: 1
>  
> +pixfmt-meta-intel-ipu3
>  pixfmt-meta-d4xx
>  pixfmt-meta-uvc
>  pixfmt-meta-vsp1-hgo
> diff --git a/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst 
> b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> new file mode 100644
> index 000..23b945b
> --- /dev/null
> +++ b/Documentation/media/uapi/v4l/pixfmt-meta-intel-ipu3.rst
> @@ -0,0 +1,181 @@
> +.. -*- coding: utf-8; mode: rst -*-
> +
> +.. _intel-ipu3:
> +
> +**
> +V4L2_META_FMT_IPU3_PARAMS ('ip3p'), V4L2_META_FMT_IPU3_3A ('ip3s')
> +**
> +
> +.. c:type:: ipu3_uapi_stats_3a
> +
> +3A statistics
> +=
> +
> +For IPU3 ImgU, the 3A statistics accelerators collect different statistics 
> over
> +an input bayer frame. Those statistics, defined in data struct
> +:c:type:`ipu3_uapi_stats_3a`, are meta output obtained from "ipu3-imgu 3a 
> stat"

I'd rephrase this:

are obtained from the "ipu3-imgu 3a stat" metadata capture video node,

> +video node, which are then passed to user space for statistics analysis
> +using :c:type:`v4l2_meta_format` interface.
> +
> +The statistics collected are AWB (Auto-white balance) RGBS (Red, Green, Blue 
> and 
> +Saturation measure) cells, AWB filter response, AF (Auto-focus) filter 
> response,
> +and AE (Auto-exposure) histogram.
> +
> +struct :c:type:`ipu3_uapi_4a_config` saves configurable parameters for all 
> above.
> +
> +
> +.. code-block:: c
> +
> +
> + struct ipu3_uapi_stats_3a {
> + struct ipu3_uapi_awb_raw_buffer awb_raw_buffer
> +  __attribute__((aligned(32)));
> + struct ipu3_uapi_ae_raw_buffer_aligned
> + ae_raw_buffer[IPU3_UAPI_MAX_STRIPES];
> + struct ipu3_uapi_af_raw_buffer af_raw_buffer;
> + struct ipu3_uapi_awb_fr_raw_buffer awb_fr_raw_buffer;
> + struct ipu3_uapi_4a_config stats_4a_config;
> + __u32 ae_join_buffers;
> + __u8 padding[28];
> + struct ipu3_uapi_stats_3a_bubble_info_per_stripe
> + stats_3a_bubble_per_stripe;
> + struct ipu3_uapi_ff_status stats_3a_status;
> + } __packed;
> +
> +
> +.. c:type:: ipu3_uapi_params
> +
> +Pipeline parameters
> +===
> +
> +IPU3 pipeline has a number of image processing stages, each of which takes a
> +set of parameters as input. The major stages of pipelines are shown here:
> +
> +Raw pixels -> Bayer Downscaling -> Optical Black Correction ->
> +
> +Linearization -> Lens Shading Correction -> White Balance / Exposure /
> +
> +Focus Apply -> Bayer Noise Reduction -> ANR -> Demosaicing -> Color
> +
> +Correction Matrix -> Gamma correction -> Color Space Conversion ->
> +
> +Chroma Down Scaling -> Chromatic Noise Reduction -> Total Color
> +
> +Correction -> XNR3 -> TNR -> DDR
> +
> +The table below presents a description of the above algorithms.
> +
> + 
> ===
> +Name  Description
> + 
> ===
> +Optical Black Correction Optical Black Correction block subtracts a 
> pre-defined
> +  value from the respective pixel values to obtain better
> +  image quality.
> +  Defined in :c:type:`ipu3_uapi_obgrid_param`.
> +Linearization This algo block uses linearization parameters 
> to
> +  address non-linearity sensor effects. The Lookup table
> +  table is defined in
> +  :c:type:`ipu3_uapi_isp_lin_vmem_params`.
> +SHD   Lens shading correction is used to correct spatial
> +  non-uniformity of the pixel response due to optical
> +  lens shading. This is done by applying a different gain
> +

Re: [PATCH v7 14/16] intel-ipu3: Add v4l2 driver based on media framework

2018-11-15 Thread Hans Verkuil 
On 10/29/18 23:23, Yong Zhi wrote:
> Implement video driver that utilizes v4l2, vb2 queue support
> and media controller APIs. The driver exposes single
> subdevice and six nodes.
> 
> Signed-off-by: Yong Zhi 
> ---
>  drivers/media/pci/intel/ipu3/ipu3-v4l2.c | 1091 
> ++
>  1 file changed, 1091 insertions(+)
>  create mode 100644 drivers/media/pci/intel/ipu3/ipu3-v4l2.c
> 
> diff --git a/drivers/media/pci/intel/ipu3/ipu3-v4l2.c 
> b/drivers/media/pci/intel/ipu3/ipu3-v4l2.c



> +int ipu3_v4l2_register(struct imgu_device *imgu)
> +{



> + /* Initialize vbq */
> + vbq->type = node->vdev_fmt.type;
> + vbq->io_modes = VB2_USERPTR | VB2_MMAP | VB2_DMABUF;

Are you sure USERPTR works? If you have alignment requirements that
the buffer starts at a multiple of more than (I think) 8 bytes, then
USERPTR won't work.

> + vbq->ops = _vb2_ops;
> + vbq->mem_ops = _dma_sg_memops;
> + if (imgu->buf_struct_size <= 0)
> + imgu->buf_struct_size = sizeof(struct ipu3_vb2_buffer);
> + vbq->buf_struct_size = imgu->buf_struct_size;
> + vbq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
> + vbq->min_buffers_needed = 0;/* Can streamon w/o buffers */
> + vbq->drv_priv = imgu;
> + vbq->lock = >lock;
> + r = vb2_queue_init(vbq);
> + if (r) {
> + dev_err(>pci_dev->dev,
> + "failed to initialize video queue (%d)\n", r);
> + goto fail_vdev;
> + }
> +
> + /* Initialize vdev */
> + snprintf(vdev->name, sizeof(vdev->name), "%s %s",
> +  IMGU_NAME, node->name);
> + vdev->release = video_device_release_empty;
> + vdev->fops = _v4l2_fops;
> + vdev->lock = >lock;
> + vdev->v4l2_dev = >v4l2_dev;
> + vdev->queue = >vbq;
> + vdev->vfl_dir = node->output ? VFL_DIR_TX : VFL_DIR_RX;
> + video_set_drvdata(vdev, imgu);
> + r = video_register_device(vdev, VFL_TYPE_GRABBER, -1);
> + if (r) {
> + dev_err(>pci_dev->dev,
> + "failed to register video device (%d)\n", r);
> + goto fail_vdev;
> + }
> +
> + /* Create link between video node and the subdev pad */
> + flags = 0;
> + if (node->enabled)
> + flags |= MEDIA_LNK_FL_ENABLED;
> + if (node->immutable)
> + flags |= MEDIA_LNK_FL_IMMUTABLE;
> + if (node->output) {
> + r = media_create_pad_link(>entity, 0,
> +   >subdev.entity,
> +  i, flags);
> + } else {
> + r = media_create_pad_link(>subdev.entity,
> +   i, >entity, 0, flags);
> + }
> + if (r)
> + goto fail_link;
> + }
> +
> + r = media_device_register(>media_dev);
> + if (r) {
> + dev_err(>pci_dev->dev,
> + "failed to register media device (%d)\n", r);
> + i--;
> + goto fail_link;
> + }
> +
> + return 0;
> +
> + for (; i >= 0; i--) {
> +fail_link:
> + video_unregister_device(>nodes[i].vdev);
> +fail_vdev:
> + media_entity_cleanup(>nodes[i].vdev.entity);
> +fail_vdev_media_entity:
> + mutex_destroy(>nodes[i].lock);
> + }
> +fail_subdevs:
> + v4l2_device_unregister_subdev(>subdev);
> +fail_subdev:
> + media_entity_cleanup(>subdev.entity);
> +fail_media_entity:
> + kfree(imgu->subdev_pads);
> +fail_subdev_pads:
> + v4l2_device_unregister(>v4l2_dev);
> +fail_v4l2_dev:
> + media_device_cleanup(>media_dev);
> +
> + return r;
> +}
> +EXPORT_SYMBOL_GPL(ipu3_v4l2_register);
> +
> +int ipu3_v4l2_unregister(struct imgu_device *imgu)
> +{
> + unsigned int i;
> +
> + media_device_unregister(>media_dev);
> + media_device_cleanup(>media_dev);
> +
> + for (i = 0; i < IMGU_NODE_NUM; i++) {
> + video_unregister_device(>nodes[i].vdev);
> + media_entity_cleanup(>nodes[i].vdev.entity);
> + mutex_destroy(>nodes[i].lock);
> + }
> +
> + v4l2_device_unregister_subdev(>subdev);
> + media_entity_cleanup(>subdev.entity);
> + kfree(imgu->subdev_pads);
> + v4l2_device_unregister(>v4l2_dev);
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(ipu3_v4l2_unregister);
> +
> +void ipu3_v4l2_buffer_done(struct vb2_buffer *vb,
> +enum vb2_buffer_state state)
> +{
> + struct ipu3_vb2_buffer *b =
> + container_of(vb, struct ipu3_vb2_buffer, vbb.vb2_buf);
> +
> + list_del(>list);
> +

Re: [PATCH v3] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers

2018-11-15 Thread Sakari Ailus 
On Thu, Nov 15, 2018 at 11:29:35AM +0100, Hans Verkuil wrote:
> On 11/14/18 20:59, Laurent Pinchart wrote:
> > Hi Philipp,
> > 
> > Thank you for the patch.
> > 
> > On Wednesday, 14 November 2018 17:04:49 EET 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.
> > 
> > While I agree that we should remove this restriction, it has helped me in 
> > the 
> > past to find missing munmap() in userspace. This patch thus has the 
> > potential 
> > of causing memory leaks in userspace. Is there a way we could assist 
> > application developers with this ?
> 
> Should we just keep the debug message? (rephrased of course)
> 
> That way you can enable debugging and see that this happens.
> 
> It sounds reasonable to me.

Makes sense IMO.

-- 
Sakari Ailus
e-mail: sakari.ai...@iki.fi

[PATCH vicodec v4 3/3] media: vicodec: Add support for 4 planes formats

2018-11-15 Thread Dafna Hirschfeld 
Add support for formats with 4 planes: V4L2_PIX_FMT_ABGR32,
V4L2_PIX_FMT_ARGB32.
Also add alpha plane related flags to the header of the encoded file.

Signed-off-by: Dafna Hirschfeld 
---
 drivers/media/platform/vicodec/codec-fwht.c   | 15 +
 drivers/media/platform/vicodec/codec-fwht.h   |  2 ++
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 32 +++
 drivers/media/platform/vicodec/vicodec-core.c | 12 +--
 4 files changed, 59 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.c 
b/drivers/media/platform/vicodec/codec-fwht.c
index 1af9af84163e..9513374e8f44 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -782,6 +782,17 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
encoding |= FWHT_CR_UNENCODED;
encoding &= ~FWHT_FRAME_UNENCODED;
}
+
+   if (frm->components_num == 4) {
+   rlco_max = rlco + size / 2 - 256;
+   encoding = encode_plane(frm->alpha, ref_frm->alpha, , 
rlco_max, cf,
+   frm->height, frm->width,
+   frm->luma_alpha_step, is_intra, next_is_intra);
+   if (encoding & FWHT_FRAME_UNENCODED)
+   encoding |= FWHT_ALPHA_UNENCODED;
+   encoding &= ~FWHT_FRAME_UNENCODED;
+   }
+
cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
return encoding;
 }
@@ -860,4 +871,8 @@ void fwht_decode_frame(struct fwht_cframe *cf, struct 
fwht_raw_frame *ref,
decode_plane(cf, , ref->cr, h, w,
hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
}
+
+   if (components_num == 4)
+   decode_plane(cf, , ref->alpha, cf->height, cf->width,
+hdr_flags & FWHT_FL_ALPHA_IS_UNCOMPRESSED);
 }
diff --git a/drivers/media/platform/vicodec/codec-fwht.h 
b/drivers/media/platform/vicodec/codec-fwht.h
index bde11fb93f26..90ff8962fca7 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -75,6 +75,7 @@
 #define FWHT_FL_CR_IS_UNCOMPRESSED BIT(6)
 #define FWHT_FL_CHROMA_FULL_HEIGHT BIT(7)
 #define FWHT_FL_CHROMA_FULL_WIDTH  BIT(8)
+#define FWHT_FL_ALPHA_IS_UNCOMPRESSED  BIT(9)
 
 /* A 4-values flag - the number of components - 1 */
 #define FWHT_FL_COMPONENTS_NUM_MSK GENMASK(17, 16)
@@ -119,6 +120,7 @@ struct fwht_raw_frame {
 #define FWHT_LUMA_UNENCODEDBIT(2)
 #define FWHT_CB_UNENCODED  BIT(3)
 #define FWHT_CR_UNENCODED  BIT(4)
+#define FWHT_ALPHA_UNENCODED   BIT(5)
 
 u32 fwht_encode_frame(struct fwht_raw_frame *frm,
  struct fwht_raw_frame *ref_frm,
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c 
b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index 7dc3918a017e..b53655a8cef6 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -33,6 +33,8 @@ static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] 
= {
{ V4L2_PIX_FMT_RGB32,   4, 4, 1, 4, 4, 1, 1, 3},
{ V4L2_PIX_FMT_XRGB32,  4, 4, 1, 4, 4, 1, 1, 3},
{ V4L2_PIX_FMT_HSV32,   4, 4, 1, 4, 4, 1, 1, 3},
+   { V4L2_PIX_FMT_ARGB32,  4, 4, 1, 4, 4, 1, 1, 4},
+   { V4L2_PIX_FMT_ABGR32,  4, 4, 1, 4, 4, 1, 1, 4},
 
 };
 
@@ -146,6 +148,18 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 
*p_in, u8 *p_out)
rf.cr = rf.cb + 2;
rf.luma++;
break;
+   case V4L2_PIX_FMT_ARGB32:
+   rf.alpha = rf.luma;
+   rf.cr = rf.luma + 1;
+   rf.cb = rf.cr + 2;
+   rf.luma += 2;
+   break;
+   case V4L2_PIX_FMT_ABGR32:
+   rf.cb = rf.luma;
+   rf.cr = rf.cb + 2;
+   rf.luma++;
+   rf.alpha = rf.cr + 1;
+   break;
default:
return -EINVAL;
}
@@ -177,6 +191,8 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 
*p_in, u8 *p_out)
flags |= FWHT_FL_CB_IS_UNCOMPRESSED;
if (encoding & FWHT_CR_UNENCODED)
flags |= FWHT_FL_CR_IS_UNCOMPRESSED;
+   if (encoding & FWHT_ALPHA_UNENCODED)
+   flags |= FWHT_FL_ALPHA_IS_UNCOMPRESSED;
if (rf.height_div == 1)
flags |= FWHT_FL_CHROMA_FULL_HEIGHT;
if (rf.width_div == 1)
@@ -356,6 +372,22 @@ int v4l2_fwht_decode(struct v4l2_fwht_state *state, u8 
*p_in, u8 *p_out)
*p++ = 0;
}
break;
+   case V4L2_PIX_FMT_ARGB32:
+   for (i = 0, p = p_out; i < size; i++) {
+   *p++ = state->ref_frame.alpha[i];
+   *p++ = state->ref_frame.cr[i];
+   *p++ = state->ref_frame.luma[i];
+   *p++ = state->ref_frame.cb[i];
+   }
+

[PATCH vicodec v4 2/3] media: vicodec: Add support of greyscale format

2018-11-15 Thread Dafna Hirschfeld 
Add support for single plane greyscale format V4L2_PIX_FMT_GREY.
Also change the header of the encoded file to include the number
of components.

Signed-off-by: Dafna Hirschfeld 
---
 drivers/media/platform/vicodec/codec-fwht.c   | 56 +++
 drivers/media/platform/vicodec/codec-fwht.h   |  8 ++-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 44 ---
 drivers/media/platform/vicodec/vicodec-core.c | 25 +++--
 4 files changed, 93 insertions(+), 40 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.c 
b/drivers/media/platform/vicodec/codec-fwht.c
index 4ee6d7e0fbe2..1af9af84163e 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -753,9 +753,6 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
__be16 *rlco = cf->rlc_data;
__be16 *rlco_max;
u32 encoding;
-   u32 chroma_h = frm->height / frm->height_div;
-   u32 chroma_w = frm->width / frm->width_div;
-   unsigned int chroma_size = chroma_h * chroma_w;
 
rlco_max = rlco + size / 2 - 256;
encoding = encode_plane(frm->luma, ref_frm->luma, , rlco_max, cf,
@@ -764,20 +761,27 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
if (encoding & FWHT_FRAME_UNENCODED)
encoding |= FWHT_LUMA_UNENCODED;
encoding &= ~FWHT_FRAME_UNENCODED;
-   rlco_max = rlco + chroma_size / 2 - 256;
-   encoding |= encode_plane(frm->cb, ref_frm->cb, , rlco_max, cf,
+
+   if (frm->components_num >= 3) {
+   u32 chroma_h = frm->height / frm->height_div;
+   u32 chroma_w = frm->width / frm->width_div;
+   unsigned int chroma_size = chroma_h * chroma_w;
+
+   rlco_max = rlco + chroma_size / 2 - 256;
+   encoding |= encode_plane(frm->cb, ref_frm->cb, , rlco_max, 
cf,
 chroma_h, chroma_w,
 frm->chroma_step, is_intra, next_is_intra);
-   if (encoding & FWHT_FRAME_UNENCODED)
-   encoding |= FWHT_CB_UNENCODED;
-   encoding &= ~FWHT_FRAME_UNENCODED;
-   rlco_max = rlco + chroma_size / 2 - 256;
-   encoding |= encode_plane(frm->cr, ref_frm->cr, , rlco_max, cf,
+   if (encoding & FWHT_FRAME_UNENCODED)
+   encoding |= FWHT_CB_UNENCODED;
+   encoding &= ~FWHT_FRAME_UNENCODED;
+   rlco_max = rlco + chroma_size / 2 - 256;
+   encoding |= encode_plane(frm->cr, ref_frm->cr, , rlco_max, 
cf,
 chroma_h, chroma_w,
 frm->chroma_step, is_intra, next_is_intra);
-   if (encoding & FWHT_FRAME_UNENCODED)
-   encoding |= FWHT_CR_UNENCODED;
-   encoding &= ~FWHT_FRAME_UNENCODED;
+   if (encoding & FWHT_FRAME_UNENCODED)
+   encoding |= FWHT_CR_UNENCODED;
+   encoding &= ~FWHT_FRAME_UNENCODED;
+   }
cf->size = (rlco - cf->rlc_data) * sizeof(*rlco);
return encoding;
 }
@@ -836,20 +840,24 @@ static void decode_plane(struct fwht_cframe *cf, const 
__be16 **rlco, u8 *ref,
 }
 
 void fwht_decode_frame(struct fwht_cframe *cf, struct fwht_raw_frame *ref,
-  u32 hdr_flags)
+  u32 hdr_flags, unsigned int components_num)
 {
const __be16 *rlco = cf->rlc_data;
-   u32 h = cf->height / 2;
-   u32 w = cf->width / 2;
 
-   if (hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT)
-   h *= 2;
-   if (hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH)
-   w *= 2;
decode_plane(cf, , ref->luma, cf->height, cf->width,
 hdr_flags & FWHT_FL_LUMA_IS_UNCOMPRESSED);
-   decode_plane(cf, , ref->cb, h, w,
-hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
-   decode_plane(cf, , ref->cr, h, w,
-hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
+
+   if (components_num >= 3) {
+   u32 h = cf->height;
+   u32 w = cf->width;
+
+   if (!(hdr_flags & FWHT_FL_CHROMA_FULL_HEIGHT))
+   h /= 2;
+   if (!(hdr_flags & FWHT_FL_CHROMA_FULL_WIDTH))
+   w /= 2;
+   decode_plane(cf, , ref->cb, h, w,
+   hdr_flags & FWHT_FL_CB_IS_UNCOMPRESSED);
+   decode_plane(cf, , ref->cr, h, w,
+   hdr_flags & FWHT_FL_CR_IS_UNCOMPRESSED);
+   }
 }
diff --git a/drivers/media/platform/vicodec/codec-fwht.h 
b/drivers/media/platform/vicodec/codec-fwht.h
index 743d78e112f8..bde11fb93f26 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -56,7 +56,7 @@
 #define FWHT_MAGIC1 0x4f4f4f4f
 #define FWHT_MAGIC2 0x
 
-#define FWHT_VERSION 1
+#define FWHT_VERSION 2
 
 /* Set if this is an interlaced format */
 #define FWHT_FL_IS_INTERLACED  BIT(0)
@@ -76,6 +76,10 @@
 #define

[PATCH vicodec v4 1/3] media: vicodec: prepare support for various number of planes

2018-11-15 Thread Dafna Hirschfeld 
Add fields to the structs `fwht_raw_frame`, `v4l2_fwht_pixfmts`
to support various number of planes - formats
with alpha channel that have 4 planes and greyscale formats
that have 1 plane.

Signed-off-by: Dafna Hirschfeld 
---
 drivers/media/platform/vicodec/codec-fwht.c   |  2 +-
 drivers/media/platform/vicodec/codec-fwht.h   |  5 +-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 47 ++-
 .../media/platform/vicodec/codec-v4l2-fwht.h  |  3 +-
 drivers/media/platform/vicodec/vicodec-core.c |  2 +-
 5 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/drivers/media/platform/vicodec/codec-fwht.c 
b/drivers/media/platform/vicodec/codec-fwht.c
index 36656031b295..4ee6d7e0fbe2 100644
--- a/drivers/media/platform/vicodec/codec-fwht.c
+++ b/drivers/media/platform/vicodec/codec-fwht.c
@@ -760,7 +760,7 @@ u32 fwht_encode_frame(struct fwht_raw_frame *frm,
rlco_max = rlco + size / 2 - 256;
encoding = encode_plane(frm->luma, ref_frm->luma, , rlco_max, cf,
frm->height, frm->width,
-   frm->luma_step, is_intra, next_is_intra);
+   frm->luma_alpha_step, is_intra, next_is_intra);
if (encoding & FWHT_FRAME_UNENCODED)
encoding |= FWHT_LUMA_UNENCODED;
encoding &= ~FWHT_FRAME_UNENCODED;
diff --git a/drivers/media/platform/vicodec/codec-fwht.h 
b/drivers/media/platform/vicodec/codec-fwht.h
index 3e9391fec5fe..743d78e112f8 100644
--- a/drivers/media/platform/vicodec/codec-fwht.h
+++ b/drivers/media/platform/vicodec/codec-fwht.h
@@ -104,9 +104,10 @@ struct fwht_raw_frame {
unsigned int width, height;
unsigned int width_div;
unsigned int height_div;
-   unsigned int luma_step;
+   unsigned int luma_alpha_step;
unsigned int chroma_step;
-   u8 *luma, *cb, *cr;
+   unsigned int components_num;
+   u8 *luma, *cb, *cr, *alpha;
 };
 
 #define FWHT_FRAME_PCODED  BIT(0)
diff --git a/drivers/media/platform/vicodec/codec-v4l2-fwht.c 
b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
index e5b68fb38aac..b842636e71a9 100644
--- a/drivers/media/platform/vicodec/codec-v4l2-fwht.c
+++ b/drivers/media/platform/vicodec/codec-v4l2-fwht.c
@@ -11,27 +11,28 @@
 #include "codec-v4l2-fwht.h"
 
 static const struct v4l2_fwht_pixfmt_info v4l2_fwht_pixfmts[] = {
-   { V4L2_PIX_FMT_YUV420,  1, 3, 2, 1, 1, 2, 2 },
-   { V4L2_PIX_FMT_YVU420,  1, 3, 2, 1, 1, 2, 2 },
-   { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1 },
-   { V4L2_PIX_FMT_NV12,1, 3, 2, 1, 2, 2, 2 },
-   { V4L2_PIX_FMT_NV21,1, 3, 2, 1, 2, 2, 2 },
-   { V4L2_PIX_FMT_NV16,1, 2, 1, 1, 2, 2, 1 },
-   { V4L2_PIX_FMT_NV61,1, 2, 1, 1, 2, 2, 1 },
-   { V4L2_PIX_FMT_NV24,1, 3, 1, 1, 2, 1, 1 },
-   { V4L2_PIX_FMT_NV42,1, 3, 1, 1, 2, 1, 1 },
-   { V4L2_PIX_FMT_YUYV,2, 2, 1, 2, 4, 2, 1 },
-   { V4L2_PIX_FMT_YVYU,2, 2, 1, 2, 4, 2, 1 },
-   { V4L2_PIX_FMT_UYVY,2, 2, 1, 2, 4, 2, 1 },
-   { V4L2_PIX_FMT_VYUY,2, 2, 1, 2, 4, 2, 1 },
-   { V4L2_PIX_FMT_BGR24,   3, 3, 1, 3, 3, 1, 1 },
-   { V4L2_PIX_FMT_RGB24,   3, 3, 1, 3, 3, 1, 1 },
-   { V4L2_PIX_FMT_HSV24,   3, 3, 1, 3, 3, 1, 1 },
-   { V4L2_PIX_FMT_BGR32,   4, 4, 1, 4, 4, 1, 1 },
-   { V4L2_PIX_FMT_XBGR32,  4, 4, 1, 4, 4, 1, 1 },
-   { V4L2_PIX_FMT_RGB32,   4, 4, 1, 4, 4, 1, 1 },
-   { V4L2_PIX_FMT_XRGB32,  4, 4, 1, 4, 4, 1, 1 },
-   { V4L2_PIX_FMT_HSV32,   4, 4, 1, 4, 4, 1, 1 },
+   { V4L2_PIX_FMT_YUV420,  1, 3, 2, 1, 1, 2, 2, 3},
+   { V4L2_PIX_FMT_YVU420,  1, 3, 2, 1, 1, 2, 2, 3},
+   { V4L2_PIX_FMT_YUV422P, 1, 2, 1, 1, 1, 2, 1, 3},
+   { V4L2_PIX_FMT_NV12,1, 3, 2, 1, 2, 2, 2, 3},
+   { V4L2_PIX_FMT_NV21,1, 3, 2, 1, 2, 2, 2, 3},
+   { V4L2_PIX_FMT_NV16,1, 2, 1, 1, 2, 2, 1, 3},
+   { V4L2_PIX_FMT_NV61,1, 2, 1, 1, 2, 2, 1, 3},
+   { V4L2_PIX_FMT_NV24,1, 3, 1, 1, 2, 1, 1, 3},
+   { V4L2_PIX_FMT_NV42,1, 3, 1, 1, 2, 1, 1, 3},
+   { V4L2_PIX_FMT_YUYV,2, 2, 1, 2, 4, 2, 1, 3},
+   { V4L2_PIX_FMT_YVYU,2, 2, 1, 2, 4, 2, 1, 3},
+   { V4L2_PIX_FMT_UYVY,2, 2, 1, 2, 4, 2, 1, 3},
+   { V4L2_PIX_FMT_VYUY,2, 2, 1, 2, 4, 2, 1, 3},
+   { V4L2_PIX_FMT_BGR24,   3, 3, 1, 3, 3, 1, 1, 3},
+   { V4L2_PIX_FMT_RGB24,   3, 3, 1, 3, 3, 1, 1, 3},
+   { V4L2_PIX_FMT_HSV24,   3, 3, 1, 3, 3, 1, 1, 3},
+   { V4L2_PIX_FMT_BGR32,   4, 4, 1, 4, 4, 1, 1, 3},
+   { V4L2_PIX_FMT_XBGR32,  4, 4, 1, 4, 4, 1, 1, 3},
+   { V4L2_PIX_FMT_RGB32,   4, 4, 1, 4, 4, 1, 1, 3},
+   { V4L2_PIX_FMT_XRGB32,  4, 4, 1, 4, 4, 1, 1, 3},
+   { V4L2_PIX_FMT_HSV32,   4, 4, 1, 4, 4, 1, 1, 3},
+
 };
 
 const struct v4l2_fwht_pixfmt_info *v4l2_fwht_find_pixfmt(u32 pixelformat)
@@ -68,8 +69,10 @@ int v4l2_fwht_encode(struct v4l2_fwht_state *state, u8 
*p_in, u8 *p_out)
rf.luma = p_in;
rf.width_div = info->width_div;
rf.height_div =

[PATCH vicodec v4 0/3] Add support to more pixel formats in vicodec

2018-11-15 Thread Dafna Hirschfeld 
The new supported formats are
V4L2_PIX_FMT_GREY, V4L2_PIX_FMT_ARGB32, V4L2_PIX_FMT_ABGR32.

The returned encoded format is chaned to support various numbers
of planes instead of assuming 3 planes.

The first patch adds new fields to structs.
The second patch adds support for V4L2_PIX_FMT_GREY.
The third patch adds support for V4L2_PIX_FMT_ARGB32, V4L2_PIX_FMT_ABGR32.

The code used to test this patch is https://github.com/kamomil/outreachy
The script I used to test greyscale support:
https://github.com/kamomil/outreachy/blob/master/greyscale-full-example.sh 
The script I used to test argb/abgr:
https://github.com/kamomil/outreachy/blob/master/argb-and-abgr-full-example.sh

Changes from v3:

patch 1,3: - no change

patch 2:
- replace the 2-bit flag FWHT_FL_COMPONENTS_NUM_BIT[01] with GENMASK
- add TODO comment - handle the case where the encoded stream is different 
format
than the decoded
- allocate maximal space for the V4L2_PIX_FMT_FWHT format

with the test 'flags & FWHT_FL_COMPONENTS_NUM_BIT[01]'

Dafna Hirschfeld (3):
  media: vicodec: prepare support for various number of planes
  media: vicodec: Add support of greyscale format
  media: vicodec: Add support for 4 planes formats

 drivers/media/platform/vicodec/codec-fwht.c   |  73 +++
 drivers/media/platform/vicodec/codec-fwht.h   |  15 ++-
 .../media/platform/vicodec/codec-v4l2-fwht.c  | 123 +-
 .../media/platform/vicodec/codec-v4l2-fwht.h  |   3 +-
 drivers/media/platform/vicodec/vicodec-core.c |  35 -
 5 files changed, 182 insertions(+), 67 deletions(-)

-- 
2.17.1

Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI

2018-11-15 Thread Hans Verkuil 
On 11/07/18 00:27, Mani, Rajmohan wrote:
> Hi Mauro,
> 
> Thanks for the reviews.
> 
>> Subject: Re: [PATCH v7 03/16] v4l: Add Intel IPU3 meta data uAPI
>>
>> Hi Mauro,
>>
>> On Fri, Nov 2, 2018 at 10:49 PM Mauro Carvalho Chehab
>>  wrote:
>>>
>>> Em Mon, 29 Oct 2018 15:22:57 -0700
>>> Yong Zhi  escreveu:
>> [snip]
 +struct ipu3_uapi_awb_config_s {
 + __u16 rgbs_thr_gr;
 + __u16 rgbs_thr_r;
 + __u16 rgbs_thr_gb;
 + __u16 rgbs_thr_b;
 + struct ipu3_uapi_grid_config grid; }
 +__attribute__((aligned(32))) __packed;
>>>
>>> Hmm... Kernel defines a macro for aligned attribute:
>>>
>>> include/linux/compiler_types.h:#define __aligned(x)
>> __attribute__((aligned(x)))
>>>
>>
>> First, thanks for review!
>>
>> Maybe I missed something, but last time I checked, it wasn't accessible from
>> UAPI headers in userspace.
> 
> Ack. We see that's still the case.
> 
>>
>>> I'm not a gcc expert, but it sounds weird to first ask it to align
>>> with 32 bits and then have __packed (with means that pads should be
>>> removed).
>>>
>>> In other words, I *guess* is it should either be __packed or
>>> __aligned(32).
>>>
>>> Not that it would do any difference, in practice, as this specific
>>> struct has a size with is multiple of 32 bits, but let's do the right
>>> annotation here, not mixing two incompatible alignment requirements.
>>>
>>
>> My understanding was that __packed makes the compiler not insert any
>> alignment between particular fields of the struct, while __aligned makes the
>> whole struct be aligned at given boundary, if placed in another struct. If I
>> didn't miss anything, having both should make perfect sense here.
> 
> Ack
> 
> I also recall that as part of addressing review comments  (from Hans and 
> Sakari),
> on earlier versions of this patch series, we added __packed attribute to all 
> structs
> to ensure the size of the structs remains the same between 32 and 64 bit 
> builds.
> 
> The addition of structure members of the name padding[x] in some of the 
> structs
> ensures that respective members are aligned at 32 byte boundaries, while the
> overall size of the structs remain the same between 32 and 64 bit builds.

I recommend that this is documented in the header. It's not a common 
construction
so an explanation will help.

Regards,

Hans

> 
> Thanks
> Raj
> 
>>
>> Best regards,
>> Tomasz

Re: [PATCH v3] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers

2018-11-15 Thread Hans Verkuil 
On 11/14/18 16:04, 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 
> Acked-by: Sakari Ailus 
> ---
> Changes since v2:
>  - Added documentation for V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS
> ---
>  .../media/uapi/v4l/vidioc-reqbufs.rst | 15 ---
>  .../media/common/videobuf2/videobuf2-core.c   | 26 +--
>  .../media/common/videobuf2/videobuf2-v4l2.c   |  2 +-
>  include/uapi/linux/videodev2.h|  1 +
>  4 files changed, 15 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst 
> b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> index d40c60e8..d53006b938ac 100644
> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
> @@ -59,9 +59,12 @@ When the I/O method is not supported the ioctl returns an 
> ``EINVAL`` error
>  code.
>  
>  Applications can call :ref:`VIDIOC_REQBUFS` again to change the number of
> -buffers, however this cannot succeed when any buffers are still mapped.
> -A ``count`` value of zero frees all buffers, after aborting or finishing
> -any DMA in progress, an implicit
> +buffers. Note that if any buffers are still mapped or exported via DMABUF,
> +this can only succeed if the ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS`` flag
> +is set. In that case these buffers are orphaned and will be freed when they
> +are unmapped or when the exported DMABUF fds are closed.

I'd rephrase this:

Note that if any buffers are still mapped or exported via DMABUF, then
:ref:`VIDIOC_REQBUFS` can only succeed if the 
``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS``
capability is set. Otherwise :ref:`VIDIOC_REQBUFS` will return the ``EBUSY`` 
error code.

If ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS`` is set, then these buffers are 
orphaned
and will be freed when they are unmapped or when the exported DMABUF fds are 
closed.

> +A ``count`` value of zero frees or orphans all buffers, after aborting or
> +finishing any DMA in progress, an implicit
>  :ref:`VIDIOC_STREAMOFF `.
>  
>  
> @@ -132,6 +135,12 @@ any DMA in progress, an implicit
>  * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
>- 0x0008
>- This buffer type supports :ref:`requests `.
> +* - ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS``
> +  - 0x0010
> +  - The kernel allows calling :ref:`VIDIOC_REQBUFS` with a ``count`` 
> value
> +of zero while buffers are still mapped or exported via DMABUF. These

Not quite true. This isn't related to the count value, so just drop the
'with a ``count`` value of zero' part of the sentence.

Regards,

Hans

> +orphaned buffers will be freed when they are unmapped or when the
> +exported DMABUF fds are closed.
>  
>  Return Value
>  
> 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

Re: [PATCH v3] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers

2018-11-15 Thread Hans Verkuil 
On 11/14/18 20:59, Laurent Pinchart wrote:
> Hi Philipp,
> 
> Thank you for the patch.
> 
> On Wednesday, 14 November 2018 17:04:49 EET 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.
> 
> While I agree that we should remove this restriction, it has helped me in the 
> past to find missing munmap() in userspace. This patch thus has the potential 
> of causing memory leaks in userspace. Is there a way we could assist 
> application developers with this ?

Should we just keep the debug message? (rephrased of course)

That way you can enable debugging and see that this happens.

It sounds reasonable to me.

Regards,

Hans

> 
>> 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 
>> Acked-by: Sakari Ailus 
>> ---
>> Changes since v2:
>>  - Added documentation for V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS
>> ---
>>  .../media/uapi/v4l/vidioc-reqbufs.rst | 15 ---
>>  .../media/common/videobuf2/videobuf2-core.c   | 26 +--
>>  .../media/common/videobuf2/videobuf2-v4l2.c   |  2 +-
>>  include/uapi/linux/videodev2.h|  1 +
>>  4 files changed, 15 insertions(+), 29 deletions(-)
>>
>> diff --git a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst index
>> d40c60e8..d53006b938ac 100644
>> --- a/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> +++ b/Documentation/media/uapi/v4l/vidioc-reqbufs.rst
>> @@ -59,9 +59,12 @@ When the I/O method is not supported the ioctl returns an
>> ``EINVAL`` error code.
>>
>>  Applications can call :ref:`VIDIOC_REQBUFS` again to change the number of
>> -buffers, however this cannot succeed when any buffers are still mapped.
>> -A ``count`` value of zero frees all buffers, after aborting or finishing
>> -any DMA in progress, an implicit
>> +buffers. Note that if any buffers are still mapped or exported via DMABUF,
>> +this can only succeed if the ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS`` flag
>> +is set. In that case these buffers are orphaned and will be freed when they
>> +are unmapped or when the exported DMABUF fds are closed.
>> +A ``count`` value of zero frees or orphans all buffers, after aborting or
>> +finishing any DMA in progress, an implicit
>>
>>  :ref:`VIDIOC_STREAMOFF `.
>>
>> @@ -132,6 +135,12 @@ any DMA in progress, an implicit
>>  * - ``V4L2_BUF_CAP_SUPPORTS_REQUESTS``
>>- 0x0008
>>- This buffer type supports :ref:`requests `.
>> +* - ``V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS``
>> +  - 0x0010
>> +  - The kernel allows calling :ref:`VIDIOC_REQBUFS` with a ``count``
>> value +of zero while buffers are still mapped or exported via
>> DMABUF. These +orphaned buffers will be freed when they are
>> unmapped or when the +exported DMABUF fds are closed.
>>
>>  Return Value
>>  
>> 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 -

Re: [PATCH v4l-utils] v4l2-compliance: test orphaned buffer support

2018-11-15 Thread Hans Verkuil 
On 11/14/18 15:38, Philipp Zabel wrote:
> Test that V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS is reported equally for
> both MMAP and DMABUF memory types. If supported, try to orphan buffers
> by calling reqbufs(0) before unmapping or closing DMABUF fds.
> 
> Also close exported DMABUF fds and free buffers in testDmaBuf if
> orphaned buffers are not supported.
> 
> Signed-off-by: Philipp Zabel 
> ---
>  contrib/freebsd/include/linux/videodev2.h   |  1 +
>  include/linux/videodev2.h   |  1 +
>  utils/common/v4l2-info.cpp  |  1 +
>  utils/v4l2-compliance/v4l2-compliance.h |  1 +
>  utils/v4l2-compliance/v4l2-test-buffers.cpp | 35 +
>  5 files changed, 33 insertions(+), 6 deletions(-)
> 
> diff --git a/contrib/freebsd/include/linux/videodev2.h 
> b/contrib/freebsd/include/linux/videodev2.h
> index 9928c00e4b68..33153b53c175 100644
> --- a/contrib/freebsd/include/linux/videodev2.h
> +++ b/contrib/freebsd/include/linux/videodev2.h
> @@ -907,6 +907,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)
>  
>  /**
>   * struct v4l2_plane - plane info for multi-planar buffers
> diff --git a/include/linux/videodev2.h b/include/linux/videodev2.h
> index 79418cd39480..a39300cacb6a 100644
> --- a/include/linux/videodev2.h
> +++ b/include/linux/videodev2.h
> @@ -873,6 +873,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)
>  
>  /**
>   * struct v4l2_plane - plane info for multi-planar buffers
> diff --git a/utils/common/v4l2-info.cpp b/utils/common/v4l2-info.cpp
> index 258e5446f030..3699c35cb9d6 100644
> --- a/utils/common/v4l2-info.cpp
> +++ b/utils/common/v4l2-info.cpp
> @@ -200,6 +200,7 @@ static const flag_def bufcap_def[] = {
>   { V4L2_BUF_CAP_SUPPORTS_USERPTR, "userptr" },
>   { V4L2_BUF_CAP_SUPPORTS_DMABUF, "dmabuf" },
>   { V4L2_BUF_CAP_SUPPORTS_REQUESTS, "requests" },
> + { V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS, "orphaned-bufs" },
>   { 0, NULL }
>  };
>  
> diff --git a/utils/v4l2-compliance/v4l2-compliance.h 
> b/utils/v4l2-compliance/v4l2-compliance.h
> index def185f17261..88ec260a9bcc 100644
> --- a/utils/v4l2-compliance/v4l2-compliance.h
> +++ b/utils/v4l2-compliance/v4l2-compliance.h
> @@ -119,6 +119,7 @@ struct base_node {
>   __u32 valid_buftypes;
>   __u32 valid_buftype;
>   __u32 valid_memorytype;
> + bool has_orphaned_bufs;

I'd rename that to supports_orphaned_bufs.

>  };
>  
>  struct node : public base_node, public cv4l_fd {
> diff --git a/utils/v4l2-compliance/v4l2-test-buffers.cpp 
> b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> index c59a56d9ced7..6174015cb4e7 100644
> --- a/utils/v4l2-compliance/v4l2-test-buffers.cpp
> +++ b/utils/v4l2-compliance/v4l2-test-buffers.cpp
> @@ -400,8 +400,11 @@ int testReqBufs(struct node *node)
>   mmap_valid = !ret;
>   if (mmap_valid)
>   caps = q.g_capabilities();
> - if (caps)
> + if (caps) {
>   fail_on_test(mmap_valid ^ !!(caps & 
> V4L2_BUF_CAP_SUPPORTS_MMAP));
> + if (caps & V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS)
> + node->has_orphaned_bufs = true;
> + }
>  
>   q.init(i, V4L2_MEMORY_USERPTR);
>   ret = q.reqbufs(node, 0);
> @@ -418,8 +421,11 @@ int testReqBufs(struct node *node)
>   fail_on_test(!mmap_valid && dmabuf_valid);
>   // Note: dmabuf is only supported with vb2, so we can assume a
>   // non-0 caps value if dmabuf is supported.
> - if (caps || dmabuf_valid)
> + if (caps || dmabuf_valid) {
>   fail_on_test(dmabuf_valid ^ !!(caps & 
> V4L2_BUF_CAP_SUPPORTS_DMABUF));
> + if (node->has_orphaned_bufs)
> + fail_on_test(userptr_valid ^ !!(caps & 
> V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS));

Huh? I'm not sure what you are testing here.

> + }
>  
>   fail_on_test((can_stream && !is_overlay) && !mmap_valid && 
> !userptr_valid && !dmabuf_valid);
>   fail_on_test((!can_stream || is_overlay) && (mmap_valid || 
> userptr_valid || dmabuf_valid));
> @@ -967,12 +973,22 @@ int testMmap(struct node *node, unsigned frame_count)

The setupM2M function should check if m2m_q also sets the 
V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS.
I.e. this capability for m2m_q must match node->has_orphaned_bufs.

It makes no sense if it is set for the capture queue but not the output queue
for m2m devices. And since this has to be set manually in the drivers (at least
for now),

Re: [PATCH] cedrus: add action item to the TODO

2018-11-15 Thread Paul Kocialkowski 
Hi,

On Thu, 2018-11-15 at 08:49 +0100, Hans Verkuil wrote:
> Mention that the request validation should increment the memory refcount
> of reference buffers so we don't forget to do this.

Thanks for adding this item, we should definitely take care of it before
unstaging.

Acked-by: Paul Kocialkowski 

> Signed-off-by: Hans Verkuil 
> ---
> diff --git a/drivers/staging/media/sunxi/cedrus/TODO 
> b/drivers/staging/media/sunxi/cedrus/TODO
> index ec277ece47af..a951b3fd1ea1 100644
> --- a/drivers/staging/media/sunxi/cedrus/TODO
> +++ b/drivers/staging/media/sunxi/cedrus/TODO
> @@ -5,3 +5,8 @@ Before this stateless decoder driver can leave the staging 
> area:
>  * Userspace support for the Request API needs to be reviewed;
>  * Another stateless decoder driver should be submitted;
>  * At least one stateless encoder driver should be submitted.
> +* When queueing a request containing references to I frames, the
> +  refcount of the memory for those I frames needs to be incremented
> +  and decremented when the request is completed. This will likely
> +  require some help from vb2. The driver should fail the request
> +  if the memory/buffer is gone.
-- 
Paul Kocialkowski, Bootlin (formerly Free Electrons)
Embedded Linux and kernel engineering
https://bootlin.com


signature.asc
Description: This is a digitally signed message part

vim2m/vicodec: set device_caps in video_device struct

2018-11-15 Thread Hans Verkuil 
Instead of setting device_caps/capabilities in the querycap ioctl, set
it in struct video_device instead.

Signed-off-by: Hans Verkuil 
---
diff --git a/drivers/media/platform/vicodec/vicodec-core.c 
b/drivers/media/platform/vicodec/vicodec-core.c
index 72245183b077..35703c251d1b 100644
--- a/drivers/media/platform/vicodec/vicodec-core.c
+++ b/drivers/media/platform/vicodec/vicodec-core.c
@@ -387,11 +387,6 @@ static int vidioc_querycap(struct file *file, void *priv,
strncpy(cap->card, VICODEC_NAME, sizeof(cap->card) - 1);
snprintf(cap->bus_info, sizeof(cap->bus_info),
"platform:%s", VICODEC_NAME);
-   cap->device_caps =  V4L2_CAP_STREAMING |
-   (multiplanar ?
-V4L2_CAP_VIDEO_M2M_MPLANE :
-V4L2_CAP_VIDEO_M2M);
-   cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
return 0;
 }

@@ -1303,6 +1298,8 @@ static int vicodec_probe(struct platform_device *pdev)
vfd->lock = >enc_mutex;
vfd->v4l2_dev = >v4l2_dev;
strscpy(vfd->name, "vicodec-enc", sizeof(vfd->name));
+   vfd->device_caps = V4L2_CAP_STREAMING |
+   (multiplanar ? V4L2_CAP_VIDEO_M2M_MPLANE : V4L2_CAP_VIDEO_M2M);
v4l2_disable_ioctl(vfd, VIDIOC_DECODER_CMD);
v4l2_disable_ioctl(vfd, VIDIOC_TRY_DECODER_CMD);
video_set_drvdata(vfd, dev);
diff --git a/drivers/media/platform/vim2m.c b/drivers/media/platform/vim2m.c
index 9d1222f489b8..644f81568351 100644
--- a/drivers/media/platform/vim2m.c
+++ b/drivers/media/platform/vim2m.c
@@ -428,8 +428,6 @@ static int vidioc_querycap(struct file *file, void *priv,
strncpy(cap->card, MEM2MEM_NAME, sizeof(cap->card) - 1);
snprintf(cap->bus_info, sizeof(cap->bus_info),
"platform:%s", MEM2MEM_NAME);
-   cap->device_caps = V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING;
-   cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS;
return 0;
 }

@@ -991,6 +989,7 @@ static const struct video_device vim2m_videodev = {
.ioctl_ops  = _ioctl_ops,
.minor  = -1,
.release= video_device_release_empty,
+   .device_caps= V4L2_CAP_VIDEO_M2M | V4L2_CAP_STREAMING,
 };

 static const struct v4l2_m2m_ops m2m_ops = {

Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers

2018-11-15 Thread Hans Verkuil 
On 11/15/2018 08:49 AM, Laurent Pinchart wrote:
> Hi Hans,
> 
> 
> On Thursday, 15 November 2018 09:30:55 EET Hans Verkuil wrote:
>> On 11/14/2018 08:52 PM, Laurent Pinchart wrote:
>>> On Tuesday, 13 November 2018 17:43:48 EET Hans Verkuil wrote:
 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 :-)
>>>
>>> Do you have plans to refactor and document the v4l2-compliance internals
>>> to make it easier ?
>>
>> Yes. I hope to be able to set aside one or two days for that in the next two
>> weeks.
> 
> That would be great ! Let me know if you would like to discuss how the code 
> base could be refactored.
> 

I'm happy to hear your ideas, but I only have one day for refactoring, so it is
very limited what can be done.

Frankly, it is mainly v4l2-test-buffers.cpp that I would like to split up in
two parts: one for the tests that do not require actual streaming and one part
for all the streaming tests.

Documentation (esp. cv4l-helpers.h which is used a lot) is much more important
and I wanted to spend the second day on that.

Regards,

Hans