| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <f4eec2aa4856edbbb2f1e9248dffd9aff62c1a80.camel@ndufresne.ca>
Date: Mon, 07 Apr 2025 11:17:25 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Sebastian Fricke <sebastian.fricke@...labora.com>, Sakari Ailus
<sakari.ailus@...ux.intel.com>, Laurent Pinchart
<laurent.pinchart@...asonboard.com>, Mauro Carvalho Chehab
<mchehab@...nel.org>, Hans Verkuil <hverkuil@...all.nl>, Tiffany Lin
<tiffany.lin@...iatek.com>, Andrew-CT Chen <andrew-ct.chen@...iatek.com>,
Yunfei Dong <yunfei.dong@...iatek.com>, Matthias Brugger
<matthias.bgg@...il.com>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>
Cc: linux-media@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
kernel@...labora.com
Subject: Re: [PATCH 4/5] media: vcodec: Implement manual request completion
Le vendredi 14 mars 2025 à 14:26 +0100, Sebastian Fricke a écrit :
> Rework how requests are completed in the MediaTek VCodec driver, by
> implementing the new manual request completion feature, which allows to
> keep a request open while allowing to add new bitstream data.
> This is useful in this case, because the hardware has a LAT and a core
> decode work, after the LAT decode the bitstream isn't required anymore
> so the source buffer can be set done and the request stays open until
> the core decode work finishes.
>
> Signed-off-by: Sebastian Fricke <sebastian.fricke@...labora.com>
> ---
> .../mediatek/vcodec/common/mtk_vcodec_cmn_drv.h | 13 ++++++
> .../mediatek/vcodec/decoder/mtk_vcodec_dec.c | 4 +-
> .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c | 52 ++++++++++++++++++++++
> .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 4 ++
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 46 +++++++++----------
> 5 files changed, 94 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_cmn_drv.h b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_cmn_drv.h
> index 6087e27bd604d24e5d37b48de5bb37eab86fc1ab..81ec5beecfaed239ed70bee3460aae27e476231c 100644
> --- a/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_cmn_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/common/mtk_vcodec_cmn_drv.h
> @@ -105,6 +105,19 @@ enum mtk_instance_state {
> MTK_STATE_ABORT = 4,
> };
>
> +/**
> + * enum mtk_request_state - Stages of processing a request
> + * MTK_REQUEST_RECEIVED: Hardware prepared for the LAT decode
> + * MTK_REQUEST_DONE_WITH_BITSTREAM: LAT decode finished, the bitstream is not
> + * needed anymore
> + * MTK_REQUEST_COMPLETE: CORE decode finished
> + */
> +enum mtk_request_state {
> + MTK_REQUEST_RECEIVED = 0,
> + MTK_REQUEST_LAT_DONE = 1,
> + MTK_REQUEST_CORE_DONE = 2,
> +};
> +
> enum mtk_fmt_type {
> MTK_FMT_DEC = 0,
> MTK_FMT_ENC = 1,
> 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 98838217b97d45ed2b5431fdf87c94e0ff79fc57..036ad191a9c3e644fe99b4ce25d6a089292f1e57 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec.c
> @@ -889,8 +889,10 @@ void vb2ops_vdec_stop_streaming(struct vb2_queue *q)
> src_buf->vb2_buf.req_obj.req;
> v4l2_m2m_buf_done(src_buf,
> VB2_BUF_STATE_ERROR);
> - if (req)
> + if (req) {
> v4l2_ctrl_request_complete(req, &ctx->ctrl_hdl);
> + media_request_manual_complete(req);
> + }
> }
> }
> return;
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> index 9247d92d431d8570609423156b989878f7901f1c..d9c7aaec0c4515cb73b80c913b1ad5b08392dd18 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.c
> @@ -26,6 +26,58 @@
> #include "mtk_vcodec_dec_pm.h"
> #include "../common/mtk_vcodec_intr.h"
>
> +const char *state_to_str(enum mtk_request_state state)
> +{
> + switch (state) {
> + case MTK_REQUEST_RECEIVED:
> + return "RECEIVED";
> + case MTK_REQUEST_LAT_DONE:
> + return "LAT_DONE";
> + case MTK_REQUEST_CORE_DONE:
> + return "CORE_DONE";
> + default:
> + return "UNKNOWN";
> + }
> +}
> +
> +int mtk_vcodec_complete(struct mtk_vcodec_dec_ctx *ctx, enum mtk_request_state state,
> + enum vb2_buffer_state buffer_state, struct media_request *src_buf_req)
> +{
> + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +
> + mutex_lock(&ctx->lock);
> +
> + if (ctx->req_state >= state) {
> + mutex_unlock(&ctx->lock);
> + return -EINVAL;
> + }
> +
> + switch (ctx->req_state) {
> + case MTK_REQUEST_RECEIVED:
> + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
> + src_buf = v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> + v4l2_m2m_buf_done(src_buf, buffer_state);
> + ctx->req_state = MTK_REQUEST_LAT_DONE;
> + if (state == MTK_REQUEST_LAT_DONE)
> + break;
> + fallthrough;
> + case MTK_REQUEST_LAT_DONE:
> + dst_buf = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> + v4l2_m2m_buf_done(dst_buf, buffer_state);
> + media_request_manual_complete(src_buf_req);
> + ctx->req_state = MTK_REQUEST_CORE_DONE;
> + break;
> + default:
> + break;
> + }
> +
> + mtk_v4l2_vdec_dbg(3, ctx, "Switch state from %s to %s.\n",
> + state_to_str(ctx->req_state), state_to_str(state));
> + ctx->req_state = state;
> + mutex_unlock(&ctx->lock);
> + return 0;
> +}
> +
> static int mtk_vcodec_get_hw_count(struct mtk_vcodec_dec_ctx *ctx, struct mtk_vcodec_dec_dev *dev)
> {
> switch (dev->vdec_pdata->hw_arch) {
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> index ac568ed14fa257d25b533b6fd6b3cd341227ecc2..21c2301363d0151ba3cf384a2a32fff9f5d46034 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h
> @@ -185,6 +185,7 @@ struct mtk_vcodec_dec_ctx {
> struct mtk_q_data q_data[2];
> int id;
> enum mtk_instance_state state;
> + enum mtk_request_state req_state;
As you discovered before I came to do that final review, this is the
wrong version of the patchset. The request state must be per-request.
Looking forward your v2.
Nicolas
>
> const struct vdec_common_if *dec_if;
> void *drv_handle;
> @@ -326,6 +327,9 @@ wake_up_dec_ctx(struct mtk_vcodec_dec_ctx *ctx, unsigned int reason, unsigned in
> wake_up_interruptible(&ctx->queue[hw_id]);
> }
>
> +int mtk_vcodec_complete(struct mtk_vcodec_dec_ctx *ctx, enum mtk_request_state state,
> + enum vb2_buffer_state buffer_state, struct media_request *src_buf_req);
> +
> #define mtk_vdec_err(ctx, fmt, args...) \
> mtk_vcodec_err((ctx)->id, (ctx)->dev->plat_dev, fmt, ##args)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> index afa224da0f4165cf5701d6861f1f787c6317bfe4..9187d7bcfc8aea17f3fc98d94419777d8026db51 100644
> --- a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> +++ b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_stateless.c
> @@ -247,7 +247,6 @@ static const struct v4l2_frmsize_stepwise stepwise_fhd = {
> static void mtk_vdec_stateless_cap_to_disp(struct mtk_vcodec_dec_ctx *ctx, int error,
> struct media_request *src_buf_req)
> {
> - struct vb2_v4l2_buffer *vb2_dst;
> enum vb2_buffer_state state;
>
> if (error)
> @@ -255,17 +254,7 @@ static void mtk_vdec_stateless_cap_to_disp(struct mtk_vcodec_dec_ctx *ctx, int e
> else
> state = VB2_BUF_STATE_DONE;
>
> - vb2_dst = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> - if (vb2_dst) {
> - v4l2_m2m_buf_done(vb2_dst, state);
> - mtk_v4l2_vdec_dbg(2, ctx, "free frame buffer id:%d to done list",
> - vb2_dst->vb2_buf.index);
> - } else {
> - mtk_v4l2_vdec_err(ctx, "dst buffer is NULL");
> - }
> -
> - if (src_buf_req)
> - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
> + mtk_vcodec_complete(ctx, MTK_REQUEST_CORE_DONE, state, src_buf_req);
> }
>
> static struct vdec_fb *vdec_get_cap_buffer(struct mtk_vcodec_dec_ctx *ctx)
> @@ -308,6 +297,7 @@ static void vb2ops_vdec_buf_request_complete(struct vb2_buffer *vb)
> struct mtk_vcodec_dec_ctx *ctx = vb2_get_drv_priv(vb->vb2_queue);
>
> v4l2_ctrl_request_complete(vb->req_obj.req, &ctx->ctrl_hdl);
> + media_request_manual_complete(vb->req_obj.req);
> }
>
> static void mtk_vdec_worker(struct work_struct *work)
> @@ -358,12 +348,17 @@ static void mtk_vdec_worker(struct work_struct *work)
> else
> mtk_v4l2_vdec_err(ctx, "vb2 buffer media request is NULL");
>
> + mutex_lock(&ctx->lock);
> + ctx->req_state = MTK_REQUEST_RECEIVED;
> + mutex_unlock(&ctx->lock);
> +
> ret = vdec_if_decode(ctx, bs_src, NULL, &res_chg);
> +
> if (ret && ret != -EAGAIN) {
> mtk_v4l2_vdec_err(ctx,
> - "[%d] decode src_buf[%d] sz=0x%zx pts=%llu ret=%d res_chg=%d",
> + "[%d] decode src_buf[%d] sz=0x%zx pts=%llu res_chg=%d ret=%d",
> ctx->id, vb2_src->index, bs_src->size,
> - vb2_src->timestamp, ret, res_chg);
> + vb2_src->timestamp, res_chg, ret);
> if (ret == -EIO) {
> mutex_lock(&ctx->lock);
> dec_buf_src->error = true;
> @@ -372,18 +367,15 @@ static void mtk_vdec_worker(struct work_struct *work)
> }
>
> state = ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE;
> + if (ret != -EAGAIN)
> + mtk_vcodec_complete(ctx, MTK_REQUEST_LAT_DONE, state, src_buf_req);
> +
> if (!IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch) ||
> ctx->current_codec == V4L2_PIX_FMT_VP8_FRAME) {
> - v4l2_m2m_buf_done_and_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx, state);
> - if (src_buf_req)
> - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
> - } else {
> - if (ret != -EAGAIN) {
> - v4l2_m2m_src_buf_remove(ctx->m2m_ctx);
> - v4l2_m2m_buf_done(vb2_v4l2_src, state);
> - }
> - v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
> + mtk_vcodec_complete(ctx, MTK_REQUEST_CORE_DONE, state, src_buf_req);
> }
> +
> + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
> }
>
> static void vb2ops_vdec_stateless_buf_queue(struct vb2_buffer *vb)
> @@ -731,9 +723,15 @@ static int fops_media_request_validate(struct media_request *mreq)
> return vb2_request_validate(mreq);
> }
>
> +static void fops_media_request_queue(struct media_request *mreq)
> +{
> + media_request_mark_manual_completion(mreq);
> + v4l2_m2m_request_queue(mreq);
> +}
> +
> const struct media_device_ops mtk_vcodec_media_ops = {
> .req_validate = fops_media_request_validate,
> - .req_queue = v4l2_m2m_request_queue,
> + .req_queue = fops_media_request_queue,
> };
>
> static void mtk_vcodec_add_formats(unsigned int fourcc,
Powered by blists - more mailing lists