From: Dikshita Agarwal quic_dikshita@quicinc.com
[ Upstream commit b791dcfcba3a0c46fb3e2decab31d2340c5dc313 ]
Firmware attach DATACORRUPT/DROP buffer flags for the frames which needs to be dropped, handle it by setting VB2_BUF_STATE_ERROR for these buffers before calling buf_done.
Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Acked-by: Vikash Garodia quic_vgarodia@quicinc.com Tested-by: Neil Armstrong neil.armstrong@linaro.org # on SM8550-QRD Tested-by: Neil Armstrong neil.armstrong@linaro.org # on SM8550-HDK Tested-by: Neil Armstrong neil.armstrong@linaro.org # on SM8650-QRD Tested-by: Neil Armstrong neil.armstrong@linaro.org # on SM8650-HDK Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com Tested-by: Vikash Garodia quic_vgarodia@quicinc.com # on sa8775p-ride Signed-off-by: Bryan O'Donoghue bod@kernel.org Signed-off-by: Hans Verkuil hverkuil@xs4all.nl Signed-off-by: Sasha Levin sashal@kernel.org ---
LLM Generated explanations, may be completely bogus:
Now let me analyze what this patch actually does more carefully:
**YES**
This commit should be backported to stable kernel trees for the following reasons:
## Bug Fix Analysis:
1. **Clear Bug Fix**: The commit fixes a handling issue where the firmware signals corrupt or dropped frames via `HFI_BUFFERFLAG_DATACORRUPT` and `HFI_BUFFERFLAG_DROP_FRAME` flags, but the driver was not properly handling these error conditions.
2. **Data Corruption Prevention**: Without this fix, corrupt video frames marked by the firmware would be passed to userspace as valid data, potentially causing: - Display of corrupted video frames - Application crashes when processing invalid data - Incorrect timestamp/sequence handling
3. **Minimal and Contained Changes**: The fix is very small and targeted: - Adds two flag definitions (`HFI_BUFFERFLAG_DATACORRUPT`, `HFI_BUFFERFLAG_DROP_FRAME`) - Modifies error handling path to properly set `VB2_BUF_STATE_ERROR` - Clears payload and timestamp for error frames - Returns early to avoid incorrect state updates
4. **No Architectural Changes**: The patch only fixes error handling logic without changing any APIs, data structures, or architectural design.
5. **Low Risk of Regression**: The changes are defensive - they only affect frames already marked as corrupt/dropped by firmware, not the normal video processing path.
6. **Important User Impact**: Video playback/recording with corrupt frames is a visible user-facing issue that affects quality of service.
## Specific Code Analysis:
The key fix in `iris_buffer.c`: ```c if (buf->flags & V4L2_BUF_FLAG_ERROR) { state = VB2_BUF_STATE_ERROR; vb2_set_plane_payload(vb2, 0, 0); // Clear payload vb2->timestamp = 0; // Clear timestamp v4l2_m2m_buf_done(vbuf, state); return 0; // Early return } ```
And in `iris_hfi_gen1_response.c`: ```c if (hfi_flags & HFI_BUFFERFLAG_DATACORRUPT) flags |= V4L2_BUF_FLAG_ERROR;
if (hfi_flags & HFI_BUFFERFLAG_DROP_FRAME) flags |= V4L2_BUF_FLAG_ERROR; ```
This ensures corrupt frames are properly marked as errors and handled appropriately rather than being passed as valid data.
## Note on Driver Maturity: While the iris driver is very new (introduced in February 2025), this makes the fix even more important for stable backporting as it addresses a fundamental error handling issue in a newly deployed driver that users may encounter immediately upon adoption.
drivers/media/platform/qcom/iris/iris_buffer.c | 11 ++++++++--- .../media/platform/qcom/iris/iris_hfi_gen1_defines.h | 2 ++ .../media/platform/qcom/iris/iris_hfi_gen1_response.c | 6 ++++++ 3 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..7dd5730a867a 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -593,10 +593,13 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf)
vb2 = &vbuf->vb2_buf;
- if (buf->flags & V4L2_BUF_FLAG_ERROR) + if (buf->flags & V4L2_BUF_FLAG_ERROR) { state = VB2_BUF_STATE_ERROR; - else - state = VB2_BUF_STATE_DONE; + vb2_set_plane_payload(vb2, 0, 0); + vb2->timestamp = 0; + v4l2_m2m_buf_done(vbuf, state); + return 0; + }
vbuf->flags |= buf->flags;
@@ -616,6 +619,8 @@ int iris_vb2_buffer_done(struct iris_inst *inst, struct iris_buffer *buf) v4l2_m2m_mark_stopped(m2m_ctx); } } + + state = VB2_BUF_STATE_DONE; vb2->timestamp = buf->timestamp; v4l2_m2m_buf_done(vbuf, state);
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h index 9f246816a286..93b5f838c290 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h @@ -117,6 +117,8 @@ #define HFI_FRAME_NOTCODED 0x7f002000 #define HFI_FRAME_YUV 0x7f004000 #define HFI_UNUSED_PICT 0x10000000 +#define HFI_BUFFERFLAG_DATACORRUPT 0x00000008 +#define HFI_BUFFERFLAG_DROP_FRAME 0x20000000
struct hfi_pkt_hdr { u32 size; diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c index b72d503dd740..91d95eed68aa 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c @@ -481,6 +481,12 @@ static void iris_hfi_gen1_session_ftb_done(struct iris_inst *inst, void *packet) buf->attr |= BUF_ATTR_DEQUEUED; buf->attr |= BUF_ATTR_BUFFER_DONE;
+ if (hfi_flags & HFI_BUFFERFLAG_DATACORRUPT) + flags |= V4L2_BUF_FLAG_ERROR; + + if (hfi_flags & HFI_BUFFERFLAG_DROP_FRAME) + flags |= V4L2_BUF_FLAG_ERROR; + buf->flags |= flags;
iris_vb2_buffer_done(inst, buf);