linux-kernel - [PATCH v2 3/7] media: mtk-vcodec: enc: use "stream_started" flag for "stop/start

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20211117130635.11633-4-dafna.hirschfeld@collabora.com>
Date:   Wed, 17 Nov 2021 15:06:31 +0200
From:   Dafna Hirschfeld <dafna.hirschfeld@...labora.com>
To:     linux-media@...r.kernel.org
Cc:     Dafna Hirschfeld <dafna.hirschfeld@...labora.com>,
        kernel@...labora.com, acourbot@...omium.org,
        andrew-ct.chen@...iatek.com, courbot@...omium.org,
        dafna3@...il.com, eizan@...omium.org, houlong.wei@...iatek.com,
        hsinyi@...omium.org, hverkuil@...all.nl, irui.wang@...iatek.com,
        linux-kernel@...r.kernel.org, linux-mediatek@...ts.infradead.org,
        maoguang.meng@...iatek.com, matthias.bgg@...il.com,
        mchehab@...nel.org, minghsiu.tsai@...iatek.com, tfiga@...omium.org,
        tiffany.lin@...iatek.com
Subject: [PATCH v2 3/7] media: mtk-vcodec: enc: use "stream_started" flag for "stop/start_streaming"

Currently the mtk-vcodec encoder does runtime pm resume
in "start_streaming" cb if both queues are streaming
and does runtime pm 'put' in "stop_streaming" if both
queues are not streaming.
This is wrong since the same queue might be started and
then stopped causing the driver to turn off the hardware
without turning it on. This cause for example unbalanced
calls to pm_runtime_*

Fixes: 4e855a6efa547 ("[media] vcodec: mediatek: Add Mediatek V4L2 Video Encoder Driver")
Signed-off-by: Dafna Hirschfeld <dafna.hirschfeld@...labora.com>
---
to reproduce the issue:
patch v4l-utils as follow:

static void stateful_m2m(cv4l_fd &fd, cv4l_queue &in, cv4l_queue &out,

 	if (fd.streamon(out.g_type()))
 		return;
+	if (fd.streamoff(out.g_type()))
+		return;
+
+	exit(1);

 	fd.s_trace(0);
 	if (exp_fd_p)

and call:
v4l2-ctl -x width=160,height=128,pixelformat=NM12 -v pixelformat=VP80 --stream-mmap --stream-out-mmap -d5
then the file /sys/devices/platform/soc/19002000.vcodec/power/runtime_usage
will show a negative number and further streaming (with e.g, gstreamer) is not possible.

 drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h | 4 ++++
 drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c | 8 ++++++++
 2 files changed, 12 insertions(+)

diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
index 9d36e3d27369..84c5289f872b 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_drv.h
@@ -259,6 +259,9 @@ struct vdec_pic_info {
  * @decoded_frame_cnt: number of decoded frames
  * @lock: protect variables accessed by V4L2 threads and worker thread such as
  *	  mtk_video_dec_buf.
+ * @stream_started: this flag is turned on when both queues (cap and out) starts streaming
+ *	  and it is turned off once both queues stop streaming. It is used for a correct
+ *	  setup and set-down of the hardware when starting and stopping streaming.
  */
 struct mtk_vcodec_ctx {
 	enum mtk_instance_type type;
@@ -301,6 +304,7 @@ struct mtk_vcodec_ctx {
 
 	int decoded_frame_cnt;
 	struct mutex lock;
+	bool stream_started;
 
 };
 
diff --git a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
index 87a5114bf680..fb3cf804c96a 100644
--- a/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
+++ b/drivers/media/platform/mtk-vcodec/mtk_vcodec_enc.c
@@ -890,6 +890,9 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
 		goto err_start_stream;
 	}
 
+	if (ctx->stream_started)
+		return 0;
+
 	/* Do the initialization when both start_streaming have been called */
 	if (V4L2_TYPE_IS_OUTPUT(q->type)) {
 		if (!vb2_start_streaming_called(&ctx->m2m_ctx->cap_q_ctx.q))
@@ -928,6 +931,7 @@ static int vb2ops_venc_start_streaming(struct vb2_queue *q, unsigned int count)
 		ctx->state = MTK_STATE_HEADER;
 	}
 
+	ctx->stream_started = true;
 	return 0;
 
 err_set_param:
@@ -1002,6 +1006,9 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
 		}
 	}
 
+	if (!ctx->stream_started)
+		return;
+
 	if ((q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
 	     vb2_is_streaming(&ctx->m2m_ctx->out_q_ctx.q)) ||
 	    (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
@@ -1023,6 +1030,7 @@ static void vb2ops_venc_stop_streaming(struct vb2_queue *q)
 		mtk_v4l2_err("pm_runtime_put fail %d", ret);
 
 	ctx->state = MTK_STATE_FREE;
+	ctx->stream_started = false;
 }
 
 static int vb2ops_venc_buf_out_validate(struct vb2_buffer *vb)
-- 
2.17.1