linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/8] Venus stateful encoder compliance
@ 2020-11-11 14:37 Stanimir Varbanov
  2020-11-11 14:37 ` [PATCH v2 1/8] venus: hfi: Use correct state in unload resources Stanimir Varbanov
                   ` (8 more replies)
  0 siblings, 9 replies; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

Hello,

Here is v2 of the subject patchset. 

The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
The last 8/8 just delete not needed helper function.

The major changes are:
 * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
 * Use null encoder buffer to signal end-of-stream in 6/8.

Comments are welcome!

regards,
Stan

Dikshita Agarwal (1):
  venus: venc: add handling for VIDIOC_ENCODER_CMD

Stanimir Varbanov (7):
  venus: hfi: Use correct state in unload resources
  venus: helpers: Add a new helper for buffer processing
  venus: hfi_cmds: Allow null buffer address on encoder input
  venus: helpers: Calculate properly compressed buffer size
  venus: pm_helpers: Check instance state when calculate instance
    frequency
  venus: venc: Handle reset encoder state
  venus: helpers: Delete unused stop streaming helper

 drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
 drivers/media/platform/qcom/venus/helpers.h   |   2 +-
 drivers/media/platform/qcom/venus/hfi.c       |   2 +-
 drivers/media/platform/qcom/venus/hfi.h       |   1 -
 drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
 .../media/platform/qcom/venus/pm_helpers.c    |   3 +
 drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
 7 files changed, 226 insertions(+), 81 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH v2 1/8] venus: hfi: Use correct state in unload resources
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:02   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 2/8] venus: helpers: Add a new helper for buffer processing Stanimir Varbanov
                   ` (7 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

INST_RELEASE_RESOURCES state is set but not used, correct this
by enter into INIT state once the unload resources is done.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/hfi.c | 2 +-
 drivers/media/platform/qcom/venus/hfi.h | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/hfi.c b/drivers/media/platform/qcom/venus/hfi.c
index 638ed5cfe05e..4c87228e8e1d 100644
--- a/drivers/media/platform/qcom/venus/hfi.c
+++ b/drivers/media/platform/qcom/venus/hfi.c
@@ -388,7 +388,7 @@ int hfi_session_unload_res(struct venus_inst *inst)
 	if (ret)
 		return ret;
 
-	inst->state = INST_RELEASE_RESOURCES;
+	inst->state = INST_INIT;
 
 	return 0;
 }
diff --git a/drivers/media/platform/qcom/venus/hfi.h b/drivers/media/platform/qcom/venus/hfi.h
index f25d412d6553..e9c944271cc1 100644
--- a/drivers/media/platform/qcom/venus/hfi.h
+++ b/drivers/media/platform/qcom/venus/hfi.h
@@ -87,7 +87,6 @@ struct hfi_event_data {
 #define INST_LOAD_RESOURCES			4
 #define INST_START				5
 #define INST_STOP				6
-#define INST_RELEASE_RESOURCES			7
 
 struct venus_core;
 struct venus_inst;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 2/8] venus: helpers: Add a new helper for buffer processing
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
  2020-11-11 14:37 ` [PATCH v2 1/8] venus: hfi: Use correct state in unload resources Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:03   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 3/8] venus: hfi_cmds: Allow null buffer address on encoder input Stanimir Varbanov
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

The new helper will be used from encoder and decoder drivers
to enqueue buffers for processing by firmware.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/helpers.c | 20 ++++++++++++++++++++
 drivers/media/platform/qcom/venus/helpers.h |  1 +
 2 files changed, 21 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index efa2781d6f55..688e3e3e8362 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1369,6 +1369,26 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
 }
 EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue);
 
+void venus_helper_process_buf(struct vb2_buffer *vb)
+{
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
+	int ret;
+
+	cache_payload(inst, vb);
+
+	if (vb2_start_streaming_called(vb->vb2_queue)) {
+		ret = is_buf_refed(inst, vbuf);
+		if (ret)
+			return;
+
+		ret = session_process_buf(inst, vbuf);
+		if (ret)
+			return_buf_error(inst, vbuf);
+	}
+}
+EXPORT_SYMBOL_GPL(venus_helper_process_buf);
+
 void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
 			       enum vb2_buffer_state state)
 {
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index f36c9f717798..231af29667e7 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -19,6 +19,7 @@ void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
 int venus_helper_vb2_buf_init(struct vb2_buffer *vb);
 int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb);
 void venus_helper_vb2_buf_queue(struct vb2_buffer *vb);
+void venus_helper_process_buf(struct vb2_buffer *vb);
 void venus_helper_vb2_stop_streaming(struct vb2_queue *q);
 int venus_helper_vb2_start_streaming(struct venus_inst *inst);
 void venus_helper_m2m_device_run(void *priv);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 3/8] venus: hfi_cmds: Allow null buffer address on encoder input
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
  2020-11-11 14:37 ` [PATCH v2 1/8] venus: hfi: Use correct state in unload resources Stanimir Varbanov
  2020-11-11 14:37 ` [PATCH v2 2/8] venus: helpers: Add a new helper for buffer processing Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:05   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size Stanimir Varbanov
                   ` (5 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

Allow null buffer address for encoder input buffers. This will
be used to send null input buffers to signal end-of-stream.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/hfi_cmds.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
index 4f7565834469..2affaa2ed70f 100644
--- a/drivers/media/platform/qcom/venus/hfi_cmds.c
+++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
@@ -278,7 +278,7 @@ int pkt_session_etb_encoder(
 		struct hfi_session_empty_buffer_uncompressed_plane0_pkt *pkt,
 		void *cookie, struct hfi_frame_data *in_frame)
 {
-	if (!cookie || !in_frame->device_addr)
+	if (!cookie)
 		return -EINVAL;
 
 	pkt->shdr.hdr.size = sizeof(*pkt);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
                   ` (2 preceding siblings ...)
  2020-11-11 14:37 ` [PATCH v2 3/8] venus: hfi_cmds: Allow null buffer address on encoder input Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:07   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 5/8] venus: pm_helpers: Check instance state when calculate instance frequency Stanimir Varbanov
                   ` (4 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

For resolutions below 720p the size of the compressed buffer must
be bigger. Correct this by checking the resolution when calculating
buffer size and multiply by four.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/helpers.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 688e3e3e8362..490c026b58a3 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -986,6 +986,8 @@ u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 width, u32 height)
 
 	if (compressed) {
 		sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2 / 2;
+		if (width < 1280 || height < 720)
+			sz *= 8;
 		return ALIGN(sz, SZ_4K);
 	}
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 5/8] venus: pm_helpers: Check instance state when calculate instance frequency
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
                   ` (3 preceding siblings ...)
  2020-11-11 14:37 ` [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:08   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 6/8] venus: venc: add handling for VIDIOC_ENCODER_CMD Stanimir Varbanov
                   ` (3 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

Skip calculating instance frequency if it is not in running state.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/pm_helpers.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
index ca99908ca3d3..cc84dc5e371b 100644
--- a/drivers/media/platform/qcom/venus/pm_helpers.c
+++ b/drivers/media/platform/qcom/venus/pm_helpers.c
@@ -940,6 +940,9 @@ static unsigned long calculate_inst_freq(struct venus_inst *inst,
 
 	mbs_per_sec = load_per_instance(inst);
 
+	if (inst->state != INST_START)
+		return 0;
+
 	vpp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vpp_freq;
 	/* 21 / 20 is overhead factor */
 	vpp_freq += vpp_freq / 20;
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 6/8] venus: venc: add handling for VIDIOC_ENCODER_CMD
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
                   ` (4 preceding siblings ...)
  2020-11-11 14:37 ` [PATCH v2 5/8] venus: pm_helpers: Check instance state when calculate instance frequency Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:12   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 7/8] venus: venc: Handle reset encoder state Stanimir Varbanov
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig,
	Dikshita Agarwal, Stanimir Varbanov

From: Dikshita Agarwal <dikshita@codeaurora.org>

Add handling for below commands in encoder:
1. V4L2_ENC_CMD_STOP
2. V4L2_ENC_CMD_START

Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/venc.c | 77 +++++++++++++++++++++++-
 1 file changed, 76 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 99bfabf90bd2..7512e4a16270 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -507,6 +507,59 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
 	return 0;
 }
 
+static int venc_encoder_cmd(struct file *file, void *fh,
+			    struct v4l2_encoder_cmd *ec)
+{
+	struct venus_inst *inst = to_inst(file);
+	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
+	struct hfi_frame_data fdata = {0};
+	int ret = 0;
+
+	ret = v4l2_m2m_ioctl_try_encoder_cmd(file, fh, ec);
+	if (ret < 0)
+		return ret;
+
+	mutex_lock(&inst->lock);
+
+	if (!vb2_is_streaming(&m2m_ctx->cap_q_ctx.q) ||
+	    !vb2_is_streaming(&m2m_ctx->out_q_ctx.q))
+		goto unlock;
+
+	if (m2m_ctx->is_draining) {
+		ret = -EBUSY;
+		goto unlock;
+	}
+
+	if (ec->cmd == V4L2_ENC_CMD_STOP) {
+		if (v4l2_m2m_has_stopped(m2m_ctx)) {
+			ret = 0;
+			goto unlock;
+		}
+
+		m2m_ctx->is_draining = true;
+
+		fdata.buffer_type = HFI_BUFFER_INPUT;
+		fdata.flags |= HFI_BUFFERFLAG_EOS;
+		fdata.device_addr = 0;
+		fdata.clnt_data = (u32)-1;
+
+		ret = hfi_session_process_buf(inst, &fdata);
+		if (ret)
+			goto unlock;
+	}
+
+	if (ec->cmd == V4L2_ENC_CMD_START && v4l2_m2m_has_stopped(m2m_ctx)) {
+		vb2_clear_last_buffer_dequeued(&m2m_ctx->cap_q_ctx.q);
+		inst->m2m_ctx->has_stopped = false;
+		venus_helper_process_initial_out_bufs(inst);
+		venus_helper_process_initial_cap_bufs(inst);
+	}
+
+unlock:
+	mutex_unlock(&inst->lock);
+	return ret;
+}
+
 static const struct v4l2_ioctl_ops venc_ioctl_ops = {
 	.vidioc_querycap = venc_querycap,
 	.vidioc_enum_fmt_vid_cap = venc_enum_fmt,
@@ -534,6 +587,8 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops = {
 	.vidioc_enum_frameintervals = venc_enum_frameintervals,
 	.vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
 	.vidioc_unsubscribe_event = v4l2_event_unsubscribe,
+	.vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
+	.vidioc_encoder_cmd = venc_encoder_cmd,
 };
 
 static int venc_set_properties(struct venus_inst *inst)
@@ -946,9 +1001,22 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
 static void venc_vb2_buf_queue(struct vb2_buffer *vb)
 {
 	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
 	mutex_lock(&inst->lock);
-	venus_helper_vb2_buf_queue(vb);
+
+	v4l2_m2m_buf_queue(m2m_ctx, vbuf);
+
+	if (!(inst->streamon_out && inst->streamon_cap))
+		goto unlock;
+
+	if (v4l2_m2m_has_stopped(m2m_ctx))
+		goto unlock;
+
+	venus_helper_process_buf(vb);
+
+unlock:
 	mutex_unlock(&inst->lock);
 }
 
@@ -968,6 +1036,7 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
 	struct vb2_v4l2_buffer *vbuf;
 	struct vb2_buffer *vb;
 	unsigned int type;
+	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 
 	if (buf_type == HFI_BUFFER_INPUT)
 		type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
@@ -986,6 +1055,12 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
 		vb->planes[0].data_offset = data_offset;
 		vb->timestamp = timestamp_us * NSEC_PER_USEC;
 		vbuf->sequence = inst->sequence_cap++;
+
+		if ((!bytesused && m2m_ctx->is_draining) ||
+		    (vbuf->flags & V4L2_BUF_FLAG_LAST)) {
+			vbuf->flags |= V4L2_BUF_FLAG_LAST;
+			v4l2_m2m_mark_stopped(inst->m2m_ctx);
+		}
 	} else {
 		vbuf->sequence = inst->sequence_out++;
 	}
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 7/8] venus: venc: Handle reset encoder state
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
                   ` (5 preceding siblings ...)
  2020-11-11 14:37 ` [PATCH v2 6/8] venus: venc: add handling for VIDIOC_ENCODER_CMD Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2021-01-02  0:13   ` Fritz Koenig
  2020-11-11 14:37 ` [PATCH v2 8/8] venus: helpers: Delete unused stop streaming helper Stanimir Varbanov
  2020-11-29 19:17 ` [PATCH v2 0/8] Venus stateful encoder compliance Fritz Koenig
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

Redesign the encoder driver to be compliant with stateful encoder
spec - specifically adds handling of Reset state.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/venc.c | 155 ++++++++++++++++++-----
 1 file changed, 122 insertions(+), 33 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 7512e4a16270..f1ae89d45a54 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -907,6 +907,54 @@ static int venc_queue_setup(struct vb2_queue *q,
 	return ret;
 }
 
+static void venc_release_session(struct venus_inst *inst)
+{
+	int ret, abort = 0;
+
+	mutex_lock(&inst->lock);
+
+	ret = hfi_session_deinit(inst);
+	abort = (ret && ret != -EINVAL) ? 1 : 0;
+
+	if (inst->session_error)
+		abort = 1;
+
+	if (abort)
+		hfi_session_abort(inst);
+
+	venus_pm_load_scale(inst);
+	INIT_LIST_HEAD(&inst->registeredbufs);
+	mutex_unlock(&inst->lock);
+
+	venus_pm_release_core(inst);
+}
+
+static int venc_buf_init(struct vb2_buffer *vb)
+{
+	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
+
+	inst->buf_count++;
+
+	return venus_helper_vb2_buf_init(vb);
+}
+
+static void venc_buf_cleanup(struct vb2_buffer *vb)
+{
+	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
+	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
+	struct venus_buffer *buf = to_venus_buffer(vbuf);
+
+	mutex_lock(&inst->lock);
+	if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+		if (!list_empty(&inst->registeredbufs))
+			list_del_init(&buf->reg_list);
+	mutex_unlock(&inst->lock);
+
+	inst->buf_count--;
+	if (!inst->buf_count)
+		venc_release_session(inst);
+}
+
 static int venc_verify_conf(struct venus_inst *inst)
 {
 	enum hfi_version ver = inst->core->res->hfi_version;
@@ -938,49 +986,57 @@ static int venc_verify_conf(struct venus_inst *inst)
 static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
 {
 	struct venus_inst *inst = vb2_get_drv_priv(q);
+	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
 	int ret;
 
 	mutex_lock(&inst->lock);
 
-	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+	v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
+
+	if (V4L2_TYPE_IS_OUTPUT(q->type))
 		inst->streamon_out = 1;
 	else
 		inst->streamon_cap = 1;
 
-	if (!(inst->streamon_out & inst->streamon_cap)) {
-		mutex_unlock(&inst->lock);
-		return 0;
-	}
+	if (inst->streamon_out && inst->streamon_cap &&
+	    inst->state == INST_UNINIT) {
+		venus_helper_init_instance(inst);
 
-	venus_helper_init_instance(inst);
+		inst->sequence_cap = 0;
+		inst->sequence_out = 0;
 
-	inst->sequence_cap = 0;
-	inst->sequence_out = 0;
+		ret = venc_init_session(inst);
+		if (ret)
+			goto bufs_done;
 
-	ret = venc_init_session(inst);
-	if (ret)
-		goto bufs_done;
+		ret = venus_pm_acquire_core(inst);
+		if (ret)
+			goto deinit_sess;
 
-	ret = venus_pm_acquire_core(inst);
-	if (ret)
-		goto deinit_sess;
+		ret = venc_verify_conf(inst);
+		if (ret)
+			goto deinit_sess;
 
-	ret = venc_set_properties(inst);
-	if (ret)
-		goto deinit_sess;
+		ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
+						inst->num_output_bufs, 0);
+		if (ret)
+			goto deinit_sess;
 
-	ret = venc_verify_conf(inst);
-	if (ret)
-		goto deinit_sess;
+		ret = venus_helper_vb2_start_streaming(inst);
+		if (ret)
+			goto deinit_sess;
 
-	ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
-					inst->num_output_bufs, 0);
-	if (ret)
-		goto deinit_sess;
+		venus_helper_process_initial_out_bufs(inst);
+		venus_helper_process_initial_cap_bufs(inst);
+	} else if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->streamon_cap &&
+		   inst->streamon_out) {
+		ret = venus_helper_vb2_start_streaming(inst);
+		if (ret)
+			goto bufs_done;
 
-	ret = venus_helper_vb2_start_streaming(inst);
-	if (ret)
-		goto deinit_sess;
+		venus_helper_process_initial_out_bufs(inst);
+		venus_helper_process_initial_cap_bufs(inst);
+	}
 
 	mutex_unlock(&inst->lock);
 
@@ -990,15 +1046,43 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
 	hfi_session_deinit(inst);
 bufs_done:
 	venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED);
-	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
+	if (V4L2_TYPE_IS_OUTPUT(q->type))
 		inst->streamon_out = 0;
 	else
 		inst->streamon_cap = 0;
+
 	mutex_unlock(&inst->lock);
 	return ret;
 }
 
-static void venc_vb2_buf_queue(struct vb2_buffer *vb)
+static void venc_stop_streaming(struct vb2_queue *q)
+{
+	struct venus_inst *inst = vb2_get_drv_priv(q);
+	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
+	int ret = -EINVAL;
+
+	mutex_lock(&inst->lock);
+
+	v4l2_m2m_clear_state(m2m_ctx);
+
+	if (V4L2_TYPE_IS_CAPTURE(q->type)) {
+		ret = hfi_session_stop(inst);
+		ret |= hfi_session_unload_res(inst);
+		ret |= venus_helper_unregister_bufs(inst);
+		ret |= venus_helper_intbufs_free(inst);
+	}
+
+	venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
+
+	if (V4L2_TYPE_IS_OUTPUT(q->type))
+		inst->streamon_out = 0;
+	else
+		inst->streamon_cap = 0;
+
+	mutex_unlock(&inst->lock);
+}
+
+static void venc_buf_queue(struct vb2_buffer *vb)
 {
 	struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
 	struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
@@ -1022,11 +1106,12 @@ static void venc_vb2_buf_queue(struct vb2_buffer *vb)
 
 static const struct vb2_ops venc_vb2_ops = {
 	.queue_setup = venc_queue_setup,
-	.buf_init = venus_helper_vb2_buf_init,
+	.buf_init = venc_buf_init,
+	.buf_cleanup = venc_buf_cleanup,
 	.buf_prepare = venus_helper_vb2_buf_prepare,
 	.start_streaming = venc_start_streaming,
-	.stop_streaming = venus_helper_vb2_stop_streaming,
-	.buf_queue = venc_vb2_buf_queue,
+	.stop_streaming = venc_stop_streaming,
+	.buf_queue = venc_buf_queue,
 };
 
 static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
@@ -1084,8 +1169,12 @@ static const struct hfi_inst_ops venc_hfi_ops = {
 	.event_notify = venc_event_notify,
 };
 
+static void venc_m2m_device_run(void *priv)
+{
+}
+
 static const struct v4l2_m2m_ops venc_m2m_ops = {
-	.device_run = venus_helper_m2m_device_run,
+	.device_run = venc_m2m_device_run,
 	.job_abort = venus_helper_m2m_job_abort,
 };
 
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH v2 8/8] venus: helpers: Delete unused stop streaming helper
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
                   ` (6 preceding siblings ...)
  2020-11-11 14:37 ` [PATCH v2 7/8] venus: venc: Handle reset encoder state Stanimir Varbanov
@ 2020-11-11 14:37 ` Stanimir Varbanov
  2020-11-29  6:09   ` Fritz Koenig
  2020-11-29 19:17 ` [PATCH v2 0/8] Venus stateful encoder compliance Fritz Koenig
  8 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-11 14:37 UTC (permalink / raw)
  To: linux-media, linux-arm-msm, linux-kernel
  Cc: Vikash Garodia, Alexandre Courbot, Fritz Koenig, Stanimir Varbanov

After re-design of encoder driver this helper is not needed
anymore.

Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
---
 drivers/media/platform/qcom/venus/helpers.c | 43 ---------------------
 drivers/media/platform/qcom/venus/helpers.h |  1 -
 2 files changed, 44 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 490c026b58a3..51c80417f361 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1406,49 +1406,6 @@ void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
 }
 EXPORT_SYMBOL_GPL(venus_helper_buffers_done);
 
-void venus_helper_vb2_stop_streaming(struct vb2_queue *q)
-{
-	struct venus_inst *inst = vb2_get_drv_priv(q);
-	struct venus_core *core = inst->core;
-	int ret;
-
-	mutex_lock(&inst->lock);
-
-	if (inst->streamon_out & inst->streamon_cap) {
-		ret = hfi_session_stop(inst);
-		ret |= hfi_session_unload_res(inst);
-		ret |= venus_helper_unregister_bufs(inst);
-		ret |= venus_helper_intbufs_free(inst);
-		ret |= hfi_session_deinit(inst);
-
-		if (inst->session_error || core->sys_error)
-			ret = -EIO;
-
-		if (ret)
-			hfi_session_abort(inst);
-
-		venus_helper_free_dpb_bufs(inst);
-
-		venus_pm_load_scale(inst);
-		INIT_LIST_HEAD(&inst->registeredbufs);
-	}
-
-	venus_helper_buffers_done(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
-				  VB2_BUF_STATE_ERROR);
-	venus_helper_buffers_done(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
-				  VB2_BUF_STATE_ERROR);
-
-	if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
-		inst->streamon_out = 0;
-	else
-		inst->streamon_cap = 0;
-
-	venus_pm_release_core(inst);
-
-	mutex_unlock(&inst->lock);
-}
-EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
-
 int venus_helper_process_initial_cap_bufs(struct venus_inst *inst)
 {
 	struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
index 231af29667e7..3eae2acbcc8e 100644
--- a/drivers/media/platform/qcom/venus/helpers.h
+++ b/drivers/media/platform/qcom/venus/helpers.h
@@ -20,7 +20,6 @@ int venus_helper_vb2_buf_init(struct vb2_buffer *vb);
 int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb);
 void venus_helper_vb2_buf_queue(struct vb2_buffer *vb);
 void venus_helper_process_buf(struct vb2_buffer *vb);
-void venus_helper_vb2_stop_streaming(struct vb2_queue *q);
 int venus_helper_vb2_start_streaming(struct venus_inst *inst);
 void venus_helper_m2m_device_run(void *priv);
 void venus_helper_m2m_job_abort(void *priv);
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 1/8] venus: hfi: Use correct state in unload resources
  2020-11-11 14:37 ` [PATCH v2 1/8] venus: hfi: Use correct state in unload resources Stanimir Varbanov
@ 2020-11-29  6:02   ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:02 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> INST_RELEASE_RESOURCES state is set but not used, correct this
> by enter into INIT state once the unload resources is done.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/hfi.c | 2 +-
>  drivers/media/platform/qcom/venus/hfi.h | 1 -
>  2 files changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi.c b/drivers/media/platform/qcom/venus/hfi.c
> index 638ed5cfe05e..4c87228e8e1d 100644
> --- a/drivers/media/platform/qcom/venus/hfi.c
> +++ b/drivers/media/platform/qcom/venus/hfi.c
> @@ -388,7 +388,7 @@ int hfi_session_unload_res(struct venus_inst *inst)
>         if (ret)
>                 return ret;
>
> -       inst->state = INST_RELEASE_RESOURCES;
> +       inst->state = INST_INIT;
>
>         return 0;
>  }
> diff --git a/drivers/media/platform/qcom/venus/hfi.h b/drivers/media/platform/qcom/venus/hfi.h
> index f25d412d6553..e9c944271cc1 100644
> --- a/drivers/media/platform/qcom/venus/hfi.h
> +++ b/drivers/media/platform/qcom/venus/hfi.h
> @@ -87,7 +87,6 @@ struct hfi_event_data {
>  #define INST_LOAD_RESOURCES                    4
>  #define INST_START                             5
>  #define INST_STOP                              6
> -#define INST_RELEASE_RESOURCES                 7
>
>  struct venus_core;
>  struct venus_inst;
> --
> 2.17.1
>

Reviewed-by: Fritz Koenig <frkoenig@chromium.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 2/8] venus: helpers: Add a new helper for buffer processing
  2020-11-11 14:37 ` [PATCH v2 2/8] venus: helpers: Add a new helper for buffer processing Stanimir Varbanov
@ 2020-11-29  6:03   ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:03 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> The new helper will be used from encoder and decoder drivers
> to enqueue buffers for processing by firmware.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/helpers.c | 20 ++++++++++++++++++++
>  drivers/media/platform/qcom/venus/helpers.h |  1 +
>  2 files changed, 21 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index efa2781d6f55..688e3e3e8362 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -1369,6 +1369,26 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
>  }
>  EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue);
>
> +void venus_helper_process_buf(struct vb2_buffer *vb)
> +{
> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +       struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +       int ret;
> +
> +       cache_payload(inst, vb);
> +
> +       if (vb2_start_streaming_called(vb->vb2_queue)) {
> +               ret = is_buf_refed(inst, vbuf);
> +               if (ret)
> +                       return;
> +
> +               ret = session_process_buf(inst, vbuf);
> +               if (ret)
> +                       return_buf_error(inst, vbuf);
> +       }
> +}
> +EXPORT_SYMBOL_GPL(venus_helper_process_buf);
> +
>  void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
>                                enum vb2_buffer_state state)
>  {
> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> index f36c9f717798..231af29667e7 100644
> --- a/drivers/media/platform/qcom/venus/helpers.h
> +++ b/drivers/media/platform/qcom/venus/helpers.h
> @@ -19,6 +19,7 @@ void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
>  int venus_helper_vb2_buf_init(struct vb2_buffer *vb);
>  int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb);
>  void venus_helper_vb2_buf_queue(struct vb2_buffer *vb);
> +void venus_helper_process_buf(struct vb2_buffer *vb);
>  void venus_helper_vb2_stop_streaming(struct vb2_queue *q);
>  int venus_helper_vb2_start_streaming(struct venus_inst *inst);
>  void venus_helper_m2m_device_run(void *priv);
> --
> 2.17.1
>

Reviewed-by: Fritz Koenig <frkoenig@chromium.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 3/8] venus: hfi_cmds: Allow null buffer address on encoder input
  2020-11-11 14:37 ` [PATCH v2 3/8] venus: hfi_cmds: Allow null buffer address on encoder input Stanimir Varbanov
@ 2020-11-29  6:05   ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:05 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Allow null buffer address for encoder input buffers. This will
> be used to send null input buffers to signal end-of-stream.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/hfi_cmds.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/hfi_cmds.c b/drivers/media/platform/qcom/venus/hfi_cmds.c
> index 4f7565834469..2affaa2ed70f 100644
> --- a/drivers/media/platform/qcom/venus/hfi_cmds.c
> +++ b/drivers/media/platform/qcom/venus/hfi_cmds.c
> @@ -278,7 +278,7 @@ int pkt_session_etb_encoder(
>                 struct hfi_session_empty_buffer_uncompressed_plane0_pkt *pkt,
>                 void *cookie, struct hfi_frame_data *in_frame)
>  {
> -       if (!cookie || !in_frame->device_addr)
> +       if (!cookie)
>                 return -EINVAL;
>
>         pkt->shdr.hdr.size = sizeof(*pkt);
> --
> 2.17.1
>
Reviewed-by: Fritz Koenig <frkoenig@chromium.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size
  2020-11-11 14:37 ` [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size Stanimir Varbanov
@ 2020-11-29  6:07   ` Fritz Koenig
  2020-11-30  7:52     ` Stanimir Varbanov
  0 siblings, 1 reply; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:07 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> For resolutions below 720p the size of the compressed buffer must
> be bigger. Correct this by checking the resolution when calculating
> buffer size and multiply by four.

I'm confused because the commit message doesn't appear to line up with
the code.  It says multiply by four here, but the code has by eight.

>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/helpers.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 688e3e3e8362..490c026b58a3 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -986,6 +986,8 @@ u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 width, u32 height)
>
>         if (compressed) {
>                 sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2 / 2;
> +               if (width < 1280 || height < 720)
> +                       sz *= 8;
>                 return ALIGN(sz, SZ_4K);
>         }
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 5/8] venus: pm_helpers: Check instance state when calculate instance frequency
  2020-11-11 14:37 ` [PATCH v2 5/8] venus: pm_helpers: Check instance state when calculate instance frequency Stanimir Varbanov
@ 2020-11-29  6:08   ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:08 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Skip calculating instance frequency if it is not in running state.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/pm_helpers.c | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/drivers/media/platform/qcom/venus/pm_helpers.c b/drivers/media/platform/qcom/venus/pm_helpers.c
> index ca99908ca3d3..cc84dc5e371b 100644
> --- a/drivers/media/platform/qcom/venus/pm_helpers.c
> +++ b/drivers/media/platform/qcom/venus/pm_helpers.c
> @@ -940,6 +940,9 @@ static unsigned long calculate_inst_freq(struct venus_inst *inst,
>
>         mbs_per_sec = load_per_instance(inst);
>
> +       if (inst->state != INST_START)
> +               return 0;
> +
>         vpp_freq = mbs_per_sec * inst->clk_data.codec_freq_data->vpp_freq;
>         /* 21 / 20 is overhead factor */
>         vpp_freq += vpp_freq / 20;
> --
> 2.17.1
>

Reviewed-by: Fritz Koenig <frkoenig@chromium.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 8/8] venus: helpers: Delete unused stop streaming helper
  2020-11-11 14:37 ` [PATCH v2 8/8] venus: helpers: Delete unused stop streaming helper Stanimir Varbanov
@ 2020-11-29  6:09   ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:09 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> After re-design of encoder driver this helper is not needed
> anymore.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/helpers.c | 43 ---------------------
>  drivers/media/platform/qcom/venus/helpers.h |  1 -
>  2 files changed, 44 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
> index 490c026b58a3..51c80417f361 100644
> --- a/drivers/media/platform/qcom/venus/helpers.c
> +++ b/drivers/media/platform/qcom/venus/helpers.c
> @@ -1406,49 +1406,6 @@ void venus_helper_buffers_done(struct venus_inst *inst, unsigned int type,
>  }
>  EXPORT_SYMBOL_GPL(venus_helper_buffers_done);
>
> -void venus_helper_vb2_stop_streaming(struct vb2_queue *q)
> -{
> -       struct venus_inst *inst = vb2_get_drv_priv(q);
> -       struct venus_core *core = inst->core;
> -       int ret;
> -
> -       mutex_lock(&inst->lock);
> -
> -       if (inst->streamon_out & inst->streamon_cap) {
> -               ret = hfi_session_stop(inst);
> -               ret |= hfi_session_unload_res(inst);
> -               ret |= venus_helper_unregister_bufs(inst);
> -               ret |= venus_helper_intbufs_free(inst);
> -               ret |= hfi_session_deinit(inst);
> -
> -               if (inst->session_error || core->sys_error)
> -                       ret = -EIO;
> -
> -               if (ret)
> -                       hfi_session_abort(inst);
> -
> -               venus_helper_free_dpb_bufs(inst);
> -
> -               venus_pm_load_scale(inst);
> -               INIT_LIST_HEAD(&inst->registeredbufs);
> -       }
> -
> -       venus_helper_buffers_done(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
> -                                 VB2_BUF_STATE_ERROR);
> -       venus_helper_buffers_done(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE,
> -                                 VB2_BUF_STATE_ERROR);
> -
> -       if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> -               inst->streamon_out = 0;
> -       else
> -               inst->streamon_cap = 0;
> -
> -       venus_pm_release_core(inst);
> -
> -       mutex_unlock(&inst->lock);
> -}
> -EXPORT_SYMBOL_GPL(venus_helper_vb2_stop_streaming);
> -
>  int venus_helper_process_initial_cap_bufs(struct venus_inst *inst)
>  {
>         struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> diff --git a/drivers/media/platform/qcom/venus/helpers.h b/drivers/media/platform/qcom/venus/helpers.h
> index 231af29667e7..3eae2acbcc8e 100644
> --- a/drivers/media/platform/qcom/venus/helpers.h
> +++ b/drivers/media/platform/qcom/venus/helpers.h
> @@ -20,7 +20,6 @@ int venus_helper_vb2_buf_init(struct vb2_buffer *vb);
>  int venus_helper_vb2_buf_prepare(struct vb2_buffer *vb);
>  void venus_helper_vb2_buf_queue(struct vb2_buffer *vb);
>  void venus_helper_process_buf(struct vb2_buffer *vb);
> -void venus_helper_vb2_stop_streaming(struct vb2_queue *q);
>  int venus_helper_vb2_start_streaming(struct venus_inst *inst);
>  void venus_helper_m2m_device_run(void *priv);
>  void venus_helper_m2m_job_abort(void *priv);
> --
> 2.17.1
>

Reviewed-by: Fritz Koenig <frkoenig@chromium.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 6/8] venus: venc: add handling for VIDIOC_ENCODER_CMD
  2020-11-11 14:37 ` [PATCH v2 6/8] venus: venc: add handling for VIDIOC_ENCODER_CMD Stanimir Varbanov
@ 2020-11-29  6:12   ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29  6:12 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig, Dikshita Agarwal

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> From: Dikshita Agarwal <dikshita@codeaurora.org>
>
> Add handling for below commands in encoder:
> 1. V4L2_ENC_CMD_STOP
> 2. V4L2_ENC_CMD_START
>
> Signed-off-by: Dikshita Agarwal <dikshita@codeaurora.org>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/venc.c | 77 +++++++++++++++++++++++-
>  1 file changed, 76 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 99bfabf90bd2..7512e4a16270 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -507,6 +507,59 @@ static int venc_enum_frameintervals(struct file *file, void *fh,
>         return 0;
>  }
>
> +static int venc_encoder_cmd(struct file *file, void *fh,
> +                           struct v4l2_encoder_cmd *ec)
> +{
> +       struct venus_inst *inst = to_inst(file);
> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> +       struct hfi_frame_data fdata = {0};
> +       int ret = 0;
> +
> +       ret = v4l2_m2m_ioctl_try_encoder_cmd(file, fh, ec);
> +       if (ret < 0)
> +               return ret;
> +
> +       mutex_lock(&inst->lock);
> +
> +       if (!vb2_is_streaming(&m2m_ctx->cap_q_ctx.q) ||
> +           !vb2_is_streaming(&m2m_ctx->out_q_ctx.q))
> +               goto unlock;
> +
> +       if (m2m_ctx->is_draining) {
> +               ret = -EBUSY;
> +               goto unlock;
> +       }
> +
> +       if (ec->cmd == V4L2_ENC_CMD_STOP) {
> +               if (v4l2_m2m_has_stopped(m2m_ctx)) {
> +                       ret = 0;
> +                       goto unlock;
> +               }
> +
> +               m2m_ctx->is_draining = true;
> +
> +               fdata.buffer_type = HFI_BUFFER_INPUT;
> +               fdata.flags |= HFI_BUFFERFLAG_EOS;
> +               fdata.device_addr = 0;
> +               fdata.clnt_data = (u32)-1;
> +
> +               ret = hfi_session_process_buf(inst, &fdata);
> +               if (ret)
> +                       goto unlock;
> +       }
> +
> +       if (ec->cmd == V4L2_ENC_CMD_START && v4l2_m2m_has_stopped(m2m_ctx)) {
> +               vb2_clear_last_buffer_dequeued(&m2m_ctx->cap_q_ctx.q);
> +               inst->m2m_ctx->has_stopped = false;
> +               venus_helper_process_initial_out_bufs(inst);
> +               venus_helper_process_initial_cap_bufs(inst);
> +       }
> +
> +unlock:
> +       mutex_unlock(&inst->lock);
> +       return ret;
> +}
> +
>  static const struct v4l2_ioctl_ops venc_ioctl_ops = {
>         .vidioc_querycap = venc_querycap,
>         .vidioc_enum_fmt_vid_cap = venc_enum_fmt,
> @@ -534,6 +587,8 @@ static const struct v4l2_ioctl_ops venc_ioctl_ops = {
>         .vidioc_enum_frameintervals = venc_enum_frameintervals,
>         .vidioc_subscribe_event = v4l2_ctrl_subscribe_event,
>         .vidioc_unsubscribe_event = v4l2_event_unsubscribe,
> +       .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
> +       .vidioc_encoder_cmd = venc_encoder_cmd,
>  };
>
>  static int venc_set_properties(struct venus_inst *inst)
> @@ -946,9 +1001,22 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
>  static void venc_vb2_buf_queue(struct vb2_buffer *vb)
>  {
>         struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>
>         mutex_lock(&inst->lock);
> -       venus_helper_vb2_buf_queue(vb);
> +
> +       v4l2_m2m_buf_queue(m2m_ctx, vbuf);
> +
> +       if (!(inst->streamon_out && inst->streamon_cap))
> +               goto unlock;
> +
> +       if (v4l2_m2m_has_stopped(m2m_ctx))
> +               goto unlock;
> +
> +       venus_helper_process_buf(vb);
> +
> +unlock:
>         mutex_unlock(&inst->lock);
>  }
>
> @@ -968,6 +1036,7 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
>         struct vb2_v4l2_buffer *vbuf;
>         struct vb2_buffer *vb;
>         unsigned int type;
> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>
>         if (buf_type == HFI_BUFFER_INPUT)
>                 type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE;
> @@ -986,6 +1055,12 @@ static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
>                 vb->planes[0].data_offset = data_offset;
>                 vb->timestamp = timestamp_us * NSEC_PER_USEC;
>                 vbuf->sequence = inst->sequence_cap++;
> +
> +               if ((!bytesused && m2m_ctx->is_draining) ||
> +                   (vbuf->flags & V4L2_BUF_FLAG_LAST)) {
> +                       vbuf->flags |= V4L2_BUF_FLAG_LAST;
> +                       v4l2_m2m_mark_stopped(inst->m2m_ctx);
> +               }
>         } else {
>                 vbuf->sequence = inst->sequence_out++;
>         }
> --
> 2.17.1
>

Reviewed-by: Fritz Koenig <frkoenig@chromium.org>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/8] Venus stateful encoder compliance
  2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
                   ` (7 preceding siblings ...)
  2020-11-11 14:37 ` [PATCH v2 8/8] venus: helpers: Delete unused stop streaming helper Stanimir Varbanov
@ 2020-11-29 19:17 ` Fritz Koenig
  2020-11-30  7:55   ` Stanimir Varbanov
  8 siblings, 1 reply; 22+ messages in thread
From: Fritz Koenig @ 2020-11-29 19:17 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

Since this patchset adds support for V4L2_ENC_CMD_STOP and
VENUS_ENC_STATE_ENCODING it should also add support for
VIDIOC_TRY_ENCODER_CMD so that those commands are discoverable.  I've
made an attempt at that here:
https://www.spinics.net/lists/linux-media/msg182319.html

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Hello,
>
> Here is v2 of the subject patchset.
>
> The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
> 6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
> The last 8/8 just delete not needed helper function.
>
> The major changes are:
>  * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
>  * Use null encoder buffer to signal end-of-stream in 6/8.
>
> Comments are welcome!
>
> regards,
> Stan
>
> Dikshita Agarwal (1):
>   venus: venc: add handling for VIDIOC_ENCODER_CMD
>
> Stanimir Varbanov (7):
>   venus: hfi: Use correct state in unload resources
>   venus: helpers: Add a new helper for buffer processing
>   venus: hfi_cmds: Allow null buffer address on encoder input
>   venus: helpers: Calculate properly compressed buffer size
>   venus: pm_helpers: Check instance state when calculate instance
>     frequency
>   venus: venc: Handle reset encoder state
>   venus: helpers: Delete unused stop streaming helper
>
>  drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
>  drivers/media/platform/qcom/venus/helpers.h   |   2 +-
>  drivers/media/platform/qcom/venus/hfi.c       |   2 +-
>  drivers/media/platform/qcom/venus/hfi.h       |   1 -
>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
>  .../media/platform/qcom/venus/pm_helpers.c    |   3 +
>  drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
>  7 files changed, 226 insertions(+), 81 deletions(-)
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size
  2020-11-29  6:07   ` Fritz Koenig
@ 2020-11-30  7:52     ` Stanimir Varbanov
  0 siblings, 0 replies; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-30  7:52 UTC (permalink / raw)
  To: Fritz Koenig
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot



On 11/29/20 8:07 AM, Fritz Koenig wrote:
> On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>>
>> For resolutions below 720p the size of the compressed buffer must
>> be bigger. Correct this by checking the resolution when calculating
>> buffer size and multiply by four.
> 
> I'm confused because the commit message doesn't appear to line up with
> the code.  It says multiply by four here, but the code has by eight.

Yes, it is confusing. I will correct it in next version.

> 
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  drivers/media/platform/qcom/venus/helpers.c | 2 ++
>>  1 file changed, 2 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
>> index 688e3e3e8362..490c026b58a3 100644
>> --- a/drivers/media/platform/qcom/venus/helpers.c
>> +++ b/drivers/media/platform/qcom/venus/helpers.c
>> @@ -986,6 +986,8 @@ u32 venus_helper_get_framesz(u32 v4l2_fmt, u32 width, u32 height)
>>
>>         if (compressed) {
>>                 sz = ALIGN(height, 32) * ALIGN(width, 32) * 3 / 2 / 2;
>> +               if (width < 1280 || height < 720)
>> +                       sz *= 8;
>>                 return ALIGN(sz, SZ_4K);
>>         }
>>
>> --
>> 2.17.1
>>

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/8] Venus stateful encoder compliance
  2020-11-29 19:17 ` [PATCH v2 0/8] Venus stateful encoder compliance Fritz Koenig
@ 2020-11-30  7:55   ` Stanimir Varbanov
  2020-11-30 17:28     ` Fritz Koenig
  0 siblings, 1 reply; 22+ messages in thread
From: Stanimir Varbanov @ 2020-11-30  7:55 UTC (permalink / raw)
  To: Fritz Koenig
  Cc: linux-media, linux-arm-msm, linux-kernel, Vikash Garodia,
	Alexandre Courbot

Hi Fritz,

On 11/29/20 9:17 PM, Fritz Koenig wrote:
> Since this patchset adds support for V4L2_ENC_CMD_STOP and
> VENUS_ENC_STATE_ENCODING it should also add support for
> VIDIOC_TRY_ENCODER_CMD so that those commands are discoverable.  I've

6/8 is adding it:

+	.vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,

> made an attempt at that here:
> https://www.spinics.net/lists/linux-media/msg182319.html
> 
> On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>>
>> Hello,
>>
>> Here is v2 of the subject patchset.
>>
>> The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
>> 6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
>> The last 8/8 just delete not needed helper function.
>>
>> The major changes are:
>>  * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
>>  * Use null encoder buffer to signal end-of-stream in 6/8.
>>
>> Comments are welcome!
>>
>> regards,
>> Stan
>>
>> Dikshita Agarwal (1):
>>   venus: venc: add handling for VIDIOC_ENCODER_CMD
>>
>> Stanimir Varbanov (7):
>>   venus: hfi: Use correct state in unload resources
>>   venus: helpers: Add a new helper for buffer processing
>>   venus: hfi_cmds: Allow null buffer address on encoder input
>>   venus: helpers: Calculate properly compressed buffer size
>>   venus: pm_helpers: Check instance state when calculate instance
>>     frequency
>>   venus: venc: Handle reset encoder state
>>   venus: helpers: Delete unused stop streaming helper
>>
>>  drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
>>  drivers/media/platform/qcom/venus/helpers.h   |   2 +-
>>  drivers/media/platform/qcom/venus/hfi.c       |   2 +-
>>  drivers/media/platform/qcom/venus/hfi.h       |   1 -
>>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
>>  .../media/platform/qcom/venus/pm_helpers.c    |   3 +
>>  drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
>>  7 files changed, 226 insertions(+), 81 deletions(-)
>>
>> --
>> 2.17.1
>>

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 0/8] Venus stateful encoder compliance
  2020-11-30  7:55   ` Stanimir Varbanov
@ 2020-11-30 17:28     ` Fritz Koenig
  0 siblings, 0 replies; 22+ messages in thread
From: Fritz Koenig @ 2020-11-30 17:28 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Fritz Koenig, linux-media, linux-arm-msm, linux-kernel,
	Vikash Garodia, Alexandre Courbot

On Sun, Nov 29, 2020 at 11:55 PM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Hi Fritz,
>
> On 11/29/20 9:17 PM, Fritz Koenig wrote:
> > Since this patchset adds support for V4L2_ENC_CMD_STOP and
> > VENUS_ENC_STATE_ENCODING it should also add support for
> > VIDIOC_TRY_ENCODER_CMD so that those commands are discoverable.  I've
>
> 6/8 is adding it:
>
> +       .vidioc_try_encoder_cmd = v4l2_m2m_ioctl_try_encoder_cmd,
>

Ahh, thanks.  I need to work on my reading comprehension.

> > made an attempt at that here:
> > https://www.spinics.net/lists/linux-media/msg182319.html
> >
> > On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
> > <stanimir.varbanov@linaro.org> wrote:
> >>
> >> Hello,
> >>
> >> Here is v2 of the subject patchset.
> >>
> >> The patchset starts with few small preparation and fix patches, 1/8 to 5/8.
> >> 6/8 is redesigned Dikshita's patch and 7/8 add Reset encoder state handling.
> >> The last 8/8 just delete not needed helper function.
> >>
> >> The major changes are:
> >>  * An attempt to reuse m2m helpers for drain and reset state in 6/8 and 7/8.
> >>  * Use null encoder buffer to signal end-of-stream in 6/8.
> >>
> >> Comments are welcome!
> >>
> >> regards,
> >> Stan
> >>
> >> Dikshita Agarwal (1):
> >>   venus: venc: add handling for VIDIOC_ENCODER_CMD
> >>
> >> Stanimir Varbanov (7):
> >>   venus: hfi: Use correct state in unload resources
> >>   venus: helpers: Add a new helper for buffer processing
> >>   venus: hfi_cmds: Allow null buffer address on encoder input
> >>   venus: helpers: Calculate properly compressed buffer size
> >>   venus: pm_helpers: Check instance state when calculate instance
> >>     frequency
> >>   venus: venc: Handle reset encoder state
> >>   venus: helpers: Delete unused stop streaming helper
> >>
> >>  drivers/media/platform/qcom/venus/helpers.c   |  65 ++---
> >>  drivers/media/platform/qcom/venus/helpers.h   |   2 +-
> >>  drivers/media/platform/qcom/venus/hfi.c       |   2 +-
> >>  drivers/media/platform/qcom/venus/hfi.h       |   1 -
> >>  drivers/media/platform/qcom/venus/hfi_cmds.c  |   2 +-
> >>  .../media/platform/qcom/venus/pm_helpers.c    |   3 +
> >>  drivers/media/platform/qcom/venus/venc.c      | 232 +++++++++++++++---
> >>  7 files changed, 226 insertions(+), 81 deletions(-)
> >>
> >> --
> >> 2.17.1
> >>
>
> --
> regards,
> Stan

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 7/8] venus: venc: Handle reset encoder state
  2020-11-11 14:37 ` [PATCH v2 7/8] venus: venc: Handle reset encoder state Stanimir Varbanov
@ 2021-01-02  0:13   ` Fritz Koenig
  2021-01-07 10:09     ` Stanimir Varbanov
  0 siblings, 1 reply; 22+ messages in thread
From: Fritz Koenig @ 2021-01-02  0:13 UTC (permalink / raw)
  To: Stanimir Varbanov
  Cc: Linux Media Mailing List, linux-arm-msm, LKML, Vikash Garodia,
	Alexandre Courbot, Fritz Koenig

How should we resolve this patch in relation to the "venus: venc: Init
the session only once in queue_setup" [1] patch?

"venus: venc: Init the session only once in queue_setup" comes after
and reworks |venc_start_streaming| substantially.  This patch
implements |venc_stop_streaming|, but maybe that is not needed with
the newer patch?  Can this one just be dropped, or does it need
rework?

-Fritz

[1]: https://lore.kernel.org/patchwork/patch/1349416/

On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
<stanimir.varbanov@linaro.org> wrote:
>
> Redesign the encoder driver to be compliant with stateful encoder
> spec - specifically adds handling of Reset state.
>
> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
> ---
>  drivers/media/platform/qcom/venus/venc.c | 155 ++++++++++++++++++-----
>  1 file changed, 122 insertions(+), 33 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
> index 7512e4a16270..f1ae89d45a54 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -907,6 +907,54 @@ static int venc_queue_setup(struct vb2_queue *q,
>         return ret;
>  }
>
> +static void venc_release_session(struct venus_inst *inst)
> +{
> +       int ret, abort = 0;
> +
> +       mutex_lock(&inst->lock);
> +
> +       ret = hfi_session_deinit(inst);
> +       abort = (ret && ret != -EINVAL) ? 1 : 0;
> +
> +       if (inst->session_error)
> +               abort = 1;
> +
> +       if (abort)
> +               hfi_session_abort(inst);
> +
> +       venus_pm_load_scale(inst);
> +       INIT_LIST_HEAD(&inst->registeredbufs);
> +       mutex_unlock(&inst->lock);
> +
> +       venus_pm_release_core(inst);
> +}
> +
> +static int venc_buf_init(struct vb2_buffer *vb)
> +{
> +       struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +
> +       inst->buf_count++;
> +
> +       return venus_helper_vb2_buf_init(vb);
> +}
> +
> +static void venc_buf_cleanup(struct vb2_buffer *vb)
> +{
> +       struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> +       struct venus_buffer *buf = to_venus_buffer(vbuf);
> +
> +       mutex_lock(&inst->lock);
> +       if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +               if (!list_empty(&inst->registeredbufs))
> +                       list_del_init(&buf->reg_list);
> +       mutex_unlock(&inst->lock);
> +
> +       inst->buf_count--;
> +       if (!inst->buf_count)
> +               venc_release_session(inst);
> +}
> +
>  static int venc_verify_conf(struct venus_inst *inst)
>  {
>         enum hfi_version ver = inst->core->res->hfi_version;
> @@ -938,49 +986,57 @@ static int venc_verify_conf(struct venus_inst *inst)
>  static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
>  {
>         struct venus_inst *inst = vb2_get_drv_priv(q);
> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>         int ret;
>
>         mutex_lock(&inst->lock);
>
> -       if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +       v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
> +
> +       if (V4L2_TYPE_IS_OUTPUT(q->type))
>                 inst->streamon_out = 1;
>         else
>                 inst->streamon_cap = 1;
>
> -       if (!(inst->streamon_out & inst->streamon_cap)) {
> -               mutex_unlock(&inst->lock);
> -               return 0;
> -       }
> +       if (inst->streamon_out && inst->streamon_cap &&
> +           inst->state == INST_UNINIT) {
> +               venus_helper_init_instance(inst);
>
> -       venus_helper_init_instance(inst);
> +               inst->sequence_cap = 0;
> +               inst->sequence_out = 0;
>
> -       inst->sequence_cap = 0;
> -       inst->sequence_out = 0;
> +               ret = venc_init_session(inst);
> +               if (ret)
> +                       goto bufs_done;
>
> -       ret = venc_init_session(inst);
> -       if (ret)
> -               goto bufs_done;
> +               ret = venus_pm_acquire_core(inst);
> +               if (ret)
> +                       goto deinit_sess;
>
> -       ret = venus_pm_acquire_core(inst);
> -       if (ret)
> -               goto deinit_sess;
> +               ret = venc_verify_conf(inst);
> +               if (ret)
> +                       goto deinit_sess;
>
> -       ret = venc_set_properties(inst);
> -       if (ret)
> -               goto deinit_sess;
> +               ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
> +                                               inst->num_output_bufs, 0);
> +               if (ret)
> +                       goto deinit_sess;
>
> -       ret = venc_verify_conf(inst);
> -       if (ret)
> -               goto deinit_sess;
> +               ret = venus_helper_vb2_start_streaming(inst);
> +               if (ret)
> +                       goto deinit_sess;
>
> -       ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
> -                                       inst->num_output_bufs, 0);
> -       if (ret)
> -               goto deinit_sess;
> +               venus_helper_process_initial_out_bufs(inst);
> +               venus_helper_process_initial_cap_bufs(inst);
> +       } else if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->streamon_cap &&
> +                  inst->streamon_out) {
> +               ret = venus_helper_vb2_start_streaming(inst);
> +               if (ret)
> +                       goto bufs_done;
>
> -       ret = venus_helper_vb2_start_streaming(inst);
> -       if (ret)
> -               goto deinit_sess;
> +               venus_helper_process_initial_out_bufs(inst);
> +               venus_helper_process_initial_cap_bufs(inst);
> +       }
>
>         mutex_unlock(&inst->lock);
>
> @@ -990,15 +1046,43 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
>         hfi_session_deinit(inst);
>  bufs_done:
>         venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED);
> -       if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
> +       if (V4L2_TYPE_IS_OUTPUT(q->type))
>                 inst->streamon_out = 0;
>         else
>                 inst->streamon_cap = 0;
> +
>         mutex_unlock(&inst->lock);
>         return ret;
>  }
>
> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
> +static void venc_stop_streaming(struct vb2_queue *q)
> +{
> +       struct venus_inst *inst = vb2_get_drv_priv(q);
> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
> +       int ret = -EINVAL;
> +
> +       mutex_lock(&inst->lock);
> +
> +       v4l2_m2m_clear_state(m2m_ctx);
> +
> +       if (V4L2_TYPE_IS_CAPTURE(q->type)) {
> +               ret = hfi_session_stop(inst);
> +               ret |= hfi_session_unload_res(inst);
> +               ret |= venus_helper_unregister_bufs(inst);
> +               ret |= venus_helper_intbufs_free(inst);
> +       }
> +
> +       venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> +
> +       if (V4L2_TYPE_IS_OUTPUT(q->type))
> +               inst->streamon_out = 0;
> +       else
> +               inst->streamon_cap = 0;
> +
> +       mutex_unlock(&inst->lock);
> +}
> +
> +static void venc_buf_queue(struct vb2_buffer *vb)
>  {
>         struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
>         struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
> @@ -1022,11 +1106,12 @@ static void venc_vb2_buf_queue(struct vb2_buffer *vb)
>
>  static const struct vb2_ops venc_vb2_ops = {
>         .queue_setup = venc_queue_setup,
> -       .buf_init = venus_helper_vb2_buf_init,
> +       .buf_init = venc_buf_init,
> +       .buf_cleanup = venc_buf_cleanup,
>         .buf_prepare = venus_helper_vb2_buf_prepare,
>         .start_streaming = venc_start_streaming,
> -       .stop_streaming = venus_helper_vb2_stop_streaming,
> -       .buf_queue = venc_vb2_buf_queue,
> +       .stop_streaming = venc_stop_streaming,
> +       .buf_queue = venc_buf_queue,
>  };
>
>  static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
> @@ -1084,8 +1169,12 @@ static const struct hfi_inst_ops venc_hfi_ops = {
>         .event_notify = venc_event_notify,
>  };
>
> +static void venc_m2m_device_run(void *priv)
> +{
> +}
> +
>  static const struct v4l2_m2m_ops venc_m2m_ops = {
> -       .device_run = venus_helper_m2m_device_run,
> +       .device_run = venc_m2m_device_run,
>         .job_abort = venus_helper_m2m_job_abort,
>  };
>
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH v2 7/8] venus: venc: Handle reset encoder state
  2021-01-02  0:13   ` Fritz Koenig
@ 2021-01-07 10:09     ` Stanimir Varbanov
  0 siblings, 0 replies; 22+ messages in thread
From: Stanimir Varbanov @ 2021-01-07 10:09 UTC (permalink / raw)
  To: Fritz Koenig, Stanimir Varbanov
  Cc: Linux Media Mailing List, linux-arm-msm, LKML, Vikash Garodia,
	Alexandre Courbot

Hi Fritz,

On 1/2/21 2:13 AM, Fritz Koenig wrote:
> How should we resolve this patch in relation to the "venus: venc: Init
> the session only once in queue_setup" [1] patch?

I plan to make a pull request including 4/8 and 5/8 patches from this
series which are infact fixes.

After that I will send v2 on top of this pull-request. Unfortunately  I
have to postpone this series because I see issues when execute in a row
my tests for drain and reset encoder states.

> 
> "venus: venc: Init the session only once in queue_setup" comes after
> and reworks |venc_start_streaming| substantially.  This patch
> implements |venc_stop_streaming|, but maybe that is not needed with
> the newer patch?  Can this one just be dropped, or does it need
> rework?
> 
> -Fritz
> 
> [1]: https://lore.kernel.org/patchwork/patch/1349416/
> 
> On Wed, Nov 11, 2020 at 6:38 AM Stanimir Varbanov
> <stanimir.varbanov@linaro.org> wrote:
>>
>> Redesign the encoder driver to be compliant with stateful encoder
>> spec - specifically adds handling of Reset state.
>>
>> Signed-off-by: Stanimir Varbanov <stanimir.varbanov@linaro.org>
>> ---
>>  drivers/media/platform/qcom/venus/venc.c | 155 ++++++++++++++++++-----
>>  1 file changed, 122 insertions(+), 33 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
>> index 7512e4a16270..f1ae89d45a54 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -907,6 +907,54 @@ static int venc_queue_setup(struct vb2_queue *q,
>>         return ret;
>>  }
>>
>> +static void venc_release_session(struct venus_inst *inst)
>> +{
>> +       int ret, abort = 0;
>> +
>> +       mutex_lock(&inst->lock);
>> +
>> +       ret = hfi_session_deinit(inst);
>> +       abort = (ret && ret != -EINVAL) ? 1 : 0;
>> +
>> +       if (inst->session_error)
>> +               abort = 1;
>> +
>> +       if (abort)
>> +               hfi_session_abort(inst);
>> +
>> +       venus_pm_load_scale(inst);
>> +       INIT_LIST_HEAD(&inst->registeredbufs);
>> +       mutex_unlock(&inst->lock);
>> +
>> +       venus_pm_release_core(inst);
>> +}
>> +
>> +static int venc_buf_init(struct vb2_buffer *vb)
>> +{
>> +       struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
>> +
>> +       inst->buf_count++;
>> +
>> +       return venus_helper_vb2_buf_init(vb);
>> +}
>> +
>> +static void venc_buf_cleanup(struct vb2_buffer *vb)
>> +{
>> +       struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
>> +       struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> +       struct venus_buffer *buf = to_venus_buffer(vbuf);
>> +
>> +       mutex_lock(&inst->lock);
>> +       if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>> +               if (!list_empty(&inst->registeredbufs))
>> +                       list_del_init(&buf->reg_list);
>> +       mutex_unlock(&inst->lock);
>> +
>> +       inst->buf_count--;
>> +       if (!inst->buf_count)
>> +               venc_release_session(inst);
>> +}
>> +
>>  static int venc_verify_conf(struct venus_inst *inst)
>>  {
>>         enum hfi_version ver = inst->core->res->hfi_version;
>> @@ -938,49 +986,57 @@ static int venc_verify_conf(struct venus_inst *inst)
>>  static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
>>  {
>>         struct venus_inst *inst = vb2_get_drv_priv(q);
>> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>>         int ret;
>>
>>         mutex_lock(&inst->lock);
>>
>> -       if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>> +       v4l2_m2m_update_start_streaming_state(m2m_ctx, q);
>> +
>> +       if (V4L2_TYPE_IS_OUTPUT(q->type))
>>                 inst->streamon_out = 1;
>>         else
>>                 inst->streamon_cap = 1;
>>
>> -       if (!(inst->streamon_out & inst->streamon_cap)) {
>> -               mutex_unlock(&inst->lock);
>> -               return 0;
>> -       }
>> +       if (inst->streamon_out && inst->streamon_cap &&
>> +           inst->state == INST_UNINIT) {
>> +               venus_helper_init_instance(inst);
>>
>> -       venus_helper_init_instance(inst);
>> +               inst->sequence_cap = 0;
>> +               inst->sequence_out = 0;
>>
>> -       inst->sequence_cap = 0;
>> -       inst->sequence_out = 0;
>> +               ret = venc_init_session(inst);
>> +               if (ret)
>> +                       goto bufs_done;
>>
>> -       ret = venc_init_session(inst);
>> -       if (ret)
>> -               goto bufs_done;
>> +               ret = venus_pm_acquire_core(inst);
>> +               if (ret)
>> +                       goto deinit_sess;
>>
>> -       ret = venus_pm_acquire_core(inst);
>> -       if (ret)
>> -               goto deinit_sess;
>> +               ret = venc_verify_conf(inst);
>> +               if (ret)
>> +                       goto deinit_sess;
>>
>> -       ret = venc_set_properties(inst);
>> -       if (ret)
>> -               goto deinit_sess;
>> +               ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
>> +                                               inst->num_output_bufs, 0);
>> +               if (ret)
>> +                       goto deinit_sess;
>>
>> -       ret = venc_verify_conf(inst);
>> -       if (ret)
>> -               goto deinit_sess;
>> +               ret = venus_helper_vb2_start_streaming(inst);
>> +               if (ret)
>> +                       goto deinit_sess;
>>
>> -       ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
>> -                                       inst->num_output_bufs, 0);
>> -       if (ret)
>> -               goto deinit_sess;
>> +               venus_helper_process_initial_out_bufs(inst);
>> +               venus_helper_process_initial_cap_bufs(inst);
>> +       } else if (V4L2_TYPE_IS_CAPTURE(q->type) && inst->streamon_cap &&
>> +                  inst->streamon_out) {
>> +               ret = venus_helper_vb2_start_streaming(inst);
>> +               if (ret)
>> +                       goto bufs_done;
>>
>> -       ret = venus_helper_vb2_start_streaming(inst);
>> -       if (ret)
>> -               goto deinit_sess;
>> +               venus_helper_process_initial_out_bufs(inst);
>> +               venus_helper_process_initial_cap_bufs(inst);
>> +       }
>>
>>         mutex_unlock(&inst->lock);
>>
>> @@ -990,15 +1046,43 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
>>         hfi_session_deinit(inst);
>>  bufs_done:
>>         venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED);
>> -       if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>> +       if (V4L2_TYPE_IS_OUTPUT(q->type))
>>                 inst->streamon_out = 0;
>>         else
>>                 inst->streamon_cap = 0;
>> +
>>         mutex_unlock(&inst->lock);
>>         return ret;
>>  }
>>
>> -static void venc_vb2_buf_queue(struct vb2_buffer *vb)
>> +static void venc_stop_streaming(struct vb2_queue *q)
>> +{
>> +       struct venus_inst *inst = vb2_get_drv_priv(q);
>> +       struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
>> +       int ret = -EINVAL;
>> +
>> +       mutex_lock(&inst->lock);
>> +
>> +       v4l2_m2m_clear_state(m2m_ctx);
>> +
>> +       if (V4L2_TYPE_IS_CAPTURE(q->type)) {
>> +               ret = hfi_session_stop(inst);
>> +               ret |= hfi_session_unload_res(inst);
>> +               ret |= venus_helper_unregister_bufs(inst);
>> +               ret |= venus_helper_intbufs_free(inst);
>> +       }
>> +
>> +       venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
>> +
>> +       if (V4L2_TYPE_IS_OUTPUT(q->type))
>> +               inst->streamon_out = 0;
>> +       else
>> +               inst->streamon_cap = 0;
>> +
>> +       mutex_unlock(&inst->lock);
>> +}
>> +
>> +static void venc_buf_queue(struct vb2_buffer *vb)
>>  {
>>         struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
>>         struct vb2_v4l2_buffer *vbuf = to_vb2_v4l2_buffer(vb);
>> @@ -1022,11 +1106,12 @@ static void venc_vb2_buf_queue(struct vb2_buffer *vb)
>>
>>  static const struct vb2_ops venc_vb2_ops = {
>>         .queue_setup = venc_queue_setup,
>> -       .buf_init = venus_helper_vb2_buf_init,
>> +       .buf_init = venc_buf_init,
>> +       .buf_cleanup = venc_buf_cleanup,
>>         .buf_prepare = venus_helper_vb2_buf_prepare,
>>         .start_streaming = venc_start_streaming,
>> -       .stop_streaming = venus_helper_vb2_stop_streaming,
>> -       .buf_queue = venc_vb2_buf_queue,
>> +       .stop_streaming = venc_stop_streaming,
>> +       .buf_queue = venc_buf_queue,
>>  };
>>
>>  static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
>> @@ -1084,8 +1169,12 @@ static const struct hfi_inst_ops venc_hfi_ops = {
>>         .event_notify = venc_event_notify,
>>  };
>>
>> +static void venc_m2m_device_run(void *priv)
>> +{
>> +}
>> +
>>  static const struct v4l2_m2m_ops venc_m2m_ops = {
>> -       .device_run = venus_helper_m2m_device_run,
>> +       .device_run = venc_m2m_device_run,
>>         .job_abort = venus_helper_m2m_job_abort,
>>  };
>>
>> --
>> 2.17.1
>>

-- 
regards,
Stan

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-01-07 10:10 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-11 14:37 [PATCH v2 0/8] Venus stateful encoder compliance Stanimir Varbanov
2020-11-11 14:37 ` [PATCH v2 1/8] venus: hfi: Use correct state in unload resources Stanimir Varbanov
2020-11-29  6:02   ` Fritz Koenig
2020-11-11 14:37 ` [PATCH v2 2/8] venus: helpers: Add a new helper for buffer processing Stanimir Varbanov
2020-11-29  6:03   ` Fritz Koenig
2020-11-11 14:37 ` [PATCH v2 3/8] venus: hfi_cmds: Allow null buffer address on encoder input Stanimir Varbanov
2020-11-29  6:05   ` Fritz Koenig
2020-11-11 14:37 ` [PATCH v2 4/8] venus: helpers: Calculate properly compressed buffer size Stanimir Varbanov
2020-11-29  6:07   ` Fritz Koenig
2020-11-30  7:52     ` Stanimir Varbanov
2020-11-11 14:37 ` [PATCH v2 5/8] venus: pm_helpers: Check instance state when calculate instance frequency Stanimir Varbanov
2020-11-29  6:08   ` Fritz Koenig
2020-11-11 14:37 ` [PATCH v2 6/8] venus: venc: add handling for VIDIOC_ENCODER_CMD Stanimir Varbanov
2020-11-29  6:12   ` Fritz Koenig
2020-11-11 14:37 ` [PATCH v2 7/8] venus: venc: Handle reset encoder state Stanimir Varbanov
2021-01-02  0:13   ` Fritz Koenig
2021-01-07 10:09     ` Stanimir Varbanov
2020-11-11 14:37 ` [PATCH v2 8/8] venus: helpers: Delete unused stop streaming helper Stanimir Varbanov
2020-11-29  6:09   ` Fritz Koenig
2020-11-29 19:17 ` [PATCH v2 0/8] Venus stateful encoder compliance Fritz Koenig
2020-11-30  7:55   ` Stanimir Varbanov
2020-11-30 17:28     ` Fritz Koenig

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).