Re: [PATCH v2 4/5] media: vcodec: Implement manual request completion
From: AngeloGioacchino Del Regno
Date: Mon Apr 14 2025 - 05:11:17 EST
Il 10/04/25 17:39, Nicolas Dufresne ha scritto:
From: Sebastian Fricke <sebastian.fricke@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>
Co-developed-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
Signed-off-by: Nicolas Dufresne <nicolas.dufresne@xxxxxxxxxxxxx>
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@xxxxxxxxxxxxx>