This is needed for the next patch where the dma-sg alloc memop needs
to know the dma_dir.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

The 'write' argument is very ambiguous. I first assumed that if it is 1,
then we're doing video output but instead it meant the reverse.

Since it is used to setup the dma_dir value anyway it is now replaced by
the correct dma_dir value which is unambiguous.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

The recent conversion of saa7134 to vb2 unconvered a poll() bug that
broke the teletext applications alevt and mtt. These applications
expect that calling poll() without having called VIDIOC_STREAMON will
cause poll() to return POLLERR. That did not happen in vb2.

This patch fixes that behavior. It also fixes what should happen when
poll() is called when STREAMON is called but no buffers have been
queued. In that case poll() will also return POLLERR, but only for
capture queues since output queues will always return POLLOUT
anyway in that situation.

This brings the vb2 behavior in line with the old videobuf behavior.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

It's also invalid when plane_no is equal to vb->num_planes
Signed-off-by: NZhaowei Yuan <zhaowei.yuan@samsung.com>
Cc: stable@vger.kernel.org      # for v3.7 and up
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

Commit bd994ddb (vb2: Fix stream start and
buffer completion race) broke the buffer state check in vb2_buffer_done.

So accept all three possible states there since I can no longer tell the
difference between vb2_buffer_done called from start_streaming or from
elsewhere.

Instead add a WARN_ON at the end of start_streaming that will check whether
any buffers were added to the done list, since that implies that the wrong
state was used as well.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: stable@vger.kernel.org      # for v3.15 and up
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

Recently WARN_ON() calls have been added to warn if the driver is not
properly returning buffers to vb2 in start_streaming (if it fails) or
stop_streaming(). Add comments before those WARN_ON calls that refer
to the videobuf2-core.h header that explains what drivers are supposed
to do in these situations. That should help point developers in the
right direction if they see these warnings.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

(Changes since v2: dropped local variable as suggested by Laurent)

Commit f035eb4e (videobuf2: fix lockdep warning)
unfortunately removed the mmap_sem lock that is needed around the call to
__qbuf_userptr. Amazingly nobody noticed this (especially me as the author)
until Jan Kara pointed this out to me.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Reported-by: NJan Kara <jack@suse.cz>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: NMauro Carvalho Chehab <mchehab@osg.samsung.com>

If this is a multiplanar buf_type and the plane we want to read has a
non-zero data_offset, then that data_offset was not taken into account.

Note that read() or write() for formats with more than one plane is currently
not allowed, hence the use of 'planes[0]' since this is only relevant for a
single-plane format.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

Modern kernels enable dynamic printk support, which is fine, except when it is
combined with a debug module option. Enabling debug in videobuf2-core now produces
no debugging unless it is also enabled through the dynamic printk support in debugfs.

Either use a debug module option + pr_info, or use pr_debug without a debug module
option. In this case the fact that you can set various debug levels is very useful,
so I believe that for videobuf2-core.c we should use pr_info.

The mix of the two is very confusing: I've spent too much time already trying to
figure out why I am not seeing any debug output in the kernel log when I do:

	echo 1 >/sys/modules/videobuf2_core/parameters/debug
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The following lockdep warning has been there ever since commit a517cca6
one year ago:

[  403.117947] ======================================================
[  403.117949] [ INFO: possible circular locking dependency detected ]
[  403.117953] 3.16.0-rc6-test-media #961 Not tainted
[  403.117954] -------------------------------------------------------
[  403.117956] v4l2-ctl/15377 is trying to acquire lock:
[  403.117959]  (&dev->mutex#3){+.+.+.}, at: [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.117974]
[  403.117974] but task is already holding lock:
[  403.117976]  (&mm->mmap_sem){++++++}, at: [<ffffffff8118291f>] vm_mmap_pgoff+0x6f/0xc0
[  403.117987]
[  403.117987] which lock already depends on the new lock.
[  403.117987]
[  403.117990]
[  403.117990] the existing dependency chain (in reverse order) is:
[  403.117992]
[  403.117992] -> #1 (&mm->mmap_sem){++++++}:
[  403.117997]        [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
[  403.118006]        [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
[  403.118010]        [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
[  403.118014]        [<ffffffff8118c9ec>] might_fault+0x7c/0xb0
[  403.118018]        [<ffffffffa0028a25>] video_usercopy+0x425/0x610 [videodev]
[  403.118028]        [<ffffffffa0028c25>] video_ioctl2+0x15/0x20 [videodev]
[  403.118034]        [<ffffffffa0022764>] v4l2_ioctl+0x184/0x1a0 [videodev]
[  403.118040]        [<ffffffff811d77d0>] do_vfs_ioctl+0x2f0/0x4f0
[  403.118307]        [<ffffffff811d7a51>] SyS_ioctl+0x81/0xa0
[  403.118311]        [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
[  403.118319]
[  403.118319] -> #0 (&dev->mutex#3){+.+.+.}:
[  403.118324]        [<ffffffff810d6a96>] check_prevs_add+0x746/0x9f0
[  403.118329]        [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
[  403.118333]        [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
[  403.118336]        [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
[  403.118340]        [<ffffffff81999664>] mutex_lock_interruptible_nested+0x64/0x640
[  403.118344]        [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.118349]        [<ffffffffa0022122>] v4l2_mmap+0x62/0xa0 [videodev]
[  403.118354]        [<ffffffff81197270>] mmap_region+0x3d0/0x5d0
[  403.118359]        [<ffffffff8119778d>] do_mmap_pgoff+0x31d/0x400
[  403.118363]        [<ffffffff81182940>] vm_mmap_pgoff+0x90/0xc0
[  403.118366]        [<ffffffff81195cef>] SyS_mmap_pgoff+0x1df/0x2a0
[  403.118369]        [<ffffffff810085c2>] SyS_mmap+0x22/0x30
[  403.118376]        [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b
[  403.118381]
[  403.118381] other info that might help us debug this:
[  403.118381]
[  403.118383]  Possible unsafe locking scenario:
[  403.118383]
[  403.118385]        CPU0                    CPU1
[  403.118387]        ----                    ----
[  403.118388]   lock(&mm->mmap_sem);
[  403.118391]                                lock(&dev->mutex#3);
[  403.118394]                                lock(&mm->mmap_sem);
[  403.118397]   lock(&dev->mutex#3);
[  403.118400]
[  403.118400]  *** DEADLOCK ***
[  403.118400]
[  403.118403] 1 lock held by v4l2-ctl/15377:
[  403.118405]  #0:  (&mm->mmap_sem){++++++}, at: [<ffffffff8118291f>] vm_mmap_pgoff+0x6f/0xc0
[  403.118411]
[  403.118411] stack backtrace:
[  403.118415] CPU: 0 PID: 15377 Comm: v4l2-ctl Not tainted 3.16.0-rc6-test-media #961
[  403.118418] Hardware name: VMware, Inc. VMware Virtual Platform/440BX Desktop Reference Platform, BIOS 6.00 07/31/2013
[  403.118420]  ffffffff82a6c9d0 ffff8800af37fb00 ffffffff819916a2 ffffffff82a6c9d0
[  403.118425]  ffff8800af37fb40 ffffffff810d5715 ffff8802308e4200 0000000000000000
[  403.118429]  ffff8802308e4a48 ffff8802308e4a48 ffff8802308e4200 0000000000000001
[  403.118433] Call Trace:
[  403.118441]  [<ffffffff819916a2>] dump_stack+0x4e/0x7a
[  403.118445]  [<ffffffff810d5715>] print_circular_bug+0x1d5/0x2a0
[  403.118449]  [<ffffffff810d6a96>] check_prevs_add+0x746/0x9f0
[  403.118455]  [<ffffffff8119c172>] ? find_vmap_area+0x42/0x70
[  403.118459]  [<ffffffff810d733c>] validate_chain.isra.39+0x5fc/0x9a0
[  403.118463]  [<ffffffff810d8bc3>] __lock_acquire+0x4d3/0xd30
[  403.118468]  [<ffffffff810d9da7>] lock_acquire+0xa7/0x160
[  403.118472]  [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.118476]  [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.118480]  [<ffffffff81999664>] mutex_lock_interruptible_nested+0x64/0x640
[  403.118484]  [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.118488]  [<ffffffffa005a6c3>] ? vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.118493]  [<ffffffff810d8055>] ? mark_held_locks+0x75/0xa0
[  403.118497]  [<ffffffffa005a6c3>] vb2_fop_mmap+0x33/0x90 [videobuf2_core]
[  403.118502]  [<ffffffffa0022122>] v4l2_mmap+0x62/0xa0 [videodev]
[  403.118506]  [<ffffffff81197270>] mmap_region+0x3d0/0x5d0
[  403.118510]  [<ffffffff8119778d>] do_mmap_pgoff+0x31d/0x400
[  403.118513]  [<ffffffff81182940>] vm_mmap_pgoff+0x90/0xc0
[  403.118517]  [<ffffffff81195cef>] SyS_mmap_pgoff+0x1df/0x2a0
[  403.118521]  [<ffffffff810085c2>] SyS_mmap+0x22/0x30
[  403.118525]  [<ffffffff8199dc69>] system_call_fastpath+0x16/0x1b

The reason is that vb2_fop_mmap and vb2_fop_get_unmapped_area take the core lock
while they are called with the mmap_sem semaphore held. But elsewhere in the code
the core lock is taken first but calls to copy_to/from_user() can take the mmap_sem
semaphore as well, potentially causing a classical A-B/B-A deadlock.

However, the mmap/get_unmapped_area calls really shouldn't take the core lock
at all. So what would happen if they don't take the core lock anymore?

There are two situations that need to be taken into account: calling mmap while
new buffers are being added and calling mmap while buffers are being deleted.

The first case works almost fine without a lock: in all cases mmap relies on
correctly filled-in q->num_buffers/q->num_planes values and those are only
updated by reqbufs and create_buffers *after* any new buffers have been
initialized completely. Except in one case: if an error occurred while allocating
the buffers it will increase num_buffers and rely on __vb2_queue_free to
decrease it again. So there is a short period where the buffer information
may be wrong.

The second case definitely does pose a problem: buffers may be in the process
of being deleted, without the internal structure being updated.

In order to fix this a new mutex is added to vb2_queue that is taken when
buffers are allocated or deleted, and in vb2_mmap. That way vb2_mmap won't
get stale buffer data. Note that this is a problem only for MEMORY_MMAP, so
even though __qbuf_userptr and __qbuf_dmabuf also mess around with buffers
(mem_priv in particular), this doesn't clash with vb2_mmap or
vb2_get_unmapped_area since those are MMAP specific.

As an additional bonus the hack in __buf_prepare, the USERPTR case, can be
removed as well since mmap() no longer takes the core lock.

All in all a much cleaner solution.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: NMarek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

vb2_poll should always return POLLOUT | POLLWRNORM as long as there
are fewer buffers queued than there are buffers available. Poll for
an output stream should only wait if all buffers are queued and nobody
is dequeuing them.
Signed-off-by: NHans Verkuil <hansverk@cisco.com>
Acked-by: NMarek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The original report from Nikhil was that if data_offset > 0 and bytesused == 0,
then the check in __verify_length() would fail, even though the spec says that
if bytes_used == 0, then it will be replaced by the actual length of the
buffer.

After digging into it a bit more I realized that there were several other
things wrong:

- in __verify_length() it would use the application-provided length value
  for USERPTR and the vb2 core length for other memory models, but it
  should have used the application-provided length as well for DMABUF.

- in __fill_vb2_buffer() on the other hand it would replace bytesused == 0
  by the application-provided length, even for MMAP buffers where the
  length is determined by the vb2 core.

- in __fill_vb2_buffer() it tries to figure out if all the planes have
  bytesused == 0 before it will decide to replace bytesused by length.
  However, the spec makes no such provision, and it makes for convoluted
  code. So just replace any bytesused == 0 by the proper length.
  The idea behind this was that you could use bytesused to signal empty
  planes, something that is currently not supported. But that is better
  done in the future by using one of the reserved fields in strucy v4l2_plane.

This patch fixes all these issues.

Regards,

	Hans
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Reported-by: NNikhil Devshatwar <nikhil.nd@ti.com>
Cc: Nikhil Devshatwar <nikhil.nd@ti.com>
Acked-by: NMarek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

When a fatal error occurs that render the device unusable, the only
options for a driver to signal the error condition to userspace is to
set the V4L2_BUF_FLAG_ERROR flag when dequeuing buffers and to return an
error from the buffer prepare handler when queuing buffers.

The buffer error flag indicates a transient error and can't be used by
applications to detect fatal errors. Returning an error from vb2_qbuf()
is thus the only real indication that a fatal error occurred. However,
this is difficult to handle for multithreaded applications that requeue
buffers from a thread other than the control thread. In particular the
poll() call in the control thread will not notify userspace of the
error.

This patch adds an explicit mechanism to report fatal errors to
userspace. Drivers can call the vb2_queue_error() function to signal a
fatal error. From this moment on, buffer preparation will return -EIO to
userspace, and vb2_poll() will set the POLLERR flag and return
immediately. The error flag is cleared when cancelling the queue, either
at stream off time (through vb2_streamoff) or when releasing the queue
with vb2_queue_release().
Signed-off-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The V4L2 specification states that

"When the application did not call VIDIOC_QBUF or VIDIOC_STREAMON yet
the poll() function succeeds, but sets the POLLERR flag in the revents
field."

The vb2_poll() function sets POLLERR when the queued buffers list is
empty, regardless of whether this is caused by the stream not being
active yet, or by a transient buffer underrun.

Bring the implementation in line with the specification by returning
POLLERR if no buffer has been queued only when the queue is not
streaming. Buffer underruns during streaming are not treated specially
anymore and just result in poll() blocking until the next event.
Signed-off-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

videobuf2 stores the driver streaming state internally in the queue in
the start_streaming_called variable. The state is set right after the
driver start_stream operation returns, and checked in the
vb2_buffer_done() function, typically called from the frame completion
interrupt handler. A race condition exists if the hardware finishes
processing the first frame before the start_stream operation returns.

Fix this by setting start_streaming_called to 1 before calling the
start_stream operation, and resetting it to 0 if the operation fails.

Cc: stable@vger.kernel.org      # for v3.15 and up
Signed-off-by: NLaurent Pinchart <laurent.pinchart+renesas@ideasonboard.com>
Reviewed-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

When suspending a device while a video stream is active all buffers
marked as done but not dequeued yet will be kept across suspend and
given back to userspace after resume. This will result in outdated
buffers being dequeued.

Introduce a new vb2 function to mark all done buffers as erroneous
instead, to be used by drivers at resume time.
Signed-off-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Acked-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NSakari Ailus <sakari.ailus@iki.fi>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

Remove duplicated test of buffer presence at streamon
Signed-off-by: NVictor Lambret <victor.lambret.ext@parrot.com>
Acked-by: NMarek Szyprowski <m.szyprowski@samsung.com>
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

num_buffers can't be bigger than VIDEO_MAX_FRAME. This is assured by:
  num_buffers = min_t(unsigned int, req->count, VIDEO_MAX_FRAME);

However, this value is overriden by:
  num_buffers = max_t(unsigned int, req->count, q->min_buffers_needed);

It should, instead, use the previously calculated value as an input
to max_t:
  num_buffers = max_t(unsigned int, num_buffers, q->min_buffers_needed);
Signed-off-by: NPhilipp Zabel <p.zabel@pengutronix.de>
Reviewed-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The __vb2_queue_cancel function marks the queue as not streaming and
then WARNs when buffers are still owned by the driver. It proceeds to
complete all active buffers by calling vb2_buffer_done with the new
buffer state set to VB2_BUF_STATE_ERROR in that case. This triggers
another WARN_ON due to as new state not being VB2_BUF_STATE_QUEUED while
the queue is not streaming.

Check buffer ownership and complete all active buffers before marking
the queue as not streaming to avoid the double WARN_on.
Signed-off-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Reviewed-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

When compiling this for older kernels using the compatibility build
the compiler complains about uninitialized variables:

In file included from include/linux/kernel.h:20:0,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/input.h:13,
                 from /home/hans/work/build/media_build/v4l/compat.h:9,
                 from <command-line>:0:
/home/hans/work/build/media_build/v4l/videobuf2-core.c: In function 'vb2_mmap':
include/linux/dynamic_debug.h:60:9: warning: 'plane' may be used uninitialized in this function [-Wmaybe-uninitialized]
   printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);  \
         ^
/home/hans/work/build/media_build/v4l/videobuf2-core.c:2381:23: note: 'plane' was declared here
  unsigned int buffer, plane;
                       ^
In file included from include/linux/kernel.h:20:0,
                 from include/linux/cache.h:4,
                 from include/linux/time.h:7,
                 from include/linux/input.h:13,
                 from /home/hans/work/build/media_build/v4l/compat.h:9,
                 from <command-line>:0:
include/linux/dynamic_debug.h:60:9: warning: 'buffer' may be used uninitialized in this function [-Wmaybe-uninitialized]
   printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__);  \
         ^
/home/hans/work/build/media_build/v4l/videobuf2-core.c:2381:15: note: 'buffer' was declared here
  unsigned int buffer, plane;
               ^

While these warnings are bogus (the call to __find_plane_by_offset will
set buffer and plane), it doesn't hurt to initialize these variables.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The vb2 core ignores any return code from the stop_streaming op.
And there really isn't anything it can do anyway in case of an error.
So change the return type to void and update any drivers that implement it.

The int return gave drivers the idea that this operation could actually
fail, but that's really not the case.

The pwc amd sdr-msi3101 drivers both had this construction:

        if (mutex_lock_interruptible(&s->v4l2_lock))
                return -ERESTARTSYS;

This has been updated to just call mutex_lock(). The stop_streaming op
expects this to really stop streaming and I very much doubt this will
work reliably if stop_streaming just returns without really stopping the
DMA.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

In order to implement vb2 DVB support you need to be able to start
a kernel thread that queues and dequeues buffers, calling a callback
function for every buffer. This patch adds support for that.

It's based on drivers/media/v4l2-core/videobuf-dvb.c, but with all the DVB
specific stuff stripped out, thus making it much more generic.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The kernel debug messages produced by vb2 started either with a
lower or an upper case character. Switched all to use lower-case
which seemed to be what was used in the majority of the messages.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

It was impossible to read() or write() a frame if the queue type was multiplanar.
Even if the current format is single planar. Change this to just check whether
the number of planes is 1 or more.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

Added a vb2_fileio_is_active inline function that returns true if fileio
is in progress. Check for this too in mmap() (you don't want apps mmap()ing
buffers used by fileio) and expbuf() (same reason).

In addition drivers should be able to check for this in queue_setup() to
return an error if an attempt is made to read() or write() with
V4L2_FIELD_ALTERNATE being configured. This is illegal (there is no way
to pass the TOP/BOTTOM information around using file I/O).

However, in order to be able to check for this the init_fileio function
needs to set q->fileio early on, before the buffers are allocated. So switch
to using internal functions (__reqbufs, vb2_internal_qbuf and
vb2_internal_streamon) to skip the fileio check. Well, that's why the internal
functions were created...
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

q->start_streaming_called is always true, so the WARN_ON check against
it being false can be dropped.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

This is not allowed by the spec and does in fact not make any sense.
Return -EINVAL if this is the case.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

When using write() to write data to an output video node the vb2 core
should set timestamps if V4L2_BUF_FLAG_TIMESTAMP_COPY is set. Nobody
else is able to provide this information with the write() operation.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

__qbuf_mmap was sort of hidden in between the much larger __qbuf_userptr
and __qbuf_dmabuf functions. Move it before __qbuf_userptr which is
also conform the usual order these memory models are implemented: first
mmap, then userptr, then dmabuf.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

Many dprintk's in vb2 use a hardcoded prefix with the function name. In
many cases that is now outdated. To keep things consistent the dprintk
macro has been changed to print the function name in addition to the "vb2:"
prefix. Superfluous prefixes elsewhere in the code have been removed.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The application should really always fill in bytesused for output
buffers, unfortunately the vb2 framework never checked for that.

So for single planar formats replace a bytesused of 0 by the length
of the buffer, and for multiplanar format do the same if bytesused is
0 for ALL planes.

This seems to be what the user really intended if v4l2_buffer was
just memset to 0.

I'm afraid that just checking for this and returning an error would
break too many applications. Quite a few drivers never check for bytesused
at all and just use the buffer length instead.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

The videobuf2-core did not zero the 'planes' array in __qbuf_userptr()
and __qbuf_dmabuf(). That's now memset to 0. Without this the reserved
array in struct v4l2_plane would be non-zero, causing v4l2-compliance
errors.

More serious is the fact that data_offset was not handled correctly:

- for capture devices it was never zeroed, which meant that it was
  uninitialized. Unless the driver sets it it was a completely random
  number. With the memset above this is now fixed.

- __qbuf_dmabuf had a completely incorrect length check that included
  data_offset.

- in __fill_vb2_buffer in the DMABUF case the data_offset field was
  unconditionally copied from v4l2_buffer to v4l2_plane when this
  should only happen in the output case.

- in the single-planar case data_offset was never correctly set to 0.
  The single-planar API doesn't support data_offset, so setting it
  to 0 is the right thing to do. This too is now solved by the memset.

All these issues were found with v4l2-compliance.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

Sparse generated a bunch of errors like this:

drivers/media/v4l2-core/videobuf2-core.c:2045:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:136:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:151:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:168:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:183:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:185:9: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:385:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1115:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1268:33: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1270:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1315:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1324:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1396:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1457:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1482:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1484:9: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1523:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1525:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1815:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1828:17: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1914:25: error: incompatible types in conditional expression (different base types)
drivers/media/v4l2-core/videobuf2-core.c:1944:9: error: incompatible types in conditional expression (different base types)

These are caused by the call*op defines which do something like this:

        (ops->op) ? ops->op(args) : 0

which is OK as long as op is not a void function, because in that case one part
of the conditional expression returns void, the other an integer. Hence the sparse
errors.

I've replaced this by introducing three variants of the call_ macros:
call_*op for int returns, call_void_*op for void returns and call_ptr_*op for
pointer returns.

That's the bad news. The good news is that the fail_*op macros could be removed
since the call_*op macros now have enough information to determine if the op
succeeded or not and can increment the op counter only on success. This at least
makes it more robust w.r.t. future changes.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

Don't call buf_finish unless we know that the buffer is in a valid state.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

If you request buffers, then queue buffers and then call STREAMOFF
those buffers are not returned to their dequeued state because streamoff
will just return if q->streaming was 0.

This means that afterwards you can never QBUF that same buffer again unless
you do STREAMON, REQBUFS or close the filehandle first.

It is clear that if you do STREAMOFF even if no STREAMON was called before,
you still want to have all buffers returned to their proper dequeued state.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

No need to oops for this, WARN_ON is good enough.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

If __reqbufs was called then existing buffers are freed. However, if that
happens without ever having started STREAMON, but if buffers have been queued,
then the buf_finish op is never called.

Add a call to __vb2_queue_cancel in __reqbufs so that these buffers are
cleaned up there as well.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NSakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

In commit 02f142ec support was added to
start_streaming to return -ENOBUFS if insufficient buffers were queued
for the DMA engine to start. The vb2 core would attempt calling
start_streaming again if another buffer would be queued up.

Later analysis uncovered problems with the queue management if start_streaming
would return an error: the buffers are enqueued to the driver before the
start_streaming op is called, so after an error they are never returned to
the vb2 core. The solution for this is to let the driver return them to
the vb2 core in case of an error while starting the DMA engine. However,
in the case of -ENOBUFS that would be weird: it is not a real error, it
just says that more buffers are needed. Requiring start_streaming to give
them back only to have them requeued again the next time the application
calls QBUF is inefficient.

This patch changes this mechanism: it adds a 'min_buffers_needed' field
to vb2_queue that drivers can set with the minimum number of buffers
required to start the DMA engine. The start_streaming op is only called
if enough buffers are queued. The -ENOBUFS handling has been dropped in
favor of this new method.

Drivers are expected to return buffers back to vb2 core with state QUEUED
if start_streaming would return an error. The vb2 core checks for this
and produces a warning if that didn't happen and it will forcefully
reclaim such buffers to ensure that the internal vb2 core state remains
consistent and all buffer-related resources have been correctly freed
and all op calls have been balanced.

__reqbufs() has been updated to check that at least min_buffers_needed
buffers could be allocated. If fewer buffers were allocated then __reqbufs
will free what was allocated and return -ENOMEM. Based on a suggestion from
Pawel Osciak.

__create_bufs() doesn't do that check, since the use of __create_bufs
assumes some advance scenario where the user might want more control.
Instead streamon will check if enough buffers were allocated to prevent
streaming with fewer than the minimum required number of buffers.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

__vb2_queue_free() would init the queued_list at all times, even if
q->num_buffers > 0. This should only happen if num_buffers == 0.

This situation can happen if a CREATE_BUFFERS call couldn't allocate
enough buffers and had to free those it did manage to allocate before
returning an error.

While we're at it: __vb2_queue_alloc() returns the number of buffers
allocated, not an error code. So stick the result in allocated_buffers
instead of ret as that's very confusing.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>

'queued_count' is a bit vague since it is not clear to which queue it
refers to: the vb2 internal list of buffers or the driver-owned list
of buffers.

Rename to make it explicit.
Signed-off-by: NHans Verkuil <hans.verkuil@cisco.com>
Acked-by: NPawel Osciak <pawel@osciak.com>
Acked-by: NLaurent Pinchart <laurent.pinchart@ideasonboard.com>
Signed-off-by: NMauro Carvalho Chehab <m.chehab@samsung.com>