Re: [PATCH v2 4/6] Revert "media: iris: Add sanity check for stop streaming"

[Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>]

On 12/30/2025 3:55 PM, Bryan O'Donoghue wrote:
> On 29/12/2025 06:31, Dikshita Agarwal wrote:
>> Revert the check that skipped stop_streaming when the instance was in
>> IRIS_INST_ERROR, as it caused multiple regressions:
>>
>> 1. Buffers were not returned to vb2 when the instance was already in
>>     error state, triggering warnings in the vb2 core because buffer
>>     completion was skipped.
>>
>> 2. If a session failed early (e.g. unsupported configuration), the
>>     instance transitioned to IRIS_INST_ERROR. When userspace attempted
>>     to stop streaming for cleanup, stop_streaming was skipped due to the
>>     added check, preventing proper teardown and leaving the firmware
>>     in an inconsistent state.
>>
>> Fixes: ad699fa78b59 ("media: iris: Add sanity check for stop streaming")
>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
>> ---
>>   drivers/media/platform/qcom/iris/iris_vb2.c | 8 +++-----
>>   1 file changed, 3 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c
>> b/drivers/media/platform/qcom/iris/iris_vb2.c
>> index
>> db8768d8a8f61c9ceb04e423d0a769d35114e20e..139b821f7952feb33b21a7045aef9e8a4782aa3c 100644
>> --- a/drivers/media/platform/qcom/iris/iris_vb2.c
>> +++ b/drivers/media/platform/qcom/iris/iris_vb2.c
>> @@ -231,8 +231,6 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
>>           return;
>>         mutex_lock(&inst->lock);
>> -    if (inst->state == IRIS_INST_ERROR)
>> -        goto exit;
>>         if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
>>           !V4L2_TYPE_IS_CAPTURE(q->type))
>> @@ -243,10 +241,10 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
>>           goto exit;
>>     exit:
>> -    if (ret) {
>> -        iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
>> +    iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
>> +    if (ret)
>>           iris_inst_change_state(inst, IRIS_INST_ERROR);
>> -    }
>> +
>>       mutex_unlock(&inst->lock);
>>   }
>>  
> 
> This revert looks strange, should be something like:
> 
> commit 9b6b11d31918722b4522b8982141d7b9646c0e48 (HEAD -> next-6.19-camss-v2)
> Author: Bryan O'Donoghue <bod@kernel.org>
> Date:   Tue Dec 30 10:20:01 2025 +0000
> 
>     Revert "media: iris: Add sanity check for stop streaming"
> 
>     This reverts commit ad699fa78b59241c9d71a8cafb51525f3dab04d4.
> 
>     Everything is broken I give up.
> 
>     Signed-off-by: Bryan O'Donoghue <bod@kernel.org>
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c
> b/drivers/media/platform/qcom/iris/iris_vb2.c
> index db8768d8a8f61..139b821f7952f 100644
> --- a/drivers/media/platform/qcom/iris/iris_vb2.c
> +++ b/drivers/media/platform/qcom/iris/iris_vb2.c
> @@ -231,8 +231,6 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
>                 return;
> 
>         mutex_lock(&inst->lock);
> -       if (inst->state == IRIS_INST_ERROR)
> -               goto exit;
> 
>         if (!V4L2_TYPE_IS_OUTPUT(q->type) &&
>             !V4L2_TYPE_IS_CAPTURE(q->type))
> @@ -243,10 +241,10 @@ void iris_vb2_stop_streaming(struct vb2_queue *q)
>                 goto exit;
> 
>  exit:
> -       if (ret) {
> -               iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> +       iris_helper_buffers_done(inst, q->type, VB2_BUF_STATE_ERROR);
> +       if (ret)
>                 iris_inst_change_state(inst, IRIS_INST_ERROR);
> -       }
> +
>         mutex_unlock(&inst->lock);
>  }
> 
> Just `git revert ad699fa78b59241c9d71a8cafb51525f3dab04d4` and add your
> commit log ?!

Yeah I did the same, revert and changed the commit message.

BTW, I don't see any difference in my change and your commit, anything I am
missing here?

Thanks,
Dikshita

> 
> ---
> bod