1. 04 12月, 2014 2 次提交
  2. 25 11月, 2014 2 次提交
  3. 22 9月, 2014 5 次提交
  4. 03 9月, 2014 1 次提交
  5. 22 8月, 2014 2 次提交
    • H
      [media] vb2: use pr_info instead of pr_debug · 0821344d
      Hans Verkuil 提交于
      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>
      0821344d
    • H
      [media] videobuf2: fix lockdep warning · f035eb4e
      Hans Verkuil 提交于
      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>
      f035eb4e
  6. 26 7月, 2014 1 次提交
  7. 22 7月, 2014 1 次提交
    • H
      [media] vb2: fix bytesused == 0 handling · 8a75ffb8
      Hans Verkuil 提交于
      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>
      8a75ffb8
  8. 18 7月, 2014 1 次提交
    • L
      [media] v4l: vb2: Add fatal error condition flag · 4bb7267d
      Laurent Pinchart 提交于
      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>
      4bb7267d
  9. 17 7月, 2014 2 次提交
  10. 25 5月, 2014 2 次提交
  11. 24 5月, 2014 2 次提交
  12. 23 4月, 2014 2 次提交
    • H
      [media] vb2: fix compiler warning · ce9c2244
      Hans Verkuil 提交于
      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>
      ce9c2244
    • H
      [media] vb2: stop_streaming should return void · e37559b2
      Hans Verkuil 提交于
      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>
      e37559b2
  13. 17 4月, 2014 12 次提交
  14. 11 3月, 2014 5 次提交
    • H
      [media] vb2: call buf_finish after the state check · 9cf3c31a
      Hans Verkuil 提交于
      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>
      9cf3c31a
    • H
      [media] vb2: fix streamoff handling if streamon wasn't called · 3f1a9a33
      Hans Verkuil 提交于
      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>
      3f1a9a33
    • H
      [media] vb2: replace BUG by WARN_ON · e4d25816
      Hans Verkuil 提交于
      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>
      e4d25816
    • H
      [media] vb2: properly clean up PREPARED and QUEUED buffers · fb64dca8
      Hans Verkuil 提交于
      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>
      fb64dca8
    • H
      [media] vb2: only call start_streaming if sufficient buffers are queued · b3379c62
      Hans Verkuil 提交于
      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>
      b3379c62