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

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

On 1/5/2026 6:20 PM, Bryan O'Donoghue wrote:
> On 05/01/2026 09:51, Dikshita Agarwal wrote:
>>
>>
>> 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?
> 
> Take this example, I believe the "This reverts commit xxx" is added by the
> revert command and its best practice to include it.
> 
> commit afb9917d9b374ecb77d478c2a052e20875c6e232
> Author: Christian Brauner <brauner@kernel.org>
> Date:   Fri Dec 5 13:50:31 2025 +0100
> 
>     Revert "net/socket: convert sock_map_fd() to FD_ADD()"
> 
>     This reverts commit 245f0d1c622b0183ce4f44b3e39aeacf78fae594.
> 
> I can just add it back in with the PR though its NBD.

Ah ok, got it. Thanks!

> 
> ---
> bod