[PATCH] cedrus: add action item to the TODO
Mention that the request validation should increment the memory refcount of reference buffers so we don't forget to do this. 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.
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
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. -- Regards, Laurent Pinchart
Re: [PATCH v3] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
On 11/14/2018 08:59 PM, 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 ?
>
>> 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 - * 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
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
On 11/14/2018 08:52 PM, Laurent Pinchart wrote: > Hi Hans, > > 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. Regards, Hans
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: Thu Nov 15 05:00:18 CET 2018 media-tree git hash:fbe57dde7126d1b2712ab5ea93fb9d15f89de708 media_build git hash: a8aef9cea0a4a2f3ea86c0b37bd6a1378018c0c1 v4l-utils git hash: ccfb7ede73ed073d8e47541793129187a99bfe12 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: OK 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/Thursday.log Full logs are available here: http://www.xs4all.nl/~hverkuil/logs/Thursday.tar.bz2 The Media Infrastructure API from this daily build is here: http://www.xs4all.nl/~hverkuil/spec/index.html
Re: [PATCH v3] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
On Thu, Nov 15, 2018 at 4:59 AM 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 ? > I'm not very convinced that it's something we need to have, but possibly one could have it as a settable capability, that's given to REQBUF/CREATE_BUFS at allocation (count > 0) time? > > 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) } > >
Re: TechnoTrend CT2-4500 remote not working
Martin, On Wed, Nov 14, 2018 at 09:51:38PM +0100, martin.kono...@mknetz.de wrote: > > It would be interesting to see what the device is sending. Please can you > > turn > > on dynamic debug for ir-kbd-i2c.c: > > > > echo "file ir-kbd-i2.c +p" > /sys/kernel/debug/dynamic_debug/control > > > > Try the remote again and report what in the kernel messages. > > > > Sean > > > > 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. 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). What's the output of the lspci -vvv? Maybe it's reported via gpio and not i2c. Thanks Sean
Re: TechnoTrend CT2-4500 remote not working
It would be interesting to see what the device is sending. Please can you turn on dynamic debug for ir-kbd-i2c.c: echo "file ir-kbd-i2.c +p" > /sys/kernel/debug/dynamic_debug/control Try the remote again and report what in the kernel messages. Sean 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. Martin
Re: [PATCH v3] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
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 ?
> 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 - * 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;
> -
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Hi Hans,
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 ?
> 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
Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate
Hi Maxime,
many thanks for re-sending this updated version
On Tue, Nov 13, 2018 at 02:03:15PM +0100, Maxime Ripard wrote:
> 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
As you've squashed in my MIPI CSI-2 fixes, do you think it is
appropriate adding my So-b tag here?
> ---
> 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,
>
[PATCH] media: v4l: constify v4l2_ioctls[]
From: Eric Biggers
v4l2_ioctls[] is never modified, so mark it 'const'.
This way it gets placed in .rodata.
Signed-off-by: Eric Biggers
---
drivers/media/v4l2-core/v4l2-ioctl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
b/drivers/media/v4l2-core/v4l2-ioctl.c
index c63746968fa3d..fc23455820336 100644
--- a/drivers/media/v4l2-core/v4l2-ioctl.c
+++ b/drivers/media/v4l2-core/v4l2-ioctl.c
@@ -2586,7 +2586,7 @@ DEFINE_V4L_STUB_FUNC(enum_dv_timings)
DEFINE_V4L_STUB_FUNC(query_dv_timings)
DEFINE_V4L_STUB_FUNC(dv_timings_cap)
-static struct v4l2_ioctl_info v4l2_ioctls[] = {
+static const struct v4l2_ioctl_info v4l2_ioctls[] = {
IOCTL_INFO(VIDIOC_QUERYCAP, v4l_querycap, v4l_print_querycap, 0),
IOCTL_INFO(VIDIOC_ENUM_FMT, v4l_enum_fmt, v4l_print_fmtdesc,
INFO_FL_CLEAR(v4l2_fmtdesc, type)),
IOCTL_INFO(VIDIOC_G_FMT, v4l_g_fmt, v4l_print_format, 0),
--
2.19.1.930.g4563a0d9d0-goog
Re: cec kernel oops with pulse8 usb cec adapter
On 2018-11-14 14:23, Hans Verkuil wrote:
On 11/14/18 14:00, Torbjorn Jansson wrote:
since there now is a patch that appears to be working and fixing this problem
i'd like to ask for some troubleshooting advice with another cec issue i have
that i haven't figured out why it is happening and exactly whats causing it.
i'm not sure if it is a hardware issue or a software issue or both.
this is what is happening:
i have a script that polls the tv for current status by running:
cec-ctl --to=0 --give-device-power-status
after a fresh reboot this works fine for a while then sometime later it stops
working and errors with:
-
# cec-ctl --to=0 --give-device-power-status
Driver Info:
Driver Name: pulse8-cec
Adapter Name : serio0
Capabilities : 0x003f
Physical Address
Logical Addresses
Transmit
Passthrough
Remote Control Support
Monitor All
Driver version : 4.18.16
Available Logical Addresses: 1
Physical Address : 1.4.0.0
Logical Address Mask : 0x0800
CEC Version: 2.0
Vendor ID : 0x000c03
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 11 (Playback Device 3)
Primary Device Type: Playback
Logical Address Type : Playback
All Device Types : Playback
RC TV Profile : None
Device Features:
None
Transmit from Playback Device 3 to TV (11 to 0):
CEC_MSG_GIVE_DEVICE_POWER_STATUS (0x8f)
Sequence: 119437 Tx Timestamp: 865389.535s
Tx, Error (1), Max Retries
-
once this happens i can never get back any status and also running:
cec-ctl -M
gives me a lot of:
Transmitted by Playback Device 3 to TV (11 to 0):
CEC_MSG_GIVE_DEVICE_POWER_STATUS (0x8f)
Tx, Error (1), Max Retries
once for every run of my status checking script.
i know polling like this is not the best option and a better way would be to
listen for events and then take action when status changes but that's not
easily doable with what i need it for.
anyway, once i start getting the above errors when i poll it will not give me
back any good status any more (everything errors out)
also sending commands to tv to turn on or off like:
cec-ctl --to=0 --image-view-on
or
cec-ctl --to=0 --standby
doesn't work.
BUT if i remove power to tv and wait for standby led to go out completely then
power it back on i can use above two commands to turn on/off the tv even when
they return errors and i still can't poll for current status.
so even with the errors always returned at this stage the on/off commands still
gets sent and works.
do you think this behavior is a sw or hw issue or both?
Does 'cec-ctl --to=0 --give-device-power-status' eventually fail even if the TV
is
on all the time? Or does it only fail if the TV goes to standby or has been in
standby for a very long time?
good question, tv will go to standby by itself after some time and i can't have
the tv turned on for a few days in a row (not practical).
my gut feeling is that it happens after being in standby for a long time.
it may work fine for a day or two then it fails.
This error ('Tx, Error (1), Max Retries') indicates that the pulse-eight returns
transmit errors suggesting that the CEC line is locked (always high or low).
I did some more testing and i was able to somehow trigger the error state by
having one cec-ctl -M and one cec-ctl -S going at the same time but this is not
consistent (tested several times and was only able to get it to fail like this
once)
Also i tested to pull hdmi cable from tv when in error state and see if i could
reach other devices, tested with cec-ctl -S and it did display a some of my
other hdmi devices like amp & bd player.
So i guess my tv is to blame for locking up cec.
Maybe i'll just schedule something to cut power completely to tv during night
for a few hours.
if i'm not mistaken i could unplug usb cable to pulse8 cec adapter and reinsert
to make it work properly again (no errors and correct response like a fresh
start)
but i'm not 100% sure of this.
when i tried it now i get a new kernel oops:
-
[866129.392139] usb 7-2: USB disconnect, device number 3
[866129.409568] cdc_acm 7-2:1.1: acm_start_wb - usb_submit_urb(write bulk)
failed: -19
[866129.409576] cdc_acm 7-2:1.1: acm_start_wb - usb_submit_urb(write bulk)
failed: -19
[866129.409635] WARNING: CPU: 10 PID: 1571 at drivers/media/cec/cec-adap.c:1243
Not a kernel oops, just a warning. The pulse-eight driver has a small bug
that causes this warning, but it does not affect the CEC behavior in any way.
I'll post a patch.
Ok so nothing to worry about then.
Re: [PATCH v2 1/4] dt-bindings: media: sun6i: Add A31 and H3 compatibles
On Wed, Nov 14, 2018 at 10:59 PM Maxime Ripard wrote: > > The H3 has a slightly different CSI controller (no BT656, no CCI) which > looks a lot like the original A31 controller. Add a compatible for the A31, > and more specific compatible the for the H3 to be used in combination for > the A31. > > Reviewed-by: Rob Herring > Signed-off-by: Maxime Ripard Reviewed-by: Chen-Yu Tsai
Re: [PATCH v2 2/4] media: sun6i: Add A31 compatible
On Wed, Nov 14, 2018 at 10:59 PM Maxime Ripard wrote: > > The first device that used that IP was the A31. Add it to our list of > compatibles. > > Signed-off-by: Maxime Ripard Reviewed-by: Chen-Yu Tsai
Re: [PATCH v2 3/4] ARM: dts: sun8i: Add the H3/H5 CSI controller
Hi,
On Wed, Nov 14, 2018 at 10:59 PM Maxime Ripard
wrote:
>
> From: Mylène Josserand
>
> The H3 and H5 features the same CSI controller that was initially found on
> the A31.
>
> Add a DT node for it.
>
> Signed-off-by: Mylène Josserand
> Signed-off-by: Maxime Ripard
> ---
> arch/arm/boot/dts/sunxi-h3-h5.dtsi | 22 ++
> 1 file changed, 22 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> index 4b1530ebe427..8779ee750bd8 100644
> --- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> +++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
> @@ -393,6 +393,13 @@
> interrupt-controller;
> #interrupt-cells = <3>;
>
> + csi_pins: csi {
> + pins = "PE0", "PE1", "PE2", "PE3", "PE4",
You should separate out PE1, which provides the MCLK. Not all camera modules
need it. Some might have a standalone crystal to provide the reference clock.
Designs could then use that pin for other purposes.
Otherwise,
Reviewed-by: Chen-Yu Tsai
> + "PE5", "PE6", "PE7", "PE8", "PE9",
> + "PE10", "PE11";
> + function = "csi";
> + };
> +
> emac_rgmii_pins: emac0 {
> pins = "PD0", "PD1", "PD2", "PD3", "PD4",
>"PD5", "PD7", "PD8", "PD9", "PD10",
> @@ -744,6 +751,21 @@
> interrupts = IRQ_TYPE_LEVEL_HIGH)>;
> };
>
> + csi: camera@1cb {
> + compatible = "allwinner,sun8i-h3-csi",
> +"allwinner,sun6i-a31-csi";
> + reg = <0x01cb 0x1000>;
> + interrupts = ;
> + clocks = < CLK_BUS_CSI>,
> +< CLK_CSI_SCLK>,
> +< CLK_DRAM_CSI>;
> + clock-names = "bus", "mod", "ram";
> + resets = < RST_BUS_CSI>;
> + pinctrl-names = "default";
> + pinctrl-0 = <_pins>;
> + status = "disabled";
> + };
> +
> hdmi: hdmi@1ee {
> compatible = "allwinner,sun8i-h3-dw-hdmi",
> "allwinner,sun8i-a83t-dw-hdmi";
> --
> 2.19.1
>
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
On 11/14/18 15:43, Philipp Zabel wrote:
> Hi Hans,
>
> On Tue, 2018-11-13 at 16:43 +0100, Hans Verkuil wrote:
>> 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.
>
> I have tested on v4.20-rc2 with 92539d3eda2c ("media: v4l: event: Add
> subscription to list before calling "add" operation") and this patch
> applied:
>
> $ modprobe vivid no_error_inj=1
> vivid-000: V4L2 capture device registered as video15
> vivid-000: V4L2 output device registered as video16
>
> $ v4l2-compliance -d 15 -s 1 --expbuf-device 16
> v4l2-compliance SHA: 98b4c9f276a18535b5691e5f350f59ffbf5a9aa5, 32 bits
> ...
> Total: 112, Succeeded: 112, Failed: 0, Warnings: 4
>
> The warnings are:
> warn: v4l2-test-formats.cpp(1426): doioctl(node,
> VIDIOC_CROPCAP, )
Will be fixed when https://patchwork.linuxtv.org/patch/52811/ is merged.
> test Cropping: OK
> (one per input) and:
> warn: v4l2-test-controls.cpp(845): V4L2_CID_DV_RX_POWER_PRESENT
> not found for input 3
Waiting for a volunteer to teach vivid to properly emulate an HDMI connector.
So this looks good.
Will look at the v4l2-compliance patch tomorrow.
Regards,
Hans
> test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
>
>> 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 :-)
>
> That's fair. The SHA above is actually a lie, I had one patch applied.
>
> regards
> Philipp
>
[GIT PULL FOR v4.21] Various fixes (coda, rcar, pxp, others)
The following changes since commit fbe57dde7126d1b2712ab5ea93fb9d15f89de708: media: ov7740: constify structures stored in fields of v4l2_subdev_ops structure (2018-11-06 07:17:02 -0500) are available in the Git repository at: git://linuxtv.org/hverkuil/media_tree.git tags/br-v4.21c for you to fetch changes up to fb248101254782a3939c40eaaa7e347260b27b77: pulse8-cec: return 0 when invalidating the logical address (2018-11-14 16:07:47 +0100) Tag branch Fabio Estevam (3): media: imx-pxp: Check the return value from clk_prepare_enable() media: imx-pxp: Check for pxp_soft_reset() error media: imx-pxp: Improve pxp_soft_reset() error message Hans Verkuil (3): vb2: vb2_mmap: move lock up cec-pin: fix broken tx_ignore_nack_until_eom error injection pulse8-cec: return 0 when invalidating the logical address Jacopo Mondi (6): media: dt-bindings: rcar-vin: Add R8A77990 support media: rcar-vin: Add support for R-Car R8A77990 media: dt-bindings: rcar-csi2: Add R8A77990 media: rcar-csi2: Add R8A77990 support media: rcar: rcar-csi2: Update V3M/E3 PHTW tables media: rcar-csi2: Handle per-SoC number of channels Lucas Stach (2): media: coda: limit queueing into internal bitstream buffer media: coda: don't disable IRQs across buffer meta handling Michael Tretter (1): media: coda: print SEQ_INIT error code as hex value Philipp Zabel (12): media: coda: fix memory corruption in case more than 32 instances are opened media: coda: store unmasked fifo position in meta media: coda: always hold back decoder jobs until we have enough bitstream payload media: coda: reduce minimum frame size to 48x16 pixels. media: coda: remove unused instances list media: coda: set V4L2_CAP_TIMEPERFRAME flag in coda_s_parm media: coda: implement ENUM_FRAMEINTERVALS media: coda: never set infinite timeperframe media: coda: fail S_SELECTION for read-only targets media: coda: improve queue busy error message media: coda: normalise debug output media: coda: debug output when setting visible size via crop selection Ricardo Ribalda Delgado (1): media: doc-rst: Fix broken references kbuild test robot (1): media: platform: fix platform_no_drv_owner.cocci warnings Documentation/devicetree/bindings/media/rcar_vin.txt | 1 + Documentation/devicetree/bindings/media/renesas,rcar-csi2.txt | 1 + Documentation/media/v4l-drivers/sh_mobile_ceu_camera.rst | 2 +- drivers/media/cec/cec-pin.c | 5 +- drivers/media/common/videobuf2/videobuf2-core.c | 11 ++- drivers/media/platform/coda/coda-bit.c| 113 +-- drivers/media/platform/coda/coda-common.c | 231 +- drivers/media/platform/coda/coda.h| 28 +- drivers/media/platform/coda/trace.h | 10 +- drivers/media/platform/imx-pxp.c | 17 +++- drivers/media/platform/rcar-vin/rcar-core.c | 20 drivers/media/platform/rcar-vin/rcar-csi2.c | 86 ++--- drivers/media/platform/sh_vou.c | 2 +- drivers/media/usb/pulse8-cec/pulse8-cec.c | 2 +- drivers/staging/media/sunxi/cedrus/cedrus.c | 1 - 15 files changed, 319 insertions(+), 211 deletions(-)
[PATCH v4l-utils] v4l2-compliance: limit acceptable width/height to 65536 in VIDIOC_SUBDEV_G/S_FMT test
Fail if the driver returns unrealistically large frame sizes.
Signed-off-by: Philipp Zabel
---
utils/v4l2-compliance/v4l2-test-subdevs.cpp | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
index 031fd6e78c56..29987b310448 100644
--- a/utils/v4l2-compliance/v4l2-test-subdevs.cpp
+++ b/utils/v4l2-compliance/v4l2-test-subdevs.cpp
@@ -308,8 +308,8 @@ int testSubDevFrameInterval(struct node *node, unsigned pad)
static int checkMBusFrameFmt(struct node *node, struct v4l2_mbus_framefmt )
{
fail_on_test(check_0(fmt.reserved, sizeof(fmt.reserved)));
- fail_on_test(fmt.width == 0 || fmt.width == ~0U);
- fail_on_test(fmt.height == 0 || fmt.height == ~0U);
+ fail_on_test(fmt.width == 0 || fmt.width > 65536);
+ fail_on_test(fmt.height == 0 || fmt.height > 65536);
fail_on_test(fmt.code == 0 || fmt.code == ~0U);
fail_on_test(fmt.field == ~0U);
if (!node->is_passthrough_subdev) {
--
2.19.1
[PATCH v3] 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
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
- * 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.
-
Re: [PATCH 07/15] media: coda: don't disable IRQs across buffer meta handling
On Mon, 2018-11-05 at 16:25 +0100, Philipp Zabel wrote: > From: Lucas Stach > > The CODA driver uses threaded IRQs only, so there is nothing happening > in hardirq context that could interfere with the buffer meta handling. > > Signed-off-by: Lucas Stach Signed-off-by: Philipp Zabel regards Philipp
[PATCH v2 4/4] [DO NOT MERGE] ARM: dts: sun8i: Add CAM500B camera module to the Nano Pi M1+
From: Mylène Josserand
The Nano Pi M1+ comes with an optional sensor based on the ov5640 from
Omnivision. Enable the support for it in the DT.
Signed-off-by: Mylène Josserand
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dts | 85 +++
1 file changed, 85 insertions(+)
diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dts
b/arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dts
index 06010a9afba0..2ac62d109285 100644
--- a/arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dts
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dts
@@ -52,6 +52,37 @@
ethernet1 = _wifi;
};
+ cam_xclk: cam-xclk {
+#clock-cells = <0>;
+compatible = "fixed-clock";
+clock-frequency = <2400>;
+clock-output-names = "cam-xclk";
+};
+
+reg_cam_avdd: cam-avdd {
+compatible = "regulator-fixed";
+regulator-name = "cam500b-avdd";
+regulator-min-microvolt = <280>;
+regulator-max-microvolt = <280>;
+vin-supply = <_vcc3v3>;
+};
+
+reg_cam_dovdd: cam-dovdd {
+compatible = "regulator-fixed";
+regulator-name = "cam500b-dovdd";
+regulator-min-microvolt = <180>;
+regulator-max-microvolt = <180>;
+vin-supply = <_vcc3v3>;
+};
+
+reg_cam_dvdd: cam-dvdd {
+compatible = "regulator-fixed";
+regulator-name = "cam500b-dvdd";
+regulator-min-microvolt = <150>;
+regulator-max-microvolt = <150>;
+vin-supply = <_vcc3v3>;
+};
+
reg_gmac_3v3: gmac-3v3 {
compatible = "regulator-fixed";
regulator-name = "gmac-3v3";
@@ -69,6 +100,26 @@
};
};
+ {
+status = "okay";
+
+port {
+#address-cells = <1>;
+#size-cells = <0>;
+
+/* Parallel bus endpoint */
+csi_from_ov5640: endpoint {
+remote-endpoint = <_to_csi>;
+bus-width = <8>;
+data-shift = <2>;
+hsync-active = <1>; /* Active high */
+vsync-active = <0>; /* Active low */
+data-active = <1>; /* Active high */
+pclk-sample = <1>; /* Rising */
+};
+};
+};
+
{
status = "okay";
};
@@ -94,6 +145,40 @@
};
};
+ {
+ status = "okay";
+
+ ov5640: camera@3c {
+compatible = "ovti,ov5640";
+reg = <0x3c>;
+clocks = <_xclk>;
+clock-names = "xclk";
+
+reset-gpios = < 4 14 GPIO_ACTIVE_LOW>;
+powerdown-gpios = < 4 15 GPIO_ACTIVE_HIGH>;
+AVDD-supply = <_cam_avdd>;
+DOVDD-supply = <_cam_dovdd>;
+DVDD-supply = <_cam_dvdd>;
+
+port {
+ov5640_to_csi: endpoint {
+remote-endpoint = <_from_ov5640>;
+bus-width = <8>;
+data-shift = <2>;
+hsync-active = <1>; /* Active high */
+vsync-active = <0>; /* Active low */
+data-active = <1>; /* Active high */
+pclk-sample = <1>; /* Rising */
+};
+};
+};
+
+};
+
+_pins {
+ bias-pull-up;
+};
+
{
pinctrl-names = "default";
pinctrl-0 = <_pins_a>;
--
2.19.1
[PATCH v2 3/4] ARM: dts: sun8i: Add the H3/H5 CSI controller
From: Mylène Josserand
The H3 and H5 features the same CSI controller that was initially found on
the A31.
Add a DT node for it.
Signed-off-by: Mylène Josserand
Signed-off-by: Maxime Ripard
---
arch/arm/boot/dts/sunxi-h3-h5.dtsi | 22 ++
1 file changed, 22 insertions(+)
diff --git a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
index 4b1530ebe427..8779ee750bd8 100644
--- a/arch/arm/boot/dts/sunxi-h3-h5.dtsi
+++ b/arch/arm/boot/dts/sunxi-h3-h5.dtsi
@@ -393,6 +393,13 @@
interrupt-controller;
#interrupt-cells = <3>;
+ csi_pins: csi {
+ pins = "PE0", "PE1", "PE2", "PE3", "PE4",
+ "PE5", "PE6", "PE7", "PE8", "PE9",
+ "PE10", "PE11";
+ function = "csi";
+ };
+
emac_rgmii_pins: emac0 {
pins = "PD0", "PD1", "PD2", "PD3", "PD4",
"PD5", "PD7", "PD8", "PD9", "PD10",
@@ -744,6 +751,21 @@
interrupts = ;
};
+ csi: camera@1cb {
+ compatible = "allwinner,sun8i-h3-csi",
+"allwinner,sun6i-a31-csi";
+ reg = <0x01cb 0x1000>;
+ interrupts = ;
+ clocks = < CLK_BUS_CSI>,
+< CLK_CSI_SCLK>,
+< CLK_DRAM_CSI>;
+ clock-names = "bus", "mod", "ram";
+ resets = < RST_BUS_CSI>;
+ pinctrl-names = "default";
+ pinctrl-0 = <_pins>;
+ status = "disabled";
+ };
+
hdmi: hdmi@1ee {
compatible = "allwinner,sun8i-h3-dw-hdmi",
"allwinner,sun8i-a83t-dw-hdmi";
--
2.19.1
[PATCH v2 0/4] media: sun6i: Add support for the H3 CSI controller
Hi, The H3 and H5 have a CSI controller based on the one previously found in the A31, that is currently supported by the sun6i-csi driver. Add the compatibles to the device tree bindings and to the driver to make it work properly. This obviously depends on the serie "Initial Allwinner V3s CSI Support" by Yong Deng. Let me know what you think, Maxime Changes from v1: - Rebased on top of latest Yong's series Maxime Ripard (2): dt-bindings: media: sun6i: Add A31 and H3 compatibles media: sun6i: Add A31 compatible Mylène Josserand (2): ARM: dts: sun8i: Add the H3/H5 CSI controller [DO NOT MERGE] ARM: dts: sun8i: Add CAM500B camera module to the Nano Pi M1+ .../devicetree/bindings/media/sun6i-csi.txt | 5 +- arch/arm/boot/dts/sun8i-h3-nanopi-m1-plus.dts | 85 +++ arch/arm/boot/dts/sunxi-h3-h5.dtsi| 22 + .../platform/sunxi/sun6i-csi/sun6i_csi.c | 1 + 4 files changed, 112 insertions(+), 1 deletion(-) -- 2.19.1
[PATCH v2 2/4] media: sun6i: Add A31 compatible
The first device that used that IP was the A31. Add it to our list of
compatibles.
Signed-off-by: Maxime Ripard
---
drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
index 7af55ad142dc..9813bca38939 100644
--- a/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
+++ b/drivers/media/platform/sunxi/sun6i-csi/sun6i_csi.c
@@ -892,6 +892,7 @@ static int sun6i_csi_remove(struct platform_device *pdev)
}
static const struct of_device_id sun6i_csi_of_match[] = {
+ { .compatible = "allwinner,sun6i-a31-csi", },
{ .compatible = "allwinner,sun8i-v3s-csi", },
{},
};
--
2.19.1
[PATCH v2 1/4] dt-bindings: media: sun6i: Add A31 and H3 compatibles
The H3 has a slightly different CSI controller (no BT656, no CCI) which looks a lot like the original A31 controller. Add a compatible for the A31, and more specific compatible the for the H3 to be used in combination for the A31. Reviewed-by: Rob Herring Signed-off-by: Maxime Ripard --- Documentation/devicetree/bindings/media/sun6i-csi.txt | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Documentation/devicetree/bindings/media/sun6i-csi.txt b/Documentation/devicetree/bindings/media/sun6i-csi.txt index 2ff47a9507a6..3f8eb0296382 100644 --- a/Documentation/devicetree/bindings/media/sun6i-csi.txt +++ b/Documentation/devicetree/bindings/media/sun6i-csi.txt @@ -5,7 +5,10 @@ Allwinner V3s SoC features two CSI module. CSI0 is used for MIPI CSI-2 interface and CSI1 is used for parallel interface. Required properties: - - compatible: value must be "allwinner,sun8i-v3s-csi" + - compatible: value must be one of: +* "allwinner,sun6i-a31-csi" +* "allwinner,sun8i-h3-csi", "allwinner,sun6i-a31-csi" +* "allwinner,sun8i-v3s-csi" - reg: base address and size of the memory-mapped region. - interrupts: interrupt associated to this IP - clocks: phandles to the clocks feeding the CSI -- 2.19.1
Re: [PATCH 04/15] media: coda: limit queueing into internal bitstream buffer
Hi, I forgot to add the proper SoB tag: On Mon, 2018-11-05 at 16:25 +0100, Philipp Zabel wrote: > From: Lucas Stach > > The ringbuffer used to hold the bitstream is very conservatively sized, > as keyframes can get very large and still need to fit into this buffer. > This means that the buffer is way oversized for the average stream to > the extend that it will hold a few hundred frames when the video data > is compressing well. > > The current strategy of queueing as much bitstream data as possible > leads to large delays when draining the decoder. In order to keep the > drain latency to a reasonable bound, try to only queue a full reorder > window of buffers. We can't always hit this low target for very well > compressible video data, as we might end up with less than the minimum > amount of data that needs to be available to the bitstream prefetcher, > so we must take this into account and allow more buffers to be queued > in this case. > > Signed-off-by: Lucas Stach Signed-off-by: Philipp Zabel regards Philipp
[PATCH v4l-utils] v4l2-compliance: test orphaned buffer support
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;
};
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));
+ }
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)
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()));
- q.munmap_bufs(node);
- fail_on_test(q.reqbufs(node, 0));
+ if (node->has_orphaned_bufs) {
+ fail_on_test(q.reqbufs(node, 0));
+ q.munmap_bufs(node);
+ } else {
+ q.munmap_bufs(node);
+ fail_on_test(q.reqbufs(node, 0));
+ }
if (node->is_m2m) {
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Hi Hans,
On Tue, 2018-11-13 at 16:43 +0100, Hans Verkuil wrote:
> 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.
I have tested on v4.20-rc2 with 92539d3eda2c ("media: v4l: event: Add
subscription to list before calling "add" operation") and this patch
applied:
$ modprobe vivid no_error_inj=1
vivid-000: V4L2 capture device registered as video15
vivid-000: V4L2 output device registered as video16
$ v4l2-compliance -d 15 -s 1 --expbuf-device 16
v4l2-compliance SHA: 98b4c9f276a18535b5691e5f350f59ffbf5a9aa5, 32 bits
...
Total: 112, Succeeded: 112, Failed: 0, Warnings: 4
The warnings are:
warn: v4l2-test-formats.cpp(1426): doioctl(node,
VIDIOC_CROPCAP, )
test Cropping: OK
(one per input) and:
warn: v4l2-test-controls.cpp(845): V4L2_CID_DV_RX_POWER_PRESENT
not found for input 3
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
> 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 :-)
That's fair. The SHA above is actually a lie, I had one patch applied.
regards
Philipp
Advertising enquiery to http://bootlin.com
Greetings! I’m preparing a massive promotion campaign for our website and as a part of preparations I came across your site. It looks professional, reliable and of a great standing. A great catch! I hope you offer paid promotion opportunities, because I’m greatly interested. What are the prices for all the kinds (guest post, homepage link or link placement in existing post) and terms (permanently, monthly) of advertising you offer? And in case there are some other online platforms for advertising at your disposal, please let me know. I will consider them with great pleasure. Thank you in advance!
Re: [PATCH v4 00/22] i.MX media mem2mem scaler
On Fri, Oct 19, 2018 at 7:16 AM Philipp Zabel wrote: > > Hi, > > this is the fourth version of the i.MX mem2mem scaler series. > > An alignment issue with 24-bit RGB formats has been corrected in the > seam position selection patch and a few new fixes by Steve have been > added. If there are no more issues, I'll pick up the ipu-v3 patches > via imx-drm/next. The first patch could be merged via the media tree > independently. I'd like to test this, but I am not sure how. Do you have instructions? (ideally using gstreamer?) adam > > Changes since v3: > - Fix tile_left_align for 24-bit RGB formats and reduce alignment >restrictions for U/V packed planar YUV formats > - Catch unaligned tile offsets in image-convert > - Add chroma plane offset overrides to ipu_cpmem_set_image() to >prevent a false positive warning in some cases > - Fix a race between run and unprepare and make abort reentrant. > > > Changes since v2: > - Rely on ipu_image_convert_adjust() in mem2mem_try_fmt() for format >adjustments. This makes the mem2mem driver mostly a V4L2 mem2mem API >wrapper around the IPU image converter, and independent of the >internal image converter implementation. > - Remove the source and destination buffers on error in device_run(). >Otherwise the conversion is re-attempted apparently over and over >again (with WARN() backtraces). > - Allow subscribing to control changes. > - Fix seam position selection for more corner cases: > - Switch width/height properly and align tile top left positions to 8x8 > IRT block size when rotating. > - Align input width to input burst length in case the scaling step > flips horizontally. > - Fix bottom edge calculation. > > Changes since v1: > - Fix inverted allow_overshoot logic > - Correctly switch horizontal / vertical tile alignment when >determining seam positions with the 90° rotator active. > - Fix SPDX-License-Identifier and remove superfluous license >text. > - Fix uninitialized walign in try_fmt > > Previous cover letter: > > we have image conversion code for scaling and colorspace conversion in > the IPUv3 base driver for a while. Since the IC hardware can only write > up to 1024x1024 pixel buffers, it scales to larger output buffers by > splitting the input and output frame into similarly sized tiles. > > This causes the issue that the bilinear interpolation resets at the tile > boundary: instead of smoothly interpolating across the seam, there is a > jump in the input sample position that is very apparent for high > upscaling factors. This can be avoided by slightly changing the scaling > coefficients to let the left/top tiles overshoot their input sampling > into the first pixel / line of their right / bottom neighbors. The error > can be further reduced by letting tiles be differently sized and by > selecting seam positions that minimize the input sampling position error > at tile boundaries. > This is complicated by different DMA start address, burst size, and > rotator block size alignment requirements, depending on the input and > output pixel formats, and the fact that flipping happens in different > places depending on the rotation. > > This series implements optimal seam position selection and seam hiding > with per-tile resizing coefficients and adds a scaling mem2mem device > to the imx-media driver. > > regards > Philipp > > Philipp Zabel (15): > media: imx: add mem2mem device > gpu: ipu-v3: ipu-ic: allow to manually set resize coefficients > gpu: ipu-v3: image-convert: prepare for per-tile configuration > gpu: ipu-v3: image-convert: calculate per-tile resize coefficients > gpu: ipu-v3: image-convert: reconfigure IC per tile > gpu: ipu-v3: image-convert: store tile top/left position > gpu: ipu-v3: image-convert: calculate tile dimensions and offsets > outside fill_image > gpu: ipu-v3: image-convert: move tile alignment helpers > gpu: ipu-v3: image-convert: select optimal seam positions > gpu: ipu-v3: image-convert: fix debug output for varying tile sizes > gpu: ipu-v3: image-convert: relax alignment restrictions > gpu: ipu-v3: image-convert: fix bytesperline adjustment > gpu: ipu-v3: image-convert: add some ASCII art to the exposition > gpu: ipu-v3: image-convert: disable double buffering if necessary > gpu: ipu-v3: image-convert: allow three rows or columns > > Steve Longerbeam (7): > gpu: ipu-cpmem: add WARN_ON_ONCE() for unaligned dma buffers > gpu: ipu-v3: Add chroma plane offset overrides to > ipu_cpmem_set_image() > gpu: ipu-v3: image-convert: Prevent race between run and unprepare > gpu: ipu-v3: image-convert: Only wait for abort completion if active > run > gpu: ipu-v3: image-convert: Allow reentrancy into abort > gpu: ipu-v3: image-convert: Remove need_abort flag > gpu: ipu-v3: image-convert: Catch unaligned tile offsets > > drivers/gpu/ipu-v3/ipu-cpmem.c| 52 +- > drivers/gpu/ipu-v3/ipu-ic.c
[PATCHv2 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..49103787d19a 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=%x\n", p->tag); + 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
[PATCHv2 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 115f2868223a..d2d4985fb352 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
[PATCHv2 0/9] vb2/cedrus: add tag support
From: Hans Verkuil 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 Changes since v1: - changed to a u32 tag. Using a 64 bit tag was overly complicated due to the bad layout of the v4l2_buffer struct, and there is no real need for it by applications. 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 | 32 + .../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| 9 +++- 17 files changed, 178 insertions(+), 73 deletions(-) -- 2.19.1
[PATCHv2 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..bb4feb6969d2 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_vb,
+ bool copy_frame_flags);
+
/* v4l2 request helper */
void v4l2_m2m_request_queue(struct media_request *req);
--
2.19.1
[PATCHv2 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 | 32 ++- .../media/uapi/v4l/vidioc-reqbufs.rst | 4 +++ 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/Documentation/media/uapi/v4l/buffer.rst b/Documentation/media/uapi/v4l/buffer.rst index 2e266d32470a..0d4aa83d2bce 100644 --- a/Documentation/media/uapi/v4l/buffer.rst +++ b/Documentation/media/uapi/v4l/buffer.rst @@ -220,17 +220,25 @@ 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. +* - + - __u32 + - ``tag`` + - When the ``V4L2_BUF_FLAG_TAG`` flag is set in ``flags``, this + structure contains a user-specified tag value. + + It is used by stateless codecs where this tag can be used to + refer to buffers that contain reference frames. * - __u32 - ``sequence`` - @@ -567,6 +575,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 +720,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_timecode` 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 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
[PATCHv2 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..115f2868223a 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 = b->tag;
+ 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)
+ b->tag = vbuf->tag;
+ else if (b->flags & V4L2_BUF_FLAG_TIMECODE)
+ 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, u32 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..c2a541af6b2c 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
[PATCHv2 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 | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
index c8e8ff810190..173a94d2cbef 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
@@ -923,6 +924,7 @@ struct v4l2_plane {
* @field: enum v4l2_field; field order of the image in the buffer
* @timestamp: frame timestamp
* @timecode: frame timecode
+ * @tag: buffer tag
* @sequence: sequence count of this frame
* @memory:enum v4l2_memory; the method, in which the actual video data is
* passed
@@ -950,7 +952,10 @@ struct v4l2_buffer {
__u32 flags;
__u32 field;
struct timeval timestamp;
- struct v4l2_timecodetimecode;
+ union {
+ struct v4l2_timecodetimecode;
+ __u32 tag;
+ };
__u32 sequence;
/* memory location */
@@ -988,6 +993,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 */
--
2.19.1
[PATCHv2 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
[PATCHv2 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
[PATCHv2 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..0cfd6036d0cd 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, true);
+
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] pulse8-cec: return 0 when invalidating the logical address
Return 0 when invalidating the logical address. The cec core produces a warning for drivers that do this. Signed-off-by: Hans Verkuil Reported-by: Torbjorn Jansson --- diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c b/drivers/media/usb/pulse8-cec/pulse8-cec.c index 365c78b748dd..b085b14f3f87 100644 --- a/drivers/media/usb/pulse8-cec/pulse8-cec.c +++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c @@ -586,7 +586,7 @@ static int pulse8_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr) else pulse8->config_pending = true; mutex_unlock(>config_lock); - return err; + return log_addr == CEC_LOG_ADDR_INVALID ? 0 : err; } static int pulse8_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
Re: cec kernel oops with pulse8 usb cec adapter
On 11/14/18 14:00, Torbjorn Jansson wrote:
> since there now is a patch that appears to be working and fixing this problem
> i'd like to ask for some troubleshooting advice with another cec issue i have
> that i haven't figured out why it is happening and exactly whats causing it.
>
> i'm not sure if it is a hardware issue or a software issue or both.
>
> this is what is happening:
> i have a script that polls the tv for current status by running:
> cec-ctl --to=0 --give-device-power-status
> after a fresh reboot this works fine for a while then sometime later it stops
> working and errors with:
> -
> # cec-ctl --to=0 --give-device-power-status
> Driver Info:
> Driver Name: pulse8-cec
> Adapter Name : serio0
> Capabilities : 0x003f
> Physical Address
> Logical Addresses
> Transmit
> Passthrough
> Remote Control Support
> Monitor All
> Driver version : 4.18.16
> Available Logical Addresses: 1
> Physical Address : 1.4.0.0
> Logical Address Mask : 0x0800
> CEC Version: 2.0
> Vendor ID : 0x000c03
> Logical Addresses : 1 (Allow RC Passthrough)
>
>Logical Address : 11 (Playback Device 3)
> Primary Device Type: Playback
> Logical Address Type : Playback
> All Device Types : Playback
> RC TV Profile : None
> Device Features:
> None
>
>
> Transmit from Playback Device 3 to TV (11 to 0):
> CEC_MSG_GIVE_DEVICE_POWER_STATUS (0x8f)
> Sequence: 119437 Tx Timestamp: 865389.535s
> Tx, Error (1), Max Retries
> -
>
> once this happens i can never get back any status and also running:
> cec-ctl -M
> gives me a lot of:
> Transmitted by Playback Device 3 to TV (11 to 0):
> CEC_MSG_GIVE_DEVICE_POWER_STATUS (0x8f)
> Tx, Error (1), Max Retries
> once for every run of my status checking script.
>
> i know polling like this is not the best option and a better way would be to
> listen for events and then take action when status changes but that's not
> easily doable with what i need it for.
>
> anyway, once i start getting the above errors when i poll it will not give me
> back any good status any more (everything errors out)
>
> also sending commands to tv to turn on or off like:
> cec-ctl --to=0 --image-view-on
> or
> cec-ctl --to=0 --standby
> doesn't work.
>
> BUT if i remove power to tv and wait for standby led to go out completely
> then
> power it back on i can use above two commands to turn on/off the tv even when
> they return errors and i still can't poll for current status.
>
> so even with the errors always returned at this stage the on/off commands
> still
> gets sent and works.
>
> do you think this behavior is a sw or hw issue or both?
Does 'cec-ctl --to=0 --give-device-power-status' eventually fail even if the TV
is
on all the time? Or does it only fail if the TV goes to standby or has been in
standby for a very long time?
This error ('Tx, Error (1), Max Retries') indicates that the pulse-eight returns
transmit errors suggesting that the CEC line is locked (always high or low).
>
>
> if i'm not mistaken i could unplug usb cable to pulse8 cec adapter and
> reinsert
> to make it work properly again (no errors and correct response like a fresh
> start)
> but i'm not 100% sure of this.
> when i tried it now i get a new kernel oops:
> -
> [866129.392139] usb 7-2: USB disconnect, device number 3
> [866129.409568] cdc_acm 7-2:1.1: acm_start_wb - usb_submit_urb(write bulk)
> failed: -19
> [866129.409576] cdc_acm 7-2:1.1: acm_start_wb - usb_submit_urb(write bulk)
> failed: -19
> [866129.409635] WARNING: CPU: 10 PID: 1571 at
> drivers/media/cec/cec-adap.c:1243
Not a kernel oops, just a warning. The pulse-eight driver has a small bug
that causes this warning, but it does not affect the CEC behavior in any way.
I'll post a patch.
Regards,
Hans
Re: [PATCH v5 01/11] media: ov5640: Adjust the clock based on the expected rate
On Tue, Nov 13, 2018 at 7:04 AM Maxime Ripard wrote:
>
> 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
Thanks for the patches! I tested the whole series. I am stil learning
the v4l2 stuff, but I'm trying to test what and where I can.
media-ctl shows the camera is talking at 60fps, but my imx6 is only
capturing at 30, but I don't think it's the fault of the ov5640
driver.
Tested-by: Adam Ford #imx6dq
adam
> ---
> 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,
>
Re: cec kernel oops with pulse8 usb cec adapter
On 2018-10-22 10:59, Sean Young wrote:
On Sat, Oct 20, 2018 at 11:12:16PM +0200, Hans Verkuil wrote:
Hi Sean,
Can you take a look at this, it appears to be an RC issue, see my analysis
below.
On 10/20/2018 03:26 PM, Torbjorn Jansson wrote:
Hello
i'm using the pulse8 usb cec adapter to control my tv.
i have a few scripts that poll the power status of my tv and after a while it
stops working returning errors when trying to check if tv is on or off.
this i think matches a kernel oops i'm seeing that i suspect is related to this.
i have sometimes been able to recover from this problem by completely cutting
power to my tv and also unplugging the usb cec adapter.
i have a feeling that the tv is at least partly to blame for cec-ctl not
working but in any case there shouldn't be a kernel oops.
also every now and then i see this in dmesg:
cec cec0: transmit: failed 05
cec cec0: transmit: failed 06
but that doesn't appear to do any harm as far as i can tell.
any idea whats causing the oops?
the ops:
BUG: unable to handle kernel NULL pointer dereference at 0038
PGD 0 P4D 0
Oops: [#1] SMP PTI
CPU: 9 PID: 27687 Comm: kworker/9:2 Tainted: P OE
4.18.12-200.fc28.x86_64 #1
Hardware name: Supermicro C7X99-OCE-F/C7X99-OCE-F, BIOS 2.1a 06/15/2018
Workqueue: events pulse8_irq_work_handler [pulse8_cec]
RIP: 0010:ir_lirc_scancode_event+0x3d/0xb0 [rc_core]
Huh. ir_lirc_scancode_event() calls spin_lock_irqsave(>lirc_fh_lock,
flags);
The spinlock dev->lirc_fh_lock is initialized in ir_lirc_register(), which is
called
from rc_register_device(), except when the protocol is CEC:
/* Ensure that the lirc kfifo is setup before we start the thread */
if (dev->allowed_protocols != RC_PROTO_BIT_CEC) {
rc = ir_lirc_register(dev);
if (rc < 0)
goto out_rx;
}
So it looks like ir_lirc_scancode_event() fails because dev->lirc_fh_lock was
never
initialized.
Could this be fall-out from the lirc changes you did not too long ago?
Yes, this is broken. My bad, sorry. I think this must have been broken since
v4.16. I can write a patch but I don't have a patch but I'm on the train
to ELCE in Edinburgh now, with no hardware to test on.
Sean
since there now is a patch that appears to be working and fixing this problem
i'd like to ask for some troubleshooting advice with another cec issue i have
that i haven't figured out why it is happening and exactly whats causing it.
i'm not sure if it is a hardware issue or a software issue or both.
this is what is happening:
i have a script that polls the tv for current status by running:
cec-ctl --to=0 --give-device-power-status
after a fresh reboot this works fine for a while then sometime later it stops
working and errors with:
-
# cec-ctl --to=0 --give-device-power-status
Driver Info:
Driver Name: pulse8-cec
Adapter Name : serio0
Capabilities : 0x003f
Physical Address
Logical Addresses
Transmit
Passthrough
Remote Control Support
Monitor All
Driver version : 4.18.16
Available Logical Addresses: 1
Physical Address : 1.4.0.0
Logical Address Mask : 0x0800
CEC Version: 2.0
Vendor ID : 0x000c03
Logical Addresses : 1 (Allow RC Passthrough)
Logical Address : 11 (Playback Device 3)
Primary Device Type: Playback
Logical Address Type : Playback
All Device Types : Playback
RC TV Profile : None
Device Features:
None
Transmit from Playback Device 3 to TV (11 to 0):
CEC_MSG_GIVE_DEVICE_POWER_STATUS (0x8f)
Sequence: 119437 Tx Timestamp: 865389.535s
Tx, Error (1), Max Retries
-
once this happens i can never get back any status and also running:
cec-ctl -M
gives me a lot of:
Transmitted by Playback Device 3 to TV (11 to 0):
CEC_MSG_GIVE_DEVICE_POWER_STATUS (0x8f)
Tx, Error (1), Max Retries
once for every run of my status checking script.
i know polling like this is not the best option and a better way would be to
listen for events and then take action when status changes but that's not
easily doable with what i need it for.
anyway, once i start getting the above errors when i poll it will not give me
back any good status any more (everything errors out)
also sending commands to tv to turn on or off like:
cec-ctl --to=0 --image-view-on
or
cec-ctl --to=0 --standby
doesn't work.
BUT if i remove power to tv and wait for standby led to go out completely then
power it back on i can use above two commands to turn on/off the tv even when
they return errors and i still can't poll for current status.
so even with the errors always
Re: [PATCH] media: rc: cec devices do not have a lirc chardev
On 2018-10-22 15:17, Sean Young wrote: On Mon, Oct 22, 2018 at 01:28:42PM +0100, Sean Young wrote: On Mon, Oct 22, 2018 at 12:30:29PM +0100, Hans Verkuil wrote: On 10/22/2018 11:14 AM, Sean Young wrote: Would you be able to test the following patch please? Sean, I think you should be able to test this with the vivid driver. Load the vivid driver, run: cec-ctl --tv; cec-ctl -d1 --playback Then: cec-ctl -d1 -t0 --user-control-pressed ui-cmd=F5 Ah, thanks. That will help with testing/reproducing. That said, I tried this, but it doesn't crash for me, but perhaps I need to run some RC command first... Hmm I think those commands should be enough. It probably needs CONFIG_DEBUG_SPINLOCK to detect the uninitialized spinlock. I'm trying it now. Yes, that turned out to work. With CONFIG_DEBUG_SPINLOCK on, it goes bang every time. With the patch, the problem goes away. Without CONFIG_DEBUG_SPINLOCK we're going into undefined behaviour, so Torbjorn you're only seeing the oops occassionally (and which is why it has not been observed or reported before). Thanks, Sean FYI i have tested this patch for a while now and have not seen any more kernel oops. So it appears to be working.
Re: [PATCH] media: vb2: Allow reqbufs(0) with "in use" MMAP buffers
Hi Sakari, On Wed, 2018-11-14 at 00:27 +0200, Sakari Ailus wrote: [...] > This lets the user to allocate lots of mmap'ed buffers that are pinned in > physical memory. This is already possible without this patch, by closing the fd instead of calling reqbufs(0). > 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 . Oh right, thanks. I'll add V4L2_BUF_CAP_SUPPORTS_ORPHANED_BUFS to _v4l2-buf-capabilities in v2. regards Philipp
[PATCH v2 for v4.9 1/1] v4l: event: Add subscription to list before calling "add" operation
[ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
while accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
Tested-by: Dave Stevenson
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
[Sakari Ailus: Backported to v4.9 stable]
Signed-off-by: Sakari Ailus
---
since v1 (as requested by Sasha):
- Add my final SoB
- Indicate specifically this is a backport
- Remove the extra cc stable
drivers/media/v4l2-core/v4l2-event.c | 43
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index 567d86835f001..1fda2873375f6 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -197,6 +197,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_event_pending);
+static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
+{
+ struct v4l2_fh *fh = sev->fh;
+ unsigned int i;
+
+ lockdep_assert_held(>subscribe_lock);
+ assert_spin_locked(>vdev->fh_lock);
+
+ /* Remove any pending events for this subscription */
+ for (i = 0; i < sev->in_use; i++) {
+ list_del(>events[sev_pos(sev, i)].list);
+ fh->navailable--;
+ }
+ list_del(>list);
+}
+
int v4l2_event_subscribe(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub, unsigned
elems,
const struct v4l2_subscribed_event_ops *ops)
@@ -228,27 +244,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
spin_lock_irqsave(>vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
+ if (!found_ev)
+ list_add(>list, >subscribed);
spin_unlock_irqrestore(>vdev->fh_lock, flags);
if (found_ev) {
/* Already listening */
kfree(sev);
- goto out_unlock;
- }
-
- if (sev->ops && sev->ops->add) {
+ } else if (sev->ops && sev->ops->add) {
ret = sev->ops->add(sev, elems);
if (ret) {
+ spin_lock_irqsave(>vdev->fh_lock, flags);
+ __v4l2_event_unsubscribe(sev);
+ spin_unlock_irqrestore(>vdev->fh_lock, flags);
kfree(sev);
- goto out_unlock;
}
}
- spin_lock_irqsave(>vdev->fh_lock, flags);
- list_add(>list, >subscribed);
- spin_unlock_irqrestore(>vdev->fh_lock, flags);
-
-out_unlock:
mutex_unlock(>subscribe_lock);
return ret;
@@ -283,7 +295,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
{
struct v4l2_subscribed_event *sev;
unsigned long flags;
- int i;
if (sub->type == V4L2_EVENT_ALL) {
v4l2_event_unsubscribe_all(fh);
@@ -295,14 +306,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
spin_lock_irqsave(>vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (sev != NULL) {
- /* Remove any pending events for this subscription */
- for (i = 0; i < sev->in_use; i++) {
- list_del(>events[sev_pos(sev, i)].list);
- fh->navailable--;
- }
- list_del(>list);
- }
+ if (sev != NULL)
+ __v4l2_event_unsubscribe(sev);
spin_unlock_irqrestore(>vdev->fh_lock, flags);
--
2.11.0
[PATCH v2 for v4.4 1/1] v4l: event: Add subscription to list before calling "add" operation
[ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
Patch ad608fbcf166 changed how events were subscribed to address an issue
elsewhere. As a side effect of that change, the "add" callback was called
before the event subscription was added to the list of subscribed events,
causing the first event queued by the add callback (and possibly other
events arriving soon afterwards) to be lost.
Fix this by adding the subscription to the list before calling the "add"
callback, and clean up afterwards if that fails.
Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event subscriptions
while accessed")
Reported-by: Dave Stevenson
Signed-off-by: Sakari Ailus
Tested-by: Dave Stevenson
Reviewed-by: Hans Verkuil
Tested-by: Hans Verkuil
Signed-off-by: Mauro Carvalho Chehab
[Sakari Ailus: Backported to v4.4 stable]
Signed-off-by: Sakari Ailus
---
since v1 (as requested by Sasha):
- Add my final SoB
- Indicate specifically this is a backport
- Remove the extra cc stable
drivers/media/v4l2-core/v4l2-event.c | 43
1 file changed, 24 insertions(+), 19 deletions(-)
diff --git a/drivers/media/v4l2-core/v4l2-event.c
b/drivers/media/v4l2-core/v4l2-event.c
index b47ac4e053d0e..f5c8a952f0aa3 100644
--- a/drivers/media/v4l2-core/v4l2-event.c
+++ b/drivers/media/v4l2-core/v4l2-event.c
@@ -197,6 +197,22 @@ int v4l2_event_pending(struct v4l2_fh *fh)
}
EXPORT_SYMBOL_GPL(v4l2_event_pending);
+static void __v4l2_event_unsubscribe(struct v4l2_subscribed_event *sev)
+{
+ struct v4l2_fh *fh = sev->fh;
+ unsigned int i;
+
+ lockdep_assert_held(>subscribe_lock);
+ assert_spin_locked(>vdev->fh_lock);
+
+ /* Remove any pending events for this subscription */
+ for (i = 0; i < sev->in_use; i++) {
+ list_del(>events[sev_pos(sev, i)].list);
+ fh->navailable--;
+ }
+ list_del(>list);
+}
+
int v4l2_event_subscribe(struct v4l2_fh *fh,
const struct v4l2_event_subscription *sub, unsigned
elems,
const struct v4l2_subscribed_event_ops *ops)
@@ -228,27 +244,23 @@ int v4l2_event_subscribe(struct v4l2_fh *fh,
spin_lock_irqsave(>vdev->fh_lock, flags);
found_ev = v4l2_event_subscribed(fh, sub->type, sub->id);
+ if (!found_ev)
+ list_add(>list, >subscribed);
spin_unlock_irqrestore(>vdev->fh_lock, flags);
if (found_ev) {
/* Already listening */
kfree(sev);
- goto out_unlock;
- }
-
- if (sev->ops && sev->ops->add) {
+ } else if (sev->ops && sev->ops->add) {
ret = sev->ops->add(sev, elems);
if (ret) {
+ spin_lock_irqsave(>vdev->fh_lock, flags);
+ __v4l2_event_unsubscribe(sev);
+ spin_unlock_irqrestore(>vdev->fh_lock, flags);
kfree(sev);
- goto out_unlock;
}
}
- spin_lock_irqsave(>vdev->fh_lock, flags);
- list_add(>list, >subscribed);
- spin_unlock_irqrestore(>vdev->fh_lock, flags);
-
-out_unlock:
mutex_unlock(>subscribe_lock);
return ret;
@@ -283,7 +295,6 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
{
struct v4l2_subscribed_event *sev;
unsigned long flags;
- int i;
if (sub->type == V4L2_EVENT_ALL) {
v4l2_event_unsubscribe_all(fh);
@@ -295,14 +306,8 @@ int v4l2_event_unsubscribe(struct v4l2_fh *fh,
spin_lock_irqsave(>vdev->fh_lock, flags);
sev = v4l2_event_subscribed(fh, sub->type, sub->id);
- if (sev != NULL) {
- /* Remove any pending events for this subscription */
- for (i = 0; i < sev->in_use; i++) {
- list_del(>events[sev_pos(sev, i)].list);
- fh->navailable--;
- }
- list_del(>list);
- }
+ if (sev != NULL)
+ __v4l2_event_unsubscribe(sev);
spin_unlock_irqrestore(>vdev->fh_lock, flags);
--
2.11.0
Re: [PATCH 2/5] v4l: controls: Add support for exponential bases, prefixes and units
Hi Sakari,
On Fri, Sep 28, 2018 at 11:00 PM Hans Verkuil wrote:
>
> On 09/25/2018 12:14 PM, Sakari Ailus wrote:
> > Add support for exponential bases, prefixes as well as units for V4L2
> > controls. This makes it possible to convey information on the relation
> > between the control value and the hardware feature being controlled.
> >
Sorry for being late to the party.
Thanks for the series. I think it has a potential to be very useful.
Please see my comments below.
> > Signed-off-by: Sakari Ailus
> > ---
> > include/uapi/linux/videodev2.h | 32 +++-
> > 1 file changed, 31 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> > index ae083978988f1..23b02f2db85a1 100644
> > --- a/include/uapi/linux/videodev2.h
> > +++ b/include/uapi/linux/videodev2.h
> > @@ -1652,6 +1652,32 @@ struct v4l2_queryctrl {
> > __u32reserved[2];
> > };
> >
> > +/* V4L2 control exponential bases */
> > +#define V4L2_CTRL_BASE_UNDEFINED 0
> > +#define V4L2_CTRL_BASE_LINEAR1
>
> I'm not really sure you need BASE_LINEAR. That is effectively the same
> as UNDEFINED since what else can you do? It's also weird to have this
> as 'base' if the EXPONENTIAL flag is set.
>
> I don't see why you need the EXPONENTIAL flag at all: if this is non-0,
> then you know the exponential base.
Or vice versa, we could remove UNDEFINED and LINEAR altogether and
have the EXPONENTIAL flag actually signify the presence of a valid
base? Besides that, "linear exponential base" just doesn't sound right
or am I missing some basic maths? ;)
Then we could actually have a LOGARITHMIC flag and it could reuse the
same bases enum.
>
> > +#define V4L2_CTRL_BASE_2 2
> > +#define V4L2_CTRL_BASE_1010
> > +
> > +/* V4L2 control unit prefixes */
> > +#define V4L2_CTRL_PREFIX_NANO-9
> > +#define V4L2_CTRL_PREFIX_MICRO -6
> > +#define V4L2_CTRL_PREFIX_MILLI -3
> > +#define V4L2_CTRL_PREFIX_1 0
>
> I would prefer PREFIX_NONE, since there is no prefix in this case.
>
> I assume this prefix is only valid if the unit is not UNDEFINED and not
> NONE?
>
> Is 'base' also dependent on a valid unit? (it doesn't appear to be)
>
> > +#define V4L2_CTRL_PREFIX_KILO3
> > +#define V4L2_CTRL_PREFIX_MEGA6
> > +#define V4L2_CTRL_PREFIX_GIGA9
> > +
> > +/* V4L2 control units */
> > +#define V4L2_CTRL_UNIT_UNDEFINED 0
> > +#define V4L2_CTRL_UNIT_NONE 1
Hmm, what's the meaning of NONE? How does it differ from UNDEFINED?
> > +#define V4L2_CTRL_UNIT_SECOND2
> > +#define V4L2_CTRL_UNIT_AMPERE3
> > +#define V4L2_CTRL_UNIT_LINE 4
> > +#define V4L2_CTRL_UNIT_PIXEL 5
> > +#define V4L2_CTRL_UNIT_PIXELS_PER_SEC6
> > +#define V4L2_CTRL_UNIT_HZ7
> > +
> > +
> > /* Used in the VIDIOC_QUERY_EXT_CTRL ioctl for querying extended controls
> > */
> > struct v4l2_query_ext_ctrl {
> > __u32id;
> > @@ -1666,7 +1692,10 @@ struct v4l2_query_ext_ctrl {
> > __u32elems;
> > __u32nr_of_dims;
> > __u32dims[V4L2_CTRL_MAX_DIMS];
> > - __u32reserved[32];
> > + __u8 base;
> > + __s8 prefix;
Should we make those bigger just in case, or leave some reserved
fields around so we can make them bigger when we need it?
Best regards,
Tomasz
Re: [PATCH v4.9 1/1] v4l: event: Add subscription to list before calling "add" operation
Hi Sasha,
On Thu, Nov 08, 2018 at 12:28:53PM -0500, Sasha Levin wrote:
> On Thu, Nov 08, 2018 at 01:46:06PM +0200, Sakari Ailus wrote:
> > [ upstream commit 92539d3eda2c090b382699bbb896d4b54e9bdece ]
> >
> > Patch ad608fbcf166 changed how events were subscribed to address an issue
> > elsewhere. As a side effect of that change, the "add" callback was called
> > before the event subscription was added to the list of subscribed events,
> > causing the first event queued by the add callback (and possibly other
> > events arriving soon afterwards) to be lost.
> >
> > Fix this by adding the subscription to the list before calling the "add"
> > callback, and clean up afterwards if that fails.
> >
> > Fixes: ad608fbcf166 ("media: v4l: event: Prevent freeing event
> > subscriptions while accessed")
> >
> > Reported-by: Dave Stevenson
> > Signed-off-by: Sakari Ailus
> > Tested-by: Dave Stevenson
> > Reviewed-by: Hans Verkuil
> > Tested-by: Hans Verkuil
> > Cc: sta...@vger.kernel.org (for 4.14 and up)
> > Signed-off-by: Mauro Carvalho Chehab
>
> Hi Sakari,
>
> For the sake of completeness, can you sign off on the backport too and
> indicate it was backported to 4.9 in the commit messge? Otherwise, this
> commit message says it's for 4.14+ and will suddenly appear in the 4.9
> tree, and if we have issues later it might cause confusion.
Yes; I'll fix the above issues and resend.
Thanks!
--
Sakari Ailus
sakari.ai...@linux.intel.com
[PATCH] cec-pin: fix broken tx_ignore_nack_until_eom error injection
If the tx_ignore_nack_until_eom error injection was activated,
then tx_nacked was never set instead of setting it when the last
byte of the message was transmitted.
As a result the transmit was marked as OK, when it should have
been NACKed.
Modify the condition so that it always sets tx_nacked when the
last byte of the message was transmitted.
Signed-off-by: Hans Verkuil
Cc: # for v4.17 and up
---
diff --git a/drivers/media/cec/cec-pin.c b/drivers/media/cec/cec-pin.c
index 635db8e70ead..8f987bc0dd88 100644
--- a/drivers/media/cec/cec-pin.c
+++ b/drivers/media/cec/cec-pin.c
@@ -601,8 +601,9 @@ static void cec_pin_tx_states(struct cec_pin *pin, ktime_t
ts)
break;
/* Was the message ACKed? */
ack = cec_msg_is_broadcast(>tx_msg) ? v : !v;
- if (!ack && !pin->tx_ignore_nack_until_eom &&
- pin->tx_bit / 10 < pin->tx_msg.len && !pin->tx_post_eom) {
+ if (!ack && (!pin->tx_ignore_nack_until_eom ||
+ pin->tx_bit / 10 == pin->tx_msg.len - 1) &&
+ !pin->tx_post_eom) {
/*
* Note: the CEC spec is ambiguous regarding
* what action to take when a NACK appears
Re: [RFC PATCH 0/3] Media Controller Properties
On 11/14/2018 09:09 AM, Tomasz Figa wrote: > Hi Hans, > > On Fri, Aug 3, 2018 at 11:36 PM Hans Verkuil wrote: >> >> From: Hans Verkuil >> >> This RFC patch series implements properties for the media controller. >> >> This is not finished, but I wanted to post this so people can discuss >> this further. >> >> No documentation yet (too early for that). >> >> An updated v4l2-ctl and v4l2-compliance that can report properties >> is available here: >> >> https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=props >> >> There is one main missing piece: currently the properties are effectively >> laid out in random order. My plan is to change that so they are grouped >> by object type and object owner. So first all properties for each entity, >> then for each pad, etc. I started to work on that, but it's a bit more >> work than expected and I wanted to post this before the weekend. >> >> While it is possible to have nested properties, this is not currently >> implemented. Only properties for entities and pads are supported in this >> code, but that's easy to extend to interfaces and links. >> >> I'm not sure about the G_TOPOLOGY ioctl handling: I went with the quickest >> option by renaming the old ioctl and adding a new one with property support. >> >> I think this needs to change (at the very least the old and new should >> share the same ioctl NR), but that's something for the future. >> >> Currently I support u64, s64 and const char * property types. But it >> can be anything including binary data if needed. No array support (as we >> have for controls), but there are enough reserved fields in media_v2_prop >> to add this if needed. >> >> I added properties for entities and pads to vimc, so I could test this. > > I think I'm missing the background and the description doesn't mention > it either. What's the use case for those and why not controls? Properties provide additional information about graph objects: main initial use-case is to give camera information: back/front, upside down, and whatever else Sakari can come up with :-) Also we want to use this to describe all the functions that an entity supports (right now you can describe only one function, but there are multi-function devices out there). Other use-cases are for connectors (but these are not yet modeled by the MC): name and/or color on the backplane. I forgot what the pad properties where needed for, I would have to dig. It is something Mauro wanted to use for either DVB or TV capture drivers to provide additional information about the type of data flowing through a pad. This is all information that most likely will come from the device tree. All this is read-only information. And yes, these properties will all have to be documented since they are part of the ABI. Regards, Hans
Re: [RFC PATCH 0/3] Media Controller Properties
Hi Hans, On Fri, Aug 3, 2018 at 11:36 PM Hans Verkuil wrote: > > From: Hans Verkuil > > This RFC patch series implements properties for the media controller. > > This is not finished, but I wanted to post this so people can discuss > this further. > > No documentation yet (too early for that). > > An updated v4l2-ctl and v4l2-compliance that can report properties > is available here: > > https://git.linuxtv.org/hverkuil/v4l-utils.git/log/?h=props > > There is one main missing piece: currently the properties are effectively > laid out in random order. My plan is to change that so they are grouped > by object type and object owner. So first all properties for each entity, > then for each pad, etc. I started to work on that, but it's a bit more > work than expected and I wanted to post this before the weekend. > > While it is possible to have nested properties, this is not currently > implemented. Only properties for entities and pads are supported in this > code, but that's easy to extend to interfaces and links. > > I'm not sure about the G_TOPOLOGY ioctl handling: I went with the quickest > option by renaming the old ioctl and adding a new one with property support. > > I think this needs to change (at the very least the old and new should > share the same ioctl NR), but that's something for the future. > > Currently I support u64, s64 and const char * property types. But it > can be anything including binary data if needed. No array support (as we > have for controls), but there are enough reserved fields in media_v2_prop > to add this if needed. > > I added properties for entities and pads to vimc, so I could test this. I think I'm missing the background and the description doesn't mention it either. What's the use case for those and why not controls? Best regards, Tomasz
