diff mbox series

[v2,03/12] staging: media: rkvdec: Helper for buffer queue busy check

Message ID 20230101-patch-series-v2-6-2-rc1-v2-3-fa1897efac14@collabora.com (mailing list archive)
State New, archived
Headers show
Series RkVDEC HEVC driver | expand

Commit Message

Sebastian Fricke Jan. 12, 2023, 12:56 p.m. UTC
Implement a helper function, wrapping around getting a vb2 queue and
checking whether it is busy, to reduce the amount of code duplication in
the driver.

Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
---
 drivers/staging/media/rkvdec/rkvdec.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

Comments

Andrzej Pietrasiewicz Jan. 16, 2023, 7:54 a.m. UTC | #1
Hi Sebastian,

W dniu 12.01.2023 o 13:56, Sebastian Fricke pisze:
> Implement a helper function, wrapping around getting a vb2 queue and
> checking whether it is busy, to reduce the amount of code duplication in
> the driver.
> 
> Signed-off-by: Sebastian Fricke <sebastian.fricke@collabora.com>
> ---
>   drivers/staging/media/rkvdec/rkvdec.c | 21 ++++++++++++++-------
>   1 file changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
> index 7bab7586918c..c849f6c20279 100644
> --- a/drivers/staging/media/rkvdec/rkvdec.c
> +++ b/drivers/staging/media/rkvdec/rkvdec.c
> @@ -27,6 +27,17 @@
>   #include "rkvdec.h"
>   #include "rkvdec-regs.h"
>   
> +static int rkvdec_queue_busy(struct rkvdec_ctx *ctx, enum v4l2_buf_type buf_type)
> +{
> +	struct vb2_queue *vq;
> +
> +	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, buf_type);
> +	if (vb2_is_busy(vq))
> +		return -EBUSY;
> +	else
> +		return 0;

Optionally you can use the ternary operator:

return vb2_is_busy(vg) ? -EBUSY : 0;

But then this new function essentially becomes a wrapper around extracting
the vq. The wrapper then encodes its result in the return value,
which is later decoded at call site to only return -EBUSY if the wrapper
returns -EBUSY and ignore the result if 0. This makes me think that
maybe you want a macro instead:

#define rkvdec_queue_busy(ctx, type) \
	vb2_is_busy(v4l2_m2m_get_vq((ctx)->fh.m2m_ctx, (type)))

and call it like this:

if (rkvdec_queue_busy(c, t))
	return -EBUSY;

> +}
> +
>   static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
>   {
>   	struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
> @@ -311,13 +322,10 @@ static int rkvdec_s_capture_fmt(struct file *file, void *priv,
>   				struct v4l2_format *f)
>   {
>   	struct rkvdec_ctx *ctx = fh_to_rkvdec_ctx(priv);
> -	struct vb2_queue *vq;
>   	int ret;
>   
>   	/* Change not allowed if queue is busy */
> -	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
> -			     V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> -	if (vb2_is_busy(vq))
> +	if (rkvdec_queue_busy(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) != 0)

if you keep the current version you can drop the "!= 0"

Regards,

Andrzej

>   		return -EBUSY;
>   
>   	ret = rkvdec_try_capture_fmt(file, priv, f);
> @@ -335,7 +343,7 @@ static int rkvdec_s_output_fmt(struct file *file, void *priv,
>   	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
>   	const struct rkvdec_coded_fmt_desc *desc;
>   	struct v4l2_format *cap_fmt;
> -	struct vb2_queue *peer_vq, *vq;
> +	struct vb2_queue *vq;
>   	int ret;
>   
>   	/*
> @@ -354,8 +362,7 @@ static int rkvdec_s_output_fmt(struct file *file, void *priv,
>   	 * queue, we can't allow doing so when the CAPTURE queue has buffers
>   	 * allocated.
>   	 */
> -	peer_vq = v4l2_m2m_get_vq(m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
> -	if (vb2_is_busy(peer_vq))
> +	if (rkvdec_queue_busy(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) != 0)
>   		return -EBUSY;
>   
>   	ret = rkvdec_try_output_fmt(file, priv, f);
>
diff mbox series

Patch

diff --git a/drivers/staging/media/rkvdec/rkvdec.c b/drivers/staging/media/rkvdec/rkvdec.c
index 7bab7586918c..c849f6c20279 100644
--- a/drivers/staging/media/rkvdec/rkvdec.c
+++ b/drivers/staging/media/rkvdec/rkvdec.c
@@ -27,6 +27,17 @@ 
 #include "rkvdec.h"
 #include "rkvdec-regs.h"
 
+static int rkvdec_queue_busy(struct rkvdec_ctx *ctx, enum v4l2_buf_type buf_type)
+{
+	struct vb2_queue *vq;
+
+	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, buf_type);
+	if (vb2_is_busy(vq))
+		return -EBUSY;
+	else
+		return 0;
+}
+
 static int rkvdec_try_ctrl(struct v4l2_ctrl *ctrl)
 {
 	struct rkvdec_ctx *ctx = container_of(ctrl->handler, struct rkvdec_ctx, ctrl_hdl);
@@ -311,13 +322,10 @@  static int rkvdec_s_capture_fmt(struct file *file, void *priv,
 				struct v4l2_format *f)
 {
 	struct rkvdec_ctx *ctx = fh_to_rkvdec_ctx(priv);
-	struct vb2_queue *vq;
 	int ret;
 
 	/* Change not allowed if queue is busy */
-	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
-			     V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
-	if (vb2_is_busy(vq))
+	if (rkvdec_queue_busy(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) != 0)
 		return -EBUSY;
 
 	ret = rkvdec_try_capture_fmt(file, priv, f);
@@ -335,7 +343,7 @@  static int rkvdec_s_output_fmt(struct file *file, void *priv,
 	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
 	const struct rkvdec_coded_fmt_desc *desc;
 	struct v4l2_format *cap_fmt;
-	struct vb2_queue *peer_vq, *vq;
+	struct vb2_queue *vq;
 	int ret;
 
 	/*
@@ -354,8 +362,7 @@  static int rkvdec_s_output_fmt(struct file *file, void *priv,
 	 * queue, we can't allow doing so when the CAPTURE queue has buffers
 	 * allocated.
 	 */
-	peer_vq = v4l2_m2m_get_vq(m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
-	if (vb2_is_busy(peer_vq))
+	if (rkvdec_queue_busy(ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) != 0)
 		return -EBUSY;
 
 	ret = rkvdec_try_output_fmt(file, priv, f);