RE: [RFCv4 PATCH 7/8] vb2: return ENODATA in start_streaming in case of too few buffers.

2013-12-11 Thread Kamil Debski 
Hi,

 From: Hans Verkuil [mailto:hverk...@xs4all.nl]
 Sent: Tuesday, December 10, 2013 8:52 AM
 
 As Guennadi mentioned in his review, ENODATA will be replaced by
 ENOBUFS, which is more appropriate.
 
 Prabhakar, Kamil, Tomasz, are you OK with this patch provided
 s/ENODATA/ENOBUFS/ ?

The patch looks good. However, shouldn't the documentation be changed too?

Now it says: [1]
(...) Accordingly the output hardware is disabled, no video signal is
produced until VIDIOC_STREAMON has been called. The ioctl will succeed
only when at least one output buffer is in the incoming queue. (...)

If I understand correctly, now the ioctl will succeed with no buffers
queued. Apart from the above you have my ack.

Acked-by: Kamil Debski k.deb...@samsung.com

Best wishes,
-- 
Kamil Debski
Samsung RD Institute Poland

[1] - http://linuxtv.org/downloads/v4l-dvb-apis/vidioc-streamon.html

 
 On 12/09/2013 02:43 PM, Hans Verkuil wrote:
  From: Hans Verkuil hans.verk...@cisco.com
 
  This works together with the retry_start_streaming mechanism to allow
  userspace to start streaming even if not all required buffers have
 been queued.
 
  Signed-off-by: Hans Verkuil hans.verk...@cisco.com
  Cc: Lad, Prabhakar prabhakar.cse...@gmail.com
  Cc: Tomasz Stanislawski t.stanisl...@samsung.com
  Cc: Kyungmin Park kyungmin.p...@samsung.com
  Cc: Kamil Debski k.deb...@samsung.com
  Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
  ---
   drivers/media/platform/davinci/vpbe_display.c   | 2 +-
   drivers/media/platform/davinci/vpif_capture.c   | 2 +-
   drivers/media/platform/davinci/vpif_display.c   | 2 +-
   drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 2 +-
   drivers/media/platform/s5p-tv/mixer_video.c | 2 +-
   drivers/media/platform/soc_camera/mx2_camera.c  | 2 +-
  drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 ++
   7 files changed, 8 insertions(+), 6 deletions(-)
 
  diff --git a/drivers/media/platform/davinci/vpbe_display.c
  b/drivers/media/platform/davinci/vpbe_display.c
  index eac472b..53be7fc 100644
  --- a/drivers/media/platform/davinci/vpbe_display.c
  +++ b/drivers/media/platform/davinci/vpbe_display.c
  @@ -347,7 +347,7 @@ static int vpbe_start_streaming(struct vb2_queue
 *vq, unsigned int count)
  /* If buffer queue is empty, return error */
  if (list_empty(layer-dma_queue)) {
  v4l2_err(vpbe_dev-v4l2_dev, buffer queue is empty\n);
  -   return -EINVAL;
  +   return -ENODATA;
  }
  /* Get the next frame from the buffer queue */
  layer-next_frm = layer-cur_frm = list_entry(layer-
 dma_queue.next,
  diff --git a/drivers/media/platform/davinci/vpif_capture.c
  b/drivers/media/platform/davinci/vpif_capture.c
  index 52ac5e6..4b04a27 100644
  --- a/drivers/media/platform/davinci/vpif_capture.c
  +++ b/drivers/media/platform/davinci/vpif_capture.c
  @@ -277,7 +277,7 @@ static int vpif_start_streaming(struct vb2_queue
 *vq, unsigned int count)
  if (list_empty(common-dma_queue)) {
  spin_unlock_irqrestore(common-irqlock, flags);
  vpif_dbg(1, debug, buffer queue is empty\n);
  -   return -EIO;
  +   return -ENODATA;
  }
 
  /* Get the next frame from the buffer queue */ diff --git
  a/drivers/media/platform/davinci/vpif_display.c
  b/drivers/media/platform/davinci/vpif_display.c
  index c31bcf1..c5070dc 100644
  --- a/drivers/media/platform/davinci/vpif_display.c
  +++ b/drivers/media/platform/davinci/vpif_display.c
  @@ -239,7 +239,7 @@ static int vpif_start_streaming(struct vb2_queue
 *vq, unsigned int count)
  if (list_empty(common-dma_queue)) {
  spin_unlock_irqrestore(common-irqlock, flags);
  vpif_err(buffer queue is empty\n);
  -   return -EIO;
  +   return -ENODATA;
  }
 
  /* Get the next frame from the buffer queue */ diff --git
  a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
  b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
  index 4ff3b6c..3bdfe85 100644
  --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
  +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
  @@ -1863,7 +1863,7 @@ static int s5p_mfc_start_streaming(struct
 vb2_queue *q, unsigned int count)
  if (ctx-src_bufs_cnt  ctx-pb_count) {
  mfc_err(Need minimum %d OUTPUT buffers\n,
  ctx-pb_count);
  -   return -EINVAL;
  +   return -ENODATA;
  }
  }
 
  diff --git a/drivers/media/platform/s5p-tv/mixer_video.c
  b/drivers/media/platform/s5p-tv/mixer_video.c
  index 81b97db..220ec31 100644
  --- a/drivers/media/platform/s5p-tv/mixer_video.c
  +++ b/drivers/media/platform/s5p-tv/mixer_video.c
  @@ -948,7 +948,7 @@ static int start_streaming(struct vb2_queue *vq,
  unsigned int count)
 
  if (count == 0) {
  mxr_dbg(mdev, no output buffers queued\n);
  -   return -EINVAL;
  +   return -ENODATA;
  }
 
  /* block any changes in output

Re: [RFCv4 PATCH 7/8] vb2: return ENODATA in start_streaming in case of too few buffers.

2013-12-11 Thread Hans Verkuil 
Hi Kamil,

On 12/11/13 11:27, Kamil Debski wrote:
 Hi,
 
 From: Hans Verkuil [mailto:hverk...@xs4all.nl]
 Sent: Tuesday, December 10, 2013 8:52 AM

 As Guennadi mentioned in his review, ENODATA will be replaced by
 ENOBUFS, which is more appropriate.

 Prabhakar, Kamil, Tomasz, are you OK with this patch provided
 s/ENODATA/ENOBUFS/ ?
 
 The patch looks good. However, shouldn't the documentation be changed too?
 
 Now it says: [1]
 (...) Accordingly the output hardware is disabled, no video signal is
 produced until VIDIOC_STREAMON has been called. The ioctl will succeed
 only when at least one output buffer is in the incoming queue. (...)
 
 If I understand correctly, now the ioctl will succeed with no buffers
 queued.

That's true *only* for drivers using vb2. As long as not all drivers are
converted (which is a *very* long-term project) I don't think i can change
the documentation.

Regards,

Hans

 Apart from the above you have my ack.
 
 Acked-by: Kamil Debski k.deb...@samsung.com
 
 Best wishes,
 
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv4 PATCH 7/8] vb2: return ENODATA in start_streaming in case of too few buffers.

2013-12-10 Thread Prabhakar Lad 
Hi Hans,

On Tue, Dec 10, 2013 at 1:21 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 As Guennadi mentioned in his review, ENODATA will be replaced by ENOBUFS, 
 which is
 more appropriate.

 Prabhakar, Kamil, Tomasz, are you OK with this patch provided 
 s/ENODATA/ENOBUFS/ ?

+1 for ENOBUFS.

Regards,
--Prabhakar Lad

 Regards,

 Hans

 On 12/09/2013 02:43 PM, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com

 This works together with the retry_start_streaming mechanism to allow 
 userspace
 to start streaming even if not all required buffers have been queued.

 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Lad, Prabhakar prabhakar.cse...@gmail.com
 Cc: Tomasz Stanislawski t.stanisl...@samsung.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Kamil Debski k.deb...@samsung.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---
  drivers/media/platform/davinci/vpbe_display.c   | 2 +-
  drivers/media/platform/davinci/vpif_capture.c   | 2 +-
  drivers/media/platform/davinci/vpif_display.c   | 2 +-
  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 2 +-
  drivers/media/platform/s5p-tv/mixer_video.c | 2 +-
  drivers/media/platform/soc_camera/mx2_camera.c  | 2 +-
  drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 ++
  7 files changed, 8 insertions(+), 6 deletions(-)

 diff --git a/drivers/media/platform/davinci/vpbe_display.c 
 b/drivers/media/platform/davinci/vpbe_display.c
 index eac472b..53be7fc 100644
 --- a/drivers/media/platform/davinci/vpbe_display.c
 +++ b/drivers/media/platform/davinci/vpbe_display.c
 @@ -347,7 +347,7 @@ static int vpbe_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   /* If buffer queue is empty, return error */
   if (list_empty(layer-dma_queue)) {
   v4l2_err(vpbe_dev-v4l2_dev, buffer queue is empty\n);
 - return -EINVAL;
 + return -ENODATA;
   }
   /* Get the next frame from the buffer queue */
   layer-next_frm = layer-cur_frm = list_entry(layer-dma_queue.next,
 diff --git a/drivers/media/platform/davinci/vpif_capture.c 
 b/drivers/media/platform/davinci/vpif_capture.c
 index 52ac5e6..4b04a27 100644
 --- a/drivers/media/platform/davinci/vpif_capture.c
 +++ b/drivers/media/platform/davinci/vpif_capture.c
 @@ -277,7 +277,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   if (list_empty(common-dma_queue)) {
   spin_unlock_irqrestore(common-irqlock, flags);
   vpif_dbg(1, debug, buffer queue is empty\n);
 - return -EIO;
 + return -ENODATA;
   }

   /* Get the next frame from the buffer queue */
 diff --git a/drivers/media/platform/davinci/vpif_display.c 
 b/drivers/media/platform/davinci/vpif_display.c
 index c31bcf1..c5070dc 100644
 --- a/drivers/media/platform/davinci/vpif_display.c
 +++ b/drivers/media/platform/davinci/vpif_display.c
 @@ -239,7 +239,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   if (list_empty(common-dma_queue)) {
   spin_unlock_irqrestore(common-irqlock, flags);
   vpif_err(buffer queue is empty\n);
 - return -EIO;
 + return -ENODATA;
   }

   /* Get the next frame from the buffer queue */
 diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c 
 b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
 index 4ff3b6c..3bdfe85 100644
 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
 +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
 @@ -1863,7 +1863,7 @@ static int s5p_mfc_start_streaming(struct vb2_queue 
 *q, unsigned int count)
   if (ctx-src_bufs_cnt  ctx-pb_count) {
   mfc_err(Need minimum %d OUTPUT buffers\n,
   ctx-pb_count);
 - return -EINVAL;
 + return -ENODATA;
   }
   }

 diff --git a/drivers/media/platform/s5p-tv/mixer_video.c 
 b/drivers/media/platform/s5p-tv/mixer_video.c
 index 81b97db..220ec31 100644
 --- a/drivers/media/platform/s5p-tv/mixer_video.c
 +++ b/drivers/media/platform/s5p-tv/mixer_video.c
 @@ -948,7 +948,7 @@ static int start_streaming(struct vb2_queue *vq, 
 unsigned int count)

   if (count == 0) {
   mxr_dbg(mdev, no output buffers queued\n);
 - return -EINVAL;
 + return -ENODATA;
   }

   /* block any changes in output configuration */
 diff --git a/drivers/media/platform/soc_camera/mx2_camera.c 
 b/drivers/media/platform/soc_camera/mx2_camera.c
 index 45a0276..587e3d1 100644
 --- a/drivers/media/platform/soc_camera/mx2_camera.c
 +++ b/drivers/media/platform/soc_camera/mx2_camera.c
 @@ -659,7 +659,7 @@ static int mx2_start_streaming(struct vb2_queue *q, 
 unsigned int count)
   unsigned long flags;

   if (count  2)
 - return -EINVAL;
 + return -ENODATA;

   spin_lock_irqsave(pcdev-lock, flags);

 diff --git

Re: [RFCv4 PATCH 7/8] vb2: return ENODATA in start_streaming in case of too few buffers.

2013-12-10 Thread Prabhakar Lad 
On Tue, Dec 10, 2013 at 3:26 PM, Prabhakar Lad
prabhakar.cse...@gmail.com wrote:
 Hi Hans,

 On Tue, Dec 10, 2013 at 1:21 PM, Hans Verkuil hverk...@xs4all.nl wrote:
 As Guennadi mentioned in his review, ENODATA will be replaced by ENOBUFS, 
 which is
 more appropriate.

 Prabhakar, Kamil, Tomasz, are you OK with this patch provided 
 s/ENODATA/ENOBUFS/ ?

 +1 for ENOBUFS.

Acked-by: Lad, Prabhakar prabhakar.cse...@gmail.com

Regrads,
--Prabhakar Lad
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv4 PATCH 7/8] vb2: return ENODATA in start_streaming in case of too few buffers.

2013-12-09 Thread Guennadi Liakhovetski 
On Mon, 9 Dec 2013, Hans Verkuil wrote:

 From: Hans Verkuil hans.verk...@cisco.com
 
 This works together with the retry_start_streaming mechanism to allow 
 userspace
 to start streaming even if not all required buffers have been queued.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Lad, Prabhakar prabhakar.cse...@gmail.com
 Cc: Tomasz Stanislawski t.stanisl...@samsung.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Kamil Debski k.deb...@samsung.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---

[snip]

  drivers/media/platform/soc_camera/mx2_camera.c  | 2 +-

Provided ENOBUFS is used instead of ENODATA:

Acked-by: Guennadi Liakhovetski g.liakhovet...@gmx.de

Thanks
Guennadi
---
Guennadi Liakhovetski, Ph.D.
Freelance Open-Source Software Developer
http://www.open-technology.de/
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [RFCv4 PATCH 7/8] vb2: return ENODATA in start_streaming in case of too few buffers.

2013-12-09 Thread Hans Verkuil 
As Guennadi mentioned in his review, ENODATA will be replaced by ENOBUFS, which 
is
more appropriate.

Prabhakar, Kamil, Tomasz, are you OK with this patch provided 
s/ENODATA/ENOBUFS/ ?

Regards,

Hans

On 12/09/2013 02:43 PM, Hans Verkuil wrote:
 From: Hans Verkuil hans.verk...@cisco.com
 
 This works together with the retry_start_streaming mechanism to allow 
 userspace
 to start streaming even if not all required buffers have been queued.
 
 Signed-off-by: Hans Verkuil hans.verk...@cisco.com
 Cc: Lad, Prabhakar prabhakar.cse...@gmail.com
 Cc: Tomasz Stanislawski t.stanisl...@samsung.com
 Cc: Kyungmin Park kyungmin.p...@samsung.com
 Cc: Kamil Debski k.deb...@samsung.com
 Cc: Guennadi Liakhovetski g.liakhovet...@gmx.de
 ---
  drivers/media/platform/davinci/vpbe_display.c   | 2 +-
  drivers/media/platform/davinci/vpif_capture.c   | 2 +-
  drivers/media/platform/davinci/vpif_display.c   | 2 +-
  drivers/media/platform/s5p-mfc/s5p_mfc_enc.c| 2 +-
  drivers/media/platform/s5p-tv/mixer_video.c | 2 +-
  drivers/media/platform/soc_camera/mx2_camera.c  | 2 +-
  drivers/staging/media/davinci_vpfe/vpfe_video.c | 2 ++
  7 files changed, 8 insertions(+), 6 deletions(-)
 
 diff --git a/drivers/media/platform/davinci/vpbe_display.c 
 b/drivers/media/platform/davinci/vpbe_display.c
 index eac472b..53be7fc 100644
 --- a/drivers/media/platform/davinci/vpbe_display.c
 +++ b/drivers/media/platform/davinci/vpbe_display.c
 @@ -347,7 +347,7 @@ static int vpbe_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   /* If buffer queue is empty, return error */
   if (list_empty(layer-dma_queue)) {
   v4l2_err(vpbe_dev-v4l2_dev, buffer queue is empty\n);
 - return -EINVAL;
 + return -ENODATA;
   }
   /* Get the next frame from the buffer queue */
   layer-next_frm = layer-cur_frm = list_entry(layer-dma_queue.next,
 diff --git a/drivers/media/platform/davinci/vpif_capture.c 
 b/drivers/media/platform/davinci/vpif_capture.c
 index 52ac5e6..4b04a27 100644
 --- a/drivers/media/platform/davinci/vpif_capture.c
 +++ b/drivers/media/platform/davinci/vpif_capture.c
 @@ -277,7 +277,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   if (list_empty(common-dma_queue)) {
   spin_unlock_irqrestore(common-irqlock, flags);
   vpif_dbg(1, debug, buffer queue is empty\n);
 - return -EIO;
 + return -ENODATA;
   }
  
   /* Get the next frame from the buffer queue */
 diff --git a/drivers/media/platform/davinci/vpif_display.c 
 b/drivers/media/platform/davinci/vpif_display.c
 index c31bcf1..c5070dc 100644
 --- a/drivers/media/platform/davinci/vpif_display.c
 +++ b/drivers/media/platform/davinci/vpif_display.c
 @@ -239,7 +239,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, 
 unsigned int count)
   if (list_empty(common-dma_queue)) {
   spin_unlock_irqrestore(common-irqlock, flags);
   vpif_err(buffer queue is empty\n);
 - return -EIO;
 + return -ENODATA;
   }
  
   /* Get the next frame from the buffer queue */
 diff --git a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c 
 b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
 index 4ff3b6c..3bdfe85 100644
 --- a/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
 +++ b/drivers/media/platform/s5p-mfc/s5p_mfc_enc.c
 @@ -1863,7 +1863,7 @@ static int s5p_mfc_start_streaming(struct vb2_queue *q, 
 unsigned int count)
   if (ctx-src_bufs_cnt  ctx-pb_count) {
   mfc_err(Need minimum %d OUTPUT buffers\n,
   ctx-pb_count);
 - return -EINVAL;
 + return -ENODATA;
   }
   }
  
 diff --git a/drivers/media/platform/s5p-tv/mixer_video.c 
 b/drivers/media/platform/s5p-tv/mixer_video.c
 index 81b97db..220ec31 100644
 --- a/drivers/media/platform/s5p-tv/mixer_video.c
 +++ b/drivers/media/platform/s5p-tv/mixer_video.c
 @@ -948,7 +948,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned 
 int count)
  
   if (count == 0) {
   mxr_dbg(mdev, no output buffers queued\n);
 - return -EINVAL;
 + return -ENODATA;
   }
  
   /* block any changes in output configuration */
 diff --git a/drivers/media/platform/soc_camera/mx2_camera.c 
 b/drivers/media/platform/soc_camera/mx2_camera.c
 index 45a0276..587e3d1 100644
 --- a/drivers/media/platform/soc_camera/mx2_camera.c
 +++ b/drivers/media/platform/soc_camera/mx2_camera.c
 @@ -659,7 +659,7 @@ static int mx2_start_streaming(struct vb2_queue *q, 
 unsigned int count)
   unsigned long flags;
  
   if (count  2)
 - return -EINVAL;
 + return -ENODATA;
  
   spin_lock_irqsave(pcdev-lock, flags);
  
 diff --git a/drivers/staging/media/davinci_vpfe/vpfe_video.c 
 b/drivers/staging/media/davinci_vpfe/vpfe_video.c
 index 24d98a6..a81b0ab 100644
 ---