2024-03-15 07:29:20

by Yunfei Dong

[permalink] [raw]
Subject: [PATCH] media: mediatek: vcodec: add decoder command to support stateless decoder

The supported decoder commands are different for stateless and
stateful architecture. Adding stateless decoder commands to fix
below v4l2-compliance test error.

Codec ioctls:
VIDIOC_ENCODER_CMD returned -1 (Inappropriate ioctl for device)
VIDIOC_TRY_ENCODER_CMD returned -1 (Inappropriate ioctl for device)
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
VIDIOC_G_ENC_INDEX returned -1 (Inappropriate ioctl for device)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
VIDIOC_DECODER_CMD returned -1 (Invalid argument)
VIDIOC_TRY_DECODER_CMD returned -1 (Invalid argument)
VIDIOC_TRY_DECODER_CMD returned -1 (Invalid argument)
fail: v4l2-test-codecs.cpp(126): ret
test VIDIOC_(TRY_)DECODER_CMD: FAIL

Signed-off-by: Yunfei Dong <[email protected]>
---
.../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 65 +++++++++++++++++--
1 file changed, 59 insertions(+), 6 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
index ba742f0e391d..90579dd92cae 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
@@ -80,21 +80,20 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_dec_ctx *ctx,
return &ctx->q_data[MTK_Q_DATA_DST];
}

-static int vidioc_try_decoder_cmd(struct file *file, void *priv,
- struct v4l2_decoder_cmd *cmd)
+static int mtk_vcodec_stateful_try_decoder_cmd(struct file *file, void *priv,
+ struct v4l2_decoder_cmd *cmd)
{
return v4l2_m2m_ioctl_try_decoder_cmd(file, priv, cmd);
}

-
-static int vidioc_decoder_cmd(struct file *file, void *priv,
- struct v4l2_decoder_cmd *cmd)
+static int mtk_vcodec_stateful_decoder_cmd(struct file *file, void *priv,
+ struct v4l2_decoder_cmd *cmd)
{
struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
struct vb2_queue *src_vq, *dst_vq;
int ret;

- ret = vidioc_try_decoder_cmd(file, priv, cmd);
+ ret = mtk_vcodec_stateful_try_decoder_cmd(file, priv, cmd);
if (ret)
return ret;

@@ -128,6 +127,60 @@ static int vidioc_decoder_cmd(struct file *file, void *priv,
return 0;
}

+static int mtk_vcodec_stateless_try_decoder_cmd(struct file *file, void *priv,
+ struct v4l2_decoder_cmd *cmd)
+{
+ return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, cmd);
+}
+
+static int mtk_vcodec_stateless_decoder_cmd(struct file *file, void *priv,
+ struct v4l2_decoder_cmd *cmd)
+{
+ struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
+ int ret;
+
+ ret = v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, cmd);
+ if (ret)
+ return ret;
+
+ mtk_v4l2_vdec_dbg(3, ctx, "decoder cmd=%u", cmd->cmd);
+ switch (cmd->cmd) {
+ case V4L2_DEC_CMD_FLUSH:
+ /*
+ * If the flag of output buffer is set with V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF,
+ * this command will prevent dequeueing the capture buffer containing the last
+ * decoded frame. Or do nothing
+ */
+ break;
+
+ default:
+ mtk_v4l2_vdec_err(ctx, "invalid stateless decoder cmd=%u", cmd->cmd);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+static int vidioc_try_decoder_cmd(struct file *file, void *priv, struct v4l2_decoder_cmd *cmd)
+{
+ struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
+
+ if (ctx->dev->vdec_pdata->uses_stateless_api)
+ return mtk_vcodec_stateless_try_decoder_cmd(file, priv, cmd);
+ else
+ return mtk_vcodec_stateful_try_decoder_cmd(file, priv, cmd);
+}
+
+static int vidioc_decoder_cmd(struct file *file, void *priv, struct v4l2_decoder_cmd *cmd)
+{
+ struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
+
+ if (ctx->dev->vdec_pdata->uses_stateless_api)
+ return mtk_vcodec_stateless_decoder_cmd(file, priv, cmd);
+ else
+ return mtk_vcodec_stateful_decoder_cmd(file, priv, cmd);
+}
+
void mtk_vdec_unlock(struct mtk_vcodec_dec_ctx *ctx)
{
mutex_unlock(&ctx->dev->dec_mutex[ctx->hw_id]);
--
2.18.0



2024-03-15 10:30:37

by Sebastian Fricke

[permalink] [raw]
Subject: Re: [PATCH] media: mediatek: vcodec: add decoder command to support stateless decoder

Hey Yunfei,

On 15.03.2024 15:26, Yunfei Dong wrote:
>The supported decoder commands are different for stateless and
>stateful architecture. Adding stateless decoder commands to fix

s/Adding/Add/

>below v4l2-compliance test error.

s/below v4l2-compliance test error./the v4l2-compliance test error below./

>
>Codec ioctls:
> VIDIOC_ENCODER_CMD returned -1 (Inappropriate ioctl for device)
> VIDIOC_TRY_ENCODER_CMD returned -1 (Inappropriate ioctl for device)
> test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
> VIDIOC_G_ENC_INDEX returned -1 (Inappropriate ioctl for device)
> test VIDIOC_G_ENC_INDEX: OK (Not Supported)
> VIDIOC_DECODER_CMD returned -1 (Invalid argument)
> VIDIOC_TRY_DECODER_CMD returned -1 (Invalid argument)
> VIDIOC_TRY_DECODER_CMD returned -1 (Invalid argument)
> fail: v4l2-test-codecs.cpp(126): ret
> test VIDIOC_(TRY_)DECODER_CMD: FAIL
>
>Signed-off-by: Yunfei Dong <[email protected]>
>---
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 65 +++++++++++++++++--
> 1 file changed, 59 insertions(+), 6 deletions(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>index ba742f0e391d..90579dd92cae 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
>@@ -80,21 +80,20 @@ static struct mtk_q_data *mtk_vdec_get_q_data(struct mtk_vcodec_dec_ctx *ctx,
> return &ctx->q_data[MTK_Q_DATA_DST];
> }
>
>-static int vidioc_try_decoder_cmd(struct file *file, void *priv,
>- struct v4l2_decoder_cmd *cmd)
>+static int mtk_vcodec_stateful_try_decoder_cmd(struct file *file, void *priv,
>+ struct v4l2_decoder_cmd *cmd)

In some cases you seem to name these functions with the prefix
`mtk_vdec` and sometimes with the prefix `mtk_vcodec` but all of these
are for decoders, so could you settle for one naming scheme? Also as
these functions are static I don't think it is strictly necessary to add
a prefix for each function.

> {
> return v4l2_m2m_ioctl_try_decoder_cmd(file, priv, cmd);
> }
>
>-
>-static int vidioc_decoder_cmd(struct file *file, void *priv,
>- struct v4l2_decoder_cmd *cmd)
>+static int mtk_vcodec_stateful_decoder_cmd(struct file *file, void *priv,
>+ struct v4l2_decoder_cmd *cmd)
> {
> struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
> struct vb2_queue *src_vq, *dst_vq;
> int ret;
>
>- ret = vidioc_try_decoder_cmd(file, priv, cmd);
>+ ret = mtk_vcodec_stateful_try_decoder_cmd(file, priv, cmd);
> if (ret)
> return ret;
>
>@@ -128,6 +127,60 @@ static int vidioc_decoder_cmd(struct file *file, void *priv,
> return 0;
> }
>
>+static int mtk_vcodec_stateless_try_decoder_cmd(struct file *file, void *priv,
>+ struct v4l2_decoder_cmd *cmd)
>+{
>+ return v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, cmd);
>+}
>+
>+static int mtk_vcodec_stateless_decoder_cmd(struct file *file, void *priv,
>+ struct v4l2_decoder_cmd *cmd)
>+{
>+ struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
>+ int ret;
>+
>+ ret = v4l2_m2m_ioctl_stateless_try_decoder_cmd(file, priv, cmd);
>+ if (ret)
>+ return ret;
>+
>+ mtk_v4l2_vdec_dbg(3, ctx, "decoder cmd=%u", cmd->cmd);
>+ switch (cmd->cmd) {
>+ case V4L2_DEC_CMD_FLUSH:
>+ /*
>+ * If the flag of output buffer is set with V4L2_BUF_FLAG_M2M_HOLD_CAPTURE_BUF,

s/output/the output/
s/is set with/equals/

>+ * this command will prevent dequeueing the capture buffer containing the last
>+ * decoded frame. Or do nothing
>+ */
>+ break;
>+

Please remove this newline.

>+ default:
>+ mtk_v4l2_vdec_err(ctx, "invalid stateless decoder cmd=%u", cmd->cmd);
>+ return -EINVAL;
>+ }
>+
>+ return 0;
>+}
>+
>+static int vidioc_try_decoder_cmd(struct file *file, void *priv, struct v4l2_decoder_cmd *cmd)
>+{
>+ struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
>+
>+ if (ctx->dev->vdec_pdata->uses_stateless_api)
>+ return mtk_vcodec_stateless_try_decoder_cmd(file, priv, cmd);
>+ else

As these conditional branches contain return statements you can skip the
else.
E.g.
if (ctx->dev->vdec_pdata->uses_stateless_api)
return mtk_vcodec_stateless_try_decoder_cmd(file, priv, cmd);
return mtk_vcodec_stateful_try_decoder_cmd(file, priv, cmd);

>+ return mtk_vcodec_stateful_try_decoder_cmd(file, priv, cmd);
>+}
>+
>+static int vidioc_decoder_cmd(struct file *file, void *priv, struct v4l2_decoder_cmd *cmd)
>+{
>+ struct mtk_vcodec_dec_ctx *ctx = fh_to_dec_ctx(priv);
>+
>+ if (ctx->dev->vdec_pdata->uses_stateless_api)
>+ return mtk_vcodec_stateless_decoder_cmd(file, priv, cmd);
>+ else

Same as mentioned above.

>+ return mtk_vcodec_stateful_decoder_cmd(file, priv, cmd);
>+}
>+
> void mtk_vdec_unlock(struct mtk_vcodec_dec_ctx *ctx)
> {
> mutex_unlock(&ctx->dev->dec_mutex[ctx->hw_id]);
>--
>2.18.0
>