Hi All,
This patch series adds initial support for the HEVC(H.265) and VP9 codecs in iris decoder. The objective of this work is to extend the decoder's capabilities to handle HEVC and VP9 codec streams, including necessary format handling and buffer management. In addition, the series also includes a set of fixes to address issues identified during testing of these additional codecs.
These patches also address the comments and feedback received from the RFC patches previously sent. I have made the necessary improvements based on the community's suggestions.
Changes in v2: - Added Changes to make sure all buffers are released in session close (bryna) - Added tracking for flush responses to fix a timing issue. - Added a handling to fix timing issue in reconfig - Splitted patch 06/20 in two patches (Bryan) - Added missing fixes tag (bryan) - Updated fluster report (Nicolas) - Link to v1: https://lore.kernel.org/r/20250408-iris-dec-hevc-vp9-v1-0-acd258778bd6@quici...
Changes sinces RFC: - Added additional fixes to address issues identified during further testing. - Moved typo fix to a seperate patch [Neil] - Reordered the patches for better logical flow and clarity [Neil, Dmitry] - Added fixes tag wherever applicable [Neil, Dmitry] - Removed the default case in the switch statement for codecs [Bryan] - Replaced if-else statements with switch-case [Bryan] - Added comments for mbpf [Bryan] - RFC: https://lore.kernel.org/linux-media/20250305104335.3629945-1-quic_dikshita@q...
This patch series depends on [1] & [2] [1] https://lore.kernel.org/linux-media/20250417-topic-sm8x50-iris-v10-v7-0-f020... [2] https://lore.kernel.org/linux-media/20250424-qcs8300_iris-v5-0-f118f505c300@...
These patches are tested on SM8250 and SM8550 with v4l2-ctl and Gstreamer for HEVC and VP9 decoders, at the same time ensured that the existing H264 decoder functionality remains uneffected.
Note: 1 of the fluster compliance test is fixed with firmware [3] [3]: https://lore.kernel.org/linux-firmware/1a511921-446d-cdc4-0203-084c88a5dc1e@...
The result of fluster test on SM8550: 131/147 testcases passed while testing JCT-VC-HEVC_V1 with GStreamer-H.265-V4L2-Gst1.0. The failing test case: - 10 testcases failed due to unsupported 10 bit format. - DBLK_A_MAIN10_VIXS_4 - INITQP_B_Main10_Sony_1 - TSUNEQBD_A_MAIN10_Technicolor_2 - WP_A_MAIN10_Toshiba_3 - WP_MAIN10_B_Toshiba_3 - WPP_A_ericsson_MAIN10_2 - WPP_B_ericsson_MAIN10_2 - WPP_C_ericsson_MAIN10_2 - WPP_E_ericsson_MAIN10_2 - WPP_F_ericsson_MAIN10_2 - 4 testcase failed due to unsupported resolution - PICSIZE_A_Bossen_1 - PICSIZE_B_Bossen_1 - WPP_D_ericsson_MAIN10_2 - WPP_D_ericsson_MAIN_2 - 2 testcase failed due to CRC mismatch - RAP_A_docomo_6 - RAP_B_Bossen_2 - BUG reported: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4392 Analysis - First few frames in this discarded by firmware and are sent to driver with 0 filled length. Driver send such buffers to client with timestamp 0 and payload set to 0 and make buf state to VB2_BUF_STATE_ERROR. Such buffers should be dropped by GST. But instead, the first frame displayed as green frame and when a valid buffer is sent to client later with same 0 timestamp, its dropped, leading to CRC mismatch for first frame.
235/305 testcases passed while testing VP9-TEST-VECTORS with GStreamer-VP9-V4L2-Gst1.0. The failing test case: - 64 testcases failed due to unsupported resolution - vp90-2-02-size-08x08.webm - vp90-2-02-size-08x10.webm - vp90-2-02-size-08x16.webm - vp90-2-02-size-08x18.webm - vp90-2-02-size-08x32.webm - vp90-2-02-size-08x34.webm - vp90-2-02-size-08x64.webm - vp90-2-02-size-08x66.webm - vp90-2-02-size-10x08.webm - vp90-2-02-size-10x10.webm - vp90-2-02-size-10x16.webm - vp90-2-02-size-10x18.webm - vp90-2-02-size-10x32.webm - vp90-2-02-size-10x34.webm - vp90-2-02-size-10x64.webm - vp90-2-02-size-10x66.webm - vp90-2-02-size-16x08.webm - vp90-2-02-size-16x10.webm - vp90-2-02-size-16x16.webm - vp90-2-02-size-16x18.webm - vp90-2-02-size-16x32.webm - vp90-2-02-size-16x34.webm - vp90-2-02-size-16x64.webm - vp90-2-02-size-16x66.webm - vp90-2-02-size-18x08.webm - vp90-2-02-size-18x10.webm - vp90-2-02-size-18x16.webm - vp90-2-02-size-18x18.webm - vp90-2-02-size-18x32.webm - vp90-2-02-size-18x34.webm - vp90-2-02-size-18x64.webm - vp90-2-02-size-18x66.webm - vp90-2-02-size-32x08.webm - vp90-2-02-size-32x10.webm - vp90-2-02-size-32x16.webm - vp90-2-02-size-32x18.webm - vp90-2-02-size-32x32.webm - vp90-2-02-size-32x34.webm - vp90-2-02-size-32x64.webm - vp90-2-02-size-32x66.webm - vp90-2-02-size-34x08.webm - vp90-2-02-size-34x10.webm - vp90-2-02-size-34x16.webm - vp90-2-02-size-34x18.webm - vp90-2-02-size-34x32.webm - vp90-2-02-size-34x34.webm - vp90-2-02-size-34x64.webm - vp90-2-02-size-34x66.webm - vp90-2-02-size-64x08.webm - vp90-2-02-size-64x10.webm - vp90-2-02-size-64x16.webm - vp90-2-02-size-64x18.webm - vp90-2-02-size-64x32.webm - vp90-2-02-size-64x34.webm - vp90-2-02-size-64x64.webm - vp90-2-02-size-64x66.webm - vp90-2-02-size-66x08.webm - vp90-2-02-size-66x10.webm - vp90-2-02-size-66x16.webm - vp90-2-02-size-66x18.webm - vp90-2-02-size-66x32.webm - vp90-2-02-size-66x34.webm - vp90-2-02-size-66x64.webm - vp90-2-02-size-66x66.webm - 2 testcases failed due to unsupported format - vp91-2-04-yuv422.webm - vp91-2-04-yuv444.webm - 1 testcase failed with CRC mismatch - vp90-2-22-svc_1280x720_3.ivf - Bug reported: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4371 - 2 testcase failed due to unsupported resolution after sequence change - vp90-2-21-resize_inter_320x180_5_1-2.webm - vp90-2-21-resize_inter_320x180_7_1-2.webm - 1 testcase failed due to unsupported stream - vp90-2-16-intra-only.webm
The result of fluster test on SM8250: 133/147 testcases passed while testing JCT-VC-HEVC_V1 with GStreamer-H.265-V4L2-Gst1.0. The failing test case: - 10 testcases failed due to unsupported 10 bit format. - DBLK_A_MAIN10_VIXS_4 - INITQP_B_Main10_Sony_1 - TSUNEQBD_A_MAIN10_Technicolor_2 - WP_A_MAIN10_Toshiba_3 - WP_MAIN10_B_Toshiba_3 - WPP_A_ericsson_MAIN10_2 - WPP_B_ericsson_MAIN10_2 - WPP_C_ericsson_MAIN10_2 - WPP_E_ericsson_MAIN10_2 - WPP_F_ericsson_MAIN10_2 - 4 testcase failed due to unsupported resolution - PICSIZE_A_Bossen_1 - PICSIZE_B_Bossen_1 - WPP_D_ericsson_MAIN10_2 - WPP_D_ericsson_MAIN_2
232/305 testcases passed while testing VP9-TEST-VECTORS with GStreamer-VP9-V4L2-Gst1.0. The failing test case: - 64 testcases failed due to unsupported resolution - vp90-2-02-size-08x08.webm - vp90-2-02-size-08x10.webm - vp90-2-02-size-08x16.webm - vp90-2-02-size-08x18.webm - vp90-2-02-size-08x32.webm - vp90-2-02-size-08x34.webm - vp90-2-02-size-08x64.webm - vp90-2-02-size-08x66.webm - vp90-2-02-size-10x08.webm - vp90-2-02-size-10x10.webm - vp90-2-02-size-10x16.webm - vp90-2-02-size-10x18.webm - vp90-2-02-size-10x32.webm - vp90-2-02-size-10x34.webm - vp90-2-02-size-10x64.webm - vp90-2-02-size-10x66.webm - vp90-2-02-size-16x08.webm - vp90-2-02-size-16x10.webm - vp90-2-02-size-16x16.webm - vp90-2-02-size-16x18.webm - vp90-2-02-size-16x32.webm - vp90-2-02-size-16x34.webm - vp90-2-02-size-16x64.webm - vp90-2-02-size-16x66.webm - vp90-2-02-size-18x08.webm - vp90-2-02-size-18x10.webm - vp90-2-02-size-18x16.webm - vp90-2-02-size-18x18.webm - vp90-2-02-size-18x32.webm - vp90-2-02-size-18x34.webm - vp90-2-02-size-18x64.webm - vp90-2-02-size-18x66.webm - vp90-2-02-size-32x08.webm - vp90-2-02-size-32x10.webm - vp90-2-02-size-32x16.webm - vp90-2-02-size-32x18.webm - vp90-2-02-size-32x32.webm - vp90-2-02-size-32x34.webm - vp90-2-02-size-32x64.webm - vp90-2-02-size-32x66.webm - vp90-2-02-size-34x08.webm - vp90-2-02-size-34x10.webm - vp90-2-02-size-34x16.webm - vp90-2-02-size-34x18.webm - vp90-2-02-size-34x32.webm - vp90-2-02-size-34x34.webm - vp90-2-02-size-34x64.webm - vp90-2-02-size-34x66.webm - vp90-2-02-size-64x08.webm - vp90-2-02-size-64x10.webm - vp90-2-02-size-64x16.webm - vp90-2-02-size-64x18.webm - vp90-2-02-size-64x32.webm - vp90-2-02-size-64x34.webm - vp90-2-02-size-64x64.webm - vp90-2-02-size-64x66.webm - vp90-2-02-size-66x08.webm - vp90-2-02-size-66x10.webm - vp90-2-02-size-66x16.webm - vp90-2-02-size-66x18.webm - vp90-2-02-size-66x32.webm - vp90-2-02-size-66x34.webm - vp90-2-02-size-66x64.webm - vp90-2-02-size-66x66.webm - 2 testcases failed due to unsupported format - vp91-2-04-yuv422.webm - vp91-2-04-yuv444.webm - 1 testcase failed with CRC mismatch - vp90-2-22-svc_1280x720_3.ivf - Bug raised: https://gitlab.freedesktop.org/gstreamer/gstreamer/-/issues/4371 - 5 testcase failed due to unsupported resolution after sequence change - vp90-2-21-resize_inter_320x180_5_1-2.webm - vp90-2-21-resize_inter_320x180_7_1-2.webm - vp90-2-21-resize_inter_320x240_5_1-2.webm - vp90-2-21-resize_inter_320x240_7_1-2.webm - vp90-2-18-resize.ivf - 1 testcase failed with CRC mismatch - vp90-2-16-intra-only.webm Analysis: First few frames are marked by firmware as NO_SHOW frame. Driver make buf state to VB2_BUF_STATE_ERROR for such frames. Such buffers should be dropped by GST. But instead, the first frame is being displayed and when a valid buffer is sent to client later with same timestamp, its dropped, leading to CRC mismatch for first frame.
Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- Dikshita Agarwal (23): media: iris: Skip destroying internal buffer if not dequeued media: iris: Update CAPTURE format info based on OUTPUT format media: iris: Add handling for corrupt and drop frames media: iris: Avoid updating frame size to firmware during reconfig media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length media: iris: Drop port check for session property response media: iris: Add handling for no show frames media: iris: Improve last flag handling media: iris: Skip flush on first sequence change media: iris: Prevent HFI queue writes when core is in deinit state media: iris: Remove redundant buffer count check in stream off media: iris: Remove deprecated property setting to firmware media: iris: Fix missing function pointer initialization media: iris: Fix NULL pointer dereference media: iris: Fix typo in depth variable media: iris: Add a comment to explain usage of MBPS media: iris: Track flush responses to prevent premature completion media: iris: Fix buffer preparation failure during resolution change media: iris: Add HEVC and VP9 formats for decoder media: iris: Add platform capabilities for HEVC and VP9 decoders media: iris: Set mandatory properties for HEVC and VP9 decoders. media: iris: Add internal buffer calculation for HEVC and VP9 decoders media: iris: Add codec specific check for VP9 decoder drain handling
drivers/media/platform/qcom/iris/iris_buffer.c | 52 ++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 +- drivers/media/platform/qcom/iris/iris_ctrls.c | 35 +- drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 + .../platform/qcom/iris/iris_hfi_gen1_command.c | 48 ++- .../platform/qcom/iris/iris_hfi_gen1_defines.h | 5 +- .../platform/qcom/iris/iris_hfi_gen1_response.c | 39 +- .../platform/qcom/iris/iris_hfi_gen2_command.c | 143 +++++++- .../platform/qcom/iris/iris_hfi_gen2_defines.h | 5 + .../platform/qcom/iris/iris_hfi_gen2_response.c | 58 ++- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- drivers/media/platform/qcom/iris/iris_instance.h | 8 + .../platform/qcom/iris/iris_platform_common.h | 28 +- .../media/platform/qcom/iris/iris_platform_gen2.c | 198 ++++++++-- .../platform/qcom/iris/iris_platform_qcs8300.h | 126 +++++-- .../platform/qcom/iris/iris_platform_sm8250.c | 15 +- drivers/media/platform/qcom/iris/iris_vb2.c | 18 +- drivers/media/platform/qcom/iris/iris_vdec.c | 117 +++--- drivers/media/platform/qcom/iris/iris_vdec.h | 11 + drivers/media/platform/qcom/iris/iris_vidc.c | 9 +- drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 397 ++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_vpu_buffer.h | 46 ++- 22 files changed, 1154 insertions(+), 210 deletions(-) --- base-commit: 398a1b33f1479af35ca915c5efc9b00d6204f8fa change-id: 20250428-qcom-iris-hevc-vp9-eb31f30c3390 prerequisite-message-id: 20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@linaro.org prerequisite-patch-id: 35f8dae1416977e88c2db7c767800c01822e266e prerequisite-patch-id: 2bba98151ca103aa62a513a0fbd0df7ae64d9868 prerequisite-patch-id: 0e43a6d758b5fa5ab921c6aa3c19859e312b47d0 prerequisite-patch-id: b7b50aa1657be59fd51c3e53d73382a1ee75a08e prerequisite-patch-id: 30960743105a36f20b3ec4a9ff19e7bca04d6add prerequisite-patch-id: b93c37dc7e09d1631b75387dc1ca90e3066dce17 prerequisite-patch-id: afffe7096c8e110a8da08c987983bc4441d39578 prerequisite-message-id: 20250424-qcs8300_iris-v5-0-f118f505c300@quicinc.com prerequisite-patch-id: 2e72fe4d11d264db3d42fa450427d30171303c6f prerequisite-patch-id: 3398937a7fabb45934bb98a530eef73252231132 prerequisite-patch-id: feda620f147ca14a958c92afdc85a1dc507701ac prerequisite-patch-id: 07ba0745c7d72796567e0a57f5c8e5355a8d2046 prerequisite-patch-id: e35b05c527217206ae871aef0d7b0261af0319ea
Best regards,
Firmware might hold the DPB buffers for reference in case of sequence change, so skip destroying buffers for which QUEUED flag is not removed. Also, make sure that all buffers are released during streamoff.
Cc: stable@vger.kernel.org Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++- drivers/media/platform/qcom/iris/iris_vdec.c | 4 +-- drivers/media/platform/qcom/iris/iris_vidc.c | 6 +++-- 4 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..606d76b10be2 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf return 0; }
-int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) { const struct iris_platform_data *platform_data = inst->core->iris_platform_data; struct iris_buffer *buf, *next; @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) for (i = 0; i < len; i++) { buffers = &inst->buffers[internal_buf_type[i]]; list_for_each_entry_safe(buf, next, &buffers->list, list) { + /* + * during stream on, skip destroying internal(DPB) buffer + * if firmware did not return it. + * during close, destroy all buffers irrespectively. + */ + if (!force && buf->attr & BUF_ATTR_QUEUED) + continue; + ret = iris_destroy_internal_buffer(inst, buf); if (ret) return ret; @@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) return 0; }
+void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) +{ + const struct iris_platform_data *platform_data = inst->core->iris_platform_data; + struct iris_buffer *buf, *next; + struct iris_buffers *buffers; + const u32 *internal_buf_type; + u32 internal_buffer_count, i; + u32 count = 0; + + if (V4L2_TYPE_IS_OUTPUT(plane)) { + internal_buf_type = platform_data->dec_ip_int_buf_tbl; + internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size; + } else { + internal_buf_type = platform_data->dec_op_int_buf_tbl; + internal_buffer_count = platform_data->dec_op_int_buf_tbl_size; + } + + for (i = 0; i < internal_buffer_count; i++) { + buffers = &inst->buffers[internal_buf_type[i]]; + list_for_each_entry_safe(buf, next, &buffers->list, list) + count++; + if (count) + dev_err(inst->core->dev, "%d buffer of type %d not released", + count, internal_buf_type[i]); + } +} + int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) { struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..03a32b91cf21 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.h +++ b/drivers/media/platform/qcom/iris/iris_buffer.h @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane); int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 4143acedfc57..2c1a7162d2da 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst)
iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); + ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false); if (ret) return ret;
@@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst)
iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); + ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false); if (ret) return ret;
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ca0f4e310f77..56531a7f0dfe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -233,8 +233,10 @@ int iris_close(struct file *filp) iris_session_close(inst); iris_inst_change_state(inst, IRIS_INST_DEINIT); iris_v4l2_fh_deinit(inst); - iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); - iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); + iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true); + iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true); + iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); + iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); iris_remove_session(inst); mutex_unlock(&inst->lock); mutex_destroy(&inst->ctx_q_lock);
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
Firmware might hold the DPB buffers for reference in case of sequence change, so skip destroying buffers for which QUEUED flag is not removed. Also, make sure that all buffers are released during streamoff.
Cc: stable@vger.kernel.org Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++- drivers/media/platform/qcom/iris/iris_vdec.c | 4 +-- drivers/media/platform/qcom/iris/iris_vidc.c | 6 +++-- 4 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..606d76b10be2 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf return 0; } -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) { const struct iris_platform_data *platform_data = inst->core->iris_platform_data; struct iris_buffer *buf, *next; @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) for (i = 0; i < len; i++) { buffers = &inst->buffers[internal_buf_type[i]]; list_for_each_entry_safe(buf, next, &buffers->list, list) {
/** during stream on, skip destroying internal(DPB) buffer* if firmware did not return it.* during close, destroy all buffers irrespectively.*/if (!force && buf->attr & BUF_ATTR_QUEUED)continue;ret = iris_destroy_internal_buffer(inst, buf); if (ret) return ret;@@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) return 0; } +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane)
name this iris_check_num_queued_internal_buffers..
+{
- const struct iris_platform_data *platform_data = inst->core->iris_platform_data;
- struct iris_buffer *buf, *next;
- struct iris_buffers *buffers;
- const u32 *internal_buf_type;
- u32 internal_buffer_count, i;
- u32 count = 0;
- if (V4L2_TYPE_IS_OUTPUT(plane)) {
internal_buf_type = platform_data->dec_ip_int_buf_tbl;internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size;- } else {
internal_buf_type = platform_data->dec_op_int_buf_tbl;internal_buffer_count = platform_data->dec_op_int_buf_tbl_size;- }
- for (i = 0; i < internal_buffer_count; i++) {
buffers = &inst->buffers[internal_buf_type[i]];list_for_each_entry_safe(buf, next, &buffers->list, list)count++;if (count)dev_err(inst->core->dev, "%d buffer of type %d not released",count, internal_buf_type[i]);- }
+}
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) { struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..03a32b91cf21 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.h +++ b/drivers/media/platform/qcom/iris/iris_buffer.h @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane);
make this static
Regards, Vikash
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 4143acedfc57..2c1a7162d2da 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false); if (ret) return ret;
@@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false); if (ret) return ret;
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ca0f4e310f77..56531a7f0dfe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -233,8 +233,10 @@ int iris_close(struct file *filp) iris_session_close(inst); iris_inst_change_state(inst, IRIS_INST_DEINIT); iris_v4l2_fh_deinit(inst);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); iris_remove_session(inst); mutex_unlock(&inst->lock); mutex_destroy(&inst->ctx_q_lock);
On 4/29/2025 2:54 PM, Vikash Garodia wrote:
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
Firmware might hold the DPB buffers for reference in case of sequence change, so skip destroying buffers for which QUEUED flag is not removed. Also, make sure that all buffers are released during streamoff.
Cc: stable@vger.kernel.org Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++- drivers/media/platform/qcom/iris/iris_vdec.c | 4 +-- drivers/media/platform/qcom/iris/iris_vidc.c | 6 +++-- 4 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..606d76b10be2 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf return 0; } -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) { const struct iris_platform_data *platform_data = inst->core->iris_platform_data; struct iris_buffer *buf, *next; @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) for (i = 0; i < len; i++) { buffers = &inst->buffers[internal_buf_type[i]]; list_for_each_entry_safe(buf, next, &buffers->list, list) {
/** during stream on, skip destroying internal(DPB) buffer* if firmware did not return it.* during close, destroy all buffers irrespectively.*/if (!force && buf->attr & BUF_ATTR_QUEUED)continue;ret = iris_destroy_internal_buffer(inst, buf); if (ret) return ret;@@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) return 0; } +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane)
name this iris_check_num_queued_internal_buffers..
+{
- const struct iris_platform_data *platform_data = inst->core->iris_platform_data;
- struct iris_buffer *buf, *next;
- struct iris_buffers *buffers;
- const u32 *internal_buf_type;
- u32 internal_buffer_count, i;
- u32 count = 0;
- if (V4L2_TYPE_IS_OUTPUT(plane)) {
internal_buf_type = platform_data->dec_ip_int_buf_tbl;internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size;- } else {
internal_buf_type = platform_data->dec_op_int_buf_tbl;internal_buffer_count = platform_data->dec_op_int_buf_tbl_size;- }
- for (i = 0; i < internal_buffer_count; i++) {
buffers = &inst->buffers[internal_buf_type[i]];list_for_each_entry_safe(buf, next, &buffers->list, list)count++;if (count)dev_err(inst->core->dev, "%d buffer of type %d not released",count, internal_buf_type[i]);- }
+}
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) { struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..03a32b91cf21 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.h +++ b/drivers/media/platform/qcom/iris/iris_buffer.h @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane);
make this static
With the above changes, you can mark it Reviewed-by: Vikash Garodia quic_vgarodia@quicinc.com
Regards, Vikash
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 4143acedfc57..2c1a7162d2da 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false); if (ret) return ret;
@@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false); if (ret) return ret;
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ca0f4e310f77..56531a7f0dfe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -233,8 +233,10 @@ int iris_close(struct file *filp) iris_session_close(inst); iris_inst_change_state(inst, IRIS_INST_DEINIT); iris_v4l2_fh_deinit(inst);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); iris_remove_session(inst); mutex_unlock(&inst->lock); mutex_destroy(&inst->ctx_q_lock);
On 4/29/2025 2:54 PM, Vikash Garodia wrote:
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
Firmware might hold the DPB buffers for reference in case of sequence change, so skip destroying buffers for which QUEUED flag is not removed. Also, make sure that all buffers are released during streamoff.
Cc: stable@vger.kernel.org Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++- drivers/media/platform/qcom/iris/iris_vdec.c | 4 +-- drivers/media/platform/qcom/iris/iris_vidc.c | 6 +++-- 4 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..606d76b10be2 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf return 0; } -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) { const struct iris_platform_data *platform_data = inst->core->iris_platform_data; struct iris_buffer *buf, *next; @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) for (i = 0; i < len; i++) { buffers = &inst->buffers[internal_buf_type[i]]; list_for_each_entry_safe(buf, next, &buffers->list, list) {
/** during stream on, skip destroying internal(DPB) buffer* if firmware did not return it.* during close, destroy all buffers irrespectively.*/if (!force && buf->attr & BUF_ATTR_QUEUED)continue;ret = iris_destroy_internal_buffer(inst, buf); if (ret) return ret;@@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) return 0; } +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane)
name this iris_check_num_queued_internal_buffers..
Ack.
Thanks, Dikshita
+{
- const struct iris_platform_data *platform_data = inst->core->iris_platform_data;
- struct iris_buffer *buf, *next;
- struct iris_buffers *buffers;
- const u32 *internal_buf_type;
- u32 internal_buffer_count, i;
- u32 count = 0;
- if (V4L2_TYPE_IS_OUTPUT(plane)) {
internal_buf_type = platform_data->dec_ip_int_buf_tbl;internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size;- } else {
internal_buf_type = platform_data->dec_op_int_buf_tbl;internal_buffer_count = platform_data->dec_op_int_buf_tbl_size;- }
- for (i = 0; i < internal_buffer_count; i++) {
buffers = &inst->buffers[internal_buf_type[i]];list_for_each_entry_safe(buf, next, &buffers->list, list)count++;if (count)dev_err(inst->core->dev, "%d buffer of type %d not released",count, internal_buf_type[i]);- }
+}
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) { struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..03a32b91cf21 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.h +++ b/drivers/media/platform/qcom/iris/iris_buffer.h @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane);
make this static
Regards, Vikash
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 4143acedfc57..2c1a7162d2da 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false); if (ret) return ret;
@@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false); if (ret) return ret;
diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ca0f4e310f77..56531a7f0dfe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -233,8 +233,10 @@ int iris_close(struct file *filp) iris_session_close(inst); iris_inst_change_state(inst, IRIS_INST_DEINIT); iris_v4l2_fh_deinit(inst);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE); iris_remove_session(inst); mutex_unlock(&inst->lock); mutex_destroy(&inst->ctx_q_lock);
On 28/04/2025 10:28, Dikshita Agarwal wrote:
/** during stream on, skip destroying internal(DPB) buffer* if firmware did not return it.* during close, destroy all buffers irrespectively.*/if (!force && buf->attr & BUF_ATTR_QUEUED)continue;
What's the effect of the firmware not having dequeued the buffer though ?
My main concern here is APSS and firmware have a different view of DMA memory.
We release on the APSS side but firmware has not.
Surely failure to release buffers by the time we get to Linux::close() is a failure of the software contract sufficient to require resetting the firmware ?
i.e. we release memory on the APSS side but firmware writes into it anyway ...
?
--- bod
On 4/29/2025 3:13 PM, Bryan O'Donoghue wrote:
On 28/04/2025 10:28, Dikshita Agarwal wrote:
+Â Â Â Â Â Â Â Â Â Â Â /* +Â Â Â Â Â Â Â Â Â Â Â Â * during stream on, skip destroying internal(DPB) buffer +Â Â Â Â Â Â Â Â Â Â Â Â * if firmware did not return it. +Â Â Â Â Â Â Â Â Â Â Â Â * during close, destroy all buffers irrespectively. +Â Â Â Â Â Â Â Â Â Â Â Â */ +Â Â Â Â Â Â Â Â Â Â Â if (!force && buf->attr & BUF_ATTR_QUEUED) +Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â continue;
What's the effect of the firmware not having dequeued the buffer though ?
My main concern here is APSS and firmware have a different view of DMA memory.
We release on the APSS side but firmware has not.
Surely failure to release buffers by the time we get to Linux::close() is a failure of the software contract sufficient to require resetting the firmware ?
i.e. we release memory on the APSS side but firmware writes into it anyway ...
As I mentioned earlier as well, during STOP issued before close, firmware is expected to release all these internal buffers and the check I added now in this patch will also ensures that. Firmware also has a check to make sure all buffers queued are released as part of STOP and then only firmware sends the STOP_DONE response to driver. STOP is a synchronous call and driver doesn't proceed without receiving the response for the same from firmware. So we will never run into this issue where driver release buffers which are still being accessed by firmware. Please note, these are internal buffers which are allocated and managed by driver only.
Thanks, Dikshita
?
bod
Not mine to review, but wanted to highlight some best practices,
comment below...
Le lundi 28 avril 2025 à 14:58 +0530, Dikshita Agarwal a écrit :
Firmware might hold the DPB buffers for reference in case of sequence change, so skip destroying buffers for which QUEUED flag is not removed. Also, make sure that all buffers are released during streamoff.
Cc: stable@vger.kernel.org Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++-  drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++-  drivers/media/platform/qcom/iris/iris_vdec.c  | 4 +--  drivers/media/platform/qcom/iris/iris_vidc.c  | 6 +++--  4 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..606d76b10be2 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf  return 0;  }  -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force)
Its always tempting to just glue a boolean at the end of a parameter list. But this has huge downside in code readability, see below...
{ Â const struct iris_platform_data *platform_data = inst->core->iris_platform_data; Â struct iris_buffer *buf, *next; @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) Â for (i = 0; i < len; i++) { Â buffers = &inst->buffers[internal_buf_type[i]]; Â list_for_each_entry_safe(buf, next, &buffers->list, list) {
/** during stream on, skip destroying internal(DPB) buffer* if firmware did not return it.* during close, destroy all buffers irrespectively.*/if (!force && buf->attr & BUF_ATTR_QUEUED)continue;ret = iris_destroy_internal_buffer(inst, buf); Â if (ret) Â return ret; @@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) Â return 0; Â } Â +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) +{
- const struct iris_platform_data *platform_data = inst->core->iris_platform_data;
- struct iris_buffer *buf, *next;
- struct iris_buffers *buffers;
- const u32 *internal_buf_type;
- u32 internal_buffer_count, i;
- u32 count = 0;
- if (V4L2_TYPE_IS_OUTPUT(plane)) {
internal_buf_type = platform_data->dec_ip_int_buf_tbl;internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size;- } else {
internal_buf_type = platform_data->dec_op_int_buf_tbl;internal_buffer_count = platform_data->dec_op_int_buf_tbl_size;- }
- for (i = 0; i < internal_buffer_count; i++) {
buffers = &inst->buffers[internal_buf_type[i]];list_for_each_entry_safe(buf, next, &buffers->list, list)count++;if (count)dev_err(inst->core->dev, "%d buffer of type %d not released",count, internal_buf_type[i]);- }
+}
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) Â { Â struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..03a32b91cf21 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.h +++ b/drivers/media/platform/qcom/iris/iris_buffer.h @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); Â int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); Â int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 4143acedfc57..2c1a7162d2da 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) Â Â iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false);
if (ret) Â return ret; Â @@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) Â Â iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false);
If I was reviewing some changes (or even debugging) this specific C file, I would not be able to understanding what this "false" means. I would have to spend extra time, opening the declaration, going back and forth, and breaking the flow.
An alternative approach is to keep the boolean parameter in a static function (c local), and then add two function wrappers that have explicit names.
regards, Nicolas
if (ret) Â return ret; Â diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ca0f4e310f77..56531a7f0dfe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -233,8 +233,10 @@ int iris_close(struct file *filp) Â iris_session_close(inst); Â iris_inst_change_state(inst, IRIS_INST_DEINIT); Â iris_v4l2_fh_deinit(inst);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
iris_remove_session(inst); Â mutex_unlock(&inst->lock); Â mutex_destroy(&inst->ctx_q_lock);
On 4/29/2025 6:17 PM, Nicolas Dufresne wrote:
Not mine to review, but wanted to highlight some best practices,
comment below...
Le lundi 28 avril 2025 à 14:58 +0530, Dikshita Agarwal a écrit :
Firmware might hold the DPB buffers for reference in case of sequence change, so skip destroying buffers for which QUEUED flag is not removed. Also, make sure that all buffers are released during streamoff.
Cc: stable@vger.kernel.org Fixes: 73702f45db81 ("media: iris: allocate, initialize and queue internal buffers") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_buffer.c | 37 +++++++++++++++++++++++++-  drivers/media/platform/qcom/iris/iris_buffer.h | 3 ++-  drivers/media/platform/qcom/iris/iris_vdec.c  | 4 +--  drivers/media/platform/qcom/iris/iris_vidc.c  | 6 +++--  4 files changed, 44 insertions(+), 6 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c index e5c5a564fcb8..606d76b10be2 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.c +++ b/drivers/media/platform/qcom/iris/iris_buffer.c @@ -376,7 +376,7 @@ int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buf  return 0;  }  -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force)
Its always tempting to just glue a boolean at the end of a parameter list. But this has huge downside in code readability, see below...
{ Â const struct iris_platform_data *platform_data = inst->core->iris_platform_data; Â struct iris_buffer *buf, *next; @@ -396,6 +396,14 @@ int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane) Â for (i = 0; i < len; i++) { Â buffers = &inst->buffers[internal_buf_type[i]]; Â list_for_each_entry_safe(buf, next, &buffers->list, list) {
/** during stream on, skip destroying internal(DPB) buffer* if firmware did not return it.* during close, destroy all buffers irrespectively.*/if (!force && buf->attr & BUF_ATTR_QUEUED)continue;ret = iris_destroy_internal_buffer(inst, buf); Â if (ret) Â return ret; @@ -446,6 +454,33 @@ static int iris_release_input_internal_buffers(struct iris_inst *inst) Â return 0; Â } Â +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane) +{
- const struct iris_platform_data *platform_data = inst->core->iris_platform_data;
- struct iris_buffer *buf, *next;
- struct iris_buffers *buffers;
- const u32 *internal_buf_type;
- u32 internal_buffer_count, i;
- u32 count = 0;
- if (V4L2_TYPE_IS_OUTPUT(plane)) {
internal_buf_type = platform_data->dec_ip_int_buf_tbl;internal_buffer_count = platform_data->dec_ip_int_buf_tbl_size;- } else {
internal_buf_type = platform_data->dec_op_int_buf_tbl;internal_buffer_count = platform_data->dec_op_int_buf_tbl_size;- }
- for (i = 0; i < internal_buffer_count; i++) {
buffers = &inst->buffers[internal_buf_type[i]];list_for_each_entry_safe(buf, next, &buffers->list, list)count++;if (count)dev_err(inst->core->dev, "%d buffer of type %d not released",count, internal_buf_type[i]);- }
+}
int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst) Â { Â struct iris_buffers *buffers = &inst->buffers[BUF_PERSIST]; diff --git a/drivers/media/platform/qcom/iris/iris_buffer.h b/drivers/media/platform/qcom/iris/iris_buffer.h index c36b6347b077..03a32b91cf21 100644 --- a/drivers/media/platform/qcom/iris/iris_buffer.h +++ b/drivers/media/platform/qcom/iris/iris_buffer.h @@ -106,7 +106,8 @@ void iris_get_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_create_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_queue_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_destroy_internal_buffer(struct iris_inst *inst, struct iris_buffer *buffer); -int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane); +int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force); +void iris_get_num_queued_internal_buffers(struct iris_inst *inst, u32 plane); Â int iris_alloc_and_queue_persist_bufs(struct iris_inst *inst); Â int iris_alloc_and_queue_input_int_bufs(struct iris_inst *inst); Â int iris_queue_buffer(struct iris_inst *inst, struct iris_buffer *buf); diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 4143acedfc57..2c1a7162d2da 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -408,7 +408,7 @@ int iris_vdec_streamon_input(struct iris_inst *inst) Â Â iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, false);
if (ret) Â return ret; Â @@ -496,7 +496,7 @@ int iris_vdec_streamon_output(struct iris_inst *inst) Â Â iris_get_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- ret = iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, false);
If I was reviewing some changes (or even debugging) this specific C file, I would not be able to understanding what this "false" means. I would have to spend extra time, opening the declaration, going back and forth, and breaking the flow.
An alternative approach is to keep the boolean parameter in a static function (c local), and then add two function wrappers that have explicit names.
Sure, I can implement the alternative approach, if recommended.
If I understand correctly, you are suggesting to have one static helper and two wrappers around this. like:
static int iris_destroy_internal_buffers(struct iris_inst *inst, u32 plane, bool force) { ... list_for_each_entry_safe(buf, next, &buffers->list, list) { if (!force && buf->attr & BUF_ATTR_QUEUED) continue; ... } ... }
//called during stream on int iris_destroy_dequeued_internal_buffers(struct iris_inst *inst, u32 plane) { return iris_destroy_internal_buffers(inst, plane, false) }
//called during close int iris_destroy_all_internal_buffers(struct iris_inst *inst, u32 plane) { return iris_destroy_internal_buffers(inst, plane, true) }
Thanks, Dikshita
regards, Nicolas
if (ret) Â return ret; Â diff --git a/drivers/media/platform/qcom/iris/iris_vidc.c b/drivers/media/platform/qcom/iris/iris_vidc.c index ca0f4e310f77..56531a7f0dfe 100644 --- a/drivers/media/platform/qcom/iris/iris_vidc.c +++ b/drivers/media/platform/qcom/iris/iris_vidc.c @@ -233,8 +233,10 @@ int iris_close(struct file *filp) Â iris_session_close(inst); Â iris_inst_change_state(inst, IRIS_INST_DEINIT); Â iris_v4l2_fh_deinit(inst);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE, true);
- iris_destroy_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE, true);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
- iris_get_num_queued_internal_buffers(inst, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
iris_remove_session(inst); Â mutex_unlock(&inst->lock); Â mutex_destroy(&inst->ctx_q_lock);
Update the width, height and buffer size of CAPTURE based on the resolution set to OUTPUT via VIDIOC_S_FMT. This is required to set the updated capture resolution to firmware when S_FMT is called only for OUTPUT.
Cc: stable@vger.kernel.org Fixes: b530b95de22c ("media: iris: implement s_fmt, g_fmt and try_fmt ioctls") Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_vdec.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 2c1a7162d2da..71751365b000 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -171,6 +171,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f) output_fmt->fmt.pix_mp.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; output_fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization;
+ /* Update capture format based on new ip w/h */ + output_fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128); + output_fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32); + inst->buffers[BUF_OUTPUT].size = iris_get_buffer_size(inst, BUF_OUTPUT); + inst->crop.left = 0; inst->crop.top = 0; inst->crop.width = f->fmt.pix_mp.width;
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
Update the width, height and buffer size of CAPTURE based on the resolution set to OUTPUT via VIDIOC_S_FMT. This is required to set the updated capture resolution to firmware when S_FMT is called only for OUTPUT.
Cc: stable@vger.kernel.org Fixes: b530b95de22c ("media: iris: implement s_fmt, g_fmt and try_fmt ioctls") Reviewed-by: Bryan O'Donoghue bryan.odonoghue@linaro.org Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_vdec.c | 5 +++++ 1 file changed, 5 insertions(+)
diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 2c1a7162d2da..71751365b000 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -171,6 +171,11 @@ int iris_vdec_s_fmt(struct iris_inst *inst, struct v4l2_format *f) output_fmt->fmt.pix_mp.ycbcr_enc = f->fmt.pix_mp.ycbcr_enc; output_fmt->fmt.pix_mp.quantization = f->fmt.pix_mp.quantization;
/* Update capture format based on new ip w/h */output_fmt->fmt.pix_mp.width = ALIGN(f->fmt.pix_mp.width, 128);output_fmt->fmt.pix_mp.height = ALIGN(f->fmt.pix_mp.height, 32);inst->buffers[BUF_OUTPUT].size = iris_get_buffer_size(inst, BUF_OUTPUT);- inst->crop.left = 0; inst->crop.top = 0; inst->crop.width = f->fmt.pix_mp.width;
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
During reconfig, the firmware sends the resolution aligned to 8 bytes. If the driver sends the same resolution back to the firmware the resolution will be aligned to 16 bytes not 8.
The alignment mismatch would then subsequently cause the firmware to send another redundant sequence change.
Fix this by not setting the resolution property during reconfig.
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 15 ++++++++------- drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c | 1 + drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 1 + drivers/media/platform/qcom/iris/iris_instance.h | 2 ++ drivers/media/platform/qcom/iris/iris_vdec.c | 1 + 5 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c index 64f887d9a17d..2239708d2d7e 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c @@ -546,14 +546,15 @@ static int iris_hfi_gen1_set_resolution(struct iris_inst *inst) struct hfi_framesize fs; int ret;
- fs.buffer_type = HFI_BUFFER_INPUT; - fs.width = inst->fmt_src->fmt.pix_mp.width; - fs.height = inst->fmt_src->fmt.pix_mp.height; - - ret = hfi_gen1_set_property(inst, ptype, &fs, sizeof(fs)); - if (ret) - return ret; + if (!inst->in_reconfig) { + fs.buffer_type = HFI_BUFFER_INPUT; + fs.width = inst->fmt_src->fmt.pix_mp.width; + fs.height = inst->fmt_src->fmt.pix_mp.height;
+ ret = hfi_gen1_set_property(inst, ptype, &fs, sizeof(fs)); + if (ret) + return ret; + } fs.buffer_type = HFI_BUFFER_OUTPUT2; fs.width = inst->fmt_dst->fmt.pix_mp.width; fs.height = inst->fmt_dst->fmt.pix_mp.height; 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 91d95eed68aa..6576496fdbdf 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c @@ -155,6 +155,7 @@ static void iris_hfi_gen1_read_changed_params(struct iris_inst *inst, inst->crop.height = event.height; }
+ inst->in_reconfig = true; inst->fw_min_count = event.buf_count; inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst, BUF_OUTPUT); inst->buffers[BUF_OUTPUT].size = pixmp_op->plane_fmt[0].sizeimage; 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 b75a01641d5d..0e889d07e997 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -508,6 +508,7 @@ static void iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst) struct vb2_queue *dst_q; struct v4l2_ctrl *ctrl;
+ inst->in_reconfig = true; subsc_params = inst_hfi_gen2->src_subcr_params; width = (subsc_params.bitstream_resolution & HFI_BITMASK_BITSTREAM_WIDTH) >> 16; diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h index caa3c6507006..a893751766ca 100644 --- a/drivers/media/platform/qcom/iris/iris_instance.h +++ b/drivers/media/platform/qcom/iris/iris_instance.h @@ -42,6 +42,7 @@ * @sequence_out: a sequence counter for output queue * @tss: timestamp metadata * @metadata_idx: index for metadata buffer + * @in_reconfig: a flag raised by decoder when the stream resolution changes */
struct iris_inst { @@ -72,6 +73,7 @@ struct iris_inst { u32 sequence_out; struct iris_ts_metadata tss[VIDEO_MAX_FRAME]; u32 metadata_idx; + bool in_reconfig; };
#endif diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 71751365b000..46abdc47420f 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -487,6 +487,7 @@ static int iris_vdec_process_streamon_output(struct iris_inst *inst) if (ret) return ret;
+ inst->in_reconfig = false; return iris_inst_change_sub_state(inst, clear_sub_state, 0); }
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
During reconfig, the firmware sends the resolution aligned to 8 bytes. If the driver sends the same resolution back to the firmware the resolution will be aligned to 16 bytes not 8.
The alignment mismatch would then subsequently cause the firmware to send another redundant sequence change.
change event.
Fix this by not setting the resolution property during reconfig.
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 15 ++++++++------- drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c | 1 + drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 1 + drivers/media/platform/qcom/iris/iris_instance.h | 2 ++ drivers/media/platform/qcom/iris/iris_vdec.c | 1 + 5 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c index 64f887d9a17d..2239708d2d7e 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c @@ -546,14 +546,15 @@ static int iris_hfi_gen1_set_resolution(struct iris_inst *inst) struct hfi_framesize fs; int ret;
- fs.buffer_type = HFI_BUFFER_INPUT;
- fs.width = inst->fmt_src->fmt.pix_mp.width;
- fs.height = inst->fmt_src->fmt.pix_mp.height;
- ret = hfi_gen1_set_property(inst, ptype, &fs, sizeof(fs));
- if (ret)
return ret;
- if (!inst->in_reconfig) {
fs.buffer_type = HFI_BUFFER_INPUT;fs.width = inst->fmt_src->fmt.pix_mp.width;fs.height = inst->fmt_src->fmt.pix_mp.height;
ret = hfi_gen1_set_property(inst, ptype, &fs, sizeof(fs));if (ret)return ret;- } fs.buffer_type = HFI_BUFFER_OUTPUT2; fs.width = inst->fmt_dst->fmt.pix_mp.width; fs.height = inst->fmt_dst->fmt.pix_mp.height;
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 91d95eed68aa..6576496fdbdf 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c @@ -155,6 +155,7 @@ static void iris_hfi_gen1_read_changed_params(struct iris_inst *inst, inst->crop.height = event.height; }
- inst->in_reconfig = true; inst->fw_min_count = event.buf_count; inst->buffers[BUF_OUTPUT].min_count = iris_vpu_buf_count(inst, BUF_OUTPUT); inst->buffers[BUF_OUTPUT].size = pixmp_op->plane_fmt[0].sizeimage;
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 b75a01641d5d..0e889d07e997 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -508,6 +508,7 @@ static void iris_hfi_gen2_read_input_subcr_params(struct iris_inst *inst) struct vb2_queue *dst_q; struct v4l2_ctrl *ctrl;
- inst->in_reconfig = true; subsc_params = inst_hfi_gen2->src_subcr_params; width = (subsc_params.bitstream_resolution & HFI_BITMASK_BITSTREAM_WIDTH) >> 16;
diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h index caa3c6507006..a893751766ca 100644 --- a/drivers/media/platform/qcom/iris/iris_instance.h +++ b/drivers/media/platform/qcom/iris/iris_instance.h @@ -42,6 +42,7 @@
- @sequence_out: a sequence counter for output queue
- @tss: timestamp metadata
- @metadata_idx: index for metadata buffer
*/
- @in_reconfig: a flag raised by decoder when the stream resolution changes
use the DRC sub-state instead of in_reconfig flag ?
Regards, Vikash
struct iris_inst { @@ -72,6 +73,7 @@ struct iris_inst { u32 sequence_out; struct iris_ts_metadata tss[VIDEO_MAX_FRAME]; u32 metadata_idx;
- bool in_reconfig;
}; #endif diff --git a/drivers/media/platform/qcom/iris/iris_vdec.c b/drivers/media/platform/qcom/iris/iris_vdec.c index 71751365b000..46abdc47420f 100644 --- a/drivers/media/platform/qcom/iris/iris_vdec.c +++ b/drivers/media/platform/qcom/iris/iris_vdec.c @@ -487,6 +487,7 @@ static int iris_vdec_process_streamon_output(struct iris_inst *inst) if (ret) return ret;
- inst->in_reconfig = false; return iris_inst_change_sub_state(inst, clear_sub_state, 0);
}
Currently, port check enforces that session property response must arrive only on the BITSTREAM port. However, firmware can send some responses on other port as well.
Remove the strict port validation to correctly handle session property responses from the firmware.
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 --- 1 file changed, 3 deletions(-)
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 0eb7549da606..5bb20ec0d67f 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -643,9 +643,6 @@ static int iris_hfi_gen2_handle_session_property(struct iris_inst *inst, { struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
- if (pkt->port != HFI_PORT_BITSTREAM) - return 0; - if (pkt->flags & HFI_FW_FLAGS_INFORMATION) return 0;
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
Currently, port check enforces that session property response must arrive only on the BITSTREAM port. However, firmware can send some responses on other port as well.
Remove the strict port validation to correctly handle session property responses from the firmware.
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 --- 1 file changed, 3 deletions(-)
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 0eb7549da606..5bb20ec0d67f 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -643,9 +643,6 @@ static int iris_hfi_gen2_handle_session_property(struct iris_inst *inst, { struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst);
- if (pkt->port != HFI_PORT_BITSTREAM)
return 0;- if (pkt->flags & HFI_FW_FLAGS_INFORMATION) return 0;
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
The current check only considers the core error state before allowing writes to the HFI queues. However, the core can also transition to the deinit state due to a system error triggered by the response thread. In such cases, writing to the HFI queues should not be allowed.
Fix this by adding a check for the core deinit state, ensuring that writes are rejected when core is not in a valid state.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_queue.c b/drivers/media/platform/qcom/iris/iris_hfi_queue.c index fac7df0c4d1a..221dcd09e1e1 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_queue.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_queue.c @@ -113,7 +113,7 @@ int iris_hfi_queue_cmd_write_locked(struct iris_core *core, void *pkt, u32 pkt_s { struct iris_iface_q_info *q_info = &core->command_queue;
- if (core->state == IRIS_CORE_ERROR) + if (core->state == IRIS_CORE_ERROR || core->state == IRIS_CORE_DEINIT) return -EINVAL;
if (!iris_hfi_queue_write(q_info, pkt, pkt_size)) {
On 4/28/2025 2:58 PM, Dikshita Agarwal wrote:
The current check only considers the core error state before allowing writes to the HFI queues. However, the core can also transition to the deinit state due to a system error triggered by the response thread. In such cases, writing to the HFI queues should not be allowed.
Fix this by adding a check for the core deinit state, ensuring that writes are rejected when core is not in a valid state.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_queue.c b/drivers/media/platform/qcom/iris/iris_hfi_queue.c index fac7df0c4d1a..221dcd09e1e1 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_queue.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_queue.c @@ -113,7 +113,7 @@ int iris_hfi_queue_cmd_write_locked(struct iris_core *core, void *pkt, u32 pkt_s { struct iris_iface_q_info *q_info = &core->command_queue;
- if (core->state == IRIS_CORE_ERROR)
- if (core->state == IRIS_CORE_ERROR || core->state == IRIS_CORE_DEINIT) return -EINVAL;
if (!iris_hfi_queue_write(q_info, pkt, pkt_size)) {
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER is deprecated and no longer supported on current firmware, remove setting the same to firmware.
At the same time, remove the check for non-zero number of v4l2 controls as some SOC might not expose any capability which requires v4l2 control.
Cc: stable@vger.kernel.org Fixes: 79865252acb6 ("media: iris: enable video driver probe of SM8250 SoC") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_ctrls.c | 6 ------ drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 8 -------- drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h | 1 - drivers/media/platform/qcom/iris/iris_platform_common.h | 2 +- drivers/media/platform/qcom/iris/iris_platform_sm8250.c | 9 --------- 5 files changed, 1 insertion(+), 25 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c index b690578256d5..915de101fcba 100644 --- a/drivers/media/platform/qcom/iris/iris_ctrls.c +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c @@ -17,8 +17,6 @@ static inline bool iris_valid_cap_id(enum platform_inst_fw_cap_type cap_id) static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id) { switch (id) { - case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER: - return DEBLOCK; case V4L2_CID_MPEG_VIDEO_H264_PROFILE: return PROFILE; case V4L2_CID_MPEG_VIDEO_H264_LEVEL: @@ -34,8 +32,6 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id) return 0;
switch (cap_id) { - case DEBLOCK: - return V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER; case PROFILE: return V4L2_CID_MPEG_VIDEO_H264_PROFILE; case LEVEL: @@ -84,8 +80,6 @@ int iris_ctrls_init(struct iris_inst *inst) if (iris_get_v4l2_id(cap[idx].cap_id)) num_ctrls++; } - if (!num_ctrls) - return -EINVAL;
/* Adding 1 to num_ctrls to include V4L2_CID_MIN_BUFFERS_FOR_CAPTURE */
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c index 2239708d2d7e..f9f3e2d2ce29 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c @@ -490,14 +490,6 @@ iris_hfi_gen1_packet_session_set_property(struct hfi_session_set_property_pkt *p packet->shdr.hdr.size += sizeof(u32) + sizeof(*wm); break; } - case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: { - struct hfi_enable *en = prop_data; - u32 *in = pdata; - - en->enable = *in; - packet->shdr.hdr.size += sizeof(u32) + sizeof(*en); - break; - } default: return -EINVAL; } 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 93b5f838c290..adffcead58ea 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h @@ -65,7 +65,6 @@
#define HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS 0x202001
-#define HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER 0x1200001 #define HFI_PROPERTY_PARAM_VDEC_DPB_COUNTS 0x120300e #define HFI_PROPERTY_CONFIG_VDEC_ENTROPY 0x1204004
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h index ac76d9e1ef9c..1dab276431c7 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_common.h +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h @@ -89,7 +89,7 @@ enum platform_inst_fw_cap_type { CODED_FRAMES, BIT_DEPTH, RAP_FRAME, - DEBLOCK, + TIER, INST_FW_CAP_MAX, };
diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c index 5c86fd7b7b6f..543fa2661539 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c @@ -30,15 +30,6 @@ static struct platform_inst_fw_cap inst_fw_cap_sm8250[] = { .hfi_id = HFI_PROPERTY_PARAM_WORK_MODE, .set = iris_set_stage, }, - { - .cap_id = DEBLOCK, - .min = 0, - .max = 1, - .step_or_mask = 1, - .value = 0, - .hfi_id = HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER, - .set = iris_set_u32, - }, };
static struct platform_inst_caps platform_inst_cap_sm8250 = {
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote:
HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER is deprecated and no longer supported on current firmware, remove setting the same to firmware.
At the same time, remove the check for non-zero number of v4l2 controls as some SOC might not expose any capability which requires v4l2 control.
Cc: stable@vger.kernel.org Fixes: 79865252acb6 ("media: iris: enable video driver probe of SM8250 SoC") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_ctrls.c | 6 ------ drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c | 8 -------- drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h | 1 - drivers/media/platform/qcom/iris/iris_platform_common.h | 2 +- drivers/media/platform/qcom/iris/iris_platform_sm8250.c | 9 --------- 5 files changed, 1 insertion(+), 25 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c index b690578256d5..915de101fcba 100644 --- a/drivers/media/platform/qcom/iris/iris_ctrls.c +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c @@ -17,8 +17,6 @@ static inline bool iris_valid_cap_id(enum platform_inst_fw_cap_type cap_id) static enum platform_inst_fw_cap_type iris_get_cap_id(u32 id) { switch (id) {
- case V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER:
return DEBLOCK;@@ -34,8 +32,6 @@ static u32 iris_get_v4l2_id(enum platform_inst_fw_cap_type cap_id) return 0; switch (cap_id) {
- case DEBLOCK:
return V4L2_CID_MPEG_VIDEO_DECODER_MPEG4_DEBLOCK_FILTER;@@ -84,8 +80,6 @@ int iris_ctrls_init(struct iris_inst *inst) if (iris_get_v4l2_id(cap[idx].cap_id)) num_ctrls++; }
- if (!num_ctrls)
return -EINVAL;/* Adding 1 to num_ctrls to include V4L2_CID_MIN_BUFFERS_FOR_CAPTURE */ diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c index 2239708d2d7e..f9f3e2d2ce29 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c @@ -490,14 +490,6 @@ iris_hfi_gen1_packet_session_set_property(struct hfi_session_set_property_pkt *p packet->shdr.hdr.size += sizeof(u32) + sizeof(*wm); break; }
- case HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER: {
struct hfi_enable *en = prop_data;u32 *in = pdata;en->enable = *in;packet->shdr.hdr.size += sizeof(u32) + sizeof(*en);break;- } default: return -EINVAL; }
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 93b5f838c290..adffcead58ea 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_defines.h @@ -65,7 +65,6 @@ #define HFI_PROPERTY_CONFIG_BUFFER_REQUIREMENTS 0x202001 -#define HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER 0x1200001 #define HFI_PROPERTY_PARAM_VDEC_DPB_COUNTS 0x120300e #define HFI_PROPERTY_CONFIG_VDEC_ENTROPY 0x1204004 diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h index ac76d9e1ef9c..1dab276431c7 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_common.h +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h @@ -89,7 +89,7 @@ enum platform_inst_fw_cap_type { CODED_FRAMES, BIT_DEPTH, RAP_FRAME,
- DEBLOCK,
- TIER, INST_FW_CAP_MAX,
}; diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c index 5c86fd7b7b6f..543fa2661539 100644 --- a/drivers/media/platform/qcom/iris/iris_platform_sm8250.c +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8250.c @@ -30,15 +30,6 @@ static struct platform_inst_fw_cap inst_fw_cap_sm8250[] = { .hfi_id = HFI_PROPERTY_PARAM_WORK_MODE, .set = iris_set_stage, },
- {
.cap_id = DEBLOCK,.min = 0,.max = 1,.step_or_mask = 1,.value = 0,.hfi_id = HFI_PROPERTY_CONFIG_VDEC_POST_LOOP_DEBLOCKER,.set = iris_set_u32,- },
}; static struct platform_inst_caps platform_inst_cap_sm8250 = {
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
The function pointers responsible for setting firmware properties were never initialized in the instance capability structure, causing it to remain NULL. As a result, the firmware properties were not being set correctly.
Fix this by properly assigning the function pointers from the core capability to the instance capability, ensuring that the properties are correctly applied to the firmware.
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_ctrls.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c index 915de101fcba..13f5cf0d0e8a 100644 --- a/drivers/media/platform/qcom/iris/iris_ctrls.c +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c @@ -157,6 +157,7 @@ void iris_session_init_caps(struct iris_core *core) core->inst_fw_caps[cap_id].value = caps[i].value; core->inst_fw_caps[cap_id].flags = caps[i].flags; core->inst_fw_caps[cap_id].hfi_id = caps[i].hfi_id; + core->inst_fw_caps[cap_id].set = caps[i].set; } }
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote:
The function pointers responsible for setting firmware properties were never initialized in the instance capability structure, causing it to remain NULL. As a result, the firmware properties were not being set correctly.
Fix this by properly assigning the function pointers from the core capability to the instance capability, ensuring that the properties are correctly applied to the firmware.
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_ctrls.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/drivers/media/platform/qcom/iris/iris_ctrls.c b/drivers/media/platform/qcom/iris/iris_ctrls.c index 915de101fcba..13f5cf0d0e8a 100644 --- a/drivers/media/platform/qcom/iris/iris_ctrls.c +++ b/drivers/media/platform/qcom/iris/iris_ctrls.c @@ -157,6 +157,7 @@ void iris_session_init_caps(struct iris_core *core) core->inst_fw_caps[cap_id].value = caps[i].value; core->inst_fw_caps[cap_id].flags = caps[i].flags; core->inst_fw_caps[cap_id].hfi_id = caps[i].hfi_id;
core->inst_fw_caps[cap_id].set = caps[i].set;}
Would certainly make encoder fail later without this.
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
A warning reported by smatch indicated a possible null pointer dereference where one of the arguments to API "iris_hfi_gen2_handle_system_error" could sometimes be null.
To fix this, add a check to validate that the argument passed is not null before accessing its members.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/linux-media/634cc9b8-f099-4b54-8556-d879fb2b5169@sta... Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 1ed798d31a3f..cba71b5db943 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -267,7 +267,8 @@ static int iris_hfi_gen2_handle_system_error(struct iris_core *core, { struct iris_inst *instance;
- dev_err(core->dev, "received system error of type %#x\n", pkt->type); + if (pkt) + dev_err(core->dev, "received system error of type %#x\n", pkt->type);
core->state = IRIS_CORE_ERROR;
On Mon, Apr 28, 2025 at 02:59:02PM +0530, Dikshita Agarwal wrote:
A warning reported by smatch indicated a possible null pointer dereference where one of the arguments to API "iris_hfi_gen2_handle_system_error" could sometimes be null.
To fix this, add a check to validate that the argument passed is not null before accessing its members.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/linux-media/634cc9b8-f099-4b54-8556-d879fb2b5169@sta... Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 1ed798d31a3f..cba71b5db943 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -267,7 +267,8 @@ static int iris_hfi_gen2_handle_system_error(struct iris_core *core, { struct iris_inst *instance;
- dev_err(core->dev, "received system error of type %#x\n", pkt->type);
- if (pkt)
dev_err(core->dev, "received system error of type %#x\n", pkt->type);
I feel like it would be better to do:
dev_err(core->dev, "received system error of type %#x\n", pkt ? pkt->type: -1);
regards, dan carpenter
On 4/28/2025 3:10 PM, Dan Carpenter wrote:
On Mon, Apr 28, 2025 at 02:59:02PM +0530, Dikshita Agarwal wrote:
A warning reported by smatch indicated a possible null pointer dereference where one of the arguments to API "iris_hfi_gen2_handle_system_error" could sometimes be null.
To fix this, add a check to validate that the argument passed is not null before accessing its members.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/linux-media/634cc9b8-f099-4b54-8556-d879fb2b5169@sta... Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 1ed798d31a3f..cba71b5db943 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -267,7 +267,8 @@ static int iris_hfi_gen2_handle_system_error(struct iris_core *core, { struct iris_inst *instance;
- dev_err(core->dev, "received system error of type %#x\n", pkt->type);
- if (pkt)
dev_err(core->dev, "received system error of type %#x\n", pkt->type);I feel like it would be better to do:
dev_err(core->dev, "received system error of type %#x\n", pkt ? pkt->type: -1);
we don't need to print anything if pkt is NULL.
Thanks, Dikshita
regards, dan carpenter
On Mon, Apr 28, 2025 at 05:40:01PM +0530, Dikshita Agarwal wrote:
On 4/28/2025 3:10 PM, Dan Carpenter wrote:
On Mon, Apr 28, 2025 at 02:59:02PM +0530, Dikshita Agarwal wrote:
A warning reported by smatch indicated a possible null pointer dereference where one of the arguments to API "iris_hfi_gen2_handle_system_error" could sometimes be null.
To fix this, add a check to validate that the argument passed is not null before accessing its members.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/linux-media/634cc9b8-f099-4b54-8556-d879fb2b5169@sta... Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 1ed798d31a3f..cba71b5db943 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -267,7 +267,8 @@ static int iris_hfi_gen2_handle_system_error(struct iris_core *core, { struct iris_inst *instance;
- dev_err(core->dev, "received system error of type %#x\n", pkt->type);
- if (pkt)
dev_err(core->dev, "received system error of type %#x\n", pkt->type);I feel like it would be better to do:
dev_err(core->dev, "received system error of type %#x\n", pkt ? pkt->type: -1);
we don't need to print anything if pkt is NULL.
Okay, fine by me then.
regards, dan carpenter
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote:
A warning reported by smatch indicated a possible null pointer dereference where one of the arguments to API "iris_hfi_gen2_handle_system_error" could sometimes be null.
To fix this, add a check to validate that the argument passed is not null before accessing its members.
Cc: stable@vger.kernel.org Fixes: fb583a214337 ("media: iris: introduce host firmware interface with necessary hooks") Reported-by: Dan Carpenter dan.carpenter@linaro.org Closes: https://lore.kernel.org/linux-media/634cc9b8-f099-4b54-8556-d879fb2b5169@sta... Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
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 1ed798d31a3f..cba71b5db943 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_response.c @@ -267,7 +267,8 @@ static int iris_hfi_gen2_handle_system_error(struct iris_core *core, { struct iris_inst *instance;
- dev_err(core->dev, "received system error of type %#x\n", pkt->type);
- if (pkt)
dev_err(core->dev, "received system error of type %#x\n", pkt->type);core->state = IRIS_CORE_ERROR;
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
Correct a typo from "dpeth" to "depth".
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c index a908b41e2868..802fa62c26eb 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c @@ -178,7 +178,7 @@ static int iris_hfi_gen2_set_crop_offsets(struct iris_inst *inst) sizeof(u64)); }
-static int iris_hfi_gen2_set_bit_dpeth(struct iris_inst *inst) +static int iris_hfi_gen2_set_bit_depth(struct iris_inst *inst) { struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); u32 port = iris_hfi_gen2_get_port(V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); @@ -378,7 +378,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p {HFI_PROP_BITSTREAM_RESOLUTION, iris_hfi_gen2_set_bitstream_resolution }, {HFI_PROP_CROP_OFFSETS, iris_hfi_gen2_set_crop_offsets }, {HFI_PROP_CODED_FRAMES, iris_hfi_gen2_set_coded_frames }, - {HFI_PROP_LUMA_CHROMA_BIT_DEPTH, iris_hfi_gen2_set_bit_dpeth }, + {HFI_PROP_LUMA_CHROMA_BIT_DEPTH, iris_hfi_gen2_set_bit_depth }, {HFI_PROP_BUFFER_FW_MIN_OUTPUT_COUNT, iris_hfi_gen2_set_min_output_count }, {HFI_PROP_PIC_ORDER_CNT_TYPE, iris_hfi_gen2_set_picture_order_count }, {HFI_PROP_SIGNAL_COLOR_INFO, iris_hfi_gen2_set_colorspace },
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote:
Correct a typo from "dpeth" to "depth".
Cc: stable@vger.kernel.org Fixes: 3a19d7b9e08b ("media: iris: implement set properties to firmware during streamon") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c index a908b41e2868..802fa62c26eb 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen2_command.c @@ -178,7 +178,7 @@ static int iris_hfi_gen2_set_crop_offsets(struct iris_inst *inst) sizeof(u64)); } -static int iris_hfi_gen2_set_bit_dpeth(struct iris_inst *inst) +static int iris_hfi_gen2_set_bit_depth(struct iris_inst *inst) { struct iris_inst_hfi_gen2 *inst_hfi_gen2 = to_iris_inst_hfi_gen2(inst); u32 port = iris_hfi_gen2_get_port(V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE); @@ -378,7 +378,7 @@ static int iris_hfi_gen2_session_set_config_params(struct iris_inst *inst, u32 p {HFI_PROP_BITSTREAM_RESOLUTION, iris_hfi_gen2_set_bitstream_resolution }, {HFI_PROP_CROP_OFFSETS, iris_hfi_gen2_set_crop_offsets }, {HFI_PROP_CODED_FRAMES, iris_hfi_gen2_set_coded_frames },
{HFI_PROP_LUMA_CHROMA_BIT_DEPTH, iris_hfi_gen2_set_bit_dpeth },
{HFI_PROP_LUMA_CHROMA_BIT_DEPTH, iris_hfi_gen2_set_bit_depth },
Acked-by: Vikash Garodia quic_vgarodia@quicinc.com
Currently, two types of flush commands are queued to the firmware, the first flush queued as part of sequence change, does not wait for a response, while the second flush queued as part of stop, expects a completion response before proceeding further.
Due to timing issue, the flush response corresponding to the first command could arrive after the second flush is issued. This casuses the driver to incorrectly assume that the second flush has completed, leading to the premature signaling of flush_completion.
To address this, introduce a counter to track the number of pending flush responses and signal flush completion only when all expected responses are received.
Cc: stable@vger.kernel.org Fixes: 11712ce70f8e ("media: iris: implement vb2 streaming ops") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- .../media/platform/qcom/iris/iris_hfi_gen1_command.c | 4 +++- .../media/platform/qcom/iris/iris_hfi_gen1_response.c | 17 +++++++++++------ drivers/media/platform/qcom/iris/iris_instance.h | 2 ++ 3 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c index f9f3e2d2ce29..ef3ca676d2ea 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_command.c @@ -208,8 +208,10 @@ static int iris_hfi_gen1_session_stop(struct iris_inst *inst, u32 plane) flush_pkt.flush_type = flush_type;
ret = iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size); - if (!ret) + if (!ret) { + inst->flush_responses_pending++; ret = iris_wait_for_session_response(inst, true); + } }
return ret; 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 dfca45d85759..01338baf3788 100644 --- a/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c +++ b/drivers/media/platform/qcom/iris/iris_hfi_gen1_response.c @@ -207,7 +207,8 @@ static void iris_hfi_gen1_event_seq_changed(struct iris_inst *inst, flush_pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_FLUSH; flush_pkt.shdr.session_id = inst->session_id; flush_pkt.flush_type = HFI_FLUSH_OUTPUT; - iris_hfi_queue_cmd_write(inst->core, &flush_pkt, flush_pkt.shdr.hdr.size); + if (!iris_hfi_queue_cmd_write(inst->core, &flush_pkt, flush_pkt.shdr.hdr.size)) + inst->flush_responses_pending++; }
iris_vdec_src_change(inst); @@ -408,7 +409,9 @@ static void iris_hfi_gen1_session_ftb_done(struct iris_inst *inst, void *packet) flush_pkt.shdr.hdr.pkt_type = HFI_CMD_SESSION_FLUSH; flush_pkt.shdr.session_id = inst->session_id; flush_pkt.flush_type = HFI_FLUSH_OUTPUT; - iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size); + if (!iris_hfi_queue_cmd_write(core, &flush_pkt, flush_pkt.shdr.hdr.size)) + inst->flush_responses_pending++; + iris_inst_sub_state_change_drain_last(inst);
return; @@ -570,7 +573,6 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response const struct iris_hfi_gen1_response_pkt_info *pkt_info; struct device *dev = core->dev; struct hfi_session_pkt *pkt; - struct completion *done; struct iris_inst *inst; bool found = false; u32 i; @@ -631,9 +633,12 @@ static void iris_hfi_gen1_handle_response(struct iris_core *core, void *response if (shdr->error_type != HFI_ERR_NONE) iris_inst_change_state(inst, IRIS_INST_ERROR);
- done = pkt_info->pkt == HFI_MSG_SESSION_FLUSH ? - &inst->flush_completion : &inst->completion; - complete(done); + if (pkt_info->pkt == HFI_MSG_SESSION_FLUSH) { + if (--inst->flush_responses_pending <= 0) + complete(&inst->flush_completion); + } else { + complete(&inst->completion); + } } mutex_unlock(&inst->lock);
diff --git a/drivers/media/platform/qcom/iris/iris_instance.h b/drivers/media/platform/qcom/iris/iris_instance.h index 5150237f0020..9ed197799ee7 100644 --- a/drivers/media/platform/qcom/iris/iris_instance.h +++ b/drivers/media/platform/qcom/iris/iris_instance.h @@ -27,6 +27,7 @@ * @crop: structure of crop info * @completion: structure of signal completions * @flush_completion: structure of signal completions for flush cmd + * @flush_responses_pending: counter to track number of pending flush responses * @fw_caps: array of supported instance firmware capabilities * @buffers: array of different iris buffers * @fw_min_count: minimnum count of buffers needed by fw @@ -59,6 +60,7 @@ struct iris_inst { struct iris_hfi_rect_desc crop; struct completion completion; struct completion flush_completion; + u32 flush_responses_pending; struct platform_inst_fw_cap fw_caps[INST_FW_CAP_MAX]; struct iris_buffers buffers[BUF_TYPE_MAX]; u32 fw_min_count;
When the resolution changes, the driver internally updates the width and height, but the client continue to queue buffers with the older resolution until the last flag is received. This results in a mismatch when the buffers are prepared, causing failure due to outdated size.
Introduce a check to prevent size validation during buffer preparation if a resolution reconfiguration is in progress, to handle this.
Cc: stable@vger.kernel.org Fixes: 17f2a485ca67 ("media: iris: implement vb2 ops for buf_queue and firmware response") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com --- drivers/media/platform/qcom/iris/iris_vb2.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c index 23473cbd0b2e..7671df0e1c69 100644 --- a/drivers/media/platform/qcom/iris/iris_vb2.c +++ b/drivers/media/platform/qcom/iris/iris_vb2.c @@ -259,13 +259,14 @@ int iris_vb2_buf_prepare(struct vb2_buffer *vb) return -EINVAL; }
- if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE && - vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_OUTPUT)) - return -EINVAL; - if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE && - vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_INPUT)) - return -EINVAL; - + if (!inst->in_reconfig) { + if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE && + vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_OUTPUT)) + return -EINVAL; + if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE && + vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_INPUT)) + return -EINVAL; + } return 0; }
On 4/28/2025 2:59 PM, Dikshita Agarwal wrote:
When the resolution changes, the driver internally updates the width and height, but the client continue to queue buffers with the older resolution until the last flag is received. This results in a mismatch when the buffers are prepared, causing failure due to outdated size.
Introduce a check to prevent size validation during buffer preparation if a resolution reconfiguration is in progress, to handle this.
Cc: stable@vger.kernel.org Fixes: 17f2a485ca67 ("media: iris: implement vb2 ops for buf_queue and firmware response") Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
drivers/media/platform/qcom/iris/iris_vb2.c | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_vb2.c b/drivers/media/platform/qcom/iris/iris_vb2.c index 23473cbd0b2e..7671df0e1c69 100644 --- a/drivers/media/platform/qcom/iris/iris_vb2.c +++ b/drivers/media/platform/qcom/iris/iris_vb2.c @@ -259,13 +259,14 @@ int iris_vb2_buf_prepare(struct vb2_buffer *vb) return -EINVAL; }
- if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&
vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_OUTPUT))return -EINVAL;- if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&
vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_INPUT))return -EINVAL;
- if (!inst->in_reconfig) {
Remove the flag and replace with state check. With that
Reviewed-by: Vikash Garodia quic_vgarodia@quicinc.com
if (vb->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE &&vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_OUTPUT))return -EINVAL;if (vb->type == V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE &&vb2_plane_size(vb, 0) < iris_get_buffer_size(inst, BUF_INPUT))return -EINVAL;- } return 0;
}
On Mon, Apr 28, 2025 at 02:58:48PM +0530, Dikshita Agarwal wrote:
Hi All,
This patch series adds initial support for the HEVC(H.265) and VP9 codecs in iris decoder. The objective of this work is to extend the decoder's capabilities to handle HEVC and VP9 codec streams, including necessary format handling and buffer management. In addition, the series also includes a set of fixes to address issues identified during testing of these additional codecs.
These patches also address the comments and feedback received from the RFC patches previously sent. I have made the necessary improvements based on the community's suggestions.
Changes in v2:
- Added Changes to make sure all buffers are released in session close
(bryna)
- Added tracking for flush responses to fix a timing issue.
- Added a handling to fix timing issue in reconfig
- Splitted patch 06/20 in two patches (Bryan)
- Added missing fixes tag (bryan)
- Updated fluster report (Nicolas)
- Link to v1:
https://lore.kernel.org/r/20250408-iris-dec-hevc-vp9-v1-0-acd258778bd6@quici...
Changes sinces RFC:
- Added additional fixes to address issues identified during further
testing.
- Moved typo fix to a seperate patch [Neil]
- Reordered the patches for better logical flow and clarity [Neil,
Dmitry]
- Added fixes tag wherever applicable [Neil, Dmitry]
- Removed the default case in the switch statement for codecs [Bryan]
- Replaced if-else statements with switch-case [Bryan]
- Added comments for mbpf [Bryan]
- RFC:
https://lore.kernel.org/linux-media/20250305104335.3629945-1-quic_dikshita@q...
This patch series depends on [1] & [2] [1] https://lore.kernel.org/linux-media/20250417-topic-sm8x50-iris-v10-v7-0-f020... [2] https://lore.kernel.org/linux-media/20250424-qcs8300_iris-v5-0-f118f505c300@...
[...]
Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
Dikshita Agarwal (23): media: iris: Skip destroying internal buffer if not dequeued media: iris: Update CAPTURE format info based on OUTPUT format media: iris: Add handling for corrupt and drop frames media: iris: Avoid updating frame size to firmware during reconfig media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length media: iris: Drop port check for session property response media: iris: Add handling for no show frames media: iris: Improve last flag handling media: iris: Skip flush on first sequence change media: iris: Prevent HFI queue writes when core is in deinit state media: iris: Remove redundant buffer count check in stream off
Please move all fixes patches to the beginning of the series. This helps maintainers to pick them up for the -fixes branches.
media: iris: Remove deprecated property setting to firmware media: iris: Fix missing function pointer initialization media: iris: Fix NULL pointer dereference media: iris: Fix typo in depth variable media: iris: Add a comment to explain usage of MBPS media: iris: Track flush responses to prevent premature completion media: iris: Fix buffer preparation failure during resolution change media: iris: Add HEVC and VP9 formats for decoder media: iris: Add platform capabilities for HEVC and VP9 decoders media: iris: Set mandatory properties for HEVC and VP9 decoders. media: iris: Add internal buffer calculation for HEVC and VP9 decoders media: iris: Add codec specific check for VP9 decoder drain handlingdrivers/media/platform/qcom/iris/iris_buffer.c | 52 ++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 +- drivers/media/platform/qcom/iris/iris_ctrls.c | 35 +- drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 + .../platform/qcom/iris/iris_hfi_gen1_command.c | 48 ++- .../platform/qcom/iris/iris_hfi_gen1_defines.h | 5 +- .../platform/qcom/iris/iris_hfi_gen1_response.c | 39 +- .../platform/qcom/iris/iris_hfi_gen2_command.c | 143 +++++++- .../platform/qcom/iris/iris_hfi_gen2_defines.h | 5 + .../platform/qcom/iris/iris_hfi_gen2_response.c | 58 ++- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- drivers/media/platform/qcom/iris/iris_instance.h | 8 + .../platform/qcom/iris/iris_platform_common.h | 28 +- .../media/platform/qcom/iris/iris_platform_gen2.c | 198 ++++++++-- .../platform/qcom/iris/iris_platform_qcs8300.h | 126 +++++-- .../platform/qcom/iris/iris_platform_sm8250.c | 15 +- drivers/media/platform/qcom/iris/iris_vb2.c | 18 +- drivers/media/platform/qcom/iris/iris_vdec.c | 117 +++--- drivers/media/platform/qcom/iris/iris_vdec.h | 11 + drivers/media/platform/qcom/iris/iris_vidc.c | 9 +- drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 397 ++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_vpu_buffer.h | 46 ++- 22 files changed, 1154 insertions(+), 210 deletions(-)
base-commit: 398a1b33f1479af35ca915c5efc9b00d6204f8fa change-id: 20250428-qcom-iris-hevc-vp9-eb31f30c3390 prerequisite-message-id: 20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@linaro.org prerequisite-patch-id: 35f8dae1416977e88c2db7c767800c01822e266e prerequisite-patch-id: 2bba98151ca103aa62a513a0fbd0df7ae64d9868 prerequisite-patch-id: 0e43a6d758b5fa5ab921c6aa3c19859e312b47d0 prerequisite-patch-id: b7b50aa1657be59fd51c3e53d73382a1ee75a08e prerequisite-patch-id: 30960743105a36f20b3ec4a9ff19e7bca04d6add prerequisite-patch-id: b93c37dc7e09d1631b75387dc1ca90e3066dce17 prerequisite-patch-id: afffe7096c8e110a8da08c987983bc4441d39578 prerequisite-message-id: 20250424-qcs8300_iris-v5-0-f118f505c300@quicinc.com prerequisite-patch-id: 2e72fe4d11d264db3d42fa450427d30171303c6f prerequisite-patch-id: 3398937a7fabb45934bb98a530eef73252231132 prerequisite-patch-id: feda620f147ca14a958c92afdc85a1dc507701ac prerequisite-patch-id: 07ba0745c7d72796567e0a57f5c8e5355a8d2046 prerequisite-patch-id: e35b05c527217206ae871aef0d7b0261af0319ea
Best regards,
Dikshita Agarwal quic_dikshita@quicinc.com
On 4/28/2025 4:37 PM, Dmitry Baryshkov wrote:
On Mon, Apr 28, 2025 at 02:58:48PM +0530, Dikshita Agarwal wrote:
Hi All,
This patch series adds initial support for the HEVC(H.265) and VP9 codecs in iris decoder. The objective of this work is to extend the decoder's capabilities to handle HEVC and VP9 codec streams, including necessary format handling and buffer management. In addition, the series also includes a set of fixes to address issues identified during testing of these additional codecs.
These patches also address the comments and feedback received from the RFC patches previously sent. I have made the necessary improvements based on the community's suggestions.
Changes in v2:
- Added Changes to make sure all buffers are released in session close
(bryna)
- Added tracking for flush responses to fix a timing issue.
- Added a handling to fix timing issue in reconfig
- Splitted patch 06/20 in two patches (Bryan)
- Added missing fixes tag (bryan)
- Updated fluster report (Nicolas)
- Link to v1:
https://lore.kernel.org/r/20250408-iris-dec-hevc-vp9-v1-0-acd258778bd6@quici...
Changes sinces RFC:
- Added additional fixes to address issues identified during further
testing.
- Moved typo fix to a seperate patch [Neil]
- Reordered the patches for better logical flow and clarity [Neil,
Dmitry]
- Added fixes tag wherever applicable [Neil, Dmitry]
- Removed the default case in the switch statement for codecs [Bryan]
- Replaced if-else statements with switch-case [Bryan]
- Added comments for mbpf [Bryan]
- RFC:
https://lore.kernel.org/linux-media/20250305104335.3629945-1-quic_dikshita@q...
This patch series depends on [1] & [2] [1] https://lore.kernel.org/linux-media/20250417-topic-sm8x50-iris-v10-v7-0-f020... [2] https://lore.kernel.org/linux-media/20250424-qcs8300_iris-v5-0-f118f505c300@...
[...]
Signed-off-by: Dikshita Agarwal quic_dikshita@quicinc.com
Dikshita Agarwal (23): media: iris: Skip destroying internal buffer if not dequeued media: iris: Update CAPTURE format info based on OUTPUT format media: iris: Add handling for corrupt and drop frames media: iris: Avoid updating frame size to firmware during reconfig media: iris: Send V4L2_BUF_FLAG_ERROR for buffers with 0 filled length media: iris: Drop port check for session property response media: iris: Add handling for no show frames media: iris: Improve last flag handling media: iris: Skip flush on first sequence change media: iris: Prevent HFI queue writes when core is in deinit state media: iris: Remove redundant buffer count check in stream off
Please move all fixes patches to the beginning of the series. This helps maintainers to pick them up for the -fixes branches.
Ack.
Thanks, Dikshita
media: iris: Remove deprecated property setting to firmware media: iris: Fix missing function pointer initialization media: iris: Fix NULL pointer dereference media: iris: Fix typo in depth variable media: iris: Add a comment to explain usage of MBPS media: iris: Track flush responses to prevent premature completion media: iris: Fix buffer preparation failure during resolution change media: iris: Add HEVC and VP9 formats for decoder media: iris: Add platform capabilities for HEVC and VP9 decoders media: iris: Set mandatory properties for HEVC and VP9 decoders. media: iris: Add internal buffer calculation for HEVC and VP9 decoders media: iris: Add codec specific check for VP9 decoder drain handlingdrivers/media/platform/qcom/iris/iris_buffer.c | 52 ++- drivers/media/platform/qcom/iris/iris_buffer.h | 3 +- drivers/media/platform/qcom/iris/iris_ctrls.c | 35 +- drivers/media/platform/qcom/iris/iris_hfi_common.h | 1 + .../platform/qcom/iris/iris_hfi_gen1_command.c | 48 ++- .../platform/qcom/iris/iris_hfi_gen1_defines.h | 5 +- .../platform/qcom/iris/iris_hfi_gen1_response.c | 39 +- .../platform/qcom/iris/iris_hfi_gen2_command.c | 143 +++++++- .../platform/qcom/iris/iris_hfi_gen2_defines.h | 5 + .../platform/qcom/iris/iris_hfi_gen2_response.c | 58 ++- drivers/media/platform/qcom/iris/iris_hfi_queue.c | 2 +- drivers/media/platform/qcom/iris/iris_instance.h | 8 + .../platform/qcom/iris/iris_platform_common.h | 28 +- .../media/platform/qcom/iris/iris_platform_gen2.c | 198 ++++++++-- .../platform/qcom/iris/iris_platform_qcs8300.h | 126 +++++-- .../platform/qcom/iris/iris_platform_sm8250.c | 15 +- drivers/media/platform/qcom/iris/iris_vb2.c | 18 +- drivers/media/platform/qcom/iris/iris_vdec.c | 117 +++--- drivers/media/platform/qcom/iris/iris_vdec.h | 11 + drivers/media/platform/qcom/iris/iris_vidc.c | 9 +- drivers/media/platform/qcom/iris/iris_vpu_buffer.c | 397 ++++++++++++++++++++- drivers/media/platform/qcom/iris/iris_vpu_buffer.h | 46 ++- 22 files changed, 1154 insertions(+), 210 deletions(-)
base-commit: 398a1b33f1479af35ca915c5efc9b00d6204f8fa change-id: 20250428-qcom-iris-hevc-vp9-eb31f30c3390 prerequisite-message-id: 20250417-topic-sm8x50-iris-v10-v7-0-f020cb1d0e98@linaro.org prerequisite-patch-id: 35f8dae1416977e88c2db7c767800c01822e266e prerequisite-patch-id: 2bba98151ca103aa62a513a0fbd0df7ae64d9868 prerequisite-patch-id: 0e43a6d758b5fa5ab921c6aa3c19859e312b47d0 prerequisite-patch-id: b7b50aa1657be59fd51c3e53d73382a1ee75a08e prerequisite-patch-id: 30960743105a36f20b3ec4a9ff19e7bca04d6add prerequisite-patch-id: b93c37dc7e09d1631b75387dc1ca90e3066dce17 prerequisite-patch-id: afffe7096c8e110a8da08c987983bc4441d39578 prerequisite-message-id: 20250424-qcs8300_iris-v5-0-f118f505c300@quicinc.com prerequisite-patch-id: 2e72fe4d11d264db3d42fa450427d30171303c6f prerequisite-patch-id: 3398937a7fabb45934bb98a530eef73252231132 prerequisite-patch-id: feda620f147ca14a958c92afdc85a1dc507701ac prerequisite-patch-id: 07ba0745c7d72796567e0a57f5c8e5355a8d2046 prerequisite-patch-id: e35b05c527217206ae871aef0d7b0261af0319ea
Best regards,
Dikshita Agarwal quic_dikshita@quicinc.com
linux-stable-mirror@lists.linaro.org
-
Bryan O'Donoghue -
Dan Carpenter -
Dikshita Agarwal -
Dmitry Baryshkov -
Nicolas Dufresne -
Vikash Garodia