Re: [PATCH v3 13/23] media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length

[Dikshita Agarwal]

On 5/2/2025 6:19 PM, Bryan O'Donoghue wrote:
> On 01/05/2025 20:13, Dikshita Agarwal wrote:
>> Firmware sends buffers with 0 filled length which needs to be dropped,
>> to achieve the same, add V4L2_BUF_FLAG_ERROR to such buffers.
>> Also make sure:
>> - These 0 length buffers are not returned as result of flush.
>> - Its not a buffer with LAST flag enabled which will also have 0 filled
>>    length.
>>
>> Acked-by: Vikash Garodia <quic_vgarodia@xxxxxxxxxxx>
>> Signed-off-by: Dikshita Agarwal <quic_dikshita@xxxxxxxxxxx>
>> ---
>>   drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> index 4488540d1d41..3bb326843a7b 100644
>> --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c
>> @@ -378,6 +378,12 @@ static int iris_hfi_gen2_handle_output_buffer(struct
>> iris_inst *inst,
>>         buf->flags = iris_hfi_gen2_get_driver_buffer_flags(inst,
>> hfi_buffer->flags);
>>   +    if (!buf->data_size && inst->state == IRIS_INST_STREAMING &&
>> +        !(hfi_buffer->flags & HFI_BUF_FW_FLAG_LAST) &&
>> +        !(inst->sub_state & IRIS_INST_SUB_DRC)) {
>> +        buf->flags |= V4L2_BUF_FLAG_ERROR;
>> +    }
>> +
>>       return 0;
>>   }
>>  
> 
> This is a pretty complex conjunctive clause.
> 
> Is it possible for say 1/3 of these logical criteria to be false ?
> 
> i.e. if you get to:
> 
> 1. buf->data_size && inst->state == IRIS_INST_STREAMING
This makes sure that the 0 length buffers are not returned as response to
flush.
> 2. !(hfi_buffer->flags & HFI_BUF_FW_FLAG_LAST)
This makes sure that we don't mark the buffer with last flag as error since
this will be also be a zero length buffer but a valid one.
> 
> is it possible to get to
> 
> 3. !(inst->sub_state & IRIS_INST_SUB_DRC)
I can remove this condition as first one captures the essence of this one.

Thanks,
Dikshita
> 
> ?
> 
> This also feels like a bugfix ?
> 
> ---
> bod