Re: [PATCH 1/3] venus: venc: Init the session only once in queue_setup

2020-12-01 Thread dikshita 

Hi Stan,

On 2020-11-20 05:40, Stanimir Varbanov wrote:

Init the hfi session only once in queue_setup and also cover that
with inst->lock.

Signed-off-by: Stanimir Varbanov 
---
 drivers/media/platform/qcom/venus/venc.c | 98 ++--
 1 file changed, 73 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c
b/drivers/media/platform/qcom/venus/venc.c
index 4ecf78e30b59..3a2e449663d8 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -725,8 +725,10 @@ static int venc_init_session(struct venus_inst 
*inst)

int ret;

ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
-   if (ret)
-   return ret;
+   if (ret == -EINVAL)
+   return 0;
+   else if (ret)
+   goto deinit;

ret = venus_helper_set_input_resolution(inst, inst->width,
inst->height);
@@ -762,17 +764,13 @@ static int venc_out_num_buffers(struct
venus_inst *inst, unsigned int *num)
struct hfi_buffer_requirements bufreq;
int ret;

-   ret = venc_init_session(inst);
+   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
if (ret)
return ret;

-   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
-
*num = bufreq.count_actual;

-   hfi_session_deinit(inst);
-
-   return ret;
+   return 0;
 }

 static int venc_queue_setup(struct vb2_queue *q,
@@ -781,7 +779,7 @@ static int venc_queue_setup(struct vb2_queue *q,
 {
struct venus_inst *inst = vb2_get_drv_priv(q);
unsigned int num, min = 4;
-   int ret = 0;
+   int ret;

if (*num_planes) {
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
@@ -803,6 +801,17 @@ static int venc_queue_setup(struct vb2_queue *q,
return 0;
}

+   ret = mutex_lock_interruptible(>lock);
+   if (ret)
+   return ret;
+
+   ret = venc_init_session(inst);
+
+   mutex_unlock(>lock);
+
+   if (ret)
+   return ret;
+
switch (q->type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
*num_planes = inst->fmt_out->num_planes;
@@ -838,6 +847,54 @@ static int venc_queue_setup(struct vb2_queue *q,
return ret;
 }

+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_release_session(struct venus_inst *inst)
+{
+   int ret, abort = 0;
+
+   mutex_lock(>lock);
+
+   ret = hfi_session_deinit(inst);
+   abort = (ret && ret != -EINVAL) ? 1 : 0;
+
+   if (inst->session_error)
+   abort = 1;
+
+   if (abort)
+   hfi_session_abort(inst);
+
+   mutex_unlock(>lock);
+
+   venus_pm_load_scale(inst);
+   INIT_LIST_HEAD(>registeredbufs);
+   venus_pm_release_core(inst);
+}
+
+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(>lock);
+   if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   if (!list_empty(>registeredbufs))
+   list_del_init(>reg_list);
+   mutex_unlock(>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;
@@ -888,38 +945,28 @@ static int venc_start_streaming(struct vb2_queue
*q, unsigned int count)
inst->sequence_cap = 0;
inst->sequence_out = 0;

-   ret = venc_init_session(inst);
-   if (ret)
-   goto bufs_done;
-
ret = venus_pm_acquire_core(inst);
if (ret)
-   goto deinit_sess;
-
-   ret = venc_set_properties(inst);
-   if (ret)
-   goto deinit_sess;


With this change, if set ctrl for target bitrate is called after queue 
setup and before streaming,
the new bitrate won’t be set to FW. which is not right and can cause 
quality issues.

The same might apply to other encoder parameters as well.
Please fix this in the next version.


+   goto error;

ret = venc_verify_conf(inst);
if (ret)
-   goto deinit_sess;
+   goto error;

ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
inst->num_output_bufs, 0);
if (ret)
-   goto deinit_sess;
+   goto error;

ret = venus_helper_vb2_start_streaming(inst);
if (ret)
-   goto deinit_sess;
+   goto error;

mutex_unlock(>lock);

return

Re: [PATCH 1/3] venus: venc: Init the session only once in queue_setup

2020-11-26 Thread Stanimir Varbanov 



On 11/25/20 5:13 AM, Alexandre Courbot wrote:
> Hi Stan,
> 
> On Fri, Nov 20, 2020 at 9:12 AM Stanimir Varbanov
>  wrote:
>>
>> Init the hfi session only once in queue_setup and also cover that
>> with inst->lock.
>>
>> Signed-off-by: Stanimir Varbanov 
>> ---
>>  drivers/media/platform/qcom/venus/venc.c | 98 ++--
>>  1 file changed, 73 insertions(+), 25 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/venus/venc.c 
>> b/drivers/media/platform/qcom/venus/venc.c
>> index 4ecf78e30b59..3a2e449663d8 100644
>> --- a/drivers/media/platform/qcom/venus/venc.c
>> +++ b/drivers/media/platform/qcom/venus/venc.c
>> @@ -725,8 +725,10 @@ static int venc_init_session(struct venus_inst *inst)
>> int ret;
>>
>> ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
>> -   if (ret)
>> -   return ret;
>> +   if (ret == -EINVAL)
>> +   return 0;
> 
> Why is it safe to ignore EINVAL here?

The confusion comes from hfi_session_init() return values. Presently
hfi_session_init will return EINVAL when the session is already init.
Maybe EINVAL is not fitting well with the expected behavior of the
function. I thought about EALREADY, EBUSY but it doesn't fit well to me too.

> 
>> +   else if (ret)
>> +   goto deinit;
>>
>> ret = venus_helper_set_input_resolution(inst, inst->width,
>> inst->height);
>> @@ -762,17 +764,13 @@ static int venc_out_num_buffers(struct venus_inst 
>> *inst, unsigned int *num)
>> struct hfi_buffer_requirements bufreq;
>> int ret;
>>
>> -   ret = venc_init_session(inst);
>> +   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
>> if (ret)
>> return ret;
>>
>> -   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
>> -
>> *num = bufreq.count_actual;
>>
>> -   hfi_session_deinit(inst);
>> -
>> -   return ret;
>> +   return 0;
>>  }
>>
>>  static int venc_queue_setup(struct vb2_queue *q,
>> @@ -781,7 +779,7 @@ static int venc_queue_setup(struct vb2_queue *q,
>>  {
>> struct venus_inst *inst = vb2_get_drv_priv(q);
>> unsigned int num, min = 4;
>> -   int ret = 0;
>> +   int ret;
>>
>> if (*num_planes) {
>> if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
>> @@ -803,6 +801,17 @@ static int venc_queue_setup(struct vb2_queue *q,
>> return 0;
>> }
>>
>> +   ret = mutex_lock_interruptible(>lock);

I'll keep original mutex_lock here in next version.

>> +   if (ret)
>> +   return ret;
>> +
>> +   ret = venc_init_session(inst);
>> +
>> +   mutex_unlock(>lock);
>> +
>> +   if (ret)
>> +   return ret;
>> +
>> switch (q->type) {
>> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
>> *num_planes = inst->fmt_out->num_planes;
>> @@ -838,6 +847,54 @@ static int venc_queue_setup(struct vb2_queue *q,
>> return ret;
>>  }
>>
>> +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_release_session(struct venus_inst *inst)
>> +{
>> +   int ret, abort = 0;
>> +
>> +   mutex_lock(>lock);
>> +
>> +   ret = hfi_session_deinit(inst);
>> +   abort = (ret && ret != -EINVAL) ? 1 : 0;
> 
> Here as well, I think a comment is warranted to explain why we can
> ignore EINVAL.

OK, will update that.

> 
>> +
>> +   if (inst->session_error)
>> +   abort = 1;
>> +
>> +   if (abort)
>> +   hfi_session_abort(inst);
>> +
>> +   mutex_unlock(>lock);
>> +
>> +   venus_pm_load_scale(inst);
>> +   INIT_LIST_HEAD(>registeredbufs);
>> +   venus_pm_release_core(inst);
>> +}
>> +
>> +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(>lock);
>> +   if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
>> +   if (!list_empty(>registeredbufs))
>> +   list_del_init(>reg_list);
>> +   mutex_unlock(>lock);
>> +
>> +   inst->buf_count--;
>> +   if (!inst->buf_count)
>> +   venc_release_session(inst);
> 
> We are calling venc_init_session() during the queue setup but
> venc_release_session() when the last buffer is cleaned up. For
> symmetry, wouldn't it make sense to call venc_init_session() when the
> first buffer is initialized by venc_buf_init()? Otherwise we can

No, the session must be initialized in queue_setup in order to return
the number and sizes of source/destination buffers.

I raised several times the need of symmetrical operation to

Re: [PATCH 1/3] venus: venc: Init the session only once in queue_setup

2020-11-24 Thread Alexandre Courbot 
Hi Stan,

On Fri, Nov 20, 2020 at 9:12 AM Stanimir Varbanov
 wrote:
>
> Init the hfi session only once in queue_setup and also cover that
> with inst->lock.
>
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/media/platform/qcom/venus/venc.c | 98 ++--
>  1 file changed, 73 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c 
> b/drivers/media/platform/qcom/venus/venc.c
> index 4ecf78e30b59..3a2e449663d8 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -725,8 +725,10 @@ static int venc_init_session(struct venus_inst *inst)
> int ret;
>
> ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
> -   if (ret)
> -   return ret;
> +   if (ret == -EINVAL)
> +   return 0;

Why is it safe to ignore EINVAL here?

> +   else if (ret)
> +   goto deinit;
>
> ret = venus_helper_set_input_resolution(inst, inst->width,
> inst->height);
> @@ -762,17 +764,13 @@ static int venc_out_num_buffers(struct venus_inst 
> *inst, unsigned int *num)
> struct hfi_buffer_requirements bufreq;
> int ret;
>
> -   ret = venc_init_session(inst);
> +   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
> if (ret)
> return ret;
>
> -   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
> -
> *num = bufreq.count_actual;
>
> -   hfi_session_deinit(inst);
> -
> -   return ret;
> +   return 0;
>  }
>
>  static int venc_queue_setup(struct vb2_queue *q,
> @@ -781,7 +779,7 @@ static int venc_queue_setup(struct vb2_queue *q,
>  {
> struct venus_inst *inst = vb2_get_drv_priv(q);
> unsigned int num, min = 4;
> -   int ret = 0;
> +   int ret;
>
> if (*num_planes) {
> if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
> @@ -803,6 +801,17 @@ static int venc_queue_setup(struct vb2_queue *q,
> return 0;
> }
>
> +   ret = mutex_lock_interruptible(>lock);
> +   if (ret)
> +   return ret;
> +
> +   ret = venc_init_session(inst);
> +
> +   mutex_unlock(>lock);
> +
> +   if (ret)
> +   return ret;
> +
> switch (q->type) {
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> *num_planes = inst->fmt_out->num_planes;
> @@ -838,6 +847,54 @@ static int venc_queue_setup(struct vb2_queue *q,
> return ret;
>  }
>
> +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_release_session(struct venus_inst *inst)
> +{
> +   int ret, abort = 0;
> +
> +   mutex_lock(>lock);
> +
> +   ret = hfi_session_deinit(inst);
> +   abort = (ret && ret != -EINVAL) ? 1 : 0;

Here as well, I think a comment is warranted to explain why we can
ignore EINVAL.

> +
> +   if (inst->session_error)
> +   abort = 1;
> +
> +   if (abort)
> +   hfi_session_abort(inst);
> +
> +   mutex_unlock(>lock);
> +
> +   venus_pm_load_scale(inst);
> +   INIT_LIST_HEAD(>registeredbufs);
> +   venus_pm_release_core(inst);
> +}
> +
> +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(>lock);
> +   if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +   if (!list_empty(>registeredbufs))
> +   list_del_init(>reg_list);
> +   mutex_unlock(>lock);
> +
> +   inst->buf_count--;
> +   if (!inst->buf_count)
> +   venc_release_session(inst);

We are calling venc_init_session() during the queue setup but
venc_release_session() when the last buffer is cleaned up. For
symmetry, wouldn't it make sense to call venc_init_session() when the
first buffer is initialized by venc_buf_init()? Otherwise we can
potentially have a scenario where the queue is set up, but no buffer
is ever created, leading to the session never being released.

> +}
> +
>  static int venc_verify_conf(struct venus_inst *inst)
>  {
> enum hfi_version ver = inst->core->res->hfi_version;
> @@ -888,38 +945,28 @@ static int venc_start_streaming(struct vb2_queue *q, 
> unsigned int count)
> inst->sequence_cap = 0;
> inst->sequence_out = 0;
>
> -   ret = venc_init_session(inst);
> -   if (ret)
> -   goto bufs_done;
> -
> ret = venus_pm_acquire_core(inst);
> if (ret)
> -   goto deinit_sess;
> -
> -   ret = venc_set_properties(inst);
> -   if (ret)
> -   goto deinit_sess;
> +

Re: [PATCH 1/3] venus: venc: Init the session only once in queue_setup

2020-11-20 Thread Fritz Koenig 
On Thu, Nov 19, 2020 at 4:12 PM Stanimir Varbanov
 wrote:
>
> Init the hfi session only once in queue_setup and also cover that
> with inst->lock.
>
> Signed-off-by: Stanimir Varbanov 
> ---
>  drivers/media/platform/qcom/venus/venc.c | 98 ++--
>  1 file changed, 73 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/venus/venc.c 
> b/drivers/media/platform/qcom/venus/venc.c
> index 4ecf78e30b59..3a2e449663d8 100644
> --- a/drivers/media/platform/qcom/venus/venc.c
> +++ b/drivers/media/platform/qcom/venus/venc.c
> @@ -725,8 +725,10 @@ static int venc_init_session(struct venus_inst *inst)
> int ret;
>
> ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
> -   if (ret)
> -   return ret;
> +   if (ret == -EINVAL)
> +   return 0;
> +   else if (ret)
> +   goto deinit;
>
> ret = venus_helper_set_input_resolution(inst, inst->width,
> inst->height);
> @@ -762,17 +764,13 @@ static int venc_out_num_buffers(struct venus_inst 
> *inst, unsigned int *num)
> struct hfi_buffer_requirements bufreq;
> int ret;
>
> -   ret = venc_init_session(inst);
> +   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
> if (ret)
> return ret;
>
> -   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
> -
> *num = bufreq.count_actual;
>
> -   hfi_session_deinit(inst);
> -
> -   return ret;
> +   return 0;
>  }
>
>  static int venc_queue_setup(struct vb2_queue *q,
> @@ -781,7 +779,7 @@ static int venc_queue_setup(struct vb2_queue *q,
>  {
> struct venus_inst *inst = vb2_get_drv_priv(q);
> unsigned int num, min = 4;
> -   int ret = 0;
> +   int ret;
>
> if (*num_planes) {
> if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
> @@ -803,6 +801,17 @@ static int venc_queue_setup(struct vb2_queue *q,
> return 0;
> }
>
> +   ret = mutex_lock_interruptible(>lock);
> +   if (ret)
> +   return ret;
> +
> +   ret = venc_init_session(inst);
> +
> +   mutex_unlock(>lock);
> +
> +   if (ret)
> +   return ret;
> +
> switch (q->type) {
> case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
> *num_planes = inst->fmt_out->num_planes;
> @@ -838,6 +847,54 @@ static int venc_queue_setup(struct vb2_queue *q,
> return ret;
>  }
>
> +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_release_session(struct venus_inst *inst)
> +{
> +   int ret, abort = 0;
> +
> +   mutex_lock(>lock);
> +
> +   ret = hfi_session_deinit(inst);
> +   abort = (ret && ret != -EINVAL) ? 1 : 0;
> +
> +   if (inst->session_error)
> +   abort = 1;
> +
> +   if (abort)
> +   hfi_session_abort(inst);
> +
> +   mutex_unlock(>lock);
> +
> +   venus_pm_load_scale(inst);
> +   INIT_LIST_HEAD(>registeredbufs);
> +   venus_pm_release_core(inst);
> +}
> +
> +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(>lock);
> +   if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
> +   if (!list_empty(>registeredbufs))
> +   list_del_init(>reg_list);
> +   mutex_unlock(>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;
> @@ -888,38 +945,28 @@ static int venc_start_streaming(struct vb2_queue *q, 
> unsigned int count)
> inst->sequence_cap = 0;
> inst->sequence_out = 0;
>
> -   ret = venc_init_session(inst);
> -   if (ret)
> -   goto bufs_done;
> -
> ret = venus_pm_acquire_core(inst);
> if (ret)
> -   goto deinit_sess;
> -
> -   ret = venc_set_properties(inst);
> -   if (ret)
> -   goto deinit_sess;
> +   goto error;
>
> ret = venc_verify_conf(inst);
> if (ret)
> -   goto deinit_sess;
> +   goto error;
>
> ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
> inst->num_output_bufs, 0);
> if (ret)
> -   goto deinit_sess;
> +   goto error;
>
> ret = venus_helper_vb2_start_streaming(inst);
> if (ret)
> -   goto deinit_sess;
> +   goto error;
>
>

[PATCH 1/3] venus: venc: Init the session only once in queue_setup

2020-11-19 Thread Stanimir Varbanov 
Init the hfi session only once in queue_setup and also cover that
with inst->lock.

Signed-off-by: Stanimir Varbanov 
---
 drivers/media/platform/qcom/venus/venc.c | 98 ++--
 1 file changed, 73 insertions(+), 25 deletions(-)

diff --git a/drivers/media/platform/qcom/venus/venc.c 
b/drivers/media/platform/qcom/venus/venc.c
index 4ecf78e30b59..3a2e449663d8 100644
--- a/drivers/media/platform/qcom/venus/venc.c
+++ b/drivers/media/platform/qcom/venus/venc.c
@@ -725,8 +725,10 @@ static int venc_init_session(struct venus_inst *inst)
int ret;
 
ret = hfi_session_init(inst, inst->fmt_cap->pixfmt);
-   if (ret)
-   return ret;
+   if (ret == -EINVAL)
+   return 0;
+   else if (ret)
+   goto deinit;
 
ret = venus_helper_set_input_resolution(inst, inst->width,
inst->height);
@@ -762,17 +764,13 @@ static int venc_out_num_buffers(struct venus_inst *inst, 
unsigned int *num)
struct hfi_buffer_requirements bufreq;
int ret;
 
-   ret = venc_init_session(inst);
+   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
if (ret)
return ret;
 
-   ret = venus_helper_get_bufreq(inst, HFI_BUFFER_INPUT, );
-
*num = bufreq.count_actual;
 
-   hfi_session_deinit(inst);
-
-   return ret;
+   return 0;
 }
 
 static int venc_queue_setup(struct vb2_queue *q,
@@ -781,7 +779,7 @@ static int venc_queue_setup(struct vb2_queue *q,
 {
struct venus_inst *inst = vb2_get_drv_priv(q);
unsigned int num, min = 4;
-   int ret = 0;
+   int ret;
 
if (*num_planes) {
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
@@ -803,6 +801,17 @@ static int venc_queue_setup(struct vb2_queue *q,
return 0;
}
 
+   ret = mutex_lock_interruptible(>lock);
+   if (ret)
+   return ret;
+
+   ret = venc_init_session(inst);
+
+   mutex_unlock(>lock);
+
+   if (ret)
+   return ret;
+
switch (q->type) {
case V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE:
*num_planes = inst->fmt_out->num_planes;
@@ -838,6 +847,54 @@ static int venc_queue_setup(struct vb2_queue *q,
return ret;
 }
 
+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_release_session(struct venus_inst *inst)
+{
+   int ret, abort = 0;
+
+   mutex_lock(>lock);
+
+   ret = hfi_session_deinit(inst);
+   abort = (ret && ret != -EINVAL) ? 1 : 0;
+
+   if (inst->session_error)
+   abort = 1;
+
+   if (abort)
+   hfi_session_abort(inst);
+
+   mutex_unlock(>lock);
+
+   venus_pm_load_scale(inst);
+   INIT_LIST_HEAD(>registeredbufs);
+   venus_pm_release_core(inst);
+}
+
+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(>lock);
+   if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE)
+   if (!list_empty(>registeredbufs))
+   list_del_init(>reg_list);
+   mutex_unlock(>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;
@@ -888,38 +945,28 @@ static int venc_start_streaming(struct vb2_queue *q, 
unsigned int count)
inst->sequence_cap = 0;
inst->sequence_out = 0;
 
-   ret = venc_init_session(inst);
-   if (ret)
-   goto bufs_done;
-
ret = venus_pm_acquire_core(inst);
if (ret)
-   goto deinit_sess;
-
-   ret = venc_set_properties(inst);
-   if (ret)
-   goto deinit_sess;
+   goto error;
 
ret = venc_verify_conf(inst);
if (ret)
-   goto deinit_sess;
+   goto error;
 
ret = venus_helper_set_num_bufs(inst, inst->num_input_bufs,
inst->num_output_bufs, 0);
if (ret)
-   goto deinit_sess;
+   goto error;
 
ret = venus_helper_vb2_start_streaming(inst);
if (ret)
-   goto deinit_sess;
+   goto error;
 
mutex_unlock(>lock);
 
return 0;
 
-deinit_sess:
-   hfi_session_deinit(inst);
-bufs_done:
+error:
venus_helper_buffers_done(inst, q->type, VB2_BUF_STATE_QUEUED);
if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
inst->streamon_out = 0;
@@ -940,7 +987,8 @@ static void venc_vb2_buf_queue(struct vb2_buffer *vb)
 
 static