Hi Hans,
<cut>
>>>
>>>> +void vidc_vb2_stop_streaming(struct vb2_queue *q)
>>>> +{
>>>> + struct venus_inst *inst = vb2_get_drv_priv(q);
>>>> + struct venus_core *core = inst->core;
>>>> + struct device *dev = core->dev;
>>>> + struct vb2_queue *other_queue;
>>>> + struct vidc_buffer *buf, *n;
>>>> + enum vb2_buffer_state state;
>>>> + int ret;
>>>> +
>>>> + if (q->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE)
>>>> + other_queue = &inst->bufq_cap;
>>>> + else
>>>> + other_queue = &inst->bufq_out;
>>>> +
>>>> + if (!vb2_is_streaming(other_queue))
>>>> + return;
>>>
>>> This seems wrong to me: this return means that the buffers of queue q are
>>> never
>>> released. Either drop this 'if' or release both queues when the last queue
>>> stops streaming. I think dropping the 'if' is best.
>>
>> I have done this way because hfi_session_stop must be called only once,
>> and buffers will be released on first streamoff for both queues.
>
> Are you sure the buffers are released for both queues? I may have missed that
> when
> reviewing.
yes, hfi_session_stop will instruct the firmware to stop using provided
buffers and return ownership to the host driver by fill_buf_done and
empty_buf_done callbacks.
>
> I would recommend to call hfi_session_stop when the first stop_streaming is
> called,
> not when it is called for both queues. I say this because stopping streaming
> without
> releasing the buffers is likely to cause problems.
this is what I tried to implement with above
!vb2_is_streaming(other_queue) thing.
>
> Did you turn on CONFIG_VIDEO_ADV_DEBUG? If it is on, and you don't release
> buffers
> then I think you will see warnings in the kernel log.
OK I will enable it to be sure that warnings are missing.
--
regards,
Stan
