| 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: <8ff9cf88b2498e4bfd4c12b9c01cfaf046b92fd6.camel@ndufresne.ca>
Date: Fri, 30 May 2025 13:40:35 -0400
From: Nicolas Dufresne <nicolas@...fresne.ca>
To: Kyrie Wu <kyrie.wu@...iatek.com>, Hans Verkuil
<hverkuil-cisco@...all.nl>, Mauro Carvalho Chehab <mchehab@...nel.org>,
Rob Herring <robh@...nel.org>, Krzysztof Kozlowski <krzk+dt@...nel.org>,
Conor Dooley <conor+dt@...nel.org>, Matthias Brugger
<matthias.bgg@...il.com>, AngeloGioacchino Del Regno
<angelogioacchino.delregno@...labora.com>, 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
Cc: srv_heupstream@...iatek.com
Subject: Re: [PATCH v5 08/12] media: mediatek: jpeg: fix stop streaming flow
for multi-core
Hi,
Le vendredi 30 mai 2025 à 15:45 +0800, Kyrie Wu a écrit :
> 1. For multi-core jpegdec, the all hws may run at the same time,
> if one hw decoded firstly, the function of mtk_jpeg_dec_stop_streaming
> would be called, but others input buffers are decoding, this will
> cause some running buffers to be buffer done, causing errors;
> 2. add a parameter to calculate the decoding buffer counts, it
> wil decrease to 0 until the all buffers decoded and the
> mtk_jpeg_dec_stop_streaming could continue to be executed.
This one is equally unclear to me. If you run different queues per core,
why does this matter ?
Nicolas
>
> Signed-off-by: Kyrie Wu <kyrie.wu@...iatek.com>
> ---
> .../media/platform/mediatek/jpeg/mtk_jpeg_core.c | 16 ++++++++++++++++
> .../media/platform/mediatek/jpeg/mtk_jpeg_core.h | 2 ++
> .../platform/mediatek/jpeg/mtk_jpeg_dec_hw.c | 9 +++++++++
> .../platform/mediatek/jpeg/mtk_jpeg_enc_hw.c | 9 +++++++++
> 4 files changed, 36 insertions(+)
>
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> index 7e3509be6f69..1d3df1230191 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> @@ -861,8 +861,12 @@ static struct vb2_v4l2_buffer *mtk_jpeg_buf_remove(struct mtk_jpeg_ctx *ctx,
> static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
> {
> struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> + struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> struct vb2_v4l2_buffer *vb;
>
> + if (jpeg->variant->multi_core)
> + wait_event(jpeg->hw_wq, (atomic_read(&ctx->buf_list_cnt) == 0));
> +
> while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> }
> @@ -870,6 +874,7 @@ static void mtk_jpeg_enc_stop_streaming(struct vb2_queue *q)
> static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
> {
> struct mtk_jpeg_ctx *ctx = vb2_get_drv_priv(q);
> + struct mtk_jpeg_dev *jpeg = ctx->jpeg;
> struct vb2_v4l2_buffer *vb;
>
> /*
> @@ -877,6 +882,9 @@ static void mtk_jpeg_dec_stop_streaming(struct vb2_queue *q)
> * Before STREAMOFF, we still have to return the old resolution and
> * subsampling. Update capture queue when the stream is off.
> */
> + if (jpeg->variant->multi_core)
> + wait_event(jpeg->hw_wq, (atomic_read(&ctx->buf_list_cnt) == 0));
> +
> if (ctx->state == MTK_JPEG_SOURCE_CHANGE &&
> V4L2_TYPE_IS_CAPTURE(q->type)) {
> struct mtk_jpeg_src_buf *src_buf;
> @@ -1186,6 +1194,7 @@ static int mtk_jpeg_open(struct file *file)
> v4l2_fh_init(&ctx->fh, vfd);
> file->private_data = &ctx->fh;
> v4l2_fh_add(&ctx->fh);
> + atomic_set(&ctx->buf_list_cnt, 0);
>
> ctx->jpeg = jpeg;
> ctx->fh.m2m_ctx = v4l2_m2m_ctx_init(jpeg->m2m_dev, ctx,
> @@ -1568,6 +1577,11 @@ static int mtk_jpegdec_set_hw_param(struct mtk_jpeg_ctx *ctx,
> return 0;
> }
>
> +static void jpeg_buf_queue_inc(struct mtk_jpeg_ctx *ctx)
> +{
> + atomic_inc(&ctx->buf_list_cnt);
> +}
> +
> static irqreturn_t mtk_jpeg_enc_done(struct mtk_jpeg_dev *jpeg)
> {
> struct mtk_jpeg_ctx *ctx;
> @@ -1693,6 +1707,7 @@ static void mtk_jpegenc_worker(struct work_struct *work)
> &src_buf->vb2_buf);
> mtk_jpeg_set_enc_params(ctx, comp_jpeg[hw_id]->reg_base);
> mtk_jpeg_enc_start(comp_jpeg[hw_id]->reg_base);
> + jpeg_buf_queue_inc(ctx);
> v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
>
> @@ -1825,6 +1840,7 @@ static void mtk_jpegdec_worker(struct work_struct *work)
> &bs,
> &fb);
> mtk_jpeg_dec_start(comp_jpeg[hw_id]->reg_base);
> + jpeg_buf_queue_inc(ctx);
> v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> spin_unlock_irqrestore(&comp_jpeg[hw_id]->hw_lock, flags);
>
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> index 186cd1862028..6e8304680393 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> @@ -303,6 +303,7 @@ struct mtk_jpeg_q_data {
> * @dst_done_queue: encoded frame buffer queue
> * @done_queue_lock: encoded frame operation spinlock
> * @last_done_frame_num: the last encoded frame number
> + * @buf_list_cnt: the frame buffer count own by jpeg driver
> */
> struct mtk_jpeg_ctx {
> struct mtk_jpeg_dev *jpeg;
> @@ -321,6 +322,7 @@ struct mtk_jpeg_ctx {
> /* spinlock protecting the encode done buffer */
> spinlock_t done_queue_lock;
> u32 last_done_frame_num;
> + atomic_t buf_list_cnt;
> };
>
> #endif /* _MTK_JPEG_CORE_H */
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> index 2200f3b628dc..2e6da8617484 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_dec_hw.c
> @@ -522,6 +522,11 @@ static void mtk_jpegdec_put_buf(struct mtk_jpegdec_comp_dev *jpeg)
> spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> }
>
> +static void jpeg_buf_queue_dec(struct mtk_jpeg_ctx *ctx)
> +{
> + atomic_dec(&ctx->buf_list_cnt);
> +}
> +
> static void mtk_jpegdec_timeout_work(struct work_struct *work)
> {
> enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> @@ -530,9 +535,11 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
> job_timeout_work.work);
> struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> struct vb2_v4l2_buffer *src_buf, *dst_buf;
> + struct mtk_jpeg_ctx *ctx;
>
> src_buf = cjpeg->hw_param.src_buffer;
> dst_buf = cjpeg->hw_param.dst_buffer;
> + ctx = cjpeg->hw_param.curr_ctx;
> v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>
> mtk_jpeg_dec_reset(cjpeg->reg_base);
> @@ -543,6 +550,7 @@ static void mtk_jpegdec_timeout_work(struct work_struct *work)
> wake_up(&master_jpeg->hw_wq);
> v4l2_m2m_buf_done(src_buf, buf_state);
> mtk_jpegdec_put_buf(cjpeg);
> + jpeg_buf_queue_dec(ctx);
> }
>
> static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> @@ -583,6 +591,7 @@ static irqreturn_t mtk_jpegdec_hw_irq_handler(int irq, void *priv)
> buf_state = VB2_BUF_STATE_DONE;
> v4l2_m2m_buf_done(src_buf, buf_state);
> mtk_jpegdec_put_buf(jpeg);
> + jpeg_buf_queue_dec(ctx);
> pm_runtime_put(ctx->jpeg->dev);
> clk_disable_unprepare(jpeg->jdec_clk.clks->clk);
>
> diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> index 4c264c14ad83..ff73393a2417 100644
> --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_enc_hw.c
> @@ -251,6 +251,11 @@ static void mtk_jpegenc_put_buf(struct mtk_jpegenc_comp_dev *jpeg)
> spin_unlock_irqrestore(&ctx->done_queue_lock, flags);
> }
>
> +static void jpeg_buf_queue_dec(struct mtk_jpeg_ctx *ctx)
> +{
> + atomic_dec(&ctx->buf_list_cnt);
> +}
> +
> static void mtk_jpegenc_timeout_work(struct work_struct *work)
> {
> struct delayed_work *dly_work = to_delayed_work(work);
> @@ -261,9 +266,11 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
> struct mtk_jpeg_dev *master_jpeg = cjpeg->master_dev;
> enum vb2_buffer_state buf_state = VB2_BUF_STATE_ERROR;
> struct vb2_v4l2_buffer *src_buf, *dst_buf;
> + struct mtk_jpeg_ctx *ctx;
>
> src_buf = cjpeg->hw_param.src_buffer;
> dst_buf = cjpeg->hw_param.dst_buffer;
> + ctx = cjpeg->hw_param.curr_ctx;
> v4l2_m2m_buf_copy_metadata(src_buf, dst_buf, true);
>
> mtk_jpeg_enc_reset(cjpeg->reg_base);
> @@ -274,6 +281,7 @@ static void mtk_jpegenc_timeout_work(struct work_struct *work)
> wake_up(&master_jpeg->hw_wq);
> v4l2_m2m_buf_done(src_buf, buf_state);
> mtk_jpegenc_put_buf(cjpeg);
> + jpeg_buf_queue_dec(ctx);
> }
>
> static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> @@ -307,6 +315,7 @@ static irqreturn_t mtk_jpegenc_hw_irq_handler(int irq, void *priv)
> buf_state = VB2_BUF_STATE_DONE;
> v4l2_m2m_buf_done(src_buf, buf_state);
> mtk_jpegenc_put_buf(jpeg);
> + jpeg_buf_queue_dec(ctx);
> pm_runtime_put(ctx->jpeg->dev);
> clk_disable_unprepare(jpeg->venc_clk.clks->clk);
>
Powered by blists - more mailing lists