Hi,
Changes since v1:
* In 1/5
- clear next_buf_last flag before session_continue in vdec_start_capture
- on event, flush output buffers unconditiannly
* Added two more patches from Alex (4/5) and Fritz (5/5)
v1 can be found at: https://www.spinics.net/lists/linux-media/msg177801.html
regards,
Stan
Alexandre Courbot (1):
media: venus: preserve DRC state across seeks
Fritz Koenig (1):
venus: vdec: Handle DRC after drain
Stanimir Varbanov (3):
venus: vdec: Fix non reliable setting of LAST flag
venus: vdec: Make decoder return LAST flag for sufficient event
venus: helpers: Lock outside of buffer queue helper
drivers/media/platform/qcom/venus/core.h | 6 +-
drivers/media/platform/qcom/venus/helpers.c | 15 +--
drivers/media/platform/qcom/venus/vdec.c | 110 +++++++++++++-------
drivers/media/platform/qcom/venus/venc.c | 11 +-
4 files changed, 93 insertions(+), 49 deletions(-)
--
2.17.1
This makes the decoder to behaives equally for sufficient and
insufficient events. After this change the LAST buffer flag will be set
when the new resolution (in dynamic-resolution-change state) is smaller
then the old bitstream resolution.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/vdec.c | 41 ++++++++++++++++--------
1 file changed, 27 insertions(+), 14 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index eb94e167e282..4ce23c2fc6eb 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -637,6 +637,7 @@ static int vdec_output_conf(struct venus_inst *inst)
{
struct venus_core *core = inst->core;
struct hfi_enable en = { .enable = 1 };
+ struct hfi_buffer_requirements bufreq;
u32 width = inst->out_width;
u32 height = inst->out_height;
u32 out_fmt, out2_fmt;
@@ -712,6 +713,22 @@ static int vdec_output_conf(struct venus_inst *inst)
}
if (IS_V3(core) || IS_V4(core)) {
+ ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT, &bufreq);
+ if (ret)
+ return ret;
+
+ if (bufreq.size > inst->output_buf_size)
+ return -EINVAL;
+
+ if (inst->dpb_fmt) {
+ ret = venus_helper_get_bufreq(inst, HFI_BUFFER_OUTPUT2, &bufreq);
+ if (ret)
+ return ret;
+
+ if (bufreq.size > inst->output2_buf_size)
+ return -EINVAL;
+ }
+
if (inst->output2_buf_size) {
ret = venus_helper_set_bufsize(inst,
inst->output2_buf_size,
@@ -1346,19 +1363,15 @@ static void vdec_event_change(struct venus_inst *inst,
dev_dbg(dev, VDBGM "event %s sufficient resources (%ux%u)\n",
sufficient ? "" : "not", ev_data->width, ev_data->height);
- if (sufficient) {
- hfi_session_continue(inst);
- } else {
- switch (inst->codec_state) {
- case VENUS_DEC_STATE_INIT:
- inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP;
- break;
- case VENUS_DEC_STATE_DECODING:
- inst->codec_state = VENUS_DEC_STATE_DRC;
- break;
- default:
- break;
- }
+ switch (inst->codec_state) {
+ case VENUS_DEC_STATE_INIT:
+ inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP;
+ break;
+ case VENUS_DEC_STATE_DECODING:
+ inst->codec_state = VENUS_DEC_STATE_DRC;
+ break;
+ default:
+ break;
}
/*
@@ -1367,7 +1380,7 @@ static void vdec_event_change(struct venus_inst *inst,
* itself doesn't mark the last decoder output buffer with HFI EOS flag.
*/
- if (!sufficient && inst->codec_state == VENUS_DEC_STATE_DRC) {
+ if (inst->codec_state == VENUS_DEC_STATE_DRC) {
int ret;
inst->next_buf_last = true;
--
2.17.1
In real use of dynamic-resolution-change it is observed that the
LAST buffer flag (which marks the last decoded buffer with the
resolution before the resolution-change event) is not reliably set.
Fix this by set the LAST buffer flag on next queued capture buffer
after the resolution-change event.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/core.h | 5 +--
drivers/media/platform/qcom/venus/helpers.c | 6 +++
drivers/media/platform/qcom/venus/vdec.c | 45 ++++++++++++---------
3 files changed, 33 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index f03ed427accd..db0e6738281e 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -285,7 +285,6 @@ enum venus_dec_state {
VENUS_DEC_STATE_DRAIN = 5,
VENUS_DEC_STATE_DECODING = 6,
VENUS_DEC_STATE_DRC = 7,
- VENUS_DEC_STATE_DRC_FLUSH_DONE = 8,
};
struct venus_ts_metadata {
@@ -350,7 +349,7 @@ struct venus_ts_metadata {
* @priv: a private for HFI operations callbacks
* @session_type: the type of the session (decoder or encoder)
* @hprop: a union used as a holder by get property
- * @last_buf: last capture buffer for dynamic-resoluton-change
+ * @next_buf_last: a flag to mark next queued capture buffer as last
*/
struct venus_inst {
struct list_head list;
@@ -413,7 +412,7 @@ struct venus_inst {
union hfi_get_property hprop;
unsigned int core_acquired: 1;
unsigned int bit_depth;
- struct vb2_buffer *last_buf;
+ bool next_buf_last;
};
#define IS_V1(core) ((core)->res->hfi_version == HFI_VERSION_1XX)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 50439eb1ffea..5ca3920237c5 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1347,6 +1347,12 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
v4l2_m2m_buf_queue(m2m_ctx, vbuf);
+ /* Skip processing queued capture buffers after LAST flag */
+ if (inst->session_type == VIDC_SESSION_TYPE_DEC &&
+ V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) &&
+ inst->codec_state == VENUS_DEC_STATE_DRC)
+ goto unlock;
+
cache_payload(inst, vb);
if (inst->session_type == VIDC_SESSION_TYPE_ENC &&
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 8488411204c3..eb94e167e282 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -917,10 +917,6 @@ static int vdec_start_capture(struct venus_inst *inst)
return 0;
reconfigure:
- ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true);
- if (ret)
- return ret;
-
ret = vdec_output_conf(inst);
if (ret)
return ret;
@@ -948,6 +944,8 @@ static int vdec_start_capture(struct venus_inst *inst)
venus_pm_load_scale(inst);
+ inst->next_buf_last = false;
+
ret = hfi_session_continue(inst);
if (ret)
goto free_dpb_bufs;
@@ -988,6 +986,7 @@ static int vdec_start_output(struct venus_inst *inst)
venus_helper_init_instance(inst);
inst->sequence_out = 0;
inst->reconfig = false;
+ inst->next_buf_last = false;
ret = vdec_set_properties(inst);
if (ret)
@@ -1081,9 +1080,7 @@ static int vdec_stop_capture(struct venus_inst *inst)
inst->codec_state = VENUS_DEC_STATE_STOPPED;
break;
case VENUS_DEC_STATE_DRC:
- WARN_ON(1);
- fallthrough;
- case VENUS_DEC_STATE_DRC_FLUSH_DONE:
+ ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true);
inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP;
venus_helper_free_dpb_bufs(inst);
break;
@@ -1207,9 +1204,28 @@ static void vdec_buf_cleanup(struct vb2_buffer *vb)
static void vdec_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);
+ static const struct v4l2_event eos = { .type = V4L2_EVENT_EOS };
vdec_pm_get_put(inst);
+ mutex_lock(&inst->lock);
+
+ if (inst->next_buf_last && V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) &&
+ inst->codec_state == VENUS_DEC_STATE_DRC) {
+ vbuf->flags |= V4L2_BUF_FLAG_LAST;
+ vbuf->sequence = inst->sequence_cap++;
+ vbuf->field = V4L2_FIELD_NONE;
+ vb2_set_plane_payload(vb, 0, 0);
+ v4l2_m2m_buf_done(vbuf, VB2_BUF_STATE_DONE);
+ v4l2_event_queue_fh(&inst->fh, &eos);
+ inst->next_buf_last = false;
+ mutex_unlock(&inst->lock);
+ return;
+ }
+
+ mutex_unlock(&inst->lock);
+
venus_helper_vb2_buf_queue(vb);
}
@@ -1253,13 +1269,6 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
vb->timestamp = timestamp_us * NSEC_PER_USEC;
vbuf->sequence = inst->sequence_cap++;
- if (inst->last_buf == vb) {
- inst->last_buf = NULL;
- vbuf->flags |= V4L2_BUF_FLAG_LAST;
- vb2_set_plane_payload(vb, 0, 0);
- vb->timestamp = 0;
- }
-
if (vbuf->flags & V4L2_BUF_FLAG_LAST) {
const struct v4l2_event ev = { .type = V4L2_EVENT_EOS };
@@ -1359,12 +1368,9 @@ static void vdec_event_change(struct venus_inst *inst,
*/
if (!sufficient && inst->codec_state == VENUS_DEC_STATE_DRC) {
- struct vb2_v4l2_buffer *last;
int ret;
- last = v4l2_m2m_last_dst_buf(inst->m2m_ctx);
- if (last)
- inst->last_buf = &last->vb2_buf;
+ inst->next_buf_last = true;
ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, false);
if (ret)
@@ -1413,8 +1419,7 @@ static void vdec_event_notify(struct venus_inst *inst, u32 event,
static void vdec_flush_done(struct venus_inst *inst)
{
- if (inst->codec_state == VENUS_DEC_STATE_DRC)
- inst->codec_state = VENUS_DEC_STATE_DRC_FLUSH_DONE;
+ dev_dbg(inst->core->dev_dec, VDBGH "flush done\n");
}
static const struct hfi_inst_ops vdec_hfi_ops = {
--
2.17.1
From: Alexandre Courbot <[email protected]>
DRC events can happen virtually at anytime, including when we are
starting a seek. Should this happen, we must make sure to return to the
DRC state, otherwise the firmware will expect buffers of the new
resolution whereas userspace will still work with the old one.
Returning to the DRC state upon resume for seeking makes sure that the
client will get the DRC event and will reallocate the buffers to fit the
firmware's expectations.
Signed-off-by: Alexandre Courbot <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
---
No changes since v1.
drivers/media/platform/qcom/venus/vdec.c | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 9f2c7b3e7d4c..d27f4fd0ca01 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -987,7 +987,10 @@ static int vdec_start_output(struct venus_inst *inst)
if (inst->codec_state == VENUS_DEC_STATE_SEEK) {
ret = venus_helper_process_initial_out_bufs(inst);
- inst->codec_state = VENUS_DEC_STATE_DECODING;
+ if (inst->next_buf_last)
+ inst->codec_state = VENUS_DEC_STATE_DRC;
+ else
+ inst->codec_state = VENUS_DEC_STATE_DECODING;
goto done;
}
@@ -1093,8 +1096,10 @@ static int vdec_stop_capture(struct venus_inst *inst)
ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true);
fallthrough;
case VENUS_DEC_STATE_DRAIN:
- vdec_cancel_dst_buffers(inst);
inst->codec_state = VENUS_DEC_STATE_STOPPED;
+ fallthrough;
+ case VENUS_DEC_STATE_SEEK:
+ vdec_cancel_dst_buffers(inst);
break;
case VENUS_DEC_STATE_DRC:
ret = hfi_session_flush(inst, HFI_FLUSH_OUTPUT, true);
@@ -1116,6 +1121,7 @@ static int vdec_stop_output(struct venus_inst *inst)
case VENUS_DEC_STATE_DECODING:
case VENUS_DEC_STATE_DRAIN:
case VENUS_DEC_STATE_STOPPED:
+ case VENUS_DEC_STATE_DRC:
ret = hfi_session_flush(inst, HFI_FLUSH_ALL, true);
inst->codec_state = VENUS_DEC_STATE_SEEK;
break;
@@ -1389,6 +1395,7 @@ static void vdec_event_change(struct venus_inst *inst,
dev_dbg(dev, VDBGH "flush output error %d\n", ret);
}
+ inst->next_buf_last = true;
inst->reconfig = true;
v4l2_event_queue_fh(&inst->fh, &ev);
wake_up(&inst->reconf_wait);
--
2.17.1
After adding more logic in vdec buf_queue vb2 op it is not
practical to have two lock/unlock for one decoder buf_queue.
So move the instance lock in encoder and decoder vb2 buf_queue
operations.
Signed-off-by: Stanimir Varbanov <[email protected]>
---
drivers/media/platform/qcom/venus/helpers.c | 11 +++--------
drivers/media/platform/qcom/venus/vdec.c | 3 +--
drivers/media/platform/qcom/venus/venc.c | 11 ++++++++++-
3 files changed, 14 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/helpers.c b/drivers/media/platform/qcom/venus/helpers.c
index 5ca3920237c5..2b6925b6c274 100644
--- a/drivers/media/platform/qcom/venus/helpers.c
+++ b/drivers/media/platform/qcom/venus/helpers.c
@@ -1343,34 +1343,29 @@ void venus_helper_vb2_buf_queue(struct vb2_buffer *vb)
struct v4l2_m2m_ctx *m2m_ctx = inst->m2m_ctx;
int ret;
- mutex_lock(&inst->lock);
-
v4l2_m2m_buf_queue(m2m_ctx, vbuf);
/* Skip processing queued capture buffers after LAST flag */
if (inst->session_type == VIDC_SESSION_TYPE_DEC &&
V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type) &&
inst->codec_state == VENUS_DEC_STATE_DRC)
- goto unlock;
+ return;
cache_payload(inst, vb);
if (inst->session_type == VIDC_SESSION_TYPE_ENC &&
!(inst->streamon_out && inst->streamon_cap))
- goto unlock;
+ return;
if (vb2_start_streaming_called(vb->vb2_queue)) {
ret = is_buf_refed(inst, vbuf);
if (ret)
- goto unlock;
+ return;
ret = session_process_buf(inst, vbuf);
if (ret)
return_buf_error(inst, vbuf);
}
-
-unlock:
- mutex_unlock(&inst->lock);
}
EXPORT_SYMBOL_GPL(venus_helper_vb2_buf_queue);
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index 4ce23c2fc6eb..9f2c7b3e7d4c 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -1241,9 +1241,8 @@ static void vdec_vb2_buf_queue(struct vb2_buffer *vb)
return;
}
- mutex_unlock(&inst->lock);
-
venus_helper_vb2_buf_queue(vb);
+ mutex_unlock(&inst->lock);
}
static const struct vb2_ops vdec_vb2_ops = {
diff --git a/drivers/media/platform/qcom/venus/venc.c b/drivers/media/platform/qcom/venus/venc.c
index 1c61602c5de1..4ecf78e30b59 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -929,13 +929,22 @@ static int venc_start_streaming(struct vb2_queue *q, unsigned int count)
return ret;
}
+static void venc_vb2_buf_queue(struct vb2_buffer *vb)
+{
+ struct venus_inst *inst = vb2_get_drv_priv(vb->vb2_queue);
+
+ mutex_lock(&inst->lock);
+ venus_helper_vb2_buf_queue(vb);
+ mutex_unlock(&inst->lock);
+}
+
static const struct vb2_ops venc_vb2_ops = {
.queue_setup = venc_queue_setup,
.buf_init = venus_helper_vb2_buf_init,
.buf_prepare = venus_helper_vb2_buf_prepare,
.start_streaming = venc_start_streaming,
.stop_streaming = venus_helper_vb2_stop_streaming,
- .buf_queue = venus_helper_vb2_buf_queue,
+ .buf_queue = venc_vb2_buf_queue,
};
static void venc_buf_done(struct venus_inst *inst, unsigned int buf_type,
--
2.17.1
From: Fritz Koenig <[email protected]>
If the DRC is near the end of the stream the client
may send a V4L2_DEC_CMD_STOP before the DRC occurs.
V4L2_DEC_CMD_STOP puts the driver into the
VENUS_DEC_STATE_DRAIN state. DRC must be aware so
that after the DRC event the state can be restored
correctly.
Signed-off-by: Fritz Koenig <[email protected]>
Signed-off-by: Stanimir Varbanov <[email protected]>
---
Changes since v2 (from Fritz):
- moved state transition from vdec_event_notify to vdec_event_change.
drivers/media/platform/qcom/venus/core.h | 1 +
drivers/media/platform/qcom/venus/vdec.c | 14 ++++++++++++--
2 files changed, 13 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/venus/core.h b/drivers/media/platform/qcom/venus/core.h
index db0e6738281e..765ab7ed881b 100644
--- a/drivers/media/platform/qcom/venus/core.h
+++ b/drivers/media/platform/qcom/venus/core.h
@@ -413,6 +413,7 @@ struct venus_inst {
unsigned int core_acquired: 1;
unsigned int bit_depth;
bool next_buf_last;
+ bool drain_active;
};
#define IS_V1(core) ((core)->res->hfi_version == HFI_VERSION_1XX)
diff --git a/drivers/media/platform/qcom/venus/vdec.c b/drivers/media/platform/qcom/venus/vdec.c
index d27f4fd0ca01..6cc35ffe2d6e 100644
--- a/drivers/media/platform/qcom/venus/vdec.c
+++ b/drivers/media/platform/qcom/venus/vdec.c
@@ -519,8 +519,10 @@ vdec_decoder_cmd(struct file *file, void *fh, struct v4l2_decoder_cmd *cmd)
ret = hfi_session_process_buf(inst, &fdata);
- if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING)
+ if (!ret && inst->codec_state == VENUS_DEC_STATE_DECODING) {
inst->codec_state = VENUS_DEC_STATE_DRAIN;
+ inst->drain_active = true;
+ }
}
unlock:
@@ -969,9 +971,13 @@ static int vdec_start_capture(struct venus_inst *inst)
inst->codec_state = VENUS_DEC_STATE_DECODING;
+ if (inst->drain_active)
+ inst->codec_state = VENUS_DEC_STATE_DRAIN;
+
inst->streamon_cap = 1;
inst->sequence_cap = 0;
inst->reconfig = false;
+ inst->drain_active = false;
return 0;
@@ -1097,6 +1103,7 @@ static int vdec_stop_capture(struct venus_inst *inst)
fallthrough;
case VENUS_DEC_STATE_DRAIN:
inst->codec_state = VENUS_DEC_STATE_STOPPED;
+ inst->drain_active = false;
fallthrough;
case VENUS_DEC_STATE_SEEK:
vdec_cancel_dst_buffers(inst);
@@ -1296,8 +1303,10 @@ static void vdec_buf_done(struct venus_inst *inst, unsigned int buf_type,
v4l2_event_queue_fh(&inst->fh, &ev);
- if (inst->codec_state == VENUS_DEC_STATE_DRAIN)
+ if (inst->codec_state == VENUS_DEC_STATE_DRAIN) {
+ inst->drain_active = false;
inst->codec_state = VENUS_DEC_STATE_STOPPED;
+ }
}
if (!bytesused)
@@ -1373,6 +1382,7 @@ static void vdec_event_change(struct venus_inst *inst,
inst->codec_state = VENUS_DEC_STATE_CAPTURE_SETUP;
break;
case VENUS_DEC_STATE_DECODING:
+ case VENUS_DEC_STATE_DRAIN:
inst->codec_state = VENUS_DEC_STATE_DRC;
break;
default:
--
2.17.1