| 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: <d3da96bf-d1c8-4bf8-a22a-21a43f78f27d@collabora.com>
Date: Mon, 14 Apr 2025 11:11:04 +0200
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Nicolas Dufresne <nicolas.dufresne@...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>
Cc: linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
linux-mediatek@...ts.infradead.org, kernel@...labora.com,
linux-media@...r.kernel.org,
Sebastian Fricke <sebastian.fricke@...labora.com>
Subject: Re: [PATCH v2 4/5] media: vcodec: Implement manual request completion
Il 10/04/25 17:39, Nicolas Dufresne ha scritto:
> From: Sebastian Fricke <sebastian.fricke@...labora.com>
>
> 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>
> Co-developed-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
> Signed-off-by: Nicolas Dufresne <nicolas.dufresne@...labora.com>
This patch is great - but looks like it's worsening naming consistency across the
driver.
Please look below.
> ---
> .../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 | 50 +++++++++++++++++
> .../mediatek/vcodec/decoder/mtk_vcodec_dec_drv.h | 19 +++++++
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 63 +++++++++++++---------
> 5 files changed, 124 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..c5fd37cb60ca0cc5fd09c9243b36fbc716c56454 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_LAT_DONE: LAT decode finished, the bitstream is not
> + * needed anymore
> + * @MTK_REQUEST_CORE_DONE: 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..c80c1db509eaadd449bfd183c5eb9db0a1fc22bd 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,56 @@
> #include "mtk_vcodec_dec_pm.h"
> #include "../common/mtk_vcodec_intr.h"
>
> +static const char *state_to_str(enum mtk_request_state state)
static const char *mtk_vcodec_req_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";
> + }
> +}
> +
> +void mtk_request_complete(struct mtk_vcodec_dec_ctx *ctx, enum mtk_request_state state,
void mtk_vcodec_request_complete( ....)
> + enum vb2_buffer_state buffer_state, struct media_request *src_buf_req)
> +{
> + struct mtk_request *req = req_to_mtk_req(src_buf_req);
> + struct vb2_v4l2_buffer *src_buf, *dst_buf;
> +
> + mutex_lock(&ctx->lock);
> +
> + if (req->req_state >= state) {
> + mutex_unlock(&ctx->lock);
> + return;
> + }
> +
> + switch (req->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);
> + 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);
> + break;
> + default:
> + break;
> + }
> +
> + mtk_v4l2_vdec_dbg(3, ctx, "Switch state from %s to %s.\n",
> + state_to_str(req->req_state), state_to_str(state));
> + req->req_state = state;
> + mutex_unlock(&ctx->lock);
> +}
> +
> 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..cd61bf46de6918c27ed39ba64162e5f2637f93b2 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
> @@ -126,6 +126,17 @@ struct mtk_vcodec_dec_pdata {
> bool uses_stateless_api;
> };
>
> +/**
> + * struct mtk_request - Media request private data.
> + *
> + * @req_state: Request completion state
> + * @req: Media Request structure
> + */
> +struct mtk_request {
Maybe mtk_vcodec_dec_request? :-)
> + enum mtk_request_state req_state;
> + struct media_request req;
> +};
> +
> /**
> * struct mtk_vcodec_dec_ctx - Context (instance) private data.
> *
> @@ -317,6 +328,11 @@ static inline struct mtk_vcodec_dec_ctx *ctrl_to_dec_ctx(struct v4l2_ctrl *ctrl)
> return container_of(ctrl->handler, struct mtk_vcodec_dec_ctx, ctrl_hdl);
> }
>
> +static inline struct mtk_request *req_to_mtk_req(struct media_request *req)
...and this could become req_to_dec_req ... but no strong opinions on this one
specifically, so feel free to keep this as it is.
> +{
> + return container_of(req, struct mtk_request, req);
> +}
> +
> /* Wake up context wait_queue */
> static inline void
> wake_up_dec_ctx(struct mtk_vcodec_dec_ctx *ctx, unsigned int reason, unsigned int hw_id)
After which....
Reviewed-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
Powered by blists - more mailing lists