[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAGXv+5EfZUXEh6r0GaMKwTgWo8gQHsXFSF2bQzFuw=J4Gj0OGw@mail.gmail.com>
Date: Fri, 8 Nov 2024 14:02:06 +0800
From: Chen-Yu Tsai <wenst@...omium.org>
To: Yunfei Dong <yunfei.dong@...iatek.com>
Cc: Nícolas F . R . A . Prado <nfraprado@...labora.com>,
Sebastian Fricke <sebastian.fricke@...labora.com>,
Nicolas Dufresne <nicolas.dufresne@...labora.com>, Hans Verkuil <hverkuil-cisco@...all.nl>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>,
Benjamin Gaignard <benjamin.gaignard@...labora.com>, Nathan Hebert <nhebert@...omium.org>,
Hsin-Yi Wang <hsinyi@...omium.org>, Fritz Koenig <frkoenig@...omium.org>,
Daniel Vetter <daniel@...ll.ch>, Steve Cho <stevecho@...omium.org>, linux-media@...r.kernel.org,
devicetree@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-arm-kernel@...ts.infradead.org, linux-mediatek@...ts.infradead.org,
Project_Global_Chrome_Upstream_Group@...iatek.com
Subject: Re: [PATCH v6 5/5] media: mediatek: vcodec: remove media request checking
On Fri, Nov 8, 2024 at 11:32 AM Yunfei Dong <yunfei.dong@...iatek.com> wrote:
>
> Setting the buffer status to error if the media request of
> each source buffer is NULL, then schedule the work to process
> again in case of access NULL pointer.
>
> Signed-off-by: Yunfei Dong <yunfei.dong@...iatek.com>
> ---
> .../vcodec/decoder/mtk_vcodec_dec_stateless.c | 14 ++++++++------
> 1 file changed, 8 insertions(+), 6 deletions(-)
>
> 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 3f94654ebc73..251111678fd8 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
> @@ -363,10 +363,14 @@ static void mtk_vdec_worker(struct work_struct *work)
> ctx->id, bs_src->va, &bs_src->dma_addr, bs_src->size, vb2_src);
> /* Apply request controls. */
> src_buf_req = vb2_src->req_obj.req;
> - if (src_buf_req)
> + if (src_buf_req) {
> v4l2_ctrl_request_setup(src_buf_req, &ctx->ctrl_hdl);
> - else
> + } else {
> mtk_v4l2_vdec_err(ctx, "vb2 buffer media request is NULL");
> + v4l2_m2m_buf_done(vb2_v4l2_src, VB2_BUF_STATE_ERROR);
> + v4l2_m2m_job_finish(dev->m2m_dev_dec, ctx->m2m_ctx);
> + return;
Is this something that actually happens? I would assume that having
the `requires_requests` flag set on the queue would block any QBUF
call that doesn't have a request attached.
> + }
>
> ret = vdec_if_decode(ctx, bs_src, NULL, &res_chg);
> if (ret && ret != -EAGAIN) {
> @@ -384,8 +388,7 @@ static void mtk_vdec_worker(struct work_struct *work)
> state = ret ? VB2_BUF_STATE_ERROR : VB2_BUF_STATE_DONE;
> if (!IS_VDEC_LAT_ARCH(dev->vdec_pdata->hw_arch) ||
> ctx->current_codec == V4L2_PIX_FMT_VP8_FRAME) {
> - if (src_buf_req)
> - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
> + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
Unrelated change. Please do the cleanup in a separate patch.
v4l2_ctrl_request_setup() and v4l2_ctrl_request_complete() are both
no-ops if either argument is NULL, so you can do one patch going over
the whole driver to clean it up.
> vb2_v4l2_dst = v4l2_m2m_dst_buf_remove(ctx->m2m_ctx);
> v4l2_m2m_buf_done(vb2_v4l2_dst, state);
> v4l2_m2m_buf_done(vb2_v4l2_src, state);
> @@ -403,8 +406,7 @@ static void mtk_vdec_worker(struct work_struct *work)
> */
> ctx->cur_src_buffer = (ret != -EAGAIN) ? NULL : vb2_v4l2_src;
> if (ret && ret != -EAGAIN) {
> - if (src_buf_req)
> - v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
> + v4l2_ctrl_request_complete(src_buf_req, &ctx->ctrl_hdl);
Unrelated change. Same as above.
ChenYu
> v4l2_m2m_buf_done(vb2_v4l2_src, state);
> }
>
> --
> 2.46.0
>
Powered by blists - more mailing lists