[PATCH 00/31] staging: bcm2835-camera: Improvements

[PATCH 00/31] staging: bcm2835-camera: Improvements

[Stefan Wahren]

This is an attempt to help Dave Stevenson to get all the fixes and
improvements of the bcm2835-camera driver into mainline.

Mostly i only polished the commit logs for upstream.

The series based on the latest bugfix V2 of staging: bcm2835-camera: Restore
return behavior of ctrl_set_bitrate().

Dave Stevenson (31):
  staging: bcm2835-camera: Ensure H264 header bytes get a sensible
    timestamp
  staging: bcm2835-camera: Check the error for REPEAT_SEQ_HEADER
  staging: bcm2835-camera: Replace spinlock protecting context_map with
    mutex
  staging: bcm2835-camera: Do not bulk receive from service thread
  staging: bcm2835-camera: Correctly denote key frames in encoded data
  staging: bcm2835-camera: Return early on errors
  staging: bcm2835-camera: Remove dead email addresses
  staging: bcm2835-camera: Fix comment style violations.
  staging: bcm2835-camera: Fix spacing around operators
  staging: bcm2835-camera: Reduce length of enum names
  staging: bcm2835-camera: Fix multiple line dereference errors
  staging: bcm2835-camera: Fix brace style issues.
  staging: bcm2835-camera: Fix missing lines between items
  staging: bcm2835-camera: Fix open parenthesis alignment
  staging: bcm2835-camera: Ensure all buffers are returned on disable
  staging: bcm2835-camera: Remove check of the number of buffers
    supplied
  staging: bcm2835-camera: Handle empty EOS buffers whilst streaming
  staging: bcm2835-camera: Set sequence number correctly
  staging: bcm2835-camera: Ensure timestamps never go backwards.
  staging: bcm2835-camera: Add multiple inclusion protection to headers
  staging: bcm2835-camera: Unify header inclusion defines
  staging: bcm2835-camera: Fix multiple assignments should be avoided
  staging: bcm2835-camera: Fix up mmal-parameters.h
  staging: bcm2835-camera: Use enums for max value in controls
  staging: bcm2835-camera: Correct V4L2_CID_COLORFX_CBCR behaviour
  staging: bcm2835-camera: Remove/amend some obsolete comments
  staging: mmal-vchiq: Avoid use of bool in structures
  staging: bcm2835-camera: Fix stride on RGB3/BGR3 formats
  staging: bcm2835-camera: Add sanity checks for queue_setup/CREATE_BUFS
  staging: bcm2835-camera: Set the field value within ach buffer
  staging: bcm2835-camera: Correct ctrl min/max/step/def to 64bit

 .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 378 ++++++++++++---------
 .../vc04_services/bcm2835-camera/bcm2835-camera.h  |  34 +-
 .../vc04_services/bcm2835-camera/controls.c        | 184 +++++-----
 .../vc04_services/bcm2835-camera/mmal-common.h     |  12 +-
 .../vc04_services/bcm2835-camera/mmal-encodings.h  |   9 +-
 .../vc04_services/bcm2835-camera/mmal-msg-common.h |   9 +-
 .../vc04_services/bcm2835-camera/mmal-msg-format.h | 104 +++---
 .../vc04_services/bcm2835-camera/mmal-msg-port.h   | 133 ++++----
 .../vc04_services/bcm2835-camera/mmal-msg.h        | 150 ++++----
 .../vc04_services/bcm2835-camera/mmal-parameters.h | 286 +++++++++-------
 .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 159 +++++----
 .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  22 +-
 12 files changed, 826 insertions(+), 654 deletions(-)

--
2.7.4

[PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

H264 header come from VC with 0 timestamps, which means they get a
strange timestamp when processed with VC/kernel start times,
particularly if used with the inline header option.
Remember the last frame timestamp and use that if set, or otherwise
use the kernel start time.

Link: https://github.com/raspberrypi/linux/issues/1836
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 14 ++++++++++++--
 .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h  |  2 ++
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index dce6e6d..0c04815 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 		}
 	} else {
 		if (dev->capture.frame_count) {
-			if (dev->capture.vc_start_timestamp != -1 && pts) {
+			if (dev->capture.vc_start_timestamp != -1) {
+				buf->vb.vb2_buf.timestamp = ktime_get_ns();
+			} else if (pts) {
 				ktime_t timestamp;
 				s64 runtime_us = pts -
 				    dev->capture.vc_start_timestamp;
@@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 					 ktime_to_ns(timestamp));
 				buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
 			} else {
-				buf->vb.vb2_buf.timestamp = ktime_get_ns();
+				if (dev->capture.last_timestamp) {
+					buf->vb.vb2_buf.timestamp =
+						dev->capture.last_timestamp;
+				} else {
+					buf->vb.vb2_buf.timestamp =
+						ktime_to_ns(dev->capture.kernel_start_ts);
+				}
 			}
+			dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;

 			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
 			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
@@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 			 dev->capture.vc_start_timestamp, parameter_size);

 	dev->capture.kernel_start_ts = ktime_get();
+	dev->capture.last_timestamp = 0;

 	/* enable the camera port */
 	dev->capture.port->cb_ctx = dev;
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
index 2b5679e..09273b0 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
@@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
 		s64         vc_start_timestamp;
 		/* Kernel start timestamp for streaming */
 		ktime_t kernel_start_ts;
+		/* Timestamp of last frame */
+		u64		last_timestamp;

 		struct vchiq_mmal_port  *port; /* port being used for capture */
 		/* camera port being used for capture */
--
2.7.4

[PATCH 02/31] staging: bcm2835-camera: Check the error for REPEAT_SEQ_HEADER

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

When handling for V4L2_CID_MPEG_VIDEO_REPEAT_SEQ_HEADER was added
the firmware would reject the setting if H264 hadn't already been
selected. This was fixed in the firmware at that point, but to
enable backwards compatibility the returned error was ignored.

That was Dec 2013, so the chances of having a firmware that still
has that issue is so close to zero that the workaround can be
removed.

Link: https://github.com/raspberrypi/linux/pull/2782/
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/controls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index c251164..133aa6e 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -1124,7 +1124,7 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 		0, 1, NULL,
 		MMAL_PARAMETER_VIDEO_ENCODE_INLINE_HEADER,
 		ctrl_set_video_encode_param_output,
-		true	/* Errors ignored as requires latest firmware to work */
+		false
 	},
 	{
 		V4L2_CID_MPEG_VIDEO_H264_PROFILE,
--
2.7.4

[PATCH 03/31] staging: bcm2835-camera: Replace spinlock protecting context_map with mutex

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

The commit "staging: bcm2835-camera: Replace open-coded idr with a struct idr."
replaced an internal implementation of an idr with the standard functions
and a spinlock. idr_alloc(GFP_KERNEL) can sleep whilst calling kmem_cache_alloc
to allocate the new node, but this is not valid whilst in an atomic context
due to the spinlock.

There is no need for this to be a spinlock as a standard mutex is
sufficient.

Fixes: 950fd867c635 ("staging: bcm2835-camera: Replace open-coded idr with a struct idr.")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 16af735..f1bb900 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -161,7 +161,8 @@ struct vchiq_mmal_instance {
 	void *bulk_scratch;

 	struct idr context_map;
-	spinlock_t context_map_lock;
+	/* protect accesses to context_map */
+	struct mutex context_map_lock;

 	/* component to use next */
 	int component_idx;
@@ -184,10 +185,10 @@ get_msg_context(struct vchiq_mmal_instance *instance)
 	 * that when we service the VCHI reply, we can look up what
 	 * message is being replied to.
 	 */
-	spin_lock(&instance->context_map_lock);
+	mutex_lock(&instance->context_map_lock);
 	handle = idr_alloc(&instance->context_map, msg_context,
 			   0, 0, GFP_KERNEL);
-	spin_unlock(&instance->context_map_lock);
+	mutex_unlock(&instance->context_map_lock);

 	if (handle < 0) {
 		kfree(msg_context);
@@ -211,9 +212,9 @@ release_msg_context(struct mmal_msg_context *msg_context)
 {
 	struct vchiq_mmal_instance *instance = msg_context->instance;

-	spin_lock(&instance->context_map_lock);
+	mutex_lock(&instance->context_map_lock);
 	idr_remove(&instance->context_map, msg_context->handle);
-	spin_unlock(&instance->context_map_lock);
+	mutex_unlock(&instance->context_map_lock);
 	kfree(msg_context);
 }

@@ -1849,7 +1850,7 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)

 	instance->bulk_scratch = vmalloc(PAGE_SIZE);

-	spin_lock_init(&instance->context_map_lock);
+	mutex_init(&instance->context_map_lock);
 	idr_init_base(&instance->context_map, 1);

 	params.callback_param = instance;
--
2.7.4

[PATCH 04/31] staging: bcm2835-camera: Do not bulk receive from service thread

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

vchi_bulk_queue_receive will queue up to a default of 4
bulk receives on a connection before blocking.
If called from the VCHI service_callback thread, then
that thread is unable to service the VCHI_CALLBACK_BULK_RECEIVED
events that would enable the queue call to succeed.

Add a workqueue to schedule the call vchi_bulk_queue_receive
in an alternate context to avoid the lock up.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 101 ++++++++++++---------
 1 file changed, 59 insertions(+), 42 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index f1bb900..1a343d8 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -117,8 +117,10 @@ struct mmal_msg_context {

 	union {
 		struct {
-			/* work struct for defered callback - must come first */
+			/* work struct for buffer_cb callback */
 			struct work_struct work;
+			/* work struct for deferred callback */
+			struct work_struct buffer_to_host_work;
 			/* mmal instance */
 			struct vchiq_mmal_instance *instance;
 			/* mmal port */
@@ -167,6 +169,9 @@ struct vchiq_mmal_instance {
 	/* component to use next */
 	int component_idx;
 	struct vchiq_mmal_component component[VCHIQ_MMAL_MAX_COMPONENTS];
+
+	/* ordered workqueue to process all bulk operations */
+	struct workqueue_struct *bulk_wq;
 };

 static struct mmal_msg_context *
@@ -248,7 +253,44 @@ static void buffer_work_cb(struct work_struct *work)
 					    msg_context->u.bulk.mmal_flags,
 					    msg_context->u.bulk.dts,
 					    msg_context->u.bulk.pts);
+}

+/* workqueue scheduled callback to handle receiving buffers
+ *
+ * VCHI will allow up to 4 bulk receives to be scheduled before blocking.
+ * If we block in the service_callback context then we can't process the
+ * VCHI_CALLBACK_BULK_RECEIVED message that would otherwise allow the blocked
+ * vchi_bulk_queue_receive() call to complete.
+ */
+static void buffer_to_host_work_cb(struct work_struct *work)
+{
+	struct mmal_msg_context *msg_context =
+		container_of(work, struct mmal_msg_context,
+			     u.bulk.buffer_to_host_work);
+	struct vchiq_mmal_instance *instance = msg_context->instance;
+	unsigned long len = msg_context->u.bulk.buffer_used;
+	int ret;
+
+	if (!len)
+		/* Dummy receive to ensure the buffers remain in order */
+		len = 8;
+	/* queue the bulk submission */
+	vchi_service_use(instance->handle);
+	ret = vchi_bulk_queue_receive(instance->handle,
+				      msg_context->u.bulk.buffer->buffer,
+				      /* Actual receive needs to be a multiple
+				       * of 4 bytes
+				       */
+				      (len + 3) & ~3,
+				      VCHI_FLAGS_CALLBACK_WHEN_OP_COMPLETE |
+				      VCHI_FLAGS_BLOCK_UNTIL_QUEUED,
+				      msg_context);
+
+	vchi_service_release(instance->handle);
+
+	if (ret != 0)
+		pr_err("%s: ctx: %p, vchi_bulk_queue_receive failed %d\n",
+		       __func__, msg_context, ret);
 }

 /* enqueue a bulk receive for a given message context */
@@ -257,7 +299,6 @@ static int bulk_receive(struct vchiq_mmal_instance *instance,
 			struct mmal_msg_context *msg_context)
 {
 	unsigned long rd_len;
-	int ret;

 	rd_len = msg->u.buffer_from_host.buffer_header.length;

@@ -293,45 +334,10 @@ static int bulk_receive(struct vchiq_mmal_instance *instance,
 	msg_context->u.bulk.dts = msg->u.buffer_from_host.buffer_header.dts;
 	msg_context->u.bulk.pts = msg->u.buffer_from_host.buffer_header.pts;

-	/* queue the bulk submission */
-	vchi_service_use(instance->handle);
-	ret = vchi_bulk_queue_receive(instance->handle,
-				      msg_context->u.bulk.buffer->buffer,
-				      /* Actual receive needs to be a multiple
-				       * of 4 bytes
-				       */
-				      (rd_len + 3) & ~3,
-				      VCHI_FLAGS_CALLBACK_WHEN_OP_COMPLETE |
-				      VCHI_FLAGS_BLOCK_UNTIL_QUEUED,
-				      msg_context);
-
-	vchi_service_release(instance->handle);
+	queue_work(msg_context->instance->bulk_wq,
+		   &msg_context->u.bulk.buffer_to_host_work);

-	return ret;
-}
-
-/* enque a dummy bulk receive for a given message context */
-static int dummy_bulk_receive(struct vchiq_mmal_instance *instance,
-			      struct mmal_msg_context *msg_context)
-{
-	int ret;
-
-	/* zero length indicates this was a dummy transfer */
-	msg_context->u.bulk.buffer_used = 0;
-
-	/* queue the bulk submission */
-	vchi_service_use(instance->handle);
-
-	ret = vchi_bulk_queue_receive(instance->handle,
-				      instance->bulk_scratch,
-				      8,
-				      VCHI_FLAGS_CALLBACK_WHEN_OP_COMPLETE |
-				      VCHI_FLAGS_BLOCK_UNTIL_QUEUED,
-				      msg_context);
-
-	vchi_service_release(instance->handle);
-
-	return ret;
+	return 0;
 }

 /* data in message, memcpy from packet into output buffer */
@@ -379,6 +385,8 @@ buffer_from_host(struct vchiq_mmal_instance *instance,

 	/* initialise work structure ready to schedule callback */
 	INIT_WORK(&msg_context->u.bulk.work, buffer_work_cb);
+	INIT_WORK(&msg_context->u.bulk.buffer_to_host_work,
+		  buffer_to_host_work_cb);

 	/* prep the buffer from host message */
 	memset(&m, 0xbc, sizeof(m));	/* just to make debug clearer */
@@ -459,7 +467,7 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance *instance,
 		if (msg->u.buffer_from_host.buffer_header.flags &
 		    MMAL_BUFFER_HEADER_FLAG_EOS) {
 			msg_context->u.bulk.status =
-			    dummy_bulk_receive(instance, msg_context);
+			    bulk_receive(instance, msg, msg_context);
 			if (msg_context->u.bulk.status == 0)
 				return;	/* successful bulk submission, bulk
 					 * completion will trigger callback
@@ -1793,6 +1801,9 @@ int vchiq_mmal_finalise(struct vchiq_mmal_instance *instance)

 	mutex_unlock(&instance->vchiq_mutex);

+	flush_workqueue(instance->bulk_wq);
+	destroy_workqueue(instance->bulk_wq);
+
 	vfree(instance->bulk_scratch);

 	idr_destroy(&instance->context_map);
@@ -1855,6 +1866,11 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)

 	params.callback_param = instance;

+	instance->bulk_wq = alloc_ordered_workqueue("mmal-vchiq",
+						    WQ_MEM_RECLAIM);
+	if (!instance->bulk_wq)
+		goto err_free;
+
 	status = vchi_service_open(vchi_instance, &params, &instance->handle);
 	if (status) {
 		pr_err("Failed to open VCHI service connection (status=%d)\n",
@@ -1869,8 +1885,9 @@ int vchiq_mmal_init(struct vchiq_mmal_instance **out_instance)
 	return 0;

 err_close_services:
-
 	vchi_service_close(instance->handle);
+	destroy_workqueue(instance->bulk_wq);
+err_free:
 	vfree(instance->bulk_scratch);
 	kfree(instance);
 	return -ENODEV;
--
2.7.4

[PATCH 05/31] staging: bcm2835-camera: Correctly denote key frames in encoded data

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Forward MMAL key frame flags to the V4L2 buffers.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 0c04815..456b686 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -385,6 +385,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 			dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;

 			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
+			if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_KEYFRAME)
+				buf->vb.flags |= V4L2_BUF_FLAG_KEYFRAME;
+
 			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);

 			if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_EOS &&
--
2.7.4

[PATCH 06/31] staging: bcm2835-camera: Return early on errors

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix several instances where it is easier to return
early on error conditions than handle it as an else
clause. As requested by Mauro.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 109 +++++++++++----------
 1 file changed, 58 insertions(+), 51 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 456b686..b597475 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -334,7 +334,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
 		}
 		return;
-	} else if (length == 0) {
+	}
+
+	if (length == 0) {
 		/* stream ended */
 		if (buf) {
 			/* this should only ever happen if the port is
@@ -357,56 +359,59 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 			/* signal frame completion */
 			complete(&dev->capture.frame_cmplt);
 		}
-	} else {
-		if (dev->capture.frame_count) {
-			if (dev->capture.vc_start_timestamp != -1) {
-				buf->vb.vb2_buf.timestamp = ktime_get_ns();
-			} else if (pts) {
-				ktime_t timestamp;
-				s64 runtime_us = pts -
-				    dev->capture.vc_start_timestamp;
-				timestamp = ktime_add_us(dev->capture.kernel_start_ts,
-							 runtime_us);
-				v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
-					 "Convert start time %llu and %llu with offset %llu to %llu\n",
-					 ktime_to_ns(dev->capture.kernel_start_ts),
-					 dev->capture.vc_start_timestamp, pts,
-					 ktime_to_ns(timestamp));
-				buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
-			} else {
-				if (dev->capture.last_timestamp) {
-					buf->vb.vb2_buf.timestamp =
-						dev->capture.last_timestamp;
-				} else {
-					buf->vb.vb2_buf.timestamp =
-						ktime_to_ns(dev->capture.kernel_start_ts);
-				}
-			}
-			dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
+		return;
+	}

-			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
-			if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_KEYFRAME)
-				buf->vb.flags |= V4L2_BUF_FLAG_KEYFRAME;
+	if (!dev->capture.frame_count) {
+		/* signal frame completion */
+		vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
+		complete(&dev->capture.frame_cmplt);
+		return;
+	}

-			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);

-			if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_EOS &&
-			    is_capturing(dev)) {
-				v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
-					 "Grab another frame as buffer has EOS");
-				vchiq_mmal_port_parameter_set(
-					instance,
-					dev->capture.camera_port,
-					MMAL_PARAMETER_CAPTURE,
-					&dev->capture.frame_count,
-					sizeof(dev->capture.frame_count));
-			}
+	if (dev->capture.vc_start_timestamp != -1) {
+		buf->vb.vb2_buf.timestamp = ktime_get_ns();
+	} else if (pts) {
+		ktime_t timestamp;
+		s64 runtime_us = pts -
+		    dev->capture.vc_start_timestamp;
+		timestamp = ktime_add_us(dev->capture.kernel_start_ts,
+					 runtime_us);
+		v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
+			 "Convert start time %llu and %llu with offset %llu to %llu\n",
+			 ktime_to_ns(dev->capture.kernel_start_ts),
+			 dev->capture.vc_start_timestamp, pts,
+			 ktime_to_ns(timestamp));
+		buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
+	} else {
+		if (dev->capture.last_timestamp) {
+			buf->vb.vb2_buf.timestamp =
+				dev->capture.last_timestamp;
 		} else {
-			/* signal frame completion */
-			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
-			complete(&dev->capture.frame_cmplt);
+			buf->vb.vb2_buf.timestamp =
+				ktime_to_ns(dev->capture.kernel_start_ts);
 		}
 	}
+	dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
+
+	vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
+	if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_KEYFRAME)
+		buf->vb.flags |= V4L2_BUF_FLAG_KEYFRAME;
+
+	vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
+
+	if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_EOS &&
+	    is_capturing(dev)) {
+		v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
+			 "Grab another frame as buffer has EOS");
+		vchiq_mmal_port_parameter_set(
+			instance,
+			dev->capture.camera_port,
+			MMAL_PARAMETER_CAPTURE,
+			&dev->capture.frame_count,
+			sizeof(dev->capture.frame_count));
+	}
 }

 static int enable_camera(struct bm2835_mmal_dev *dev)
@@ -785,27 +790,29 @@ static int vidioc_overlay(struct file *file, void *f, unsigned int on)

 	ret = vchiq_mmal_port_set_format(dev->instance, src);
 	if (ret < 0)
-		goto error;
+		return ret;

 	ret = set_overlay_params(dev, dst);
 	if (ret < 0)
-		goto error;
+		return ret;

-	if (enable_camera(dev) < 0)
-		goto error;
+	if (enable_camera(dev) < 0) {
+		ret = -EINVAL;
+		return ret;
+	}

 	ret = vchiq_mmal_component_enable(
 			dev->instance,
 			dev->component[MMAL_COMPONENT_PREVIEW]);
 	if (ret < 0)
-		goto error;
+		return ret;

 	v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "connecting %p to %p\n",
 		 src, dst);
 	ret = vchiq_mmal_port_connect_tunnel(dev->instance, src, dst);
 	if (!ret)
 		ret = vchiq_mmal_port_enable(dev->instance, src, NULL);
-error:
+
 	return ret;
 }

--
2.7.4

[PATCH 07/31] staging: bcm2835-camera: Remove dead email addresses

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

None of the listed author email addresses were valid.
Keep list of authors and the companies they represented.
Update my email address.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h  | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/controls.c        | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-common.h     | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-encodings.h  | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-msg-common.h | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h   | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h        | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c      | 9 +++++----
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h      | 9 +++++----
 12 files changed, 60 insertions(+), 48 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index b597475..814deee 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 #include <linux/errno.h>
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
index 09273b0..9833828 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  *
  * core driver device
  */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index 133aa6e..b142130 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 #include <linux/errno.h>
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
index a20bf27..858bdcd 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-common.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  *
  * MMAL structures
  *
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-encodings.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-encodings.h
index 1292035..2be9941 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-encodings.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-encodings.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */
 #ifndef MMAL_ENCODINGS_H
 #define MMAL_ENCODINGS_H
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-common.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-common.h
index ec84556..342c9b6 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-common.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-common.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 #ifndef MMAL_MSG_COMMON_H
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h
index c9d6fbe..5ea1a1b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 #ifndef MMAL_MSG_FORMAT_H
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
index 3b3ed79..fe5768d 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 /* MMAL_PORT_TYPE_T */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
index 90793c9..332de57 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 /* all the data structures which serialise the MMAL protocol. note
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
index 184024d..96e987d 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  */

 /* common parameters */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 1a343d8..5175e2c 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  *
  * V4L2 driver MMAL vchiq interface code
  */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
index 22b839e..0e5a81b 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
@@ -4,10 +4,11 @@
  *
  * Copyright © 2013 Raspberry Pi (Trading) Ltd.
  *
- * Authors: Vincent Sanders <vincent.sanders@collabora.co.uk>
- *          Dave Stevenson <dsteve@broadcom.com>
- *          Simon Mellor <simellor@broadcom.com>
- *          Luke Diamand <luked@broadcom.com>
+ * Authors: Vincent Sanders @ Collabora
+ *          Dave Stevenson @ Broadcom
+ *		(now dave.stevenson@raspberrypi.org)
+ *          Simon Mellor @ Broadcom
+ *          Luke Diamand @ Broadcom
  *
  * MMAL interface to VCHIQ message passing
  */
--
2.7.4

[PATCH 08/31] staging: bcm2835-camera: Fix comment style violations.

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix comment style violations in the header files.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/mmal-msg-format.h |  95 ++++++++-------
 .../vc04_services/bcm2835-camera/mmal-msg-port.h   | 124 +++++++++----------
 .../vc04_services/bcm2835-camera/mmal-msg.h        | 135 +++++++++++----------
 3 files changed, 185 insertions(+), 169 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h
index 5ea1a1b..a118efd 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-format.h
@@ -19,22 +19,23 @@
 /* MMAL_ES_FORMAT_T */

 struct mmal_audio_format {
-	u32 channels;           /**< Number of audio channels */
-	u32 sample_rate;        /**< Sample rate */
+	u32 channels;		/* Number of audio channels */
+	u32 sample_rate;	/* Sample rate */

-	u32 bits_per_sample;    /**< Bits per sample */
-	u32 block_align;        /**< Size of a block of data */
+	u32 bits_per_sample;	/* Bits per sample */
+	u32 block_align;	/* Size of a block of data */
 };

 struct mmal_video_format {
-	u32 width;        /**< Width of frame in pixels */
-	u32 height;       /**< Height of frame in rows of pixels */
-	struct mmal_rect crop;         /**< Visible region of the frame */
-	struct mmal_rational frame_rate;   /**< Frame rate */
-	struct mmal_rational par;          /**< Pixel aspect ratio */
-
-	/* FourCC specifying the color space of the video stream. See the
-	 * \ref MmalColorSpace "pre-defined color spaces" for some examples.
+	u32 width;		/* Width of frame in pixels */
+	u32 height;		/* Height of frame in rows of pixels */
+	struct mmal_rect crop;	/* Visible region of the frame */
+	struct mmal_rational frame_rate;	/* Frame rate */
+	struct mmal_rational par;		/* Pixel aspect ratio */
+
+	/*
+	 * FourCC specifying the color space of the video stream. See the
+	 * MmalColorSpace "pre-defined color spaces" for some examples.
 	 */
 	u32 color_space;
 };
@@ -50,48 +51,56 @@ union mmal_es_specific_format {
 	struct mmal_subpicture_format subpicture;
 };

-/** Definition of an elementary stream format (MMAL_ES_FORMAT_T) */
+/* Definition of an elementary stream format (MMAL_ES_FORMAT_T) */
 struct mmal_es_format_local {
-	u32 type;      /* enum mmal_es_type */
-
-	u32 encoding;  /* FourCC specifying encoding of the elementary stream.*/
-	u32 encoding_variant; /* FourCC specifying the specific
-			       * encoding variant of the elementary
-			       * stream.
-			       */
-
-	union mmal_es_specific_format *es;  /* Type specific
-					     * information for the
-					     * elementary stream
-					     */
-
-	u32 bitrate;        /**< Bitrate in bits per second */
-	u32 flags; /**< Flags describing properties of the elementary stream. */
-
-	u32 extradata_size;       /**< Size of the codec specific data */
-	u8  *extradata;           /**< Codec specific data */
+	u32 type;	/* enum mmal_es_type */
+
+	u32 encoding;	/* FourCC specifying encoding of the elementary
+			 * stream.
+			 */
+	u32 encoding_variant;	/* FourCC specifying the specific
+				 * encoding variant of the elementary
+				 * stream.
+				 */
+
+	union mmal_es_specific_format *es;	/* Type specific
+						 * information for the
+						 * elementary stream
+						 */
+
+	u32 bitrate;	/* Bitrate in bits per second */
+	u32 flags;	/* Flags describing properties of the elementary
+			 * stream.
+			 */
+
+	u32 extradata_size;	/* Size of the codec specific data */
+	u8  *extradata;		/* Codec specific data */
 };

-/** Remote definition of an elementary stream format (MMAL_ES_FORMAT_T) */
+/* Remote definition of an elementary stream format (MMAL_ES_FORMAT_T) */
 struct mmal_es_format {
-	u32 type;      /* enum mmal_es_type */
+	u32 type;	/* enum mmal_es_type */

-	u32 encoding;  /* FourCC specifying encoding of the elementary stream.*/
-	u32 encoding_variant; /* FourCC specifying the specific
-			       * encoding variant of the elementary
-			       * stream.
-			       */
+	u32 encoding;	/* FourCC specifying encoding of the elementary
+			 * stream.
+			 */
+	u32 encoding_variant;	/* FourCC specifying the specific
+				 * encoding variant of the elementary
+				 * stream.
+				 */

-	u32 es; /* Type specific
+	u32 es;	/* Type specific
 		 * information for the
 		 * elementary stream
 		 */

-	u32 bitrate;        /**< Bitrate in bits per second */
-	u32 flags; /**< Flags describing properties of the elementary stream. */
+	u32 bitrate;	/* Bitrate in bits per second */
+	u32 flags;	/* Flags describing properties of the elementary
+			 * stream.
+			 */

-	u32 extradata_size;       /**< Size of the codec specific data */
-	u32 extradata;           /**< Codec specific data */
+	u32 extradata_size;	/* Size of the codec specific data */
+	u32 extradata;		/* Codec specific data */
 };

 #endif /* MMAL_MSG_FORMAT_H */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
index fe5768d..3fa3f2a 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg-port.h
@@ -13,28 +13,31 @@

 /* MMAL_PORT_TYPE_T */
 enum mmal_port_type {
-	MMAL_PORT_TYPE_UNKNOWN = 0,  /**< Unknown port type */
-	MMAL_PORT_TYPE_CONTROL,      /**< Control port */
-	MMAL_PORT_TYPE_INPUT,        /**< Input port */
-	MMAL_PORT_TYPE_OUTPUT,       /**< Output port */
-	MMAL_PORT_TYPE_CLOCK,        /**< Clock port */
+	MMAL_PORT_TYPE_UNKNOWN = 0,	/* Unknown port type */
+	MMAL_PORT_TYPE_CONTROL,		/* Control port */
+	MMAL_PORT_TYPE_INPUT,		/* Input port */
+	MMAL_PORT_TYPE_OUTPUT,		/* Output port */
+	MMAL_PORT_TYPE_CLOCK,		/* Clock port */
 };

-/** The port is pass-through and doesn't need buffer headers allocated */
+/* The port is pass-through and doesn't need buffer headers allocated */
 #define MMAL_PORT_CAPABILITY_PASSTHROUGH                       0x01
-/** The port wants to allocate the buffer payloads.
+/*
+ *The port wants to allocate the buffer payloads.
  * This signals a preference that payload allocation should be done
  * on this port for efficiency reasons.
  */
 #define MMAL_PORT_CAPABILITY_ALLOCATION                        0x02
-/** The port supports format change events.
+/*
+ * The port supports format change events.
  * This applies to input ports and is used to let the client know
  * whether the port supports being reconfigured via a format
  * change event (i.e. without having to disable the port).
  */
 #define MMAL_PORT_CAPABILITY_SUPPORTS_EVENT_FORMAT_CHANGE      0x04

-/* mmal port structure (MMAL_PORT_T)
+/*
+ * mmal port structure (MMAL_PORT_T)
  *
  * most elements are informational only, the pointer values for
  * interogation messages are generally provided as additional
@@ -42,50 +45,50 @@ enum mmal_port_type {
  * buffer_num, buffer_size and userdata parameters are writable.
  */
 struct mmal_port {
-	u32 priv; /* Private member used by the framework */
-	u32 name; /* Port name. Used for debugging purposes (RO) */
-
-	u32 type;      /* Type of the port (RO) enum mmal_port_type */
-	u16 index;     /* Index of the port in its type list (RO) */
-	u16 index_all; /* Index of the port in the list of all ports (RO) */
-
-	u32 is_enabled; /* Indicates whether the port is enabled or not (RO) */
-	u32 format; /* Format of the elementary stream */
-
-	u32 buffer_num_min; /* Minimum number of buffers the port
-			     *   requires (RO).  This is set by the
-			     *   component.
-			     */
-
-	u32 buffer_size_min; /* Minimum size of buffers the port
-			      * requires (RO).  This is set by the
-			      * component.
-			      */
-
-	u32 buffer_alignment_min; /* Minimum alignment requirement for
-				   * the buffers (RO).  A value of
-				   * zero means no special alignment
-				   * requirements.  This is set by the
-				   * component.
-				   */
-
-	u32 buffer_num_recommended;  /* Number of buffers the port
-				      * recommends for optimal
-				      * performance (RO).  A value of
-				      * zero means no special
-				      * recommendation.  This is set
-				      * by the component.
-				      */
-
-	u32 buffer_size_recommended; /* Size of buffers the port
-				      * recommends for optimal
-				      * performance (RO).  A value of
-				      * zero means no special
-				      * recommendation.  This is set
-				      * by the component.
-				      */
-
-	u32 buffer_num; /* Actual number of buffers the port will use.
+	u32 priv;	/* Private member used by the framework */
+	u32 name;	/* Port name. Used for debugging purposes (RO) */
+
+	u32 type;	/* Type of the port (RO) enum mmal_port_type */
+	u16 index;	/* Index of the port in its type list (RO) */
+	u16 index_all;	/* Index of the port in the list of all ports (RO) */
+
+	u32 is_enabled;	/* Indicates whether the port is enabled or not (RO) */
+	u32 format;	/* Format of the elementary stream */
+
+	u32 buffer_num_min;	/* Minimum number of buffers the port
+				 *   requires (RO).  This is set by the
+				 *   component.
+				 */
+
+	u32 buffer_size_min;	/* Minimum size of buffers the port
+				 * requires (RO).  This is set by the
+				 * component.
+				 */
+
+	u32 buffer_alignment_min;/* Minimum alignment requirement for
+				  * the buffers (RO).  A value of
+				  * zero means no special alignment
+				  * requirements.  This is set by the
+				  * component.
+				  */
+
+	u32 buffer_num_recommended;	/* Number of buffers the port
+					 * recommends for optimal
+					 * performance (RO).  A value of
+					 * zero means no special
+					 * recommendation.  This is set
+					 * by the component.
+					 */
+
+	u32 buffer_size_recommended;	/* Size of buffers the port
+					 * recommends for optimal
+					 * performance (RO).  A value of
+					 * zero means no special
+					 * recommendation.  This is set
+					 * by the component.
+					 */
+
+	u32 buffer_num;	/* Actual number of buffers the port will use.
 			 * This is set by the client.
 			 */

@@ -94,14 +97,13 @@ struct mmal_port {
 			  * the client.
 			  */

-	u32 component; /* Component this port belongs to (Read Only) */
-
-	u32 userdata; /* Field reserved for use by the client */
+	u32 component;	/* Component this port belongs to (Read Only) */

-	u32 capabilities; /* Flags describing the capabilities of a
-			   * port (RO).  Bitwise combination of \ref
-			   * portcapabilities "Port capabilities"
-			   * values.
-			   */
+	u32 userdata;	/* Field reserved for use by the client */

+	u32 capabilities;	/* Flags describing the capabilities of a
+				 * port (RO).  Bitwise combination of \ref
+				 * portcapabilities "Port capabilities"
+				 * values.
+				 */
 };
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
index 332de57..f165ddf 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
@@ -11,7 +11,8 @@
  *          Luke Diamand @ Broadcom
  */

-/* all the data structures which serialise the MMAL protocol. note
+/*
+ * all the data structures which serialise the MMAL protocol. note
  * these are directly mapped onto the recived message data.
  *
  * BEWARE: They seem to *assume* pointers are u32 and that there is no
@@ -41,51 +42,51 @@ enum mmal_msg_type {
 	MMAL_MSG_TYPE_SERVICE_CLOSED,
 	MMAL_MSG_TYPE_GET_VERSION,
 	MMAL_MSG_TYPE_COMPONENT_CREATE,
-	MMAL_MSG_TYPE_COMPONENT_DESTROY, /* 5 */
+	MMAL_MSG_TYPE_COMPONENT_DESTROY,	/* 5 */
 	MMAL_MSG_TYPE_COMPONENT_ENABLE,
 	MMAL_MSG_TYPE_COMPONENT_DISABLE,
 	MMAL_MSG_TYPE_PORT_INFO_GET,
 	MMAL_MSG_TYPE_PORT_INFO_SET,
-	MMAL_MSG_TYPE_PORT_ACTION, /* 10 */
+	MMAL_MSG_TYPE_PORT_ACTION,		/* 10 */
 	MMAL_MSG_TYPE_BUFFER_FROM_HOST,
 	MMAL_MSG_TYPE_BUFFER_TO_HOST,
 	MMAL_MSG_TYPE_GET_STATS,
 	MMAL_MSG_TYPE_PORT_PARAMETER_SET,
-	MMAL_MSG_TYPE_PORT_PARAMETER_GET, /* 15 */
+	MMAL_MSG_TYPE_PORT_PARAMETER_GET,	/* 15 */
 	MMAL_MSG_TYPE_EVENT_TO_HOST,
 	MMAL_MSG_TYPE_GET_CORE_STATS_FOR_PORT,
 	MMAL_MSG_TYPE_OPAQUE_ALLOCATOR,
 	MMAL_MSG_TYPE_CONSUME_MEM,
-	MMAL_MSG_TYPE_LMK, /* 20 */
+	MMAL_MSG_TYPE_LMK,			/* 20 */
 	MMAL_MSG_TYPE_OPAQUE_ALLOCATOR_DESC,
 	MMAL_MSG_TYPE_DRM_GET_LHS32,
 	MMAL_MSG_TYPE_DRM_GET_TIME,
 	MMAL_MSG_TYPE_BUFFER_FROM_HOST_ZEROLEN,
-	MMAL_MSG_TYPE_PORT_FLUSH, /* 25 */
+	MMAL_MSG_TYPE_PORT_FLUSH,		/* 25 */
 	MMAL_MSG_TYPE_HOST_LOG,
 	MMAL_MSG_TYPE_MSG_LAST
 };

 /* port action request messages differ depending on the action type */
 enum mmal_msg_port_action_type {
-	MMAL_MSG_PORT_ACTION_TYPE_UNKNOWN = 0,      /* Unknown action */
-	MMAL_MSG_PORT_ACTION_TYPE_ENABLE,           /* Enable a port */
-	MMAL_MSG_PORT_ACTION_TYPE_DISABLE,          /* Disable a port */
-	MMAL_MSG_PORT_ACTION_TYPE_FLUSH,            /* Flush a port */
-	MMAL_MSG_PORT_ACTION_TYPE_CONNECT,          /* Connect ports */
-	MMAL_MSG_PORT_ACTION_TYPE_DISCONNECT,       /* Disconnect ports */
+	MMAL_MSG_PORT_ACTION_TYPE_UNKNOWN = 0,	/* Unknown action */
+	MMAL_MSG_PORT_ACTION_TYPE_ENABLE,	/* Enable a port */
+	MMAL_MSG_PORT_ACTION_TYPE_DISABLE,	/* Disable a port */
+	MMAL_MSG_PORT_ACTION_TYPE_FLUSH,	/* Flush a port */
+	MMAL_MSG_PORT_ACTION_TYPE_CONNECT,	/* Connect ports */
+	MMAL_MSG_PORT_ACTION_TYPE_DISCONNECT,	/* Disconnect ports */
 	MMAL_MSG_PORT_ACTION_TYPE_SET_REQUIREMENTS, /* Set buffer requirements*/
 };

 struct mmal_msg_header {
 	u32 magic;
-	u32 type; /** enum mmal_msg_type */
+	u32 type;	/* enum mmal_msg_type */

 	/* Opaque handle to the control service */
 	u32 control_service;

-	u32 context; /** a u32 per message context */
-	u32 status; /** The status of the vchiq operation */
+	u32 context;	/* a u32 per message context */
+	u32 status;	/* The status of the vchiq operation */
 	u32 padding;
 };

@@ -99,9 +100,9 @@ struct mmal_msg_version {

 /* request to VC to create component */
 struct mmal_msg_component_create {
-	u32 client_component; /* component context */
+	u32 client_component;	/* component context */
 	char name[128];
-	u32 pid;                /* For debug */
+	u32 pid;		/* For debug */
 };

 /* reply from VC to component creation request */
@@ -121,7 +122,7 @@ struct mmal_msg_component_destroy {
 };

 struct mmal_msg_component_destroy_reply {
-	u32 status; /** The component destruction status */
+	u32 status; /* The component destruction status */
 };

 /* request and reply to VC to enable a component */
@@ -130,7 +131,7 @@ struct mmal_msg_component_enable {
 };

 struct mmal_msg_component_enable_reply {
-	u32 status; /** The component enable status */
+	u32 status; /* The component enable status */
 };

 /* request and reply to VC to disable a component */
@@ -139,7 +140,7 @@ struct mmal_msg_component_disable {
 };

 struct mmal_msg_component_disable_reply {
-	u32 status; /** The component disable status */
+	u32 status; /* The component disable status */
 };

 /* request to VC to get port information */
@@ -151,12 +152,12 @@ struct mmal_msg_port_info_get {

 /* reply from VC to get port info request */
 struct mmal_msg_port_info_get_reply {
-	u32 status; /** enum mmal_msg_status */
-	u32 component_handle;  /* component handle port is associated with */
-	u32 port_type;         /* enum mmal_msg_port_type */
-	u32 port_index;        /* port indexed in query */
-	s32 found;             /* unused */
-	u32 port_handle;               /**< Handle to use for this port */
+	u32 status;		/* enum mmal_msg_status */
+	u32 component_handle;	/* component handle port is associated with */
+	u32 port_type;		/* enum mmal_msg_port_type */
+	u32 port_index;		/* port indexed in query */
+	s32 found;		/* unused */
+	u32 port_handle;	/* Handle to use for this port */
 	struct mmal_port port;
 	struct mmal_es_format format; /* elementary stream format */
 	union mmal_es_specific_format es; /* es type specific data */
@@ -166,8 +167,8 @@ struct mmal_msg_port_info_get_reply {
 /* request to VC to set port information */
 struct mmal_msg_port_info_set {
 	u32 component_handle;
-	u32 port_type;         /* enum mmal_msg_port_type */
-	u32 port_index;           /* port indexed in query */
+	u32 port_type;		/* enum mmal_msg_port_type */
+	u32 port_index;		/* port indexed in query */
 	struct mmal_port port;
 	struct mmal_es_format format;
 	union mmal_es_specific_format es;
@@ -177,11 +178,11 @@ struct mmal_msg_port_info_set {
 /* reply from VC to port info set request */
 struct mmal_msg_port_info_set_reply {
 	u32 status;
-	u32 component_handle;  /* component handle port is associated with */
-	u32 port_type;         /* enum mmal_msg_port_type */
-	u32 index;             /* port indexed in query */
-	s32 found;             /* unused */
-	u32 port_handle;               /**< Handle to use for this port */
+	u32 component_handle;	/* component handle port is associated with */
+	u32 port_type;		/* enum mmal_msg_port_type */
+	u32 index;		/* port indexed in query */
+	s32 found;		/* unused */
+	u32 port_handle;	/* Handle to use for this port */
 	struct mmal_port port;
 	struct mmal_es_format format;
 	union mmal_es_specific_format es;
@@ -192,7 +193,7 @@ struct mmal_msg_port_info_set_reply {
 struct mmal_msg_port_action_port {
 	u32 component_handle;
 	u32 port_handle;
-	u32 action; /* enum mmal_msg_port_action_type */
+	u32 action;		/* enum mmal_msg_port_action_type */
 	struct mmal_port port;
 };

@@ -200,50 +201,53 @@ struct mmal_msg_port_action_port {
 struct mmal_msg_port_action_handle {
 	u32 component_handle;
 	u32 port_handle;
-	u32 action; /* enum mmal_msg_port_action_type */
+	u32 action;		/* enum mmal_msg_port_action_type */
 	u32 connect_component_handle;
 	u32 connect_port_handle;
 };

 struct mmal_msg_port_action_reply {
-	u32 status; /** The port action operation status */
+	u32 status;	/* The port action operation status */
 };

 /* MMAL buffer transfer */

-/** Size of space reserved in a buffer message for short messages. */
+/* Size of space reserved in a buffer message for short messages. */
 #define MMAL_VC_SHORT_DATA 128

-/** Signals that the current payload is the end of the stream of data */
+/* Signals that the current payload is the end of the stream of data */
 #define MMAL_BUFFER_HEADER_FLAG_EOS                    BIT(0)
-/** Signals that the start of the current payload starts a frame */
+/* Signals that the start of the current payload starts a frame */
 #define MMAL_BUFFER_HEADER_FLAG_FRAME_START            BIT(1)
-/** Signals that the end of the current payload ends a frame */
+/* Signals that the end of the current payload ends a frame */
 #define MMAL_BUFFER_HEADER_FLAG_FRAME_END              BIT(2)
-/** Signals that the current payload contains only complete frames (>1) */
+/* Signals that the current payload contains only complete frames (>1) */
 #define MMAL_BUFFER_HEADER_FLAG_FRAME                  \
 	(MMAL_BUFFER_HEADER_FLAG_FRAME_START|MMAL_BUFFER_HEADER_FLAG_FRAME_END)
-/** Signals that the current payload is a keyframe (i.e. self decodable) */
+/* Signals that the current payload is a keyframe (i.e. self decodable) */
 #define MMAL_BUFFER_HEADER_FLAG_KEYFRAME               BIT(3)
-/** Signals a discontinuity in the stream of data (e.g. after a seek).
+/*
+ * Signals a discontinuity in the stream of data (e.g. after a seek).
  * Can be used for instance by a decoder to reset its state
  */
 #define MMAL_BUFFER_HEADER_FLAG_DISCONTINUITY          BIT(4)
-/** Signals a buffer containing some kind of config data for the component
+/*
+ * Signals a buffer containing some kind of config data for the component
  * (e.g. codec config data)
  */
 #define MMAL_BUFFER_HEADER_FLAG_CONFIG                 BIT(5)
-/** Signals an encrypted payload */
+/* Signals an encrypted payload */
 #define MMAL_BUFFER_HEADER_FLAG_ENCRYPTED              BIT(6)
-/** Signals a buffer containing side information */
+/* Signals a buffer containing side information */
 #define MMAL_BUFFER_HEADER_FLAG_CODECSIDEINFO          BIT(7)
-/** Signals a buffer which is the snapshot/postview image from a stills
+/*
+ * Signals a buffer which is the snapshot/postview image from a stills
  * capture
  */
 #define MMAL_BUFFER_HEADER_FLAGS_SNAPSHOT              BIT(8)
-/** Signals a buffer which contains data known to be corrupted */
+/* Signals a buffer which contains data known to be corrupted */
 #define MMAL_BUFFER_HEADER_FLAG_CORRUPTED              BIT(9)
-/** Signals that a buffer failed to be transmitted */
+/* Signals that a buffer failed to be transmitted */
 #define MMAL_BUFFER_HEADER_FLAG_TRANSMISSION_FAILED    BIT(10)

 struct mmal_driver_buffer {
@@ -255,8 +259,8 @@ struct mmal_driver_buffer {

 /* buffer header */
 struct mmal_buffer_header {
-	u32 next; /* next header */
-	u32 priv; /* framework private data */
+	u32 next;	/* next header */
+	u32 priv;	/* framework private data */
 	u32 cmd;
 	u32 data;
 	u32 alloc_size;
@@ -281,7 +285,8 @@ struct mmal_buffer_header_type_specific {
 };

 struct mmal_msg_buffer_from_host {
-	/* The front 32 bytes of the buffer header are copied
+	/*
+	 *The front 32 bytes of the buffer header are copied
 	 * back to us in the reply to allow for context. This
 	 * area is used to store two mmal_driver_buffer structures to
 	 * allow for multiple concurrent service users.
@@ -296,7 +301,7 @@ struct mmal_msg_buffer_from_host {
 	s32 is_zero_copy;
 	s32 has_reference;

-	/** allows short data to be xfered in control message */
+	/* allows short data to be xfered in control message */
 	u32 payload_in_message;
 	u8 short_data[MMAL_VC_SHORT_DATA];
 };
@@ -306,10 +311,10 @@ struct mmal_msg_buffer_from_host {
 #define MMAL_WORKER_PORT_PARAMETER_SPACE      96

 struct mmal_msg_port_parameter_set {
-	u32 component_handle; /* component */
-	u32 port_handle;      /* port */
-	u32 id;     /* Parameter ID  */
-	u32 size;      /* Parameter size */
+	u32 component_handle;	/* component */
+	u32 port_handle;	/* port */
+	u32 id;			/* Parameter ID  */
+	u32 size;		/* Parameter size */
 	u32 value[MMAL_WORKER_PORT_PARAMETER_SPACE];
 };

@@ -322,16 +327,16 @@ struct mmal_msg_port_parameter_set_reply {
 /* port parameter getting */

 struct mmal_msg_port_parameter_get {
-	u32 component_handle; /* component */
-	u32 port_handle;      /* port */
-	u32 id;     /* Parameter ID  */
-	u32 size;      /* Parameter size */
+	u32 component_handle;	/* component */
+	u32 port_handle;	/* port */
+	u32 id;			/* Parameter ID  */
+	u32 size;		/* Parameter size */
 };

 struct mmal_msg_port_parameter_get_reply {
-	u32 status;           /* Status of mmal_port_parameter_get call */
-	u32 id;     /* Parameter ID  */
-	u32 size;      /* Parameter size */
+	u32 status;		/* Status of mmal_port_parameter_get call */
+	u32 id;			/* Parameter ID  */
+	u32 size;		/* Parameter size */
 	u32 value[MMAL_WORKER_PORT_PARAMETER_SPACE];
 };

@@ -339,7 +344,7 @@ struct mmal_msg_port_parameter_get_reply {
 #define MMAL_WORKER_EVENT_SPACE 256

 struct mmal_msg_event_to_host {
-	u32 client_component; /* component context */
+	u32 client_component;	/* component context */

 	u32 port_type;
 	u32 port_num;
--
2.7.4

[PATCH 09/31] staging: bcm2835-camera: Fix spacing around operators

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix checkpatch warnings over spaces around operators.
Many were around operations that can be replaced with the
BIT(x) macro, so replace with that where appropriate.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/controls.c        | 32 +++++++++++-----------
 .../vc04_services/bcm2835-camera/mmal-msg.h        |  3 +-
 .../vc04_services/bcm2835-camera/mmal-parameters.h | 12 ++++----
 3 files changed, 24 insertions(+), 23 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index b142130..e79e7cd 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -1130,10 +1130,10 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 	{
 		V4L2_CID_MPEG_VIDEO_H264_PROFILE,
 		MMAL_CONTROL_TYPE_STD_MENU,
-		~((1<<V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
-			(1<<V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
-			(1<<V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
-			(1<<V4L2_MPEG_VIDEO_H264_PROFILE_HIGH)),
+		~(BIT(V4L2_MPEG_VIDEO_H264_PROFILE_BASELINE) |
+		  BIT(V4L2_MPEG_VIDEO_H264_PROFILE_CONSTRAINED_BASELINE) |
+		  BIT(V4L2_MPEG_VIDEO_H264_PROFILE_MAIN) |
+		  BIT(V4L2_MPEG_VIDEO_H264_PROFILE_HIGH)),
 		V4L2_MPEG_VIDEO_H264_PROFILE_HIGH,
 		V4L2_MPEG_VIDEO_H264_PROFILE_HIGH, 1, NULL,
 		MMAL_PARAMETER_PROFILE,
@@ -1142,18 +1142,18 @@ static const struct bm2835_mmal_v4l2_ctrl v4l2_ctrls[V4L2_CTRL_COUNT] = {
 	},
 	{
 		V4L2_CID_MPEG_VIDEO_H264_LEVEL, MMAL_CONTROL_TYPE_STD_MENU,
-		~((1<<V4L2_MPEG_VIDEO_H264_LEVEL_1_0) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_1B) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_1_1) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_1_2) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_1_3) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_2_0) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_2_1) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_2_2) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_3_1) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_3_2) |
-			(1<<V4L2_MPEG_VIDEO_H264_LEVEL_4_0)),
+		~(BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_0) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1B) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_1) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_2) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_1_3) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_2_0) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_2_1) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_2_2) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_3_0) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_3_1) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_3_2) |
+		  BIT(V4L2_MPEG_VIDEO_H264_LEVEL_4_0)),
 		V4L2_MPEG_VIDEO_H264_LEVEL_4_0,
 		V4L2_MPEG_VIDEO_H264_LEVEL_4_0, 1, NULL,
 		MMAL_PARAMETER_PROFILE,
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
index f165ddf..8e0aee8 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-msg.h
@@ -223,7 +223,8 @@ struct mmal_msg_port_action_reply {
 #define MMAL_BUFFER_HEADER_FLAG_FRAME_END              BIT(2)
 /* Signals that the current payload contains only complete frames (>1) */
 #define MMAL_BUFFER_HEADER_FLAG_FRAME                  \
-	(MMAL_BUFFER_HEADER_FLAG_FRAME_START|MMAL_BUFFER_HEADER_FLAG_FRAME_END)
+	(MMAL_BUFFER_HEADER_FLAG_FRAME_START | \
+	 MMAL_BUFFER_HEADER_FLAG_FRAME_END)
 /* Signals that the current payload is a keyframe (i.e. self decodable) */
 #define MMAL_BUFFER_HEADER_FLAG_KEYFRAME               BIT(3)
 /*
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
index 96e987d..6d21594 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-parameters.h
@@ -23,17 +23,17 @@
 #define __MMAL_PARAMETERS_H

 /** Common parameter ID group, used with many types of component. */
-#define MMAL_PARAMETER_GROUP_COMMON            (0<<16)
+#define MMAL_PARAMETER_GROUP_COMMON            (0 << 16)
 /** Camera-specific parameter ID group. */
-#define MMAL_PARAMETER_GROUP_CAMERA            (1<<16)
+#define MMAL_PARAMETER_GROUP_CAMERA            (1 << 16)
 /** Video-specific parameter ID group. */
-#define MMAL_PARAMETER_GROUP_VIDEO             (2<<16)
+#define MMAL_PARAMETER_GROUP_VIDEO             (2 << 16)
 /** Audio-specific parameter ID group. */
-#define MMAL_PARAMETER_GROUP_AUDIO             (3<<16)
+#define MMAL_PARAMETER_GROUP_AUDIO             (3 << 16)
 /** Clock-specific parameter ID group. */
-#define MMAL_PARAMETER_GROUP_CLOCK             (4<<16)
+#define MMAL_PARAMETER_GROUP_CLOCK             (4 << 16)
 /** Miracast-specific parameter ID group. */
-#define MMAL_PARAMETER_GROUP_MIRACAST       (5<<16)
+#define MMAL_PARAMETER_GROUP_MIRACAST       (5 << 16)

 /* Common parameters */
 enum mmal_parameter_common_type {
--
2.7.4

[PATCH 10/31] staging: bcm2835-camera: Reduce length of enum names

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

We have numerous lines over 80 chars, or oddly split. Many
of these are due to using long enum names such as
MMAL_COMPONENT_CAMERA.
Reduce the length of these enum names.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 156 +++++++++++----------
 .../vc04_services/bcm2835-camera/bcm2835-camera.h  |  20 +--
 .../vc04_services/bcm2835-camera/controls.c        |  47 +++----
 3 files changed, 111 insertions(+), 112 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 814deee..23f7754 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -80,7 +80,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_I420,
 		.depth = 12,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 1,
 		.remove_padding = 1,
 	}, {
@@ -89,7 +89,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_YUYV,
 		.depth = 16,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 2,
 		.remove_padding = 0,
 	}, {
@@ -98,7 +98,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_RGB24,
 		.depth = 24,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 3,
 		.remove_padding = 0,
 	}, {
@@ -107,7 +107,7 @@ static struct mmal_fmt formats[] = {
 		.flags = V4L2_FMT_FLAG_COMPRESSED,
 		.mmal = MMAL_ENCODING_JPEG,
 		.depth = 8,
-		.mmal_component = MMAL_COMPONENT_IMAGE_ENCODE,
+		.mmal_component = COMP_IMAGE_ENCODE,
 		.ybbp = 0,
 		.remove_padding = 0,
 	}, {
@@ -116,7 +116,7 @@ static struct mmal_fmt formats[] = {
 		.flags = V4L2_FMT_FLAG_COMPRESSED,
 		.mmal = MMAL_ENCODING_H264,
 		.depth = 8,
-		.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
+		.mmal_component = COMP_VIDEO_ENCODE,
 		.ybbp = 0,
 		.remove_padding = 0,
 	}, {
@@ -125,7 +125,7 @@ static struct mmal_fmt formats[] = {
 		.flags = V4L2_FMT_FLAG_COMPRESSED,
 		.mmal = MMAL_ENCODING_MJPEG,
 		.depth = 8,
-		.mmal_component = MMAL_COMPONENT_VIDEO_ENCODE,
+		.mmal_component = COMP_VIDEO_ENCODE,
 		.ybbp = 0,
 		.remove_padding = 0,
 	}, {
@@ -134,7 +134,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_YVYU,
 		.depth = 16,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 2,
 		.remove_padding = 0,
 	}, {
@@ -143,7 +143,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_VYUY,
 		.depth = 16,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 2,
 		.remove_padding = 0,
 	}, {
@@ -152,7 +152,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_UYVY,
 		.depth = 16,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 2,
 		.remove_padding = 0,
 	}, {
@@ -161,7 +161,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_NV12,
 		.depth = 12,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 1,
 		.remove_padding = 1,
 	}, {
@@ -170,7 +170,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_BGR24,
 		.depth = 24,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 3,
 		.remove_padding = 0,
 	}, {
@@ -179,7 +179,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_YV12,
 		.depth = 12,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 1,
 		.remove_padding = 1,
 	}, {
@@ -188,7 +188,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_NV21,
 		.depth = 12,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 1,
 		.remove_padding = 1,
 	}, {
@@ -197,7 +197,7 @@ static struct mmal_fmt formats[] = {
 		.flags = 0,
 		.mmal = MMAL_ENCODING_BGRA,
 		.depth = 32,
-		.mmal_component = MMAL_COMPONENT_CAMERA,
+		.mmal_component = COMP_CAMERA,
 		.ybbp = 4,
 		.remove_padding = 0,
 	},
@@ -313,7 +313,7 @@ static void buffer_cleanup(struct vb2_buffer *vb)
 static inline bool is_capturing(struct bm2835_mmal_dev *dev)
 {
 	return dev->capture.camera_port ==
-	    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_CAPTURE];
+	    &dev->component[COMP_CAMERA]->output[CAM_PORT_CAPTURE];
 }

 static void buffer_cb(struct vchiq_mmal_instance *instance,
@@ -422,7 +422,7 @@ static int enable_camera(struct bm2835_mmal_dev *dev)
 	if (!dev->camera_use_count) {
 		ret = vchiq_mmal_port_parameter_set(
 			dev->instance,
-			&dev->component[MMAL_COMPONENT_CAMERA]->control,
+			&dev->component[COMP_CAMERA]->control,
 			MMAL_PARAMETER_CAMERA_NUM, &dev->camera_num,
 			sizeof(dev->camera_num));
 		if (ret < 0) {
@@ -433,7 +433,7 @@ static int enable_camera(struct bm2835_mmal_dev *dev)

 		ret = vchiq_mmal_component_enable(
 				dev->instance,
-				dev->component[MMAL_COMPONENT_CAMERA]);
+				dev->component[COMP_CAMERA]);
 		if (ret < 0) {
 			v4l2_err(&dev->v4l2_dev,
 				 "Failed enabling camera, ret %d\n", ret);
@@ -465,7 +465,7 @@ static int disable_camera(struct bm2835_mmal_dev *dev)
 		ret =
 		    vchiq_mmal_component_disable(
 				dev->instance,
-				dev->component[MMAL_COMPONENT_CAMERA]);
+				dev->component[COMP_CAMERA]);
 		if (ret < 0) {
 			v4l2_err(&dev->v4l2_dev,
 				 "Failed disabling camera, ret %d\n", ret);
@@ -473,7 +473,7 @@ static int disable_camera(struct bm2835_mmal_dev *dev)
 		}
 		vchiq_mmal_port_parameter_set(
 			dev->instance,
-			&dev->component[MMAL_COMPONENT_CAMERA]->control,
+			&dev->component[COMP_CAMERA]->control,
 			MMAL_PARAMETER_CAMERA_NUM, &i,
 			sizeof(i));
 	}
@@ -525,7 +525,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	/* if the preview is not already running, wait for a few frames for AGC
 	 * to settle down.
 	 */
-	if (!dev->component[MMAL_COMPONENT_PREVIEW]->enabled)
+	if (!dev->component[COMP_PREVIEW]->enabled)
 		msleep(300);

 	/* enable the connection from camera to encoder (if applicable) */
@@ -748,9 +748,9 @@ static int vidioc_s_fmt_vid_overlay(struct file *file, void *priv,
 	vidioc_try_fmt_vid_overlay(file, priv, f);

 	dev->overlay = f->fmt.win;
-	if (dev->component[MMAL_COMPONENT_PREVIEW]->enabled) {
+	if (dev->component[COMP_PREVIEW]->enabled) {
 		set_overlay_params(dev,
-				   &dev->component[MMAL_COMPONENT_PREVIEW]->input[0]);
+				   &dev->component[COMP_PREVIEW]->input[0]);
 	}

 	return 0;
@@ -763,12 +763,12 @@ static int vidioc_overlay(struct file *file, void *f, unsigned int on)
 	struct vchiq_mmal_port *src;
 	struct vchiq_mmal_port *dst;

-	if ((on && dev->component[MMAL_COMPONENT_PREVIEW]->enabled) ||
-	    (!on && !dev->component[MMAL_COMPONENT_PREVIEW]->enabled))
+	if ((on && dev->component[COMP_PREVIEW]->enabled) ||
+	    (!on && !dev->component[COMP_PREVIEW]->enabled))
 		return 0;	/* already in requested state */

 	src =
-	    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_PREVIEW];
+	    &dev->component[COMP_CAMERA]->output[CAM_PORT_PREVIEW];

 	if (!on) {
 		/* disconnect preview ports and disable component */
@@ -780,14 +780,14 @@ static int vidioc_overlay(struct file *file, void *f, unsigned int on)
 		if (ret >= 0)
 			ret = vchiq_mmal_component_disable(
 					dev->instance,
-					dev->component[MMAL_COMPONENT_PREVIEW]);
+					dev->component[COMP_PREVIEW]);

 		disable_camera(dev);
 		return ret;
 	}

 	/* set preview port format and connect it to output */
-	dst = &dev->component[MMAL_COMPONENT_PREVIEW]->input[0];
+	dst = &dev->component[COMP_PREVIEW]->input[0];

 	ret = vchiq_mmal_port_set_format(dev->instance, src);
 	if (ret < 0)
@@ -804,7 +804,7 @@ static int vidioc_overlay(struct file *file, void *f, unsigned int on)

 	ret = vchiq_mmal_component_enable(
 			dev->instance,
-			dev->component[MMAL_COMPONENT_PREVIEW]);
+			dev->component[COMP_PREVIEW]);
 	if (ret < 0)
 		return ret;

@@ -825,7 +825,7 @@ static int vidioc_g_fbuf(struct file *file, void *fh,
 	 */
 	struct bm2835_mmal_dev *dev = video_drvdata(file);
 	struct vchiq_mmal_port *preview_port =
-		    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_PREVIEW];
+		&dev->component[COMP_CAMERA]->output[CAM_PORT_PREVIEW];

 	a->capability = V4L2_FBUF_CAP_EXTERNOVERLAY |
 			V4L2_FBUF_CAP_GLOBAL_ALPHA;
@@ -1023,27 +1023,29 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,
 	}
 	/* format dependent port setup */
 	switch (mfmt->mmal_component) {
-	case MMAL_COMPONENT_CAMERA:
+	case COMP_CAMERA:
 		/* Make a further decision on port based on resolution */
 		if (f->fmt.pix.width <= max_video_width &&
 		    f->fmt.pix.height <= max_video_height)
 			camera_port = port =
-			    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_VIDEO];
+			    &dev->component[COMP_CAMERA]->
+			    output[CAM_PORT_VIDEO];
 		else
 			camera_port = port =
-			    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_CAPTURE];
+			    &dev->component[COMP_CAMERA]->
+			    output[CAM_PORT_CAPTURE];
 		break;
-	case MMAL_COMPONENT_IMAGE_ENCODE:
-		encode_component = dev->component[MMAL_COMPONENT_IMAGE_ENCODE];
-		port = &dev->component[MMAL_COMPONENT_IMAGE_ENCODE]->output[0];
+	case COMP_IMAGE_ENCODE:
+		encode_component = dev->component[COMP_IMAGE_ENCODE];
+		port = &dev->component[COMP_IMAGE_ENCODE]->output[0];
 		camera_port =
-		    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_CAPTURE];
+		    &dev->component[COMP_CAMERA]->output[CAM_PORT_CAPTURE];
 		break;
-	case MMAL_COMPONENT_VIDEO_ENCODE:
-		encode_component = dev->component[MMAL_COMPONENT_VIDEO_ENCODE];
-		port = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
+	case COMP_VIDEO_ENCODE:
+		encode_component = dev->component[COMP_VIDEO_ENCODE];
+		port = &dev->component[COMP_VIDEO_ENCODE]->output[0];
 		camera_port =
-		    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_VIDEO];
+		    &dev->component[COMP_CAMERA]->output[CAM_PORT_VIDEO];
 		break;
 	default:
 		break;
@@ -1083,13 +1085,13 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,

 	ret = vchiq_mmal_port_set_format(dev->instance, camera_port);

-	if (!ret &&
-	    camera_port ==
-	    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_VIDEO]) {
+	if (!ret
+	    && camera_port ==
+	    &dev->component[COMP_CAMERA]->output[CAM_PORT_VIDEO]) {
 		bool overlay_enabled =
-		    !!dev->component[MMAL_COMPONENT_PREVIEW]->enabled;
+		    !!dev->component[COMP_PREVIEW]->enabled;
 		struct vchiq_mmal_port *preview_port =
-		    &dev->component[MMAL_COMPONENT_CAMERA]->output[MMAL_CAMERA_PORT_PREVIEW];
+		    &dev->component[COMP_CAMERA]->output[CAM_PORT_PREVIEW];
 		/* Preview and encode ports need to match on resolution */
 		if (overlay_enabled) {
 			/* Need to disable the overlay before we can update
@@ -1120,7 +1122,7 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,
 			ret = vchiq_mmal_port_connect_tunnel(
 				dev->instance,
 				preview_port,
-				&dev->component[MMAL_COMPONENT_PREVIEW]->input[0]);
+				&dev->component[COMP_PREVIEW]->input[0]);
 			if (!ret)
 				ret = vchiq_mmal_port_enable(dev->instance,
 							     preview_port,
@@ -1174,11 +1176,11 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,
 				port->format.encoding_variant = 0;
 				/* Set any encoding specific parameters */
 				switch (mfmt->mmal_component) {
-				case MMAL_COMPONENT_VIDEO_ENCODE:
+				case COMP_VIDEO_ENCODE:
 					port->format.bitrate =
 					    dev->capture.encode_bitrate;
 					break;
-				case MMAL_COMPONENT_IMAGE_ENCODE:
+				case COMP_IMAGE_ENCODE:
 					/* Could set EXIF parameters here */
 					break;
 				default:
@@ -1549,14 +1551,14 @@ static int mmal_init(struct bm2835_mmal_dev *dev)

 	/* get the camera component ready */
 	ret = vchiq_mmal_component_init(dev->instance, "ril.camera",
-					&dev->component[MMAL_COMPONENT_CAMERA]);
+					&dev->component[COMP_CAMERA]);
 	if (ret < 0)
 		goto unreg_mmal;

-	camera = dev->component[MMAL_COMPONENT_CAMERA];
-	if (camera->outputs < MMAL_CAMERA_PORT_COUNT) {
+	camera = dev->component[COMP_CAMERA];
+	if (camera->outputs < CAM_PORT_COUNT) {
 		v4l2_err(&dev->v4l2_dev, "%s: too few camera outputs %d needed %d\n",
-			 __func__, camera->outputs, MMAL_CAMERA_PORT_COUNT);
+			 __func__, camera->outputs, CAM_PORT_COUNT);
 		ret = -EINVAL;
 		goto unreg_camera;
 	}
@@ -1578,7 +1580,7 @@ static int mmal_init(struct bm2835_mmal_dev *dev)
 	dev->rgb_bgr_swapped = true;
 	param_size = sizeof(supported_encodings);
 	ret = vchiq_mmal_port_parameter_get(dev->instance,
-					    &camera->output[MMAL_CAMERA_PORT_CAPTURE],
+					    &camera->output[CAM_PORT_CAPTURE],
 					    MMAL_PARAMETER_SUPPORTED_ENCODINGS,
 					    &supported_encodings,
 					    &param_size);
@@ -1599,7 +1601,7 @@ static int mmal_init(struct bm2835_mmal_dev *dev)
 			}
 		}
 	}
-	format = &camera->output[MMAL_CAMERA_PORT_PREVIEW].format;
+	format = &camera->output[CAM_PORT_PREVIEW].format;

 	format->encoding = MMAL_ENCODING_OPAQUE;
 	format->encoding_variant = MMAL_ENCODING_I420;
@@ -1613,7 +1615,7 @@ static int mmal_init(struct bm2835_mmal_dev *dev)
 	format->es->video.frame_rate.num = 0; /* Rely on fps_range */
 	format->es->video.frame_rate.den = 1;

-	format = &camera->output[MMAL_CAMERA_PORT_VIDEO].format;
+	format = &camera->output[CAM_PORT_VIDEO].format;

 	format->encoding = MMAL_ENCODING_OPAQUE;
 	format->encoding_variant = MMAL_ENCODING_I420;
@@ -1627,7 +1629,7 @@ static int mmal_init(struct bm2835_mmal_dev *dev)
 	format->es->video.frame_rate.num = 0; /* Rely on fps_range */
 	format->es->video.frame_rate.den = 1;

-	format = &camera->output[MMAL_CAMERA_PORT_CAPTURE].format;
+	format = &camera->output[CAM_PORT_CAPTURE].format;

 	format->encoding = MMAL_ENCODING_OPAQUE;

@@ -1651,49 +1653,49 @@ static int mmal_init(struct bm2835_mmal_dev *dev)
 	/* get the preview component ready */
 	ret = vchiq_mmal_component_init(
 			dev->instance, "ril.video_render",
-			&dev->component[MMAL_COMPONENT_PREVIEW]);
+			&dev->component[COMP_PREVIEW]);
 	if (ret < 0)
 		goto unreg_camera;

-	if (dev->component[MMAL_COMPONENT_PREVIEW]->inputs < 1) {
+	if (dev->component[COMP_PREVIEW]->inputs < 1) {
 		ret = -EINVAL;
 		v4l2_err(&dev->v4l2_dev, "%s: too few input ports %d needed %d\n",
-			 __func__, dev->component[MMAL_COMPONENT_PREVIEW]->inputs, 1);
+			 __func__, dev->component[COMP_PREVIEW]->inputs, 1);
 		goto unreg_preview;
 	}

 	/* get the image encoder component ready */
 	ret = vchiq_mmal_component_init(
 		dev->instance, "ril.image_encode",
-		&dev->component[MMAL_COMPONENT_IMAGE_ENCODE]);
+		&dev->component[COMP_IMAGE_ENCODE]);
 	if (ret < 0)
 		goto unreg_preview;

-	if (dev->component[MMAL_COMPONENT_IMAGE_ENCODE]->inputs < 1) {
+	if (dev->component[COMP_IMAGE_ENCODE]->inputs < 1) {
 		ret = -EINVAL;
 		v4l2_err(&dev->v4l2_dev, "%s: too few input ports %d needed %d\n",
-			 __func__, dev->component[MMAL_COMPONENT_IMAGE_ENCODE]->inputs,
+			 __func__, dev->component[COMP_IMAGE_ENCODE]->inputs,
 			 1);
 		goto unreg_image_encoder;
 	}

 	/* get the video encoder component ready */
 	ret = vchiq_mmal_component_init(dev->instance, "ril.video_encode",
-					&dev->component[MMAL_COMPONENT_VIDEO_ENCODE]);
+					&dev->component[COMP_VIDEO_ENCODE]);
 	if (ret < 0)
 		goto unreg_image_encoder;

-	if (dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->inputs < 1) {
+	if (dev->component[COMP_VIDEO_ENCODE]->inputs < 1) {
 		ret = -EINVAL;
 		v4l2_err(&dev->v4l2_dev, "%s: too few input ports %d needed %d\n",
-			 __func__, dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->inputs,
+			 __func__, dev->component[COMP_VIDEO_ENCODE]->inputs,
 			 1);
 		goto unreg_vid_encoder;
 	}

 	{
 		struct vchiq_mmal_port *encoder_port =
-			&dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
+			&dev->component[COMP_VIDEO_ENCODE]->output[0];
 		encoder_port->format.encoding = MMAL_ENCODING_H264;
 		ret = vchiq_mmal_port_set_format(dev->instance,
 						 encoder_port);
@@ -1704,12 +1706,12 @@ static int mmal_init(struct bm2835_mmal_dev *dev)

 		vchiq_mmal_port_parameter_set(
 			dev->instance,
-			&dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->control,
+			&dev->component[COMP_VIDEO_ENCODE]->control,
 			MMAL_PARAMETER_VIDEO_IMMUTABLE_INPUT,
 			&enable, sizeof(enable));

 		vchiq_mmal_port_parameter_set(dev->instance,
-					      &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->control,
+					      &dev->component[COMP_VIDEO_ENCODE]->control,
 					      MMAL_PARAMETER_MINIMISE_FRAGMENTATION,
 					      &enable,
 					      sizeof(enable));
@@ -1727,23 +1729,23 @@ static int mmal_init(struct bm2835_mmal_dev *dev)
 	pr_err("Cleanup: Destroy video encoder\n");
 	vchiq_mmal_component_finalise(
 		dev->instance,
-		dev->component[MMAL_COMPONENT_VIDEO_ENCODE]);
+		dev->component[COMP_VIDEO_ENCODE]);

 unreg_image_encoder:
 	pr_err("Cleanup: Destroy image encoder\n");
 	vchiq_mmal_component_finalise(
 		dev->instance,
-		dev->component[MMAL_COMPONENT_IMAGE_ENCODE]);
+		dev->component[COMP_IMAGE_ENCODE]);

 unreg_preview:
 	pr_err("Cleanup: Destroy video render\n");
 	vchiq_mmal_component_finalise(dev->instance,
-				      dev->component[MMAL_COMPONENT_PREVIEW]);
+				      dev->component[COMP_PREVIEW]);

 unreg_camera:
 	pr_err("Cleanup: Destroy camera\n");
 	vchiq_mmal_component_finalise(dev->instance,
-				      dev->component[MMAL_COMPONENT_CAMERA]);
+				      dev->component[COMP_CAMERA]);

 unreg_mmal:
 	vchiq_mmal_finalise(dev->instance);
@@ -1799,19 +1801,19 @@ static void bcm2835_cleanup_instance(struct bm2835_mmal_dev *dev)
 					     dev->capture.encode_component);
 	}
 	vchiq_mmal_component_disable(dev->instance,
-				     dev->component[MMAL_COMPONENT_CAMERA]);
+				     dev->component[COMP_CAMERA]);

 	vchiq_mmal_component_finalise(dev->instance,
-				      dev->component[MMAL_COMPONENT_VIDEO_ENCODE]);
+				      dev->component[COMP_VIDEO_ENCODE]);

 	vchiq_mmal_component_finalise(dev->instance,
-				      dev->component[MMAL_COMPONENT_IMAGE_ENCODE]);
+				      dev->component[COMP_IMAGE_ENCODE]);

 	vchiq_mmal_component_finalise(dev->instance,
-				      dev->component[MMAL_COMPONENT_PREVIEW]);
+				      dev->component[COMP_PREVIEW]);

 	vchiq_mmal_component_finalise(dev->instance,
-				      dev->component[MMAL_COMPONENT_CAMERA]);
+				      dev->component[COMP_CAMERA]);

 	v4l2_ctrl_handler_free(&dev->ctrl_handler);

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
index 9833828..b8a466e 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
@@ -16,18 +16,18 @@
 #define V4L2_CTRL_COUNT 29 /* number of v4l controls */

 enum {
-	MMAL_COMPONENT_CAMERA = 0,
-	MMAL_COMPONENT_PREVIEW,
-	MMAL_COMPONENT_IMAGE_ENCODE,
-	MMAL_COMPONENT_VIDEO_ENCODE,
-	MMAL_COMPONENT_COUNT
+	COMP_CAMERA = 0,
+	COMP_PREVIEW,
+	COMP_IMAGE_ENCODE,
+	COMP_VIDEO_ENCODE,
+	COMP_COUNT
 };

 enum {
-	MMAL_CAMERA_PORT_PREVIEW = 0,
-	MMAL_CAMERA_PORT_VIDEO,
-	MMAL_CAMERA_PORT_CAPTURE,
-	MMAL_CAMERA_PORT_COUNT
+	CAM_PORT_PREVIEW = 0,
+	CAM_PORT_VIDEO,
+	CAM_PORT_CAPTURE,
+	CAM_PORT_COUNT
 };

 #define PREVIEW_LAYER      2
@@ -61,7 +61,7 @@ struct bm2835_mmal_dev {

 	/* allocated mmal instance and components */
 	struct vchiq_mmal_instance   *instance;
-	struct vchiq_mmal_component  *component[MMAL_COMPONENT_COUNT];
+	struct vchiq_mmal_component  *component[COMP_COUNT];
 	int camera_use_count;

 	struct v4l2_window overlay;
diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index e79e7cd..b3d7029 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -177,7 +177,7 @@ static int ctrl_set_rational(struct bm2835_mmal_dev *dev,
 	struct mmal_parameter_rational rational_value;
 	struct vchiq_mmal_port *control;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	rational_value.num = ctrl->val;
 	rational_value.den = 100;
@@ -195,7 +195,7 @@ static int ctrl_set_value(struct bm2835_mmal_dev *dev,
 	u32 u32_value;
 	struct vchiq_mmal_port *control;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	u32_value = ctrl->val;

@@ -220,7 +220,7 @@ static int ctrl_set_iso(struct bm2835_mmal_dev *dev,
 		dev->manual_iso_enabled =
 				(ctrl->val == V4L2_ISO_SENSITIVITY_MANUAL);

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	if (dev->manual_iso_enabled)
 		u32_value = dev->iso;
@@ -239,7 +239,7 @@ static int ctrl_set_value_ev(struct bm2835_mmal_dev *dev,
 	s32 s32_value;
 	struct vchiq_mmal_port *control;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	s32_value = (ctrl->val - 12) * 2;	/* Convert from index to 1/6ths */

@@ -256,7 +256,7 @@ static int ctrl_set_rotate(struct bm2835_mmal_dev *dev,
 	u32 u32_value;
 	struct vchiq_mmal_component *camera;

-	camera = dev->component[MMAL_COMPONENT_CAMERA];
+	camera = dev->component[COMP_CAMERA];

 	u32_value = ((ctrl->val % 360) / 90) * 90;

@@ -290,7 +290,7 @@ static int ctrl_set_flip(struct bm2835_mmal_dev *dev,
 	else
 		dev->vflip = ctrl->val;

-	camera = dev->component[MMAL_COMPONENT_CAMERA];
+	camera = dev->component[COMP_CAMERA];

 	if (dev->hflip && dev->vflip)
 		u32_value = MMAL_PARAM_MIRROR_BOTH;
@@ -327,7 +327,7 @@ static int ctrl_set_exposure(struct bm2835_mmal_dev *dev,
 	struct vchiq_mmal_port *control;
 	int ret = 0;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	if (mmal_ctrl->mmal_id == MMAL_PARAMETER_SHUTTER_SPEED)	{
 		/* V4L2 is in 100usec increments.
@@ -402,7 +402,7 @@ static int ctrl_set_metering_mode(struct bm2835_mmal_dev *dev,
 		struct vchiq_mmal_port *control;
 		u32 u32_value = dev->metering_mode;

-		control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+		control = &dev->component[COMP_CAMERA]->control;

 		return vchiq_mmal_port_parameter_set(dev->instance, control,
 					     mmal_ctrl->mmal_id,
@@ -418,7 +418,7 @@ static int ctrl_set_flicker_avoidance(struct bm2835_mmal_dev *dev,
 	u32 u32_value;
 	struct vchiq_mmal_port *control;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	switch (ctrl->val) {
 	case V4L2_CID_POWER_LINE_FREQUENCY_DISABLED:
@@ -447,7 +447,7 @@ static int ctrl_set_awb_mode(struct bm2835_mmal_dev *dev,
 	u32 u32_value;
 	struct vchiq_mmal_port *control;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	switch (ctrl->val) {
 	case V4L2_WHITE_BALANCE_MANUAL:
@@ -503,7 +503,7 @@ static int ctrl_set_awb_gains(struct bm2835_mmal_dev *dev,
 	struct vchiq_mmal_port *control;
 	struct mmal_parameter_awbgains gains;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	if (ctrl->id == V4L2_CID_RED_BALANCE)
 		dev->red_gain = ctrl->val;
@@ -551,7 +551,7 @@ static int ctrl_set_image_effect(struct bm2835_mmal_dev *dev,
 					v4l2_to_mmal_effects_values[i].v;
 			}

-			control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+			control = &dev->component[COMP_CAMERA]->control;

 			ret = vchiq_mmal_port_parameter_set(
 					dev->instance, control,
@@ -584,7 +584,7 @@ static int ctrl_set_colfx(struct bm2835_mmal_dev *dev,
 	int ret;
 	struct vchiq_mmal_port *control;

-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	dev->colourfx.enable = (ctrl->val & 0xff00) >> 8;
 	dev->colourfx.enable = ctrl->val & 0xff;
@@ -610,7 +610,7 @@ static int ctrl_set_bitrate(struct bm2835_mmal_dev *dev,

 	dev->capture.encode_bitrate = ctrl->val;

-	encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
+	encoder_out = &dev->component[COMP_VIDEO_ENCODE]->output[0];

 	ret = vchiq_mmal_port_parameter_set(dev->instance, encoder_out,
 					    mmal_ctrl->mmal_id, &ctrl->val,
@@ -636,7 +636,7 @@ static int ctrl_set_bitrate_mode(struct bm2835_mmal_dev *dev,
 	u32 bitrate_mode;
 	struct vchiq_mmal_port *encoder_out;

-	encoder_out = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
+	encoder_out = &dev->component[COMP_VIDEO_ENCODE]->output[0];

 	dev->capture.encode_bitrate_mode = ctrl->val;
 	switch (ctrl->val) {
@@ -663,7 +663,7 @@ static int ctrl_set_image_encode_output(struct bm2835_mmal_dev *dev,
 	u32 u32_value;
 	struct vchiq_mmal_port *jpeg_out;

-	jpeg_out = &dev->component[MMAL_COMPONENT_IMAGE_ENCODE]->output[0];
+	jpeg_out = &dev->component[COMP_IMAGE_ENCODE]->output[0];

 	u32_value = ctrl->val;

@@ -679,7 +679,7 @@ static int ctrl_set_video_encode_param_output(struct bm2835_mmal_dev *dev,
 	u32 u32_value;
 	struct vchiq_mmal_port *vid_enc_ctl;

-	vid_enc_ctl = &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0];
+	vid_enc_ctl = &dev->component[COMP_VIDEO_ENCODE]->output[0];

 	u32_value = ctrl->val;

@@ -792,7 +792,7 @@ static int ctrl_set_video_encode_profile_level(struct bm2835_mmal_dev *dev,
 		}

 		ret = vchiq_mmal_port_parameter_set(dev->instance,
-						    &dev->component[MMAL_COMPONENT_VIDEO_ENCODE]->output[0],
+						    &dev->component[COMP_VIDEO_ENCODE]->output[0],
 			mmal_ctrl->mmal_id,
 			&param, sizeof(param));
 	}
@@ -810,7 +810,7 @@ static int ctrl_set_scene_mode(struct bm2835_mmal_dev *dev,
 	v4l2_dbg(0, bcm2835_v4l2_debug, &dev->v4l2_dev,
 		 "scene mode selected %d, was %d\n", ctrl->val,
 		 dev->scene_mode);
-	control = &dev->component[MMAL_COMPONENT_CAMERA]->control;
+	control = &dev->component[COMP_CAMERA]->control;

 	if (ctrl->val == dev->scene_mode)
 		return 0;
@@ -1228,18 +1228,15 @@ int set_framerate_params(struct bm2835_mmal_dev *dev)
 		 fps_range.fps_high.den);

 	ret = vchiq_mmal_port_parameter_set(dev->instance,
-					    &dev->component[MMAL_COMPONENT_CAMERA]->
-					    output[MMAL_CAMERA_PORT_PREVIEW],
+					    &dev->component[COMP_CAMERA]->output[CAM_PORT_PREVIEW],
 					    MMAL_PARAMETER_FPS_RANGE,
 					    &fps_range, sizeof(fps_range));
 	ret += vchiq_mmal_port_parameter_set(dev->instance,
-					     &dev->component[MMAL_COMPONENT_CAMERA]->
-					     output[MMAL_CAMERA_PORT_VIDEO],
+					     &dev->component[COMP_CAMERA]->output[CAM_PORT_VIDEO],
 					     MMAL_PARAMETER_FPS_RANGE,
 					     &fps_range, sizeof(fps_range));
 	ret += vchiq_mmal_port_parameter_set(dev->instance,
-					     &dev->component[MMAL_COMPONENT_CAMERA]->
-					     output[MMAL_CAMERA_PORT_CAPTURE],
+					     &dev->component[COMP_CAMERA]->output[CAM_PORT_CAPTURE],
 					     MMAL_PARAMETER_FPS_RANGE,
 					     &fps_range, sizeof(fps_range));
 	if (ret)
--
2.7.4

[PATCH 11/31] staging: bcm2835-camera: Fix multiple line dereference errors

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix checkpatch errors "Avoid multiple line dereference"

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 23f7754..bde3548 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -1028,12 +1028,10 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,
 		if (f->fmt.pix.width <= max_video_width &&
 		    f->fmt.pix.height <= max_video_height)
 			camera_port = port =
-			    &dev->component[COMP_CAMERA]->
-			    output[CAM_PORT_VIDEO];
+			    &dev->component[COMP_CAMERA]->output[CAM_PORT_VIDEO];
 		else
 			camera_port = port =
-			    &dev->component[COMP_CAMERA]->
-			    output[CAM_PORT_CAPTURE];
+			    &dev->component[COMP_CAMERA]->output[CAM_PORT_CAPTURE];
 		break;
 	case COMP_IMAGE_ENCODE:
 		encode_component = dev->component[COMP_IMAGE_ENCODE];
@@ -1224,9 +1222,8 @@ static int mmal_setup_components(struct bm2835_mmal_dev *dev,
 						 port->current_buffer.size);
 					port->current_buffer.size =
 					    (f->fmt.pix.sizeimage <
-					     (100 << 10))
-					    ? (100 << 10)
-					    : f->fmt.pix.sizeimage;
+					     (100 << 10)) ?
+					    (100 << 10) : f->fmt.pix.sizeimage;
 				}
 				v4l2_dbg(1, bcm2835_v4l2_debug,
 					 &dev->v4l2_dev,
--
2.7.4

[PATCH 12/31] staging: bcm2835-camera: Fix brace style issues.

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix mismatched or missing brace issues flagged by checkpatch.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 3 ++-
 drivers/staging/vc04_services/bcm2835-camera/controls.c       | 3 ++-
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c     | 3 ++-
 3 files changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index bde3548..c1f6141 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -553,10 +553,11 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)

 		/* Flag to indicate just to rely on kernel timestamps */
 		dev->capture.vc_start_timestamp = -1;
-	} else
+	} else {
 		v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
 			 "Start time %lld size %d\n",
 			 dev->capture.vc_start_timestamp, parameter_size);
+	}

 	dev->capture.kernel_start_ts = ktime_get();
 	dev->capture.last_timestamp = 0;
diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index b3d7029..1a7588d 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -407,8 +407,9 @@ static int ctrl_set_metering_mode(struct bm2835_mmal_dev *dev,
 		return vchiq_mmal_port_parameter_set(dev->instance, control,
 					     mmal_ctrl->mmal_id,
 					     &u32_value, sizeof(u32_value));
-	} else
+	} else {
 		return 0;
+	}
 }

 static int ctrl_set_flicker_avoidance(struct bm2835_mmal_dev *dev,
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 5175e2c..73cb295 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -1262,9 +1262,10 @@ static int port_parameter_get(struct vchiq_mmal_instance *instance,
 		memcpy(value, &rmsg->u.port_parameter_get_reply.value,
 		       *value_size);
 		*value_size = rmsg->u.port_parameter_get_reply.size;
-	} else
+	} else {
 		memcpy(value, &rmsg->u.port_parameter_get_reply.value,
 		       rmsg->u.port_parameter_get_reply.size);
+	}

 	pr_debug("%s:result:%d component:0x%x port:%d parameter:%d\n", __func__,
 		 ret, port->component->handle, port->handle, parameter_id);
--
2.7.4

[PATCH 13/31] staging: bcm2835-camera: Fix missing lines between items

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix checkpatch errors for missing blank lines after variable
or structure declarations.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
index b8a466e..bbfe8fe 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
@@ -130,6 +130,7 @@ int set_framerate_params(struct bm2835_mmal_dev *dev);
 		(pix_fmt)->pixelformat, (pix_fmt)->bytesperline,	\
 		(pix_fmt)->sizeimage, (pix_fmt)->colorspace, (pix_fmt)->priv); \
 }
+
 #define v4l2_dump_win_format(level, debug, dev, win_fmt, desc)	\
 {	\
 	v4l2_dbg(level, debug, dev,	\
--
2.7.4

[PATCH 14/31] staging: bcm2835-camera: Fix open parenthesis alignment

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Fix checkpatch "Alignment should match open parenthesis"
errors.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/bcm2835-camera.c  |  9 ++++----
 .../vc04_services/bcm2835-camera/controls.c        | 25 ++++++++++++++--------
 .../vc04_services/bcm2835-camera/mmal-vchiq.c      |  2 +-
 .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  6 +++---
 4 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index c1f6141..4968782 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -568,8 +568,8 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	    vchiq_mmal_port_enable(dev->instance, dev->capture.port, buffer_cb);
 	if (ret) {
 		v4l2_err(&dev->v4l2_dev,
-			"Failed to enable capture port - error %d. Disabling camera port again\n",
-			ret);
+			 "Failed to enable capture port - error %d. Disabling camera port again\n",
+			 ret);

 		vchiq_mmal_port_disable(dev->instance,
 					dev->capture.camera_port);
@@ -961,8 +961,7 @@ static int vidioc_try_fmt_vid_cap(struct file *file, void *priv,
 		f->fmt.pix.bytesperline =
 			(f->fmt.pix.bytesperline + align_mask) & ~align_mask;
 		v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
-			 "Not removing padding, so bytes/line = %d, "
-			 "(align_mask %d)\n",
+			 "Not removing padding, so bytes/line = %d, (align_mask %d)\n",
 			 f->fmt.pix.bytesperline, align_mask);
 	}

@@ -1308,7 +1307,7 @@ static int vidioc_s_fmt_vid_cap(struct file *file, void *priv,
 }

 static int vidioc_enum_framesizes(struct file *file, void *fh,
-			   struct v4l2_frmsizeenum *fsize)
+				  struct v4l2_frmsizeenum *fsize)
 {
 	struct bm2835_mmal_dev *dev = video_drvdata(file);
 	static const struct v4l2_frmsize_stepwise sizes = {
diff --git a/drivers/staging/vc04_services/bcm2835-camera/controls.c b/drivers/staging/vc04_services/bcm2835-camera/controls.c
index 1a7588d..bc2f3f4 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/controls.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/controls.c
@@ -1260,9 +1260,12 @@ int bm2835_mmal_init_controls(struct bm2835_mmal_dev *dev,

 		switch (ctrl->type) {
 		case MMAL_CONTROL_TYPE_STD:
-			dev->ctrls[c] = v4l2_ctrl_new_std(hdl,
-				&bm2835_mmal_ctrl_ops, ctrl->id,
-				ctrl->min, ctrl->max, ctrl->step, ctrl->def);
+			dev->ctrls[c] =
+				v4l2_ctrl_new_std(hdl,
+						  &bm2835_mmal_ctrl_ops,
+						  ctrl->id, ctrl->min,
+						  ctrl->max, ctrl->step,
+						  ctrl->def);
 			break;

 		case MMAL_CONTROL_TYPE_STD_MENU:
@@ -1286,16 +1289,20 @@ int bm2835_mmal_init_controls(struct bm2835_mmal_dev *dev,
 				mask = ~mask;
 			}

-			dev->ctrls[c] = v4l2_ctrl_new_std_menu(hdl,
-			&bm2835_mmal_ctrl_ops, ctrl->id,
-			ctrl->max, mask, ctrl->def);
+			dev->ctrls[c] =
+				v4l2_ctrl_new_std_menu(hdl,
+						       &bm2835_mmal_ctrl_ops,
+						       ctrl->id, ctrl->max,
+						       mask, ctrl->def);
 			break;
 		}

 		case MMAL_CONTROL_TYPE_INT_MENU:
-			dev->ctrls[c] = v4l2_ctrl_new_int_menu(hdl,
-				&bm2835_mmal_ctrl_ops, ctrl->id,
-				ctrl->max, ctrl->def, ctrl->imenu);
+			dev->ctrls[c] =
+				v4l2_ctrl_new_int_menu(hdl,
+						       &bm2835_mmal_ctrl_ops,
+						       ctrl->id, ctrl->max,
+						       ctrl->def, ctrl->imenu);
 			break;

 		case MMAL_CONTROL_TYPE_CLUSTER:
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 73cb295..0b95723 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -645,7 +645,7 @@ static int send_synchronous_mmal_msg(struct vchiq_mmal_instance *instance,
 	if (payload_len >
 	    (MMAL_MSG_MAX_SIZE - sizeof(struct mmal_msg_header))) {
 		pr_err("payload length %d exceeds max:%d\n", payload_len,
-		      (int)(MMAL_MSG_MAX_SIZE -
+		       (int)(MMAL_MSG_MAX_SIZE -
 			    sizeof(struct mmal_msg_header)));
 		return -EINVAL;
 	}
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
index 0e5a81b..3498555 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
@@ -128,7 +128,7 @@ int vchiq_mmal_port_enable(
  * disable a port will dequeue any pending buffers
  */
 int vchiq_mmal_port_disable(struct vchiq_mmal_instance *instance,
-			   struct vchiq_mmal_port *port);
+			    struct vchiq_mmal_port *port);

 int vchiq_mmal_port_parameter_set(struct vchiq_mmal_instance *instance,
 				  struct vchiq_mmal_port *port,
@@ -146,8 +146,8 @@ int vchiq_mmal_port_set_format(struct vchiq_mmal_instance *instance,
 			       struct vchiq_mmal_port *port);

 int vchiq_mmal_port_connect_tunnel(struct vchiq_mmal_instance *instance,
-			    struct vchiq_mmal_port *src,
-			    struct vchiq_mmal_port *dst);
+				   struct vchiq_mmal_port *src,
+				   struct vchiq_mmal_port *dst);

 int vchiq_mmal_version(struct vchiq_mmal_instance *instance,
 		       u32 *major_out,
--
2.7.4

[PATCH 15/31] staging: bcm2835-camera: Ensure all buffers are returned on disable

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

With the recent change to match MMAL and V4L2 buffers there
is a need to wait for all MMAL buffers to be returned during
stop_streaming.

Fixes: 938416707071 ("staging: bcm2835-camera: Remove V4L2/MMAL buffer remapping")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 22 ++++++++++++++++------
 .../vc04_services/bcm2835-camera/mmal-vchiq.c      |  4 ++++
 .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  3 +++
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 4968782..36eb4d6 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -595,6 +595,7 @@ static void stop_streaming(struct vb2_queue *vq)
 	int ret;
 	unsigned long timeout;
 	struct bm2835_mmal_dev *dev = vb2_get_drv_priv(vq);
+	struct vchiq_mmal_port *port = dev->capture.port;

 	v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "%s: dev:%p\n",
 		 __func__, dev);
@@ -618,12 +619,6 @@ static void stop_streaming(struct vb2_queue *vq)
 				      &dev->capture.frame_count,
 				      sizeof(dev->capture.frame_count));

-	/* wait for last frame to complete */
-	timeout = wait_for_completion_timeout(&dev->capture.frame_cmplt, HZ);
-	if (timeout == 0)
-		v4l2_err(&dev->v4l2_dev,
-			 "timed out waiting for frame completion\n");
-
 	v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
 		 "disabling connection\n");

@@ -638,6 +633,21 @@ static void stop_streaming(struct vb2_queue *vq)
 			 ret);
 	}

+	/* wait for all buffers to be returned */
+	while (atomic_read(&port->buffers_with_vpu)) {
+		v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
+			 "%s: Waiting for buffers to be returned - %d outstanding\n",
+			 __func__, atomic_read(&port->buffers_with_vpu));
+		timeout = wait_for_completion_timeout(&dev->capture.frame_cmplt,
+						      HZ);
+		if (timeout == 0) {
+			v4l2_err(&dev->v4l2_dev, "%s: Timeout waiting for buffers to be returned - %d outstanding\n",
+				 __func__,
+				 atomic_read(&port->buffers_with_vpu));
+			break;
+		}
+	}
+
 	if (disable_camera(dev) < 0)
 		v4l2_err(&dev->v4l2_dev, "Failed to disable camera\n");
 }
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 0b95723..4d63176 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -246,6 +246,8 @@ static void buffer_work_cb(struct work_struct *work)
 	struct mmal_msg_context *msg_context =
 		container_of(work, struct mmal_msg_context, u.bulk.work);

+	atomic_dec(&msg_context->u.bulk.port->buffers_with_vpu);
+
 	msg_context->u.bulk.port->buffer_cb(msg_context->u.bulk.instance,
 					    msg_context->u.bulk.port,
 					    msg_context->u.bulk.status,
@@ -389,6 +391,8 @@ buffer_from_host(struct vchiq_mmal_instance *instance,
 	INIT_WORK(&msg_context->u.bulk.buffer_to_host_work,
 		  buffer_to_host_work_cb);

+	atomic_inc(&port->buffers_with_vpu);
+
 	/* prep the buffer from host message */
 	memset(&m, 0xbc, sizeof(m));	/* just to make debug clearer */

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
index 3498555..1750ff0 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.h
@@ -72,6 +72,9 @@ struct vchiq_mmal_port {
 	struct list_head buffers;
 	/* lock to serialise adding and removing buffers from list */
 	spinlock_t slock;
+
+	/* Count of buffers the VPU has yet to return */
+	atomic_t buffers_with_vpu;
 	/* callback on buffer completion */
 	vchiq_mmal_buffer_cb buffer_cb;
 	/* callback context */
--
2.7.4

[PATCH 16/31] staging: bcm2835-camera: Remove check of the number of buffers supplied

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Before commit "staging: bcm2835-camera: Remove V4L2/MMAL buffer remapping"
there was a need to ensure that there were sufficient buffers supplied from
the user to cover those being sent to the VPU (always 1).

Now the buffers are linked 1:1 between MMAL and V4L2,
therefore there is no need for that check, and indeed it is wrong
as there is no need to submit all the buffers before starting streaming.

Fixes: 938416707071 ("staging: bcm2835-camera: Remove V4L2/MMAL buffer remapping")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 4d63176..59eb812 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -1338,16 +1338,6 @@ static int port_enable(struct vchiq_mmal_instance *instance,
 	if (port->enabled)
 		return 0;

-	/* ensure there are enough buffers queued to cover the buffer headers */
-	if (port->buffer_cb) {
-		hdr_count = 0;
-		list_for_each(buf_head, &port->buffers) {
-			hdr_count++;
-		}
-		if (hdr_count < port->current_buffer.num)
-			return -ENOSPC;
-	}
-
 	ret = port_action_port(instance, port,
 			       MMAL_MSG_PORT_ACTION_TYPE_ENABLE);
 	if (ret)
--
2.7.4

[PATCH 17/31] staging: bcm2835-camera: Handle empty EOS buffers whilst streaming

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

The change to mapping V4L2 to MMAL buffers 1:1 didn't handle
the condition we get with raw pixel buffers (eg YUV and RGB)
direct from the camera's stills port. That sends the pixel buffer
and then an empty buffer with the EOS flag set. The EOS buffer
wasn't handled and returned an error up the stack.

Handle the condition correctly by returning it to the component
if streaming, or returning with an error if stopping streaming.

Fixes: 938416707071 ("staging: bcm2835-camera: Remove V4L2/MMAL buffer remapping")
Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 .../vc04_services/bcm2835-camera/bcm2835-camera.c   | 21 ++++++++++++---------
 .../vc04_services/bcm2835-camera/mmal-vchiq.c       |  5 +++--
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 36eb4d6..f2e951c 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -339,16 +339,13 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,

 	if (length == 0) {
 		/* stream ended */
-		if (buf) {
-			/* this should only ever happen if the port is
-			 * disabled and there are buffers still queued
+		if (dev->capture.frame_count) {
+			/* empty buffer whilst capturing - expected to be an
+			 * EOS, so grab another frame
 			 */
-			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
-			pr_debug("Empty buffer");
-		} else if (dev->capture.frame_count) {
-			/* grab another frame */
 			if (is_capturing(dev)) {
-				pr_debug("Grab another frame");
+				v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
+					 "Grab another frame");
 				vchiq_mmal_port_parameter_set(
 					instance,
 					dev->capture.camera_port,
@@ -356,8 +353,14 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 					&dev->capture.frame_count,
 					sizeof(dev->capture.frame_count));
 			}
+			if (vchiq_mmal_submit_buffer(instance, port, buf))
+				v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
+					 "Failed to return EOS buffer");
 		} else {
-			/* signal frame completion */
+			/* stopping streaming.
+			 * return buffer, and signal frame completion
+			 */
+			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_ERROR);
 			complete(&dev->capture.frame_cmplt);
 		}
 		return;
diff --git a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
index 59eb812..d0f7b67 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/mmal-vchiq.c
@@ -332,8 +332,6 @@ static int bulk_receive(struct vchiq_mmal_instance *instance,

 	/* store length */
 	msg_context->u.bulk.buffer_used = rd_len;
-	msg_context->u.bulk.mmal_flags =
-	    msg->u.buffer_from_host.buffer_header.flags;
 	msg_context->u.bulk.dts = msg->u.buffer_from_host.buffer_header.dts;
 	msg_context->u.bulk.pts = msg->u.buffer_from_host.buffer_header.pts;

@@ -461,6 +459,9 @@ static void buffer_to_host_cb(struct vchiq_mmal_instance *instance,
 		return;
 	}

+	msg_context->u.bulk.mmal_flags =
+				msg->u.buffer_from_host.buffer_header.flags;
+
 	if (msg->h.status != MMAL_MSG_STATUS_SUCCESS) {
 		/* message reception had an error */
 		pr_warn("error %d in reply\n", msg->h.status);
--
2.7.4

[PATCH 18/31] staging: bcm2835-camera: Set sequence number correctly

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

Set the sequence number in vb2_v4l2_buffer mainly so the
latest v4l2-ctl reports the frame rate correctly.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 4 ++++
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h | 2 ++
 2 files changed, 6 insertions(+)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index f2e951c..9967df9 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -398,6 +398,7 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 		}
 	}
 	dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
+	buf->vb.sequence = dev->capture.sequence++;

 	vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
 	if (mmal_flags & MMAL_BUFFER_HEADER_FLAG_KEYFRAME)
@@ -525,6 +526,9 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
 	/* enable frame capture */
 	dev->capture.frame_count = 1;

+	/* reset sequence number */
+	dev->capture.sequence = 0;
+
 	/* if the preview is not already running, wait for a few frames for AGC
 	 * to settle down.
 	 */
diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
index bbfe8fe..c821513 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
@@ -93,6 +93,8 @@ struct bm2835_mmal_dev {
 		ktime_t kernel_start_ts;
 		/* Timestamp of last frame */
 		u64		last_timestamp;
+		/* Sequence number of last buffer */
+		u32		sequence;

 		struct vchiq_mmal_port  *port; /* port being used for capture */
 		/* camera port being used for capture */
--
2.7.4

[PATCH 19/31] staging: bcm2835-camera: Ensure timestamps never go backwards.

[Stefan Wahren]

From: Dave Stevenson <dave.stevenson@raspberrypi.org>

There is an awkward situation with H264 header bytes. Currently
they are returned with a PTS of 0 because they aren't associated
with a timestamped frame to encode. These are handled by either
returning the timestamp of the last buffer to have been received,
or in the case of the first buffer the timestamp taken at
start_streaming.
This results in a race where the current frame may have started
before we take the start time, which results in the first encoded
frame having an earlier timestamp than the header bytes.

Ensure that we never return a negative delta to the user by checking
against the previous timestamp.

Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
---
 drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
index 9967df9..6205793 100644
--- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
+++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
@@ -387,6 +387,11 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
 			 ktime_to_ns(dev->capture.kernel_start_ts),
 			 dev->capture.vc_start_timestamp, pts,
 			 ktime_to_ns(timestamp));
+		if (timestamp < dev->capture.last_timestamp) {
+			v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
+				 "Negative delta - using last time\n");
+			timestamp = dev->capture.last_timestamp;
+		}
 		buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
 	} else {
 		if (dev->capture.last_timestamp) {
--
2.7.4

Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 3484 bytes --]

Hi Dave,

Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit :
> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> 
> H264 header come from VC with 0 timestamps, which means they get a
> strange timestamp when processed with VC/kernel start times,
> particularly if used with the inline header option.
> Remember the last frame timestamp and use that if set, or otherwise
> use the kernel start time.

Normally H264 headers are considered to be part of the following frame.
Giving it the timestamp of the previous frame will likely confuse some
userspace and cause an off-by-one in timestamp. I understand this is a
workaround, but am wondering if this can be improved.

> 
> Link: https://github.com/raspberrypi/linux/issues/1836
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> ---
>  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 14 ++++++++++++--
>  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h  |  2 ++
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index dce6e6d..0c04815 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
>  		}
>  	} else {
>  		if (dev->capture.frame_count) {
> -			if (dev->capture.vc_start_timestamp != -1 && pts) {
> +			if (dev->capture.vc_start_timestamp != -1) {
> +				buf->vb.vb2_buf.timestamp = ktime_get_ns();
> +			} else if (pts) {
>  				ktime_t timestamp;
>  				s64 runtime_us = pts -
>  				    dev->capture.vc_start_timestamp;
> @@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
>  					 ktime_to_ns(timestamp));
>  				buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
>  			} else {
> -				buf->vb.vb2_buf.timestamp = ktime_get_ns();
> +				if (dev->capture.last_timestamp) {
> +					buf->vb.vb2_buf.timestamp =
> +						dev->capture.last_timestamp;
> +				} else {
> +					buf->vb.vb2_buf.timestamp =
> +						ktime_to_ns(dev->capture.kernel_start_ts);
> +				}
>  			}
> +			dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
> 
>  			vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
>  			vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> @@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>  			 dev->capture.vc_start_timestamp, parameter_size);
> 
>  	dev->capture.kernel_start_ts = ktime_get();
> +	dev->capture.last_timestamp = 0;
> 
>  	/* enable the camera port */
>  	dev->capture.port->cb_ctx = dev;
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> index 2b5679e..09273b0 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
>  		s64         vc_start_timestamp;
>  		/* Kernel start timestamp for streaming */
>  		ktime_t kernel_start_ts;
> +		/* Timestamp of last frame */
> +		u64		last_timestamp;
> 
>  		struct vchiq_mmal_port  *port; /* port being used for capture */
>  		/* camera port being used for capture */
> --
> 2.7.4
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 19/31] staging: bcm2835-camera: Ensure timestamps never go backwards.

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 2104 bytes --]

Le jeudi 27 juin 2019 à 20:56 +0200, Stefan Wahren a écrit :
> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> 
> There is an awkward situation with H264 header bytes. Currently
> they are returned with a PTS of 0 because they aren't associated
> with a timestamped frame to encode. These are handled by either
> returning the timestamp of the last buffer to have been received,
> or in the case of the first buffer the timestamp taken at
> start_streaming.
> This results in a race where the current frame may have started
> before we take the start time, which results in the first encoded
> frame having an earlier timestamp than the header bytes.
> 
> Ensure that we never return a negative delta to the user by checking
> against the previous timestamp.
> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> ---
>  drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> index 9967df9..6205793 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> @@ -387,6 +387,11 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
>  			 ktime_to_ns(dev->capture.kernel_start_ts),
>  			 dev->capture.vc_start_timestamp, pts,
>  			 ktime_to_ns(timestamp));
> +		if (timestamp < dev->capture.last_timestamp) {
> +			v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev,
> +				 "Negative delta - using last time\n");
> +			timestamp = dev->capture.last_timestamp;
> +		}

Instead of that, maybe you could request a minimum number of buffers,
and not let the header buffer go until you have a proper "following
timestamp" to give it. This way you don't need this hack, and you won't
have an off-by-one in timestamps.

>  		buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
>  	} else {
>  		if (dev->capture.last_timestamp) {
> --
> 2.7.4
> 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Dan Carpenter]

On Thu, Jun 27, 2019 at 08:55:58PM +0200, Stefan Wahren wrote:
> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> index 2b5679e..09273b0 100644
> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
>  		s64         vc_start_timestamp;
>  		/* Kernel start timestamp for streaming */
>  		ktime_t kernel_start_ts;
> +		/* Timestamp of last frame */
> +		u64		last_timestamp;

Not directly related to this patch but the indenting in this .h file is
all higgle-piggledy.

regards,
dan carpenter

Re: [PATCH 06/31] staging: bcm2835-camera: Return early on errors

[Dan Carpenter]

On Thu, Jun 27, 2019 at 08:56:03PM +0200, Stefan Wahren wrote:
>  	v4l2_dbg(1, bcm2835_v4l2_debug, &dev->v4l2_dev, "connecting %p to %p\n",
>  		 src, dst);
>  	ret = vchiq_mmal_port_connect_tunnel(dev->instance, src, dst);
>  	if (!ret)
>  		ret = vchiq_mmal_port_enable(dev->instance, src, NULL);
> -error:
> +
>  	return ret;

In future patches, you probably want to flip this one around as well.
Try to do error handling instead of success handling.  In other words,
keep the success patch indented one tab and the failure path indented
two tabs.  Don't make the last failure check in the function special.

	ret = vchiq_mmal_port_connect_tunnel(dev->instance, src, dst);
	if (ret)
		return ret;

	ret = vchiq_mmal_port_enable(dev->instance, src, NULL);
	if (ret)
		return ret;
	return 0;

Or you can make the last check a little special if you want...

	return vchiq_mmal_port_enable(dev->instance, src, NULL);

Either format is good.

regards,
dan carpenter

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Hans Verkuil]

Hi Stefan,

On 6/27/19 8:55 PM, Stefan Wahren wrote:
> This is an attempt to help Dave Stevenson to get all the fixes and
> improvements of the bcm2835-camera driver into mainline.
> 
> Mostly i only polished the commit logs for upstream.
> 
> The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
> re
> return behavior of ctrl_set_bitrate().

Thank you for working on this.

Three high-level questions:

1) Can you post the output of 'v4l2-compliance -s' using the latest v4l2-compliance
   from https://git.linuxtv.org/v4l-utils.git ?  I'm interested to see what the
   status is of this driver w.r.t. the compliance tests.

2) What is the status of this driver from your point of view? What is needed to
   get it out of staging?

3) Out of curiosity: is this driver still valid for RPi4?

Regards,

	Hans

> 
> Dave Stevenson (31):
>   staging: bcm2835-camera: Ensure H264 header bytes get a sensible
>     timestamp
>   staging: bcm2835-camera: Check the error for REPEAT_SEQ_HEADER
>   staging: bcm2835-camera: Replace spinlock protecting context_map with
>     mutex
>   staging: bcm2835-camera: Do not bulk receive from service thread
>   staging: bcm2835-camera: Correctly denote key frames in encoded data
>   staging: bcm2835-camera: Return early on errors
>   staging: bcm2835-camera: Remove dead email addresses
>   staging: bcm2835-camera: Fix comment style violations.
>   staging: bcm2835-camera: Fix spacing around operators
>   staging: bcm2835-camera: Reduce length of enum names
>   staging: bcm2835-camera: Fix multiple line dereference errors
>   staging: bcm2835-camera: Fix brace style issues.
>   staging: bcm2835-camera: Fix missing lines between items
>   staging: bcm2835-camera: Fix open parenthesis alignment
>   staging: bcm2835-camera: Ensure all buffers are returned on disable
>   staging: bcm2835-camera: Remove check of the number of buffers
>     supplied
>   staging: bcm2835-camera: Handle empty EOS buffers whilst streaming
>   staging: bcm2835-camera: Set sequence number correctly
>   staging: bcm2835-camera: Ensure timestamps never go backwards.
>   staging: bcm2835-camera: Add multiple inclusion protection to headers
>   staging: bcm2835-camera: Unify header inclusion defines
>   staging: bcm2835-camera: Fix multiple assignments should be avoided
>   staging: bcm2835-camera: Fix up mmal-parameters.h
>   staging: bcm2835-camera: Use enums for max value in controls
>   staging: bcm2835-camera: Correct V4L2_CID_COLORFX_CBCR behaviour
>   staging: bcm2835-camera: Remove/amend some obsolete comments
>   staging: mmal-vchiq: Avoid use of bool in structures
>   staging: bcm2835-camera: Fix stride on RGB3/BGR3 formats
>   staging: bcm2835-camera: Add sanity checks for queue_setup/CREATE_BUFS
>   staging: bcm2835-camera: Set the field value within ach buffer
>   staging: bcm2835-camera: Correct ctrl min/max/step/def to 64bit
> 
>  .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 378 ++++++++++++----=
> -----
>  .../vc04_services/bcm2835-camera/bcm2835-camera.h  |  34 +-
>  .../vc04_services/bcm2835-camera/controls.c        | 184 +++++-----
>  .../vc04_services/bcm2835-camera/mmal-common.h     |  12 +-
>  .../vc04_services/bcm2835-camera/mmal-encodings.h  |   9 +-
>  .../vc04_services/bcm2835-camera/mmal-msg-common.h |   9 +-
>  .../vc04_services/bcm2835-camera/mmal-msg-format.h | 104 +++---
>  .../vc04_services/bcm2835-camera/mmal-msg-port.h   | 133 ++++----
>  .../vc04_services/bcm2835-camera/mmal-msg.h        | 150 ++++----
>  .../vc04_services/bcm2835-camera/mmal-parameters.h | 286 +++++++++-------
>  .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 159 +++++----
>  .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  22 +-
>  12 files changed, 826 insertions(+), 654 deletions(-)
> 
> =2D-
> 2.7.4
> 

Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Dave Stevenson]

Hi Nicolas

On Thu, 27 Jun 2019 at 20:55, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>
> Hi Dave,
>
> Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit :
> > From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> >
> > H264 header come from VC with 0 timestamps, which means they get a
> > strange timestamp when processed with VC/kernel start times,
> > particularly if used with the inline header option.
> > Remember the last frame timestamp and use that if set, or otherwise
> > use the kernel start time.
>
> Normally H264 headers are considered to be part of the following frame.
> Giving it the timestamp of the previous frame will likely confuse some
> userspace and cause an off-by-one in timestamp. I understand this is a
> workaround, but am wondering if this can be improved.

Sorry, slight ambiguity in how I'm reading your comment.

Are you saying that the header bytes want to be in the same buffer as
the following frame?
I thought this had also been discussed in the V4L2 stateful codec API
threads along with how many encoded frames were allowed in a single
V4L2 buffer. I certainly hadn't seen a statement about the header
bytes being combined with the next frame.
If the behaviour required by V4L2 is that header bytes and following
frame are in the same buffer, then that is relatively easy to achieve
in the firmware. This workaround can remain for older firmware as it
will never trigger if the firmware has combined the frames.


Or are you saying that the header bytes remain in their own buffer,
but the timestamp wants to be the same as the next frame? That is
harder to achieve in the firmware, but could probably be done in the
kernel driver by holding on to the header bytes frame until the next
buffer had been received, at which point the timestamp can be copied
across. Possible, but just needs slightly careful handling to ensure
we don't lose buffers accidentally.

  Dave

> >
> > Link: https://github.com/raspberrypi/linux/issues/1836
> > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> > ---
> >  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 14 ++++++++++++--
> >  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h  |  2 ++
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > index dce6e6d..0c04815 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > @@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
> >               }
> >       } else {
> >               if (dev->capture.frame_count) {
> > -                     if (dev->capture.vc_start_timestamp != -1 && pts) {
> > +                     if (dev->capture.vc_start_timestamp != -1) {
> > +                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
> > +                     } else if (pts) {
> >                               ktime_t timestamp;
> >                               s64 runtime_us = pts -
> >                                   dev->capture.vc_start_timestamp;
> > @@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
> >                                        ktime_to_ns(timestamp));
> >                               buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
> >                       } else {
> > -                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
> > +                             if (dev->capture.last_timestamp) {
> > +                                     buf->vb.vb2_buf.timestamp =
> > +                                             dev->capture.last_timestamp;
> > +                             } else {
> > +                                     buf->vb.vb2_buf.timestamp =
> > +                                             ktime_to_ns(dev->capture.kernel_start_ts);
> > +                             }
> >                       }
> > +                     dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
> >
> >                       vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
> >                       vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> > @@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> >                        dev->capture.vc_start_timestamp, parameter_size);
> >
> >       dev->capture.kernel_start_ts = ktime_get();
> > +     dev->capture.last_timestamp = 0;
> >
> >       /* enable the camera port */
> >       dev->capture.port->cb_ctx = dev;
> > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > index 2b5679e..09273b0 100644
> > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
> >               s64         vc_start_timestamp;
> >               /* Kernel start timestamp for streaming */
> >               ktime_t kernel_start_ts;
> > +             /* Timestamp of last frame */
> > +             u64             last_timestamp;
> >
> >               struct vchiq_mmal_port  *port; /* port being used for capture */
> >               /* camera port being used for capture */
> > --
> > 2.7.4
> >

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Dave Stevenson]

Hi Stefan

Firstly a huge thank you for picking this up - it's been on my to-do
list for ages, and just hasn't made it to the top.

On Fri, 28 Jun 2019 at 09:06, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>
> Hi Stefan,
>
> On 6/27/19 8:55 PM, Stefan Wahren wrote:
> > This is an attempt to help Dave Stevenson to get all the fixes and
> > improvements of the bcm2835-camera driver into mainline.
> >
> > Mostly i only polished the commit logs for upstream.
> >
> > The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
> > re
> > return behavior of ctrl_set_bitrate().
>
> Thank you for working on this.
>
> Three high-level questions:
>
> 1) Can you post the output of 'v4l2-compliance -s' using the latest v4l2-compliance
>    from https://git.linuxtv.org/v4l-utils.git ?  I'm interested to see what the
>    status is of this driver w.r.t. the compliance tests.

Hi Hans.

Running it against the downstream driver (which should be nearly
identical based on this set of patches), 4.19, on a Pi4 I get
pi@raspberrypi:~/v4l-utils/utils/v4l2-compliance $ ./v4l2-compliance -s
v4l2-compliance SHA: b16f9e945d74aa552abdd6f873821cb77faaf13a, 32 bits

Compliance test for bm2835 mmal device /dev/video0:

Driver Info:
    Driver name      : bm2835 mmal
    Card type        : mmal service 16.1
    Bus info         : platform:bcm2835-v4l2
    Driver version   : 4.19.56
    Capabilities     : 0x85200005
        Video Capture
        Video Overlay
        Read/Write
        Streaming
        Extended Pix Format
        Device Capabilities
    Device Caps      : 0x05200005
        Video Capture
        Video Overlay
        Read/Write
        Streaming
        Extended Pix Format

Required ioctls:
    test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
    test second /dev/video0 open: OK
    test VIDIOC_QUERYCAP: OK
    test VIDIOC_G/S_PRIORITY: OK
    test for unlimited opens: OK

Debug ioctls:
    test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
    test VIDIOC_LOG_STATUS: OK

Input ioctls:
    test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
    test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
    test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
    test VIDIOC_ENUMAUDIO: OK (Not Supported)
    test VIDIOC_G/S/ENUMINPUT: OK
    test VIDIOC_G/S_AUDIO: OK (Not Supported)
    Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
    test VIDIOC_G/S_MODULATOR: OK (Not Supported)
    test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
    test VIDIOC_ENUMAUDOUT: OK (Not Supported)
    test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
    test VIDIOC_G/S_AUDOUT: OK (Not Supported)
    Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
    test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
    test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
    test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
    test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
    test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
    test VIDIOC_QUERYCTRL: OK
    test VIDIOC_G/S_CTRL: OK
    test VIDIOC_G/S/TRY_EXT_CTRLS: OK
    test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
    test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
    Standard Controls: 33 Private Controls: 0

Format ioctls (Input 0):
    test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
    test VIDIOC_G/S_PARM: OK
    test VIDIOC_G_FBUF: OK
    test VIDIOC_G_FMT: OK
    test VIDIOC_TRY_FMT: OK
    test VIDIOC_S_FMT: OK
    test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
    test Cropping: OK (Not Supported)
    test Composing: OK (Not Supported)
    test Scaling: OK

Codec ioctls (Input 0):
    test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
    test VIDIOC_G_ENC_INDEX: OK (Not Supported)
    test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
    test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
    test VIDIOC_EXPBUF: OK (Not Supported)
    test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
    test read/write: OK
    test blocking wait: OK
            warn: v4l2-test-buffers.cpp(1429): Can free buffers even
if still mmap()ed
    test MMAP (no poll): OK
            warn: v4l2-test-buffers.cpp(1429): Can free buffers even
if still mmap()ed
    test MMAP (select): OK
            warn: v4l2-test-buffers.cpp(1429): Can free buffers even
if still mmap()ed
    test MMAP (epoll): OK
    test USERPTR (no poll): OK
    test USERPTR (select): OK
    test DMABUF (no poll): OK (Not Supported)
    test DMABUF (select): OK (Not Supported)

Total for bm2835 mmal device /dev/video0: 53, Succeeded: 53, Failed:
0, Warnings: 3

The warnings are because downstream we have an early version of
"media: vb2: Allow reqbufs(0) with "in use" MMAP buffers" that doesn't
set the flag to userspace. I need to revert that and apply the
accepted one (it's not a clean cherrypick though).

I do try and run compliance every few months because I'm aware that
you frequently add extra tests.

> 2) What is the status of this driver from your point of view? What is needed to
>    get it out of staging?

I think the main issue is the dependency on vchiq. I'm hoping that the
V4L2 side of this is now relatively clean, but it probably wants a
full review when someone has the time.

> 3) Out of curiosity: is this driver still valid for RPi4?

Yes, it is still valid. The imaging side hasn't changed significantly,
it just runs a bit faster.

  Dave

> Regards,
>
>         Hans
>
> >
> > Dave Stevenson (31):
> >   staging: bcm2835-camera: Ensure H264 header bytes get a sensible
> >     timestamp
> >   staging: bcm2835-camera: Check the error for REPEAT_SEQ_HEADER
> >   staging: bcm2835-camera: Replace spinlock protecting context_map with
> >     mutex
> >   staging: bcm2835-camera: Do not bulk receive from service thread
> >   staging: bcm2835-camera: Correctly denote key frames in encoded data
> >   staging: bcm2835-camera: Return early on errors
> >   staging: bcm2835-camera: Remove dead email addresses
> >   staging: bcm2835-camera: Fix comment style violations.
> >   staging: bcm2835-camera: Fix spacing around operators
> >   staging: bcm2835-camera: Reduce length of enum names
> >   staging: bcm2835-camera: Fix multiple line dereference errors
> >   staging: bcm2835-camera: Fix brace style issues.
> >   staging: bcm2835-camera: Fix missing lines between items
> >   staging: bcm2835-camera: Fix open parenthesis alignment
> >   staging: bcm2835-camera: Ensure all buffers are returned on disable
> >   staging: bcm2835-camera: Remove check of the number of buffers
> >     supplied
> >   staging: bcm2835-camera: Handle empty EOS buffers whilst streaming
> >   staging: bcm2835-camera: Set sequence number correctly
> >   staging: bcm2835-camera: Ensure timestamps never go backwards.
> >   staging: bcm2835-camera: Add multiple inclusion protection to headers
> >   staging: bcm2835-camera: Unify header inclusion defines
> >   staging: bcm2835-camera: Fix multiple assignments should be avoided
> >   staging: bcm2835-camera: Fix up mmal-parameters.h
> >   staging: bcm2835-camera: Use enums for max value in controls
> >   staging: bcm2835-camera: Correct V4L2_CID_COLORFX_CBCR behaviour
> >   staging: bcm2835-camera: Remove/amend some obsolete comments
> >   staging: mmal-vchiq: Avoid use of bool in structures
> >   staging: bcm2835-camera: Fix stride on RGB3/BGR3 formats
> >   staging: bcm2835-camera: Add sanity checks for queue_setup/CREATE_BUFS
> >   staging: bcm2835-camera: Set the field value within ach buffer
> >   staging: bcm2835-camera: Correct ctrl min/max/step/def to 64bit
> >
> >  .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 378 ++++++++++++----=
> > -----
> >  .../vc04_services/bcm2835-camera/bcm2835-camera.h  |  34 +-
> >  .../vc04_services/bcm2835-camera/controls.c        | 184 +++++-----
> >  .../vc04_services/bcm2835-camera/mmal-common.h     |  12 +-
> >  .../vc04_services/bcm2835-camera/mmal-encodings.h  |   9 +-
> >  .../vc04_services/bcm2835-camera/mmal-msg-common.h |   9 +-
> >  .../vc04_services/bcm2835-camera/mmal-msg-format.h | 104 +++---
> >  .../vc04_services/bcm2835-camera/mmal-msg-port.h   | 133 ++++----
> >  .../vc04_services/bcm2835-camera/mmal-msg.h        | 150 ++++----
> >  .../vc04_services/bcm2835-camera/mmal-parameters.h | 286 +++++++++-------
> >  .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 159 +++++----
> >  .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  22 +-
> >  12 files changed, 826 insertions(+), 654 deletions(-)
> >
> > =2D-
> > 2.7.4
> >
>

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Hans Verkuil]

On 6/28/19 12:39 PM, Dave Stevenson wrote:
> Hi Stefan
> 
> Firstly a huge thank you for picking this up - it's been on my to-do
> list for ages, and just hasn't made it to the top.
> 
> On Fri, 28 Jun 2019 at 09:06, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>>
>> Hi Stefan,
>>
>> On 6/27/19 8:55 PM, Stefan Wahren wrote:
>>> This is an attempt to help Dave Stevenson to get all the fixes and
>>> improvements of the bcm2835-camera driver into mainline.
>>>
>>> Mostly i only polished the commit logs for upstream.
>>>
>>> The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
>>> re
>>> return behavior of ctrl_set_bitrate().
>>
>> Thank you for working on this.
>>
>> Three high-level questions:
>>
>> 1) Can you post the output of 'v4l2-compliance -s' using the latest v4l2-compliance
>>    from https://git.linuxtv.org/v4l-utils.git ?  I'm interested to see what the
>>    status is of this driver w.r.t. the compliance tests.
> 
> Hi Hans.
> 
> Running it against the downstream driver (which should be nearly
> identical based on this set of patches), 4.19, on a Pi4 I get
> pi@raspberrypi:~/v4l-utils/utils/v4l2-compliance $ ./v4l2-compliance -s
> v4l2-compliance SHA: b16f9e945d74aa552abdd6f873821cb77faaf13a, 32 bits
> 
> Compliance test for bm2835 mmal device /dev/video0:
> 
> Driver Info:
>     Driver name      : bm2835 mmal
>     Card type        : mmal service 16.1
>     Bus info         : platform:bcm2835-v4l2
>     Driver version   : 4.19.56
>     Capabilities     : 0x85200005
>         Video Capture
>         Video Overlay
>         Read/Write
>         Streaming
>         Extended Pix Format
>         Device Capabilities
>     Device Caps      : 0x05200005
>         Video Capture
>         Video Overlay
>         Read/Write
>         Streaming
>         Extended Pix Format
> 
> Required ioctls:
>     test VIDIOC_QUERYCAP: OK
> 
> Allow for multiple opens:
>     test second /dev/video0 open: OK
>     test VIDIOC_QUERYCAP: OK
>     test VIDIOC_G/S_PRIORITY: OK
>     test for unlimited opens: OK
> 
> Debug ioctls:
>     test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>     test VIDIOC_LOG_STATUS: OK
> 
> Input ioctls:
>     test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>     test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>     test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>     test VIDIOC_ENUMAUDIO: OK (Not Supported)
>     test VIDIOC_G/S/ENUMINPUT: OK
>     test VIDIOC_G/S_AUDIO: OK (Not Supported)
>     Inputs: 1 Audio Inputs: 0 Tuners: 0
> 
> Output ioctls:
>     test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>     test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>     test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>     test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>     test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>     Outputs: 0 Audio Outputs: 0 Modulators: 0
> 
> Input/Output configuration ioctls:
>     test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>     test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>     test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>     test VIDIOC_G/S_EDID: OK (Not Supported)
> 
> Control ioctls (Input 0):
>     test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>     test VIDIOC_QUERYCTRL: OK
>     test VIDIOC_G/S_CTRL: OK
>     test VIDIOC_G/S/TRY_EXT_CTRLS: OK
>     test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
>     test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>     Standard Controls: 33 Private Controls: 0
> 
> Format ioctls (Input 0):
>     test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>     test VIDIOC_G/S_PARM: OK
>     test VIDIOC_G_FBUF: OK
>     test VIDIOC_G_FMT: OK
>     test VIDIOC_TRY_FMT: OK
>     test VIDIOC_S_FMT: OK
>     test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>     test Cropping: OK (Not Supported)
>     test Composing: OK (Not Supported)
>     test Scaling: OK
> 
> Codec ioctls (Input 0):
>     test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>     test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>     test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
> 
> Buffer ioctls (Input 0):
>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>     test VIDIOC_EXPBUF: OK (Not Supported)
>     test Requests: OK (Not Supported)
> 
> Test input 0:
> 
> Streaming ioctls:
>     test read/write: OK
>     test blocking wait: OK
>             warn: v4l2-test-buffers.cpp(1429): Can free buffers even
> if still mmap()ed
>     test MMAP (no poll): OK
>             warn: v4l2-test-buffers.cpp(1429): Can free buffers even
> if still mmap()ed
>     test MMAP (select): OK
>             warn: v4l2-test-buffers.cpp(1429): Can free buffers even
> if still mmap()ed
>     test MMAP (epoll): OK
>     test USERPTR (no poll): OK
>     test USERPTR (select): OK
>     test DMABUF (no poll): OK (Not Supported)
>     test DMABUF (select): OK (Not Supported)
> 
> Total for bm2835 mmal device /dev/video0: 53, Succeeded: 53, Failed:
> 0, Warnings: 3
> 
> The warnings are because downstream we have an early version of
> "media: vb2: Allow reqbufs(0) with "in use" MMAP buffers" that doesn't
> set the flag to userspace. I need to revert that and apply the
> accepted one (it's not a clean cherrypick though).
> 
> I do try and run compliance every few months because I'm aware that
> you frequently add extra tests.

Much appreciated! That's also a great help in getting this into mainline.
If it passes these test cleanly, then that gives me a lot of confidence
about the quality of the driver.

>> 2) What is the status of this driver from your point of view? What is needed to
>>    get it out of staging?
> 
> I think the main issue is the dependency on vchiq. I'm hoping that the
> V4L2 side of this is now relatively clean, but it probably wants a
> full review when someone has the time.

A full review of the v4l2 driver? Ping me once we've merged this patch series,
and I can do a full review of the driver. I'll try to go through this series
today.

> 
>> 3) Out of curiosity: is this driver still valid for RPi4?
> 
> Yes, it is still valid. The imaging side hasn't changed significantly,
> it just runs a bit faster.

Nice!

	Hans

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Hans Verkuil]

On 6/27/19 8:55 PM, Stefan Wahren wrote:
> This is an attempt to help Dave Stevenson to get all the fixes and
> improvements of the bcm2835-camera driver into mainline.
> 
> Mostly i only polished the commit logs for upstream.
> 
> The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
> re
> return behavior of ctrl_set_bitrate().
> 
> Dave Stevenson (31):
>   staging: bcm2835-camera: Ensure H264 header bytes get a sensible
>     timestamp
>   staging: bcm2835-camera: Check the error for REPEAT_SEQ_HEADER
>   staging: bcm2835-camera: Replace spinlock protecting context_map with
>     mutex
>   staging: bcm2835-camera: Do not bulk receive from service thread
>   staging: bcm2835-camera: Correctly denote key frames in encoded data
>   staging: bcm2835-camera: Return early on errors
>   staging: bcm2835-camera: Remove dead email addresses
>   staging: bcm2835-camera: Fix comment style violations.
>   staging: bcm2835-camera: Fix spacing around operators
>   staging: bcm2835-camera: Reduce length of enum names
>   staging: bcm2835-camera: Fix multiple line dereference errors
>   staging: bcm2835-camera: Fix brace style issues.
>   staging: bcm2835-camera: Fix missing lines between items
>   staging: bcm2835-camera: Fix open parenthesis alignment
>   staging: bcm2835-camera: Ensure all buffers are returned on disable
>   staging: bcm2835-camera: Remove check of the number of buffers
>     supplied
>   staging: bcm2835-camera: Handle empty EOS buffers whilst streaming
>   staging: bcm2835-camera: Set sequence number correctly
>   staging: bcm2835-camera: Ensure timestamps never go backwards.
>   staging: bcm2835-camera: Add multiple inclusion protection to headers
>   staging: bcm2835-camera: Unify header inclusion defines
>   staging: bcm2835-camera: Fix multiple assignments should be avoided
>   staging: bcm2835-camera: Fix up mmal-parameters.h
>   staging: bcm2835-camera: Use enums for max value in controls
>   staging: bcm2835-camera: Correct V4L2_CID_COLORFX_CBCR behaviour
>   staging: bcm2835-camera: Remove/amend some obsolete comments
>   staging: mmal-vchiq: Avoid use of bool in structures
>   staging: bcm2835-camera: Fix stride on RGB3/BGR3 formats
>   staging: bcm2835-camera: Add sanity checks for queue_setup/CREATE_BUFS
>   staging: bcm2835-camera: Set the field value within ach buffer

ach -> each

>   staging: bcm2835-camera: Correct ctrl min/max/step/def to 64bit
> 
>  .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 378 ++++++++++++----=
> -----
>  .../vc04_services/bcm2835-camera/bcm2835-camera.h  |  34 +-
>  .../vc04_services/bcm2835-camera/controls.c        | 184 +++++-----
>  .../vc04_services/bcm2835-camera/mmal-common.h     |  12 +-
>  .../vc04_services/bcm2835-camera/mmal-encodings.h  |   9 +-
>  .../vc04_services/bcm2835-camera/mmal-msg-common.h |   9 +-
>  .../vc04_services/bcm2835-camera/mmal-msg-format.h | 104 +++---
>  .../vc04_services/bcm2835-camera/mmal-msg-port.h   | 133 ++++----
>  .../vc04_services/bcm2835-camera/mmal-msg.h        | 150 ++++----
>  .../vc04_services/bcm2835-camera/mmal-parameters.h | 286 +++++++++-------
>  .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 159 +++++----
>  .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  22 +-
>  12 files changed, 826 insertions(+), 654 deletions(-)
> 
> =2D-
> 2.7.4
> 

This series looks good. Others made some comments that should be addressed,
and the H264 changes should, I think, be dealt with in a separate patch
series.

I guess this should go in via Greg? When you make a v2 (excluding the H264
changes, and incorporating Dan's comments), then you can add my:

Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Thanks!

	Hans

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Mauro Carvalho Chehab]

Em Fri, 28 Jun 2019 15:13:03 +0200
Hans Verkuil <hverkuil@xs4all.nl> escreveu:

> On 6/27/19 8:55 PM, Stefan Wahren wrote:
> > This is an attempt to help Dave Stevenson to get all the fixes and
> > improvements of the bcm2835-camera driver into mainline.
> > 
> > Mostly i only polished the commit logs for upstream.
> > 
> > The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
> > re
> > return behavior of ctrl_set_bitrate().
> > 
> > Dave Stevenson (31):
> >   staging: bcm2835-camera: Ensure H264 header bytes get a sensible
> >     timestamp
> >   staging: bcm2835-camera: Check the error for REPEAT_SEQ_HEADER
> >   staging: bcm2835-camera: Replace spinlock protecting context_map with
> >     mutex
> >   staging: bcm2835-camera: Do not bulk receive from service thread
> >   staging: bcm2835-camera: Correctly denote key frames in encoded data
> >   staging: bcm2835-camera: Return early on errors
> >   staging: bcm2835-camera: Remove dead email addresses
> >   staging: bcm2835-camera: Fix comment style violations.
> >   staging: bcm2835-camera: Fix spacing around operators
> >   staging: bcm2835-camera: Reduce length of enum names
> >   staging: bcm2835-camera: Fix multiple line dereference errors
> >   staging: bcm2835-camera: Fix brace style issues.
> >   staging: bcm2835-camera: Fix missing lines between items
> >   staging: bcm2835-camera: Fix open parenthesis alignment
> >   staging: bcm2835-camera: Ensure all buffers are returned on disable
> >   staging: bcm2835-camera: Remove check of the number of buffers
> >     supplied
> >   staging: bcm2835-camera: Handle empty EOS buffers whilst streaming
> >   staging: bcm2835-camera: Set sequence number correctly
> >   staging: bcm2835-camera: Ensure timestamps never go backwards.
> >   staging: bcm2835-camera: Add multiple inclusion protection to headers
> >   staging: bcm2835-camera: Unify header inclusion defines
> >   staging: bcm2835-camera: Fix multiple assignments should be avoided
> >   staging: bcm2835-camera: Fix up mmal-parameters.h
> >   staging: bcm2835-camera: Use enums for max value in controls
> >   staging: bcm2835-camera: Correct V4L2_CID_COLORFX_CBCR behaviour
> >   staging: bcm2835-camera: Remove/amend some obsolete comments
> >   staging: mmal-vchiq: Avoid use of bool in structures
> >   staging: bcm2835-camera: Fix stride on RGB3/BGR3 formats
> >   staging: bcm2835-camera: Add sanity checks for queue_setup/CREATE_BUFS
> >   staging: bcm2835-camera: Set the field value within ach buffer  
> 
> ach -> each
> 
> >   staging: bcm2835-camera: Correct ctrl min/max/step/def to 64bit
> > 
> >  .../vc04_services/bcm2835-camera/bcm2835-camera.c  | 378 ++++++++++++----=
> > -----
> >  .../vc04_services/bcm2835-camera/bcm2835-camera.h  |  34 +-
> >  .../vc04_services/bcm2835-camera/controls.c        | 184 +++++-----
> >  .../vc04_services/bcm2835-camera/mmal-common.h     |  12 +-
> >  .../vc04_services/bcm2835-camera/mmal-encodings.h  |   9 +-
> >  .../vc04_services/bcm2835-camera/mmal-msg-common.h |   9 +-
> >  .../vc04_services/bcm2835-camera/mmal-msg-format.h | 104 +++---
> >  .../vc04_services/bcm2835-camera/mmal-msg-port.h   | 133 ++++----
> >  .../vc04_services/bcm2835-camera/mmal-msg.h        | 150 ++++----
> >  .../vc04_services/bcm2835-camera/mmal-parameters.h | 286 +++++++++-------
> >  .../vc04_services/bcm2835-camera/mmal-vchiq.c      | 159 +++++----
> >  .../vc04_services/bcm2835-camera/mmal-vchiq.h      |  22 +-
> >  12 files changed, 826 insertions(+), 654 deletions(-)
> > 
> > =2D-
> > 2.7.4
> >   
> 
> This series looks good. Others made some comments that should be addressed,
> and the H264 changes should, I think, be dealt with in a separate patch
> series.
> 
> I guess this should go in via Greg?

Works for me. I won't be able to handle this before the merge window,
as I'll be on PTO next week.

> When you make a v2 (excluding the H264
> changes, and incorporating Dan's comments), then you can add my:
> 
> Acked-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

Greg, once the issues get fixed - and if you want to pick this for this
merge window, feel fee to pick with my ack:

Acked-by: Mauro Carvalho Chehab <mchehab+samsung@kernel.org>

Otherwise, if too late for this merge window, It is probably better to
apply those against the linux-media tree after -rc1.

Thanks,
Mauro

Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 8389 bytes --]

Le vendredi 28 juin 2019 à 11:10 +0100, Dave Stevenson a écrit :
> Hi Nicolas
> 
> On Thu, 27 Jun 2019 at 20:55, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > Hi Dave,
> > 
> > Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit :
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> > > 
> > > H264 header come from VC with 0 timestamps, which means they get a
> > > strange timestamp when processed with VC/kernel start times,
> > > particularly if used with the inline header option.
> > > Remember the last frame timestamp and use that if set, or otherwise
> > > use the kernel start time.
> > 
> > Normally H264 headers are considered to be part of the following frame.
> > Giving it the timestamp of the previous frame will likely confuse some
> > userspace and cause an off-by-one in timestamp. I understand this is a
> > workaround, but am wondering if this can be improved.
> 
> Sorry, slight ambiguity in how I'm reading your comment.
> 
> Are you saying that the header bytes want to be in the same buffer as
> the following frame?
> I thought this had also been discussed in the V4L2 stateful codec API
> threads along with how many encoded frames were allowed in a single
> V4L2 buffer. I certainly hadn't seen a statement about the header
> bytes being combined with the next frame.
> If the behaviour required by V4L2 is that header bytes and following
> frame are in the same buffer, then that is relatively easy to achieve
> in the firmware. This workaround can remain for older firmware as it
> will never trigger if the firmware has combined the frames.

The frame alignment is a requirement specific to the stateful codec
API. Stateful codec must interpret _H264 format as being one full frame
per buffer (1 AU in progressive, and 1 to 2 AU in interlaced), the
first frame should include SPS/PPS and any other prefix NALs. You may
follow this rule in your capture driver if you want to make it possible
to zero-copy the encoded frames from the capture to the decoder.
Though, userspace will still have to parse as there is no indication
for capture devices of the H264 alignment being used (that imply 1
frame latency). Boris is working on a control for stateless CODEC to
control if we are running in full-frame or per slices. I do hope this
control will be extended later to allow cameras and decoders to signal
their alignment, or simply to allow enabling low-latency modes
supported by CODA and ZynMP firmwares.

> 
> Or are you saying that the header bytes remain in their own buffer,
> but the timestamp wants to be the same as the next frame? That is
> harder to achieve in the firmware, but could probably be done in the
> kernel driver by holding on to the header bytes frame until the next
> buffer had been received, at which point the timestamp can be copied
> across. Possible, but just needs slightly careful handling to ensure
> we don't lose buffers accidentally.

So this isn't specified by V4L2 itself. So instead I rely on H264 and
MPEG TS specification to advance this. This is also the interpretation
we have of timestamp in GStreamer (ffmpeg uses out-of-band headers with
full frame AVC, so this does not apply).

So H264 AUD, SPS, PPS, SEI and other prefix NAL are considered to be
the start of a frame. With this interpretation in mind, accumulating
them is considered zero-latency. This basically means that if they are
to have a timestamp, they would share that timestamp with all the
slices of the same frame. In GStreamer, we have the notion of no
timestamp, so in such a case we'd leave the timestamp empty and our
parsers would pick the first valid timestamp that formed the full frame
as being the frame timestamp (it's a bit buggier then that, but that's
what it's suppose to do).

On top of that, if you don't have any meaningful alignment in your H264
stream, the MPEG TS format states that the timestamp of a buffer should
be the timestamp of the first NAL starting within this buffer, or the
timestamp of the current NAL if there is not NAL start.

By respecting these standards you ensure that latency aware application
can work with your driver without causing delays, or worst, having to
deal with artificially late frames.

I hope this clarify and helps understand my request for "unhacking" the
headers timestamps. I had assumed the timestamp came from the driver
(instead of from the firmware), sorry if that caused confusion. If
merging full frames is easier, I think I would opt for that as it's
beneficial to performance when combined with other full frame APIs.

Nicolas

> 
>   Dave
> 
> > > Link: https://github.com/raspberrypi/linux/issues/1836
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> > > ---
> > >  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 14 ++++++++++++--
> > >  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h  |  2 ++
> > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > index dce6e6d..0c04815 100644
> > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > @@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
> > >               }
> > >       } else {
> > >               if (dev->capture.frame_count) {
> > > -                     if (dev->capture.vc_start_timestamp != -1 && pts) {
> > > +                     if (dev->capture.vc_start_timestamp != -1) {
> > > +                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
> > > +                     } else if (pts) {
> > >                               ktime_t timestamp;
> > >                               s64 runtime_us = pts -
> > >                                   dev->capture.vc_start_timestamp;
> > > @@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
> > >                                        ktime_to_ns(timestamp));
> > >                               buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
> > >                       } else {
> > > -                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
> > > +                             if (dev->capture.last_timestamp) {
> > > +                                     buf->vb.vb2_buf.timestamp =
> > > +                                             dev->capture.last_timestamp;
> > > +                             } else {
> > > +                                     buf->vb.vb2_buf.timestamp =
> > > +                                             ktime_to_ns(dev->capture.kernel_start_ts);
> > > +                             }
> > >                       }
> > > +                     dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
> > > 
> > >                       vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
> > >                       vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> > > @@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> > >                        dev->capture.vc_start_timestamp, parameter_size);
> > > 
> > >       dev->capture.kernel_start_ts = ktime_get();
> > > +     dev->capture.last_timestamp = 0;
> > > 
> > >       /* enable the camera port */
> > >       dev->capture.port->cb_ctx = dev;
> > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > > index 2b5679e..09273b0 100644
> > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > > @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
> > >               s64         vc_start_timestamp;
> > >               /* Kernel start timestamp for streaming */
> > >               ktime_t kernel_start_ts;
> > > +             /* Timestamp of last frame */
> > > +             u64             last_timestamp;
> > > 
> > >               struct vchiq_mmal_port  *port; /* port being used for capture */
> > >               /* camera port being used for capture */
> > > --
> > > 2.7.4
> > > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Hans Verkuil]

On 6/28/19 4:00 PM, Nicolas Dufresne wrote:
> Le vendredi 28 juin 2019 à 11:10 +0100, Dave Stevenson a écrit :
>> Hi Nicolas
>>
>> On Thu, 27 Jun 2019 at 20:55, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
>>> Hi Dave,
>>>
>>> Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit :
>>>> From: Dave Stevenson <dave.stevenson@raspberrypi.org>
>>>>
>>>> H264 header come from VC with 0 timestamps, which means they get a
>>>> strange timestamp when processed with VC/kernel start times,
>>>> particularly if used with the inline header option.
>>>> Remember the last frame timestamp and use that if set, or otherwise
>>>> use the kernel start time.
>>>
>>> Normally H264 headers are considered to be part of the following frame.
>>> Giving it the timestamp of the previous frame will likely confuse some
>>> userspace and cause an off-by-one in timestamp. I understand this is a
>>> workaround, but am wondering if this can be improved.
>>
>> Sorry, slight ambiguity in how I'm reading your comment.
>>
>> Are you saying that the header bytes want to be in the same buffer as
>> the following frame?
>> I thought this had also been discussed in the V4L2 stateful codec API
>> threads along with how many encoded frames were allowed in a single
>> V4L2 buffer. I certainly hadn't seen a statement about the header
>> bytes being combined with the next frame.
>> If the behaviour required by V4L2 is that header bytes and following
>> frame are in the same buffer, then that is relatively easy to achieve
>> in the firmware. This workaround can remain for older firmware as it
>> will never trigger if the firmware has combined the frames.
> 
> The frame alignment is a requirement specific to the stateful codec
> API.

Is it? I don't remember it being specified anywhere explicitly.
Here is the latest text:

https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html

I'll start a new thread about this, since this really needs to be
clarified.

Regards,

	Hans

 Stateful codec must interpret _H264 format as being one full frame
> per buffer (1 AU in progressive, and 1 to 2 AU in interlaced), the
> first frame should include SPS/PPS and any other prefix NALs. You may
> follow this rule in your capture driver if you want to make it possible
> to zero-copy the encoded frames from the capture to the decoder.
> Though, userspace will still have to parse as there is no indication
> for capture devices of the H264 alignment being used (that imply 1
> frame latency). Boris is working on a control for stateless CODEC to
> control if we are running in full-frame or per slices. I do hope this
> control will be extended later to allow cameras and decoders to signal
> their alignment, or simply to allow enabling low-latency modes
> supported by CODA and ZynMP firmwares.
> 
>>
>> Or are you saying that the header bytes remain in their own buffer,
>> but the timestamp wants to be the same as the next frame? That is
>> harder to achieve in the firmware, but could probably be done in the
>> kernel driver by holding on to the header bytes frame until the next
>> buffer had been received, at which point the timestamp can be copied
>> across. Possible, but just needs slightly careful handling to ensure
>> we don't lose buffers accidentally.
> 
> So this isn't specified by V4L2 itself. So instead I rely on H264 and
> MPEG TS specification to advance this. This is also the interpretation
> we have of timestamp in GStreamer (ffmpeg uses out-of-band headers with
> full frame AVC, so this does not apply).
> 
> So H264 AUD, SPS, PPS, SEI and other prefix NAL are considered to be
> the start of a frame. With this interpretation in mind, accumulating
> them is considered zero-latency. This basically means that if they are
> to have a timestamp, they would share that timestamp with all the
> slices of the same frame. In GStreamer, we have the notion of no
> timestamp, so in such a case we'd leave the timestamp empty and our
> parsers would pick the first valid timestamp that formed the full frame
> as being the frame timestamp (it's a bit buggier then that, but that's
> what it's suppose to do).
> 
> On top of that, if you don't have any meaningful alignment in your H264
> stream, the MPEG TS format states that the timestamp of a buffer should
> be the timestamp of the first NAL starting within this buffer, or the
> timestamp of the current NAL if there is not NAL start.
> 
> By respecting these standards you ensure that latency aware application
> can work with your driver without causing delays, or worst, having to
> deal with artificially late frames.
> 
> I hope this clarify and helps understand my request for "unhacking" the
> headers timestamps. I had assumed the timestamp came from the driver
> (instead of from the firmware), sorry if that caused confusion. If
> merging full frames is easier, I think I would opt for that as it's
> beneficial to performance when combined with other full frame APIs.
> 
> Nicolas
> 
>>
>>   Dave
>>
>>>> Link: https://github.com/raspberrypi/linux/issues/1836
>>>> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
>>>> ---
>>>>  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 14 ++++++++++++--
>>>>  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h  |  2 ++
>>>>  2 files changed, 14 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
>>>> index dce6e6d..0c04815 100644
>>>> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
>>>> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
>>>> @@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
>>>>               }
>>>>       } else {
>>>>               if (dev->capture.frame_count) {
>>>> -                     if (dev->capture.vc_start_timestamp != -1 && pts) {
>>>> +                     if (dev->capture.vc_start_timestamp != -1) {
>>>> +                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
>>>> +                     } else if (pts) {
>>>>                               ktime_t timestamp;
>>>>                               s64 runtime_us = pts -
>>>>                                   dev->capture.vc_start_timestamp;
>>>> @@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
>>>>                                        ktime_to_ns(timestamp));
>>>>                               buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
>>>>                       } else {
>>>> -                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
>>>> +                             if (dev->capture.last_timestamp) {
>>>> +                                     buf->vb.vb2_buf.timestamp =
>>>> +                                             dev->capture.last_timestamp;
>>>> +                             } else {
>>>> +                                     buf->vb.vb2_buf.timestamp =
>>>> +                                             ktime_to_ns(dev->capture.kernel_start_ts);
>>>> +                             }
>>>>                       }
>>>> +                     dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
>>>>
>>>>                       vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
>>>>                       vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
>>>> @@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
>>>>                        dev->capture.vc_start_timestamp, parameter_size);
>>>>
>>>>       dev->capture.kernel_start_ts = ktime_get();
>>>> +     dev->capture.last_timestamp = 0;
>>>>
>>>>       /* enable the camera port */
>>>>       dev->capture.port->cb_ctx = dev;
>>>> diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
>>>> index 2b5679e..09273b0 100644
>>>> --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
>>>> +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
>>>> @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
>>>>               s64         vc_start_timestamp;
>>>>               /* Kernel start timestamp for streaming */
>>>>               ktime_t kernel_start_ts;
>>>> +             /* Timestamp of last frame */
>>>> +             u64             last_timestamp;
>>>>
>>>>               struct vchiq_mmal_port  *port; /* port being used for capture */
>>>>               /* camera port being used for capture */
>>>> --
>>>> 2.7.4
>>>>

Re: [PATCH 01/31] staging: bcm2835-camera: Ensure H264 header bytes get a sensible timestamp

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 9564 bytes --]

Le vendredi 28 juin 2019 à 16:08 +0200, Hans Verkuil a écrit :
> On 6/28/19 4:00 PM, Nicolas Dufresne wrote:
> > Le vendredi 28 juin 2019 à 11:10 +0100, Dave Stevenson a écrit :
> > > Hi Nicolas
> > > 
> > > On Thu, 27 Jun 2019 at 20:55, Nicolas Dufresne <nicolas@ndufresne.ca> wrote:
> > > > Hi Dave,
> > > > 
> > > > Le jeudi 27 juin 2019 à 20:55 +0200, Stefan Wahren a écrit :
> > > > > From: Dave Stevenson <dave.stevenson@raspberrypi.org>
> > > > > 
> > > > > H264 header come from VC with 0 timestamps, which means they get a
> > > > > strange timestamp when processed with VC/kernel start times,
> > > > > particularly if used with the inline header option.
> > > > > Remember the last frame timestamp and use that if set, or otherwise
> > > > > use the kernel start time.
> > > > 
> > > > Normally H264 headers are considered to be part of the following frame.
> > > > Giving it the timestamp of the previous frame will likely confuse some
> > > > userspace and cause an off-by-one in timestamp. I understand this is a
> > > > workaround, but am wondering if this can be improved.
> > > 
> > > Sorry, slight ambiguity in how I'm reading your comment.
> > > 
> > > Are you saying that the header bytes want to be in the same buffer as
> > > the following frame?
> > > I thought this had also been discussed in the V4L2 stateful codec API
> > > threads along with how many encoded frames were allowed in a single
> > > V4L2 buffer. I certainly hadn't seen a statement about the header
> > > bytes being combined with the next frame.
> > > If the behaviour required by V4L2 is that header bytes and following
> > > frame are in the same buffer, then that is relatively easy to achieve
> > > in the firmware. This workaround can remain for older firmware as it
> > > will never trigger if the firmware has combined the frames.
> > 
> > The frame alignment is a requirement specific to the stateful codec
> > API.
> 
> Is it? I don't remember it being specified anywhere explicitly.
> Here is the latest text:
> 
> https://hverkuil.home.xs4all.nl/codec-api/uapi/v4l/dev-encoder.html
> 
> I'll start a new thread about this, since this really needs to be
> clarified.

Ok, let's clarify this, but before we start, a quick reminder that this
is what userspace assumes already, so breaking this will cause
regressions all over.

> 
> Regards,
> 
> 	Hans
> 
>  Stateful codec must interpret _H264 format as being one full frame
> > per buffer (1 AU in progressive, and 1 to 2 AU in interlaced), the
> > first frame should include SPS/PPS and any other prefix NALs. You may
> > follow this rule in your capture driver if you want to make it possible
> > to zero-copy the encoded frames from the capture to the decoder.
> > Though, userspace will still have to parse as there is no indication
> > for capture devices of the H264 alignment being used (that imply 1
> > frame latency). Boris is working on a control for stateless CODEC to
> > control if we are running in full-frame or per slices. I do hope this
> > control will be extended later to allow cameras and decoders to signal
> > their alignment, or simply to allow enabling low-latency modes
> > supported by CODA and ZynMP firmwares.
> > 
> > > Or are you saying that the header bytes remain in their own buffer,
> > > but the timestamp wants to be the same as the next frame? That is
> > > harder to achieve in the firmware, but could probably be done in the
> > > kernel driver by holding on to the header bytes frame until the next
> > > buffer had been received, at which point the timestamp can be copied
> > > across. Possible, but just needs slightly careful handling to ensure
> > > we don't lose buffers accidentally.
> > 
> > So this isn't specified by V4L2 itself. So instead I rely on H264 and
> > MPEG TS specification to advance this. This is also the interpretation
> > we have of timestamp in GStreamer (ffmpeg uses out-of-band headers with
> > full frame AVC, so this does not apply).
> > 
> > So H264 AUD, SPS, PPS, SEI and other prefix NAL are considered to be
> > the start of a frame. With this interpretation in mind, accumulating
> > them is considered zero-latency. This basically means that if they are
> > to have a timestamp, they would share that timestamp with all the
> > slices of the same frame. In GStreamer, we have the notion of no
> > timestamp, so in such a case we'd leave the timestamp empty and our
> > parsers would pick the first valid timestamp that formed the full frame
> > as being the frame timestamp (it's a bit buggier then that, but that's
> > what it's suppose to do).
> > 
> > On top of that, if you don't have any meaningful alignment in your H264
> > stream, the MPEG TS format states that the timestamp of a buffer should
> > be the timestamp of the first NAL starting within this buffer, or the
> > timestamp of the current NAL if there is not NAL start.
> > 
> > By respecting these standards you ensure that latency aware application
> > can work with your driver without causing delays, or worst, having to
> > deal with artificially late frames.
> > 
> > I hope this clarify and helps understand my request for "unhacking" the
> > headers timestamps. I had assumed the timestamp came from the driver
> > (instead of from the firmware), sorry if that caused confusion. If
> > merging full frames is easier, I think I would opt for that as it's
> > beneficial to performance when combined with other full frame APIs.
> > 
> > Nicolas
> > 
> > >   Dave
> > > 
> > > > > Link: https://github.com/raspberrypi/linux/issues/1836
> > > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.org>
> > > > > ---
> > > > >  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.c  | 14 ++++++++++++--
> > > > >  .../staging/vc04_services/bcm2835-camera/bcm2835-camera.h  |  2 ++
> > > > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > > > index dce6e6d..0c04815 100644
> > > > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.c
> > > > > @@ -359,7 +359,9 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
> > > > >               }
> > > > >       } else {
> > > > >               if (dev->capture.frame_count) {
> > > > > -                     if (dev->capture.vc_start_timestamp != -1 && pts) {
> > > > > +                     if (dev->capture.vc_start_timestamp != -1) {
> > > > > +                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
> > > > > +                     } else if (pts) {
> > > > >                               ktime_t timestamp;
> > > > >                               s64 runtime_us = pts -
> > > > >                                   dev->capture.vc_start_timestamp;
> > > > > @@ -372,8 +374,15 @@ static void buffer_cb(struct vchiq_mmal_instance *instance,
> > > > >                                        ktime_to_ns(timestamp));
> > > > >                               buf->vb.vb2_buf.timestamp = ktime_to_ns(timestamp);
> > > > >                       } else {
> > > > > -                             buf->vb.vb2_buf.timestamp = ktime_get_ns();
> > > > > +                             if (dev->capture.last_timestamp) {
> > > > > +                                     buf->vb.vb2_buf.timestamp =
> > > > > +                                             dev->capture.last_timestamp;
> > > > > +                             } else {
> > > > > +                                     buf->vb.vb2_buf.timestamp =
> > > > > +                                             ktime_to_ns(dev->capture.kernel_start_ts);
> > > > > +                             }
> > > > >                       }
> > > > > +                     dev->capture.last_timestamp = buf->vb.vb2_buf.timestamp;
> > > > > 
> > > > >                       vb2_set_plane_payload(&buf->vb.vb2_buf, 0, length);
> > > > >                       vb2_buffer_done(&buf->vb.vb2_buf, VB2_BUF_STATE_DONE);
> > > > > @@ -541,6 +550,7 @@ static int start_streaming(struct vb2_queue *vq, unsigned int count)
> > > > >                        dev->capture.vc_start_timestamp, parameter_size);
> > > > > 
> > > > >       dev->capture.kernel_start_ts = ktime_get();
> > > > > +     dev->capture.last_timestamp = 0;
> > > > > 
> > > > >       /* enable the camera port */
> > > > >       dev->capture.port->cb_ctx = dev;
> > > > > diff --git a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > > > > index 2b5679e..09273b0 100644
> > > > > --- a/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > > > > +++ b/drivers/staging/vc04_services/bcm2835-camera/bcm2835-camera.h
> > > > > @@ -90,6 +90,8 @@ struct bm2835_mmal_dev {
> > > > >               s64         vc_start_timestamp;
> > > > >               /* Kernel start timestamp for streaming */
> > > > >               ktime_t kernel_start_ts;
> > > > > +             /* Timestamp of last frame */
> > > > > +             u64             last_timestamp;
> > > > > 
> > > > >               struct vchiq_mmal_port  *port; /* port being used for capture */
> > > > >               /* camera port being used for capture */
> > > > > --
> > > > > 2.7.4
> > > > > 

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Stefan Wahren]

Hi Hans,

Am 28.06.19 um 10:06 schrieb Hans Verkuil:
> Hi Stefan,
>
> On 6/27/19 8:55 PM, Stefan Wahren wrote:
>> This is an attempt to help Dave Stevenson to get all the fixes and
>> improvements of the bcm2835-camera driver into mainline.
>>
>> Mostly i only polished the commit logs for upstream.
>>
>> The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
>> re
>> return behavior of ctrl_set_bitrate().
> Thank you for working on this.
>
> Three high-level questions:
>
> 1) Can you post the output of 'v4l2-compliance -s' using the latest v4l2-compliance
>    from https://git.linuxtv.org/v4l-utils.git ?  I'm interested to see what the
>    status is of this driver w.r.t. the compliance tests.

Before this series (Raspberry Pi 3, Camera 1.3, Linux
5.2.0-rc3-next-20190607, multi_v7_defconfig):

v4l2-compliance SHA: b16f9e945d74aa552abdd6f873821cb77faaf13a, 32 bits

Compliance test for bm2835 mmal device /dev/video0:

Driver Info:
    Driver name      : bm2835 mmal
    Card type        : mmal service 16.1
    Bus info         : platform:bcm2835-v4l2
    Driver version   : 5.2.0
    Capabilities     : 0x85200005
        Video Capture
        Video Overlay
        Read/Write
        Streaming
        Extended Pix Format
        Device Capabilities
    Device Caps      : 0x05200005
        Video Capture
        Video Overlay
        Read/Write
        Streaming
        Extended Pix Format

Required ioctls:
    test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
    test second /dev/video0 open: OK
    test VIDIOC_QUERYCAP: OK
    test VIDIOC_G/S_PRIORITY: OK
    test for unlimited opens: OK

Debug ioctls:
    test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
    test VIDIOC_LOG_STATUS: OK

Input ioctls:
    test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
    test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
    test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
    test VIDIOC_ENUMAUDIO: OK (Not Supported)
    test VIDIOC_G/S/ENUMINPUT: OK
    test VIDIOC_G/S_AUDIO: OK (Not Supported)
    Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
    test VIDIOC_G/S_MODULATOR: OK (Not Supported)
    test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
    test VIDIOC_ENUMAUDOUT: OK (Not Supported)
    test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
    test VIDIOC_G/S_AUDOUT: OK (Not Supported)
    Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
    test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
    test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
    test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
    test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
    test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
    test VIDIOC_QUERYCTRL: OK
    test VIDIOC_G/S_CTRL: OK
    test VIDIOC_G/S/TRY_EXT_CTRLS: OK
    test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
    test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
    Standard Controls: 33 Private Controls: 0

Format ioctls (Input 0):
    test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
    test VIDIOC_G/S_PARM: OK
    test VIDIOC_G_FBUF: OK
    test VIDIOC_G_FMT: OK
    test VIDIOC_TRY_FMT: OK
    test VIDIOC_S_FMT: OK
    test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
    test Cropping: OK (Not Supported)
    test Composing: OK (Not Supported)
    test Scaling: OK

Codec ioctls (Input 0):
    test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
    test VIDIOC_G_ENC_INDEX: OK (Not Supported)
    test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
        fail: v4l2-test-buffers.cpp(715): q.create_bufs(node, 1, &fmt)
!= EINVAL
    test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
    test VIDIOC_EXPBUF: OK (Not Supported)
    test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
    test read/write: OK
        fail: v4l2-test-buffers.cpp(2145): node->streamon(q.g_type())
        fail: v4l2-test-buffers.cpp(2224): testBlockingDQBuf(node, q)
    test blocking wait: FAIL
        fail: v4l2-test-buffers.cpp(1294): q.create_bufs(node, 1, &fmt)
!= EINVAL
    test MMAP (no poll): FAIL
        fail: v4l2-test-buffers.cpp(1294): q.create_bufs(node, 1, &fmt)
!= EINVAL
    test MMAP (select): FAIL
        fail: v4l2-test-buffers.cpp(1294): q.create_bufs(node, 1, &fmt)
!= EINVAL
    test MMAP (epoll): FAIL

After this series:

v4l2-compliance SHA: b16f9e945d74aa552abdd6f873821cb77faaf13a, 32 bits

Compliance test for bm2835 mmal device /dev/video0:

Driver Info:
    Driver name      : bm2835 mmal
    Card type        : mmal service 16.1
    Bus info         : platform:bcm2835-v4l2
    Driver version   : 5.2.0
    Capabilities     : 0x85200005
        Video Capture
        Video Overlay
        Read/Write
        Streaming
        Extended Pix Format
        Device Capabilities
    Device Caps      : 0x05200005
        Video Capture
        Video Overlay
        Read/Write
        Streaming
        Extended Pix Format

Required ioctls:
    test VIDIOC_QUERYCAP: OK

Allow for multiple opens:
    test second /dev/video0 open: OK
    test VIDIOC_QUERYCAP: OK
    test VIDIOC_G/S_PRIORITY: OK
    test for unlimited opens: OK

Debug ioctls:
    test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
    test VIDIOC_LOG_STATUS: OK

Input ioctls:
    test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
    test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
    test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
    test VIDIOC_ENUMAUDIO: OK (Not Supported)
    test VIDIOC_G/S/ENUMINPUT: OK
    test VIDIOC_G/S_AUDIO: OK (Not Supported)
    Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
    test VIDIOC_G/S_MODULATOR: OK (Not Supported)
    test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
    test VIDIOC_ENUMAUDOUT: OK (Not Supported)
    test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
    test VIDIOC_G/S_AUDOUT: OK (Not Supported)
    Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
    test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
    test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
    test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
    test VIDIOC_G/S_EDID: OK (Not Supported)

Control ioctls (Input 0):
    test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
    test VIDIOC_QUERYCTRL: OK
    test VIDIOC_G/S_CTRL: OK
    test VIDIOC_G/S/TRY_EXT_CTRLS: OK
    test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
    test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
    Standard Controls: 33 Private Controls: 0

Format ioctls (Input 0):
    test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
    test VIDIOC_G/S_PARM: OK
    test VIDIOC_G_FBUF: OK
    test VIDIOC_G_FMT: OK
    test VIDIOC_TRY_FMT: OK
    test VIDIOC_S_FMT: OK
    test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
    test Cropping: OK (Not Supported)
    test Composing: OK (Not Supported)
    test Scaling: OK

Codec ioctls (Input 0):
    test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
    test VIDIOC_G_ENC_INDEX: OK (Not Supported)
    test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
    test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
    test VIDIOC_EXPBUF: OK (Not Supported)
    test Requests: OK (Not Supported)

Test input 0:

Streaming ioctls:
    test read/write: OK
    test blocking wait: OK

Unfortunately in both cases the program hangs and never finish. This is
the output of strace:

ioctl(3, VIDIOC_DQBUF, {type=V4L2_BUF_TYPE_VIDEO_CAPTURE

It is possible that this is a problem with the used linux-next version.
Nevertheless the series is improvement.

> 2) What is the status of this driver from your point of view?
Sorry, i'm not a media expert. But i agree with Dan this needs
improvement of error handling. For example mapping the MMAL error codes
to Linux error codes would avoid confusion.
> What is needed to get it out of staging?

I think the driver needs more testing for 64 bit. Contrary to Raspbian a
lot of the "mainline" distributions only concentrate on arm64. But
currently i don't know of any 64 bit specific issues.

I also can't say anything about the content of the TODO file.

Regards
Stefan

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Dave Stevenson]

Hi Stefan

On Fri, 28 Jun 2019 at 17:57, Stefan Wahren <wahrenst@gmx.net> wrote:
>
> Hi Hans,
>
> Am 28.06.19 um 10:06 schrieb Hans Verkuil:
> > Hi Stefan,
> >
> > On 6/27/19 8:55 PM, Stefan Wahren wrote:
> >> This is an attempt to help Dave Stevenson to get all the fixes and
> >> improvements of the bcm2835-camera driver into mainline.
> >>
> >> Mostly i only polished the commit logs for upstream.
> >>
> >> The series based on the latest bugfix V2 of staging: bcm2835-camera: Resto=
> >> re
> >> return behavior of ctrl_set_bitrate().
> > Thank you for working on this.
> >
> > Three high-level questions:
> >
> > 1) Can you post the output of 'v4l2-compliance -s' using the latest v4l2-compliance
> >    from https://git.linuxtv.org/v4l-utils.git ?  I'm interested to see what the
> >    status is of this driver w.r.t. the compliance tests.
>
> Before this series (Raspberry Pi 3, Camera 1.3, Linux
> 5.2.0-rc3-next-20190607, multi_v7_defconfig):
>
> v4l2-compliance SHA: b16f9e945d74aa552abdd6f873821cb77faaf13a, 32 bits
>
> Compliance test for bm2835 mmal device /dev/video0:
>
> Driver Info:
>     Driver name      : bm2835 mmal
>     Card type        : mmal service 16.1
>     Bus info         : platform:bcm2835-v4l2
>     Driver version   : 5.2.0
>     Capabilities     : 0x85200005
>         Video Capture
>         Video Overlay
>         Read/Write
>         Streaming
>         Extended Pix Format
>         Device Capabilities
>     Device Caps      : 0x05200005
>         Video Capture
>         Video Overlay
>         Read/Write
>         Streaming
>         Extended Pix Format
>
> Required ioctls:
>     test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
>     test second /dev/video0 open: OK
>     test VIDIOC_QUERYCAP: OK
>     test VIDIOC_G/S_PRIORITY: OK
>     test for unlimited opens: OK
>
> Debug ioctls:
>     test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>     test VIDIOC_LOG_STATUS: OK
>
> Input ioctls:
>     test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>     test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>     test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>     test VIDIOC_ENUMAUDIO: OK (Not Supported)
>     test VIDIOC_G/S/ENUMINPUT: OK
>     test VIDIOC_G/S_AUDIO: OK (Not Supported)
>     Inputs: 1 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
>     test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>     test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>     test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>     test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>     test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>     Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
>     test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>     test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>     test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>     test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Control ioctls (Input 0):
>     test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>     test VIDIOC_QUERYCTRL: OK
>     test VIDIOC_G/S_CTRL: OK
>     test VIDIOC_G/S/TRY_EXT_CTRLS: OK
>     test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
>     test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>     Standard Controls: 33 Private Controls: 0
>
> Format ioctls (Input 0):
>     test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>     test VIDIOC_G/S_PARM: OK
>     test VIDIOC_G_FBUF: OK
>     test VIDIOC_G_FMT: OK
>     test VIDIOC_TRY_FMT: OK
>     test VIDIOC_S_FMT: OK
>     test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>     test Cropping: OK (Not Supported)
>     test Composing: OK (Not Supported)
>     test Scaling: OK
>
> Codec ioctls (Input 0):
>     test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>     test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>     test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls (Input 0):
>         fail: v4l2-test-buffers.cpp(715): q.create_bufs(node, 1, &fmt)
> != EINVAL
>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: FAIL
>     test VIDIOC_EXPBUF: OK (Not Supported)
>     test Requests: OK (Not Supported)
>
> Test input 0:
>
> Streaming ioctls:
>     test read/write: OK
>         fail: v4l2-test-buffers.cpp(2145): node->streamon(q.g_type())
>         fail: v4l2-test-buffers.cpp(2224): testBlockingDQBuf(node, q)
>     test blocking wait: FAIL
>         fail: v4l2-test-buffers.cpp(1294): q.create_bufs(node, 1, &fmt)
> != EINVAL
>     test MMAP (no poll): FAIL
>         fail: v4l2-test-buffers.cpp(1294): q.create_bufs(node, 1, &fmt)
> != EINVAL
>     test MMAP (select): FAIL
>         fail: v4l2-test-buffers.cpp(1294): q.create_bufs(node, 1, &fmt)
> != EINVAL
>     test MMAP (epoll): FAIL
>
> After this series:
>
> v4l2-compliance SHA: b16f9e945d74aa552abdd6f873821cb77faaf13a, 32 bits
>
> Compliance test for bm2835 mmal device /dev/video0:
>
> Driver Info:
>     Driver name      : bm2835 mmal
>     Card type        : mmal service 16.1
>     Bus info         : platform:bcm2835-v4l2
>     Driver version   : 5.2.0
>     Capabilities     : 0x85200005
>         Video Capture
>         Video Overlay
>         Read/Write
>         Streaming
>         Extended Pix Format
>         Device Capabilities
>     Device Caps      : 0x05200005
>         Video Capture
>         Video Overlay
>         Read/Write
>         Streaming
>         Extended Pix Format
>
> Required ioctls:
>     test VIDIOC_QUERYCAP: OK
>
> Allow for multiple opens:
>     test second /dev/video0 open: OK
>     test VIDIOC_QUERYCAP: OK
>     test VIDIOC_G/S_PRIORITY: OK
>     test for unlimited opens: OK
>
> Debug ioctls:
>     test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
>     test VIDIOC_LOG_STATUS: OK
>
> Input ioctls:
>     test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
>     test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>     test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
>     test VIDIOC_ENUMAUDIO: OK (Not Supported)
>     test VIDIOC_G/S/ENUMINPUT: OK
>     test VIDIOC_G/S_AUDIO: OK (Not Supported)
>     Inputs: 1 Audio Inputs: 0 Tuners: 0
>
> Output ioctls:
>     test VIDIOC_G/S_MODULATOR: OK (Not Supported)
>     test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
>     test VIDIOC_ENUMAUDOUT: OK (Not Supported)
>     test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
>     test VIDIOC_G/S_AUDOUT: OK (Not Supported)
>     Outputs: 0 Audio Outputs: 0 Modulators: 0
>
> Input/Output configuration ioctls:
>     test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
>     test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
>     test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
>     test VIDIOC_G/S_EDID: OK (Not Supported)
>
> Control ioctls (Input 0):
>     test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
>     test VIDIOC_QUERYCTRL: OK
>     test VIDIOC_G/S_CTRL: OK
>     test VIDIOC_G/S/TRY_EXT_CTRLS: OK
>     test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
>     test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
>     Standard Controls: 33 Private Controls: 0
>
> Format ioctls (Input 0):
>     test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
>     test VIDIOC_G/S_PARM: OK
>     test VIDIOC_G_FBUF: OK
>     test VIDIOC_G_FMT: OK
>     test VIDIOC_TRY_FMT: OK
>     test VIDIOC_S_FMT: OK
>     test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
>     test Cropping: OK (Not Supported)
>     test Composing: OK (Not Supported)
>     test Scaling: OK
>
> Codec ioctls (Input 0):
>     test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
>     test VIDIOC_G_ENC_INDEX: OK (Not Supported)
>     test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
>
> Buffer ioctls (Input 0):
>     test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
>     test VIDIOC_EXPBUF: OK (Not Supported)
>     test Requests: OK (Not Supported)
>
> Test input 0:
>
> Streaming ioctls:
>     test read/write: OK
>     test blocking wait: OK
>
> Unfortunately in both cases the program hangs and never finish. This is
> the output of strace:
>
> ioctl(3, VIDIOC_DQBUF, {type=V4L2_BUF_TYPE_VIDEO_CAPTURE
>
> It is possible that this is a problem with the used linux-next version.
> Nevertheless the series is improvement.

Could you try running
v4l2-ctl -v width=640,height=480,pixelformat=YU12
before running v4l2-compliance? The default format is JPEG, and I just
wonder if there is an issue lurking in the compression side.

I'll get a Pi3 mainline build going when I get a chance.

> > 2) What is the status of this driver from your point of view?
> Sorry, i'm not a media expert. But i agree with Dan this needs
> improvement of error handling. For example mapping the MMAL error codes
> to Linux error codes would avoid confusion.
> > What is needed to get it out of staging?
>
> I think the driver needs more testing for 64 bit. Contrary to Raspbian a
> lot of the "mainline" distributions only concentrate on arm64. But
> currently i don't know of any 64 bit specific issues.

64 bit builds were working fine (currently broken downstream due to a
different change).
Whilst the VPU and IPC are only 32bit, there are idr lookups from any
64bit values in the kernel.
Once things have calmed down again a bit I will be resuming 64bit
kernel with 32bit userland work.

> I also can't say anything about the content of the TODO file.

I think some of the items were wishlist items from others rather than
requirements.

1) For dma-bufs to be useful in the rest of the system we need to
switch from the vmalloc allocator to dma-contig. Downstream I have
drivers in place that then allow the passing of the dma_addr to the
VPU for use as a buffer handle.
It also means a switch from allocating memory as gpu_mem to cma, so
requires some careful thought on our side for how heap allocation is
done.

2) Removing the extra copy isn't trivial on the firmware side.
Multi-planar also doesn't currently fall out easily as the VPU
requirements would be for a single contiguous allocation with offsets
for the planes. The current allocators allocate independent buffers
for each plane, and put page alignment requirements on each plane.
Other than SDRAM bandwidth, it has a minimal performance hit on the
system.

3) ARM64 works. Someone else may recall better on this, but I believe
vchi is now using the correct dma-ops for everything.

Thanks again for all your efforts on this.

  Dave

Re: [PATCH 00/31] staging: bcm2835-camera: Improvements

[Stefan Wahren]

Hi Dave,

Am 28.06.19 um 19:29 schrieb Dave Stevenson:
> Hi Stefan
>
> Could you try running
> v4l2-ctl -v width=640,height=480,pixelformat=YU12
> before running v4l2-compliance? The default format is JPEG, and I just
> wonder if there is an issue lurking in the compression side.

yes, this is much better:

Streaming ioctls:
    test read/write: OK
    test blocking wait: OK
    test MMAP (no poll): OK
    test MMAP (select): OK
    test MMAP (epoll): OK
    test USERPTR (no poll): OK
    test USERPTR (select): OK
    test DMABUF (no poll): OK (Not Supported)
    test DMABUF (select): OK (Not Supported)

Total for bm2835 mmal device /dev/video0: 53, Succeeded: 53, Failed: 0,
Warnings: 0

> I'll get a Pi3 mainline build going when I get a chance.
>