[media] vb2: only call start_streaming if sufficient buffers are queued

In commit 02f142ecd24aaf891324ffba8527284c1731b561 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: Hans Verkuil <hans.verkuil@cisco.com>
Signed-off-by: Mauro Carvalho Chehab <m.chehab@samsung.com>

4 files changed, 4 insertions, 22 deletions

diff --git a/drivers/media/platform/davinci/vpbe_display.c b/drivers/media/platform/davinci/vpbe_display.c
index e512767cf7ea..7a0e40ee60e3 100644
--- a/drivers/media/platform/davinci/vpbe_display.c
+++ b/drivers/media/platform/davinci/vpbe_display.c
@@ -344,11 +344,6 @@ static int vpbe_start_streaming(struct vb2_queue *vq, unsigned int count)
 	struct vpbe_device *vpbe_dev = fh->disp_dev->vpbe_dev;
 	int ret;
 
-	/* 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 -ENOBUFS;
-	}
 	/* Get the next frame from the buffer queue */
 	layer->next_frm = layer->cur_frm = list_entry(layer->dma_queue.next,
 				struct vpbe_disp_buffer, list);
@@ -1416,6 +1411,7 @@ static int vpbe_display_reqbufs(struct file *file, void *priv,
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct vpbe_disp_buffer);
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 1;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index cd6da8b78108..756da78bac23 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -272,13 +272,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	unsigned long flags;
 	int ret;
 
-	/* If buffer queue is empty, return error */
 	spin_lock_irqsave(&common->irqlock, flags);
-	if (list_empty(&common->dma_queue)) {
-		spin_unlock_irqrestore(&common->irqlock, flags);
-		vpif_dbg(1, debug, "buffer queue is empty\n");
-		return -ENOBUFS;
-	}
 
 	/* Get the next frame from the buffer queue */
 	common->cur_frm = common->next_frm = list_entry(common->dma_queue.next,
@@ -1024,6 +1018,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct vpif_cap_buffer);
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 1;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index fd68236657c2..0ac841e35aa4 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -234,13 +234,7 @@ static int vpif_start_streaming(struct vb2_queue *vq, unsigned int count)
 	unsigned long flags;
 	int ret;
 
-	/* If buffer queue is empty, return error */
 	spin_lock_irqsave(&common->irqlock, flags);
-	if (list_empty(&common->dma_queue)) {
-		spin_unlock_irqrestore(&common->irqlock, flags);
-		vpif_err("buffer queue is empty\n");
-		return -ENOBUFS;
-	}
 
 	/* Get the next frame from the buffer queue */
 	common->next_frm = common->cur_frm =
@@ -984,6 +978,7 @@ static int vpif_reqbufs(struct file *file, void *priv,
 	q->mem_ops = &vb2_dma_contig_memops;
 	q->buf_struct_size = sizeof(struct vpif_disp_buffer);
 	q->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_MONOTONIC;
+	q->min_buffers_needed = 1;
 
 	ret = vb2_queue_init(q);
 	if (ret) {
diff --git a/drivers/media/platform/s5p-tv/mixer_video.c b/drivers/media/platform/s5p-tv/mixer_video.c
index c5059ba0d733..a1ce55fd30f3 100644
--- a/drivers/media/platform/s5p-tv/mixer_video.c
+++ b/drivers/media/platform/s5p-tv/mixer_video.c
@@ -946,11 +946,6 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 
 	mxr_dbg(mdev, "%s\n", __func__);
 
-	if (count == 0) {
-		mxr_dbg(mdev, "no output buffers queued\n");
-		return -ENOBUFS;
-	}
-
 	/* block any changes in output configuration */
 	mxr_output_get(mdev);
 
@@ -1124,6 +1119,7 @@ struct mxr_layer *mxr_base_layer_create(struct mxr_device *mdev,
 		.drv_priv = layer,
 		.buf_struct_size = sizeof(struct mxr_buffer),
 		.ops = &mxr_video_qops,
+		.min_buffers_needed = 1,
 		.mem_ops = &vb2_dma_contig_memops,
 	};