diff mbox series

media: videobuf2: Print videobuf2 buffer state by name

Message ID 20200709174336.79112-1-ezequiel@collabora.com (mailing list archive)
State New, archived
Headers show
Series media: videobuf2: Print videobuf2 buffer state by name | expand

Commit Message

Ezequiel Garcia July 9, 2020, 5:43 p.m. UTC 
For debugging purposes, seeing the state integer
representation is really inconvenient.

Improve this and be developer-friendly by printing
the state name instead.

Suggested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 .../media/common/videobuf2/videobuf2-core.c   | 32 ++++++++++++++-----
 1 file changed, 24 insertions(+), 8 deletions(-)

Comments

Hans Verkuil July 10, 2020, 10:10 a.m. UTC | #1 
On 09/07/2020 19:43, Ezequiel Garcia wrote:
> For debugging purposes, seeing the state integer
> representation is really inconvenient.
> 
> Improve this and be developer-friendly by printing
> the state name instead.
> 
> Suggested-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
> ---
>  .../media/common/videobuf2/videobuf2-core.c   | 32 ++++++++++++++-----
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
> index abaf28e057eb..8480772d58c6 100644
> --- a/drivers/media/common/videobuf2/videobuf2-core.c
> +++ b/drivers/media/common/videobuf2/videobuf2-core.c
> @@ -191,6 +191,20 @@ module_param(debug, int, 0644);
>  static void __vb2_queue_cancel(struct vb2_queue *q);
>  static void __enqueue_in_driver(struct vb2_buffer *vb);
>  
> +static const char * const vb2_state_names[] = {
> +	[VB2_BUF_STATE_DEQUEUED] = "dequeued",
> +	[VB2_BUF_STATE_IN_REQUEST] = "in request",
> +	[VB2_BUF_STATE_PREPARING] = "preparing",
> +	[VB2_BUF_STATE_QUEUED] = "queued",
> +	[VB2_BUF_STATE_ACTIVE] = "active",
> +	[VB2_BUF_STATE_DONE] = "done",
> +	[VB2_BUF_STATE_ERROR] = "error",
> +};
> +
> +#define vb2_state_name(s) \
> +	(((unsigned int)(s)) < ARRAY_SIZE(vb2_state_names) ? \
> +	 vb2_state_names[s] : "unknown")

Can you turn this into a function?

That avoids this checkpatch warning:

CHECK: Macro argument reuse 's' - possible side-effects?
#37: FILE: drivers/media/common/videobuf2/videobuf2-core.c:204:
+#define vb2_state_name(s) \
+       (((unsigned int)(s)) < ARRAY_SIZE(vb2_state_names) ? \
+        vb2_state_names[s] : "unknown")

And I think a function is cleaner as well.

This looks good otherwise.

Regards,

	Hans

> +
>  /*
>   * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
>   */
> @@ -1015,8 +1029,8 @@ void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
>  	 */
>  	vb->cnt_buf_done++;
>  #endif
> -	dprintk(q, 4, "done processing on buffer %d, state: %d\n",
> -			vb->index, state);
> +	dprintk(q, 4, "done processing on buffer %d, state: %s\n",
> +		vb->index, vb2_state_name(state));
>  
>  	if (state != VB2_BUF_STATE_QUEUED)
>  		__vb2_buf_mem_finish(vb);
> @@ -1490,8 +1504,8 @@ int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
>  
>  	vb = q->bufs[index];
>  	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
> -		dprintk(q, 1, "invalid buffer state %d\n",
> -			vb->state);
> +		dprintk(q, 1, "invalid buffer state %s\n",
> +			vb2_state_name(vb->state));
>  		return -EINVAL;
>  	}
>  	if (vb->prepared) {
> @@ -1670,7 +1684,8 @@ int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
>  		dprintk(q, 1, "buffer still being prepared\n");
>  		return -EINVAL;
>  	default:
> -		dprintk(q, 1, "invalid buffer state %d\n", vb->state);
> +		dprintk(q, 1, "invalid buffer state %s\n",
> +			vb2_state_name(vb->state));
>  		return -EINVAL;
>  	}
>  
> @@ -1884,7 +1899,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
>  		dprintk(q, 3, "returning done buffer with errors\n");
>  		break;
>  	default:
> -		dprintk(q, 1, "invalid buffer state\n");
> +		dprintk(q, 1, "invalid buffer state %s\n",
> +			vb2_state_name(vb->state));
>  		return -EINVAL;
>  	}
>  
> @@ -1915,8 +1931,8 @@ int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
>  		media_request_put(vb->request);
>  	vb->request = NULL;
>  
> -	dprintk(q, 2, "dqbuf of buffer %d, with state %d\n",
> -			vb->index, vb->state);
> +	dprintk(q, 2, "dqbuf of buffer %d, state: %s\n",
> +		vb->index, vb2_state_name(vb->state));
>  
>  	return 0;
>  
>
diff mbox series

Patch

diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c
index abaf28e057eb..8480772d58c6 100644
--- a/drivers/media/common/videobuf2/videobuf2-core.c
+++ b/drivers/media/common/videobuf2/videobuf2-core.c
@@ -191,6 +191,20 @@  module_param(debug, int, 0644);
 static void __vb2_queue_cancel(struct vb2_queue *q);
 static void __enqueue_in_driver(struct vb2_buffer *vb);
 
+static const char * const vb2_state_names[] = {
+	[VB2_BUF_STATE_DEQUEUED] = "dequeued",
+	[VB2_BUF_STATE_IN_REQUEST] = "in request",
+	[VB2_BUF_STATE_PREPARING] = "preparing",
+	[VB2_BUF_STATE_QUEUED] = "queued",
+	[VB2_BUF_STATE_ACTIVE] = "active",
+	[VB2_BUF_STATE_DONE] = "done",
+	[VB2_BUF_STATE_ERROR] = "error",
+};
+
+#define vb2_state_name(s) \
+	(((unsigned int)(s)) < ARRAY_SIZE(vb2_state_names) ? \
+	 vb2_state_names[s] : "unknown")
+
 /*
  * __vb2_buf_mem_alloc() - allocate video memory for the given buffer
  */
@@ -1015,8 +1029,8 @@  void vb2_buffer_done(struct vb2_buffer *vb, enum vb2_buffer_state state)
 	 */
 	vb->cnt_buf_done++;
 #endif
-	dprintk(q, 4, "done processing on buffer %d, state: %d\n",
-			vb->index, state);
+	dprintk(q, 4, "done processing on buffer %d, state: %s\n",
+		vb->index, vb2_state_name(state));
 
 	if (state != VB2_BUF_STATE_QUEUED)
 		__vb2_buf_mem_finish(vb);
@@ -1490,8 +1504,8 @@  int vb2_core_prepare_buf(struct vb2_queue *q, unsigned int index, void *pb)
 
 	vb = q->bufs[index];
 	if (vb->state != VB2_BUF_STATE_DEQUEUED) {
-		dprintk(q, 1, "invalid buffer state %d\n",
-			vb->state);
+		dprintk(q, 1, "invalid buffer state %s\n",
+			vb2_state_name(vb->state));
 		return -EINVAL;
 	}
 	if (vb->prepared) {
@@ -1670,7 +1684,8 @@  int vb2_core_qbuf(struct vb2_queue *q, unsigned int index, void *pb,
 		dprintk(q, 1, "buffer still being prepared\n");
 		return -EINVAL;
 	default:
-		dprintk(q, 1, "invalid buffer state %d\n", vb->state);
+		dprintk(q, 1, "invalid buffer state %s\n",
+			vb2_state_name(vb->state));
 		return -EINVAL;
 	}
 
@@ -1884,7 +1899,8 @@  int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 		dprintk(q, 3, "returning done buffer with errors\n");
 		break;
 	default:
-		dprintk(q, 1, "invalid buffer state\n");
+		dprintk(q, 1, "invalid buffer state %s\n",
+			vb2_state_name(vb->state));
 		return -EINVAL;
 	}
 
@@ -1915,8 +1931,8 @@  int vb2_core_dqbuf(struct vb2_queue *q, unsigned int *pindex, void *pb,
 		media_request_put(vb->request);
 	vb->request = NULL;
 
-	dprintk(q, 2, "dqbuf of buffer %d, with state %d\n",
-			vb->index, vb->state);
+	dprintk(q, 2, "dqbuf of buffer %d, state: %s\n",
+		vb->index, vb2_state_name(vb->state));
 
 	return 0;