* [PATCH v4 0/3] media: qcom: iris: Add generic Gen2 firmware detection and loading
@ 2026-04-29 12:09 Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 1/3] media: iris: Switch to hardware mode after firmware boot Dikshita Agarwal
` (2 more replies)
0 siblings, 3 replies; 10+ messages in thread
From: Dikshita Agarwal @ 2026-04-29 12:09 UTC (permalink / raw)
To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil
Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal,
Dmitry Baryshkov, Bryan O'Donoghue
This series enhances the Iris driver to support platforms that provide both
Gen1 and Gen2 HFI firmware by adding generic runtime firmware generation
detection and selection logic.
Some Iris platforms are capable of running either Gen1 or Gen2 HFI‑based
firmware, but the driver has historically assumed a single firmware
generation selected at build or platform‑definition time. This series
updates the firmware loading mechanism to dynamically determine the
firmware generation at runtime and select the appropriate HFI
implementation accordingly.
When no Device Tree firmware override is present, the driver now prefers
Gen2 firmware when available and falls back to Gen1 if loading Gen2
fails. When a firmware name is explicitly provided via Device Tree and
both Gen1 and Gen2 descriptors are available, the loaded firmware image
is inspected prior to authentication to determine its generation. Based
on this detection, the driver updates its firmware descriptor and
platform data so that the correct HFI implementation is used.
v4l2-compliance results on SC7280 with Gen2 firmware:
$ v4l2-compliance -d /dev/video1 -s
v4l2-compliance 1.28.1-5233, 64 bits, 64-bit time_t
v4l2-compliance SHA: fc15e229d9d3 2024-07-23 19:22:15
compliance test for iris_driver device /dev/video1:
Driver Info:
Driver name : iris_driver
Card type : Iris Encoder
Bus info : platform:aa00000.video-codec
Driver version : 6.19.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Detected Stateful Encoder
Required ioctls:
test VIDIOC_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/video1 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 38 Private Controls: 0
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK
test Composing: OK (Not Supported)
test Scaling: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test CREATE_BUFS maximum buffers: OK
test VIDIOC_REMOVE_BUFS: OK
test VIDIOC_EXPBUF: OK
test Requests: OK (Not Supported)
Test input 0:
Streaming ioctls:
test read/write: OK (Not Supported)
test blocking wait: OK
Video Capture Multiplanar: Captured 61 buffers
test MMAP (select): OK
Video Capture Multiplanar: Captured 61 buffers
test MMAP (epoll): OK
test USERPTR (select): OK (Not Supported)
test DMABUF: Cannot test, specify --expbuf-device
Total for iris_driver device /dev/video1: 52, Succeeded: 52, Failed: 0, Warnings: 0
$ v4l2-compliance -d /dev/video0 -s5 --stream-from=/media/FVDO_Freeway_720p.264
v4l2-compliance 1.28.1-5233, 64 bits, 64-bit time_t
v4l2-compliance SHA: fc15e229d9d3 2024-07-23 19:22:15
Compliance test for iris_driver device /dev/video0:
Driver Info:
Driver name : iris_driver
Card type : Iris Decoder
Bus info : platform:aa00000.video-codec
Driver version : 6.19.0
Capabilities : 0x84204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Device Capabilities
Device Caps : 0x04204000
Video Memory-to-Memory Multiplanar
Streaming
Extended Pix Format
Detected Stateful Decoder
Required ioctls:
test VIDIOC_QUERYCAP: OK
test invalid ioctls: OK
Allow for multiple opens:
test second /dev/video0 open: OK
test VIDIOC_QUERYCAP: OK
test VIDIOC_G/S_PRIORITY: OK
test for unlimited opens: OK
Debug ioctls:
test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
test VIDIOC_LOG_STATUS: OK (Not Supported)
Input ioctls:
test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
test VIDIOC_ENUMAUDIO: OK (Not Supported)
test VIDIOC_G/S/ENUMINPUT: OK (Not Supported)
test VIDIOC_G/S_AUDIO: OK (Not Supported)
Inputs: 0 Audio Inputs: 0 Tuners: 0
Output ioctls:
test VIDIOC_G/S_MODULATOR: OK (Not Supported)
test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
test VIDIOC_ENUMAUDOUT: OK (Not Supported)
test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
test VIDIOC_G/S_AUDOUT: OK (Not Supported)
Outputs: 0 Audio Outputs: 0 Modulators: 0
Input/Output configuration ioctls:
test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK (Not Supported)
test VIDIOC_DV_TIMINGS_CAP: OK (Not Supported)
test VIDIOC_G/S_EDID: OK (Not Supported)
Control ioctls:
test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
test VIDIOC_QUERYCTRL: OK
test VIDIOC_G/S_CTRL: OK
test VIDIOC_G/S/TRY_EXT_CTRLS: OK
test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
Standard Controls: 12 Private Controls: 0
Format ioctls:
test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
test VIDIOC_G/S_PARM: OK (Not Supported)
test VIDIOC_G_FBUF: OK (Not Supported)
test VIDIOC_G_FMT: OK
test VIDIOC_TRY_FMT: OK
test VIDIOC_S_FMT: OK
test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
test Cropping: OK
test Composing: OK
test Scaling: OK (Not Supported)
Codec ioctls:
test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
test VIDIOC_G_ENC_INDEX: OK (Not Supported)
test VIDIOC_(TRY_)DECODER_CMD: OK
Buffer ioctls:
test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
test CREATE_BUFS maximum buffers: OK
test VIDIOC_REMOVE_BUFS: OK
test VIDIOC_EXPBUF: OK
test Requests: OK (Not Supported)
Test input 0:
Streaming ioctls:
test read/write: OK (Not Supported)
test blocking wait: OK
Video Capture Multiplanar: Captured 65 buffers
test MMAP (select): OK
Video Capture Multiplanar: Captured 65 buffers
test MMAP (epoll): OK
test USERPTR (select): OK (Not Supported)
test DMABUF: Cannot test, specify --expbuf-device
Total for iris_driver device /dev/video0: 52, Succeeded: 52, Failed: 0, Warnings: 0
Fluster results on SC7280 with Gen2 Firmware:
./fluster.py run -ts JVT-AVC_V1 -d GStreamer-H.264-V4L2-Gst1.0 - 77/135
The failing test case:
- Unsupported profile: H.264 Extended profile is deprecated.
- BA3_SVA_C
- Interlaced content is not supported yet.
- CABREF3_Sand_D
- CAFI1_SVA_C
- CAMA1_Sony_C
- CAMA1_TOSHIBA_B
- CAMA3_Sand_E
- CAMACI3_Sony_C
- CAMANL1_TOSHIBA_B
- CAMANL2_TOSHIBA_B
- CAMANL3_Sand_E
- CAMASL3_Sony_B
- CAMP_MOT_MBAFF_L30
- CAMP_MOT_MBAFF_L31
- CANLMA2_Sony_C
- CANLMA3_Sony_C
- CAPA1_TOSHIBA_B
- CAPAMA3_Sand_F
- CVCANLMA2_Sony_C
- CVFI1_SVA_C
- CVFI1_Sony_D
- CVFI2_SVA_C
- CVFI2_Sony_H
- CVMA1_Sony_D
- CVMA1_TOSHIBA_B
- CVMANL1_TOSHIBA_B
- CVMANL2_TOSHIBA_B
- CVMAPAQP3_Sony_E
- CVMAQP2_Sony_G
- CVMAQP3_Sony_D
- CVMP_MOT_FLD_L30_B
- CVMP_MOT_FRM_L31
- CVNLFI1_Sony_C
- CVNLFI2_Sony_H
- CVPA1_TOSHIBA_B
- FI1_Sony_E
- MR6_BT_B
- MR7_BT_B
- MR8_BT_B
- MR9_BT_B
- Sharp_MP_Field_1_B
- Sharp_MP_Field_2_B
- Sharp_MP_Field_3_B
- Sharp_MP_PAFF_1r2
- Sharp_MP_PAFF_2r
- cabac_mot_fld0_full
- cabac_mot_mbaff0_full
- cabac_mot_picaff0_full
- cama1_vtc_c
- cama2_vtc_b
- cama3_vtc_b
- cavlc_mot_fld0_full_B
- cavlc_mot_mbaff0_full_B
- cavlc_mot_picaff0_full_B
- Unsupported bitstream: num_slice_group_minus1 > 0 (slice groups not supported by hardware).
- FM1_BT_B
- FM1_FT_E
- FM2_SVA_C
- Unsupported bitstream: SP slice type is not supported by hardware.
- SP1_BT_A
- sp2_bt_b
./fluster.py run -ts JCT-VC-HEVC_V1 -d GStreamer-H.265-V4L2-Gst1.0 - 131/147
The failing test case:
- 10bit content not supported yet
- DBLK_A_MAIN10_VIXS_4
- INITQP_B_Main10_Sony_1
- TSUNEQBD_A_MAIN10_Technicolor_2
- WPP_A_ericsson_MAIN10_2
- WPP_B_ericsson_MAIN10_2
- WPP_C_ericsson_MAIN10_2
- WPP_D_ericsson_MAIN10_2
- WPP_E_ericsson_MAIN10_2
- WPP_F_ericsson_MAIN10_2
- WP_A_MAIN10_Toshiba_3
- WP_MAIN10_B_Toshiba_3
- Unsupported resolution
- PICSIZE_A_Bossen_1 - resolution is higher than max supported
- PICSIZE_B_Bossen_1 - resolution is higher than max supported
- WPP_D_ericsson_MAIN_2 - resolution is lower than min supported
- CRC mismatch
- RAP_A_docomo_6
- CRC mismatch - bitstream issue - fails with ffmpeg sw decoder as well
- VPSSPSPPS_A_MainConcept_1
./fluster.py run -ts VP9-TEST-VECTORS -d GStreamer-VP9-V4L2-Gst1.0 -j1 - 235/305
The failing test case:
- 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
- Unsupported format
- vp91-2-04-yuv422.webm
- vp91-2-04-yuv444.webm
- CRC mismatch
- vp90-2-22-svc_1280x720_3.ivf
- 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
- Unsupported stream
- vp90-2-16-intra-only.webm
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
Changes in v4:
- Simplified the code handling selection between Gen2 / Gen1 and fallback (updated by Dmitry)
- Link to v3: https://lore.kernel.org/linux-media/20260331-kodiak-gen2-support-v3-0-958296fab838@oss.qualcomm.com/
Changes in v3:
- Rebased on platform rework series by Dmitry.
- Moved version detection logic inside iris_load_fw_to_memory (Dmitry).
- Make Gen2 as deafult for SC7280 and falls back to the Gen1 name only
when the Gen2 image is missing (Dmitry).
- Link to v2: https://lore.kernel.org/r/20260227-iris_sc7280_gen2_support-v2-0-7e5b13d26542@oss.qualcomm.com
Changes in v2:
- Improved the logic to detect if firmware loaded is Gen1 or Gen2 (Dmitry/Konrad)
- Added a patch to switch hardware mode after firmware boot
- Link to v1: https://lore.kernel.org/r/20260209-iris_sc7280_gen2_support-v1-0-390000a4fa39@oss.qualcomm.com
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
Dikshita Agarwal (2):
media: iris: Initialize HFI ops after firmware load in core init
media: iris: Add Gen2 firmware autodetect and fallback
Vikash Garodia (1):
media: iris: Switch to hardware mode after firmware boot
drivers/media/platform/qcom/iris/iris_core.c | 6 ++
drivers/media/platform/qcom/iris/iris_firmware.c | 105 +++++++++++++++++----
drivers/media/platform/qcom/iris/iris_hfi_common.c | 4 +
.../platform/qcom/iris/iris_platform_common.h | 6 +-
.../media/platform/qcom/iris/iris_platform_vpu2.c | 11 ++-
.../media/platform/qcom/iris/iris_platform_vpu3x.c | 8 +-
drivers/media/platform/qcom/iris/iris_probe.c | 5 -
drivers/media/platform/qcom/iris/iris_vidc.c | 3 +
drivers/media/platform/qcom/iris/iris_vpu2.c | 1 +
drivers/media/platform/qcom/iris/iris_vpu3x.c | 9 +-
drivers/media/platform/qcom/iris/iris_vpu4x.c | 24 ++---
drivers/media/platform/qcom/iris/iris_vpu_common.c | 16 ++--
drivers/media/platform/qcom/iris/iris_vpu_common.h | 3 +
13 files changed, 145 insertions(+), 56 deletions(-)
---
base-commit: 3b058d1aeeeff27a7289529c4944291613b364e9
change-id: 20260429-kodiak-gen2-support-v4-a7f055f15afb
prerequisite-message-id: <20260209-iris-venus-fix-sm8250-v5-0-0a22365d3585@oss.qualcomm.com>
prerequisite-patch-id: 8948139735836adb9fbc51d93b969911dc5b38e8
prerequisite-patch-id: 7ec91bd0149f347c479c906e73cabaa28601ab3d
prerequisite-patch-id: c711522b63f640b7504767b3af7adc05a0b36cac
prerequisite-patch-id: 42b9cd5e0fd6fd99eae267c78b239333adff7637
prerequisite-patch-id: 11c487545e2462ff0a515d689863c3f7f25f9449
prerequisite-message-id: <20260327-venus-iris-flip-switch-v5-0-2f4b6c636927@oss.qualcomm.com>
prerequisite-patch-id: 579d712ec3f942ba0c362e242c71361c151092b5
prerequisite-patch-id: fa4629a3909fbae3917d8c067cce4f673ee857c0
prerequisite-patch-id: cbbd40736f7a797ff76b0fe2b1ddfb559e14e666
prerequisite-patch-id: 5b50917dcfef01db13af320cbd1cba15fd5fa16f
prerequisite-message-id: <20260125-iris-ubwc-v4-0-1ff30644ac81@oss.qualcomm.com>
prerequisite-patch-id: 258496117b2e498200190910a37776be2ced6382
prerequisite-patch-id: 50f58e5d9c6cd2b520d17a7e7b2e657faa7d0847
prerequisite-patch-id: af2ff44a7b919da2ee06cc40893fbcd3f65d32f7
prerequisite-patch-id: f3a2b9ef97be3fa250ea0a6467b2d5a782315aa5
prerequisite-patch-id: 6bdd2119448e84aacbdc6a54d999d47fc69dac81
prerequisite-patch-id: 38cc9502c93c71324f1a11a1fd438374fc41ca84
prerequisite-patch-id: 059d1f35274246575ca4fa9b4ee33cd4801479d1
prerequisite-patch-id: 1cf4ea774a145cdba617eb8be5c1f7afe5817772
prerequisite-patch-id: 46375dcd0da4629e6031336351b9cf688691d7c5
prerequisite-message-id: <20260329-iris-platform-data-v11-0-eea672b03a95@oss.qualcomm.com>
prerequisite-patch-id: 34d473ba50399f8cfaf583f4def12de776aad65d
prerequisite-patch-id: 5a6a2b41c9312687512db5d12bac95114b8d8719
prerequisite-patch-id: e6ec4cd9eb5e93f3443f5f496a1b990a95b5d96d
prerequisite-patch-id: 4be4bbb454444d6f314c2b6ad6a73290184e6d57
prerequisite-patch-id: fd9cd7882f2a8f1b6141f48ff5c3da708839d03f
prerequisite-patch-id: 952471fa5477280d399978c05fbc9bfe6d2d33b0
prerequisite-patch-id: 01c5b37358de833f85de1954f770fe0489818a16
prerequisite-patch-id: dd14b47d6cd8ff14d1bc78c187c061f6fe262fda
prerequisite-patch-id: f4eba0865e7f91bce3fb4b2c627ee123980e0ff9
prerequisite-patch-id: 72984784b916e2d94ede8ab7d52cc0dedfa37c41
prerequisite-patch-id: 2fabf4e36b4e4f74b27fe75133ab8ba0ec9b6e3d
prerequisite-message-id: <20260330-iris-remote-fmts-v3-1-a26ab9e90101@oss.qualcomm.com>
prerequisite-patch-id: aab511a6975936fb0198697fca7b61cc2277e1b4
Best regards,
--
Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 1/3] media: iris: Switch to hardware mode after firmware boot
2026-04-29 12:09 [PATCH v4 0/3] media: qcom: iris: Add generic Gen2 firmware detection and loading Dikshita Agarwal
@ 2026-04-29 12:09 ` Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 2/3] media: iris: Initialize HFI ops after firmware load in core init Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback Dikshita Agarwal
2 siblings, 0 replies; 10+ messages in thread
From: Dikshita Agarwal @ 2026-04-29 12:09 UTC (permalink / raw)
To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil
Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal,
Dmitry Baryshkov
From: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Currently the driver switches the vcodec GDSC to hardware (HW) mode
before firmware load and boot sequence. GDSC can be powered off,
keeping in hw mode, thereby the vcodec registers programmed in TrustZone
(TZ) carry default (reset) values.
Move the transition to HW mode after firmware load and boot sequence.
The bug was exposed with driver configuring different stream ids to
different devices via iommu-map. With registers carrying reset values,
VPU would not generate desired stream-id, thereby leading to SMMU fault.
The efuse tells us which hardware blocks are actually present. If efuse
status is disabled for a block, the driver will skip powering it on or
resetting it. otherwise the driver will perform the necessary resets and
then switch that block into hardware mode. This makes sure we only touch
hardware that really exists and is enabled on the silicon.
Fixes: dde659d37036 ("media: iris: Introduce vpu ops for vpu4 with necessary hooks")
Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
Reviewed-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_core.c | 4 ++++
drivers/media/platform/qcom/iris/iris_hfi_common.c | 4 ++++
drivers/media/platform/qcom/iris/iris_vpu2.c | 1 +
drivers/media/platform/qcom/iris/iris_vpu3x.c | 9 +++-----
drivers/media/platform/qcom/iris/iris_vpu4x.c | 24 ++++++++++++----------
drivers/media/platform/qcom/iris/iris_vpu_common.c | 16 +++++++++------
drivers/media/platform/qcom/iris/iris_vpu_common.h | 3 +++
7 files changed, 38 insertions(+), 23 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_core.c b/drivers/media/platform/qcom/iris/iris_core.c
index e6141012cd3dda7e029a5659dcb3048a23cdc150..1f326f696d08014f5ebfeb0b99cfed70665fd6ab 100644
--- a/drivers/media/platform/qcom/iris/iris_core.c
+++ b/drivers/media/platform/qcom/iris/iris_core.c
@@ -74,6 +74,10 @@ int iris_core_init(struct iris_core *core)
if (ret)
goto error_unload_fw;
+ ret = iris_vpu_switch_to_hwmode(core);
+ if (ret)
+ goto error_unload_fw;
+
ret = iris_hfi_core_init(core);
if (ret)
goto error_unload_fw;
diff --git a/drivers/media/platform/qcom/iris/iris_hfi_common.c b/drivers/media/platform/qcom/iris/iris_hfi_common.c
index ad8e4ecb86052d0c4ec9338b2874293494471bc2..8769ec61f11769e004945063381d9baddb302b06 100644
--- a/drivers/media/platform/qcom/iris/iris_hfi_common.c
+++ b/drivers/media/platform/qcom/iris/iris_hfi_common.c
@@ -159,6 +159,10 @@ int iris_hfi_pm_resume(struct iris_core *core)
if (ret)
goto err_suspend_hw;
+ ret = iris_vpu_switch_to_hwmode(core);
+ if (ret)
+ goto err_suspend_hw;
+
ret = ops->sys_interframe_powercollapse(core);
if (ret)
goto err_suspend_hw;
diff --git a/drivers/media/platform/qcom/iris/iris_vpu2.c b/drivers/media/platform/qcom/iris/iris_vpu2.c
index 9c103a2e4e4eafee101a8a9b168fdc8ca76e277d..01ef40f3895743b3784464e2d5ba2de1aeca5a4a 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu2.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu2.c
@@ -44,4 +44,5 @@ const struct vpu_ops iris_vpu2_ops = {
.power_off_controller = iris_vpu_power_off_controller,
.power_on_controller = iris_vpu_power_on_controller,
.calc_freq = iris_vpu2_calc_freq,
+ .set_hwmode = iris_vpu_set_hwmode,
};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu3x.c b/drivers/media/platform/qcom/iris/iris_vpu3x.c
index fe4423b951b1e9e31d06dffc69d18071cc985731..3dad47be78b58f6cd5ed6f333b3376571a04dbf0 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu3x.c
@@ -234,14 +234,8 @@ static int iris_vpu35_power_on_hw(struct iris_core *core)
if (ret)
goto err_disable_hw_free_clk;
- ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
- if (ret)
- goto err_disable_hw_clk;
-
return 0;
-err_disable_hw_clk:
- iris_disable_unprepare_clock(core, IRIS_HW_CLK);
err_disable_hw_free_clk:
iris_disable_unprepare_clock(core, IRIS_HW_FREERUN_CLK);
err_disable_axi_clk:
@@ -266,6 +260,7 @@ const struct vpu_ops iris_vpu3_ops = {
.power_off_controller = iris_vpu_power_off_controller,
.power_on_controller = iris_vpu_power_on_controller,
.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
+ .set_hwmode = iris_vpu_set_hwmode,
};
const struct vpu_ops iris_vpu33_ops = {
@@ -274,6 +269,7 @@ const struct vpu_ops iris_vpu33_ops = {
.power_off_controller = iris_vpu33_power_off_controller,
.power_on_controller = iris_vpu_power_on_controller,
.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
+ .set_hwmode = iris_vpu_set_hwmode,
};
const struct vpu_ops iris_vpu35_ops = {
@@ -283,4 +279,5 @@ const struct vpu_ops iris_vpu35_ops = {
.power_on_controller = iris_vpu35_vpu4x_power_on_controller,
.program_bootup_registers = iris_vpu35_vpu4x_program_bootup_registers,
.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
+ .set_hwmode = iris_vpu_set_hwmode,
};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu4x.c b/drivers/media/platform/qcom/iris/iris_vpu4x.c
index a8db02ce5c5ec583c4027166b34ce51d3d683b4e..02e100a4045fced33d7a3545b632cc5f0955233f 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu4x.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu4x.c
@@ -252,21 +252,10 @@ static int iris_vpu4x_power_on_hardware(struct iris_core *core)
ret = iris_vpu4x_power_on_apv(core);
if (ret)
goto disable_hw_clocks;
-
- iris_vpu4x_ahb_sync_reset_apv(core);
}
- iris_vpu4x_ahb_sync_reset_hardware(core);
-
- ret = iris_vpu4x_genpd_set_hwmode(core, true, efuse_value);
- if (ret)
- goto disable_apv_power_domain;
-
return 0;
-disable_apv_power_domain:
- if (!(efuse_value & DISABLE_VIDEO_APV_BIT))
- iris_vpu4x_power_off_apv(core);
disable_hw_clocks:
iris_vpu4x_disable_hardware_clocks(core, efuse_value);
disable_vpp1_power_domain:
@@ -359,6 +348,18 @@ static void iris_vpu4x_power_off_hardware(struct iris_core *core)
iris_disable_power_domains(core, core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN]);
}
+static int iris_vpu4x_set_hwmode(struct iris_core *core)
+{
+ u32 efuse_value = readl(core->reg_base + WRAPPER_EFUSE_MONITOR);
+
+ if (!(efuse_value & DISABLE_VIDEO_APV_BIT))
+ iris_vpu4x_ahb_sync_reset_apv(core);
+
+ iris_vpu4x_ahb_sync_reset_hardware(core);
+
+ return iris_vpu4x_genpd_set_hwmode(core, true, efuse_value);
+}
+
const struct vpu_ops iris_vpu4x_ops = {
.power_off_hw = iris_vpu4x_power_off_hardware,
.power_on_hw = iris_vpu4x_power_on_hardware,
@@ -366,4 +367,5 @@ const struct vpu_ops iris_vpu4x_ops = {
.power_on_controller = iris_vpu35_vpu4x_power_on_controller,
.program_bootup_registers = iris_vpu35_vpu4x_program_bootup_registers,
.calc_freq = iris_vpu3x_vpu4x_calculate_frequency,
+ .set_hwmode = iris_vpu4x_set_hwmode,
};
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.c b/drivers/media/platform/qcom/iris/iris_vpu_common.c
index c6cfc1d9fd9ec5a8f69462188a03aa5cb4e1cee9..7bba3b6209c2061dce72facab7c2b58d6b3bb9b9 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.c
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.c
@@ -292,14 +292,8 @@ int iris_vpu_power_on_hw(struct iris_core *core)
if (ret && ret != -ENOENT)
goto err_disable_hw_clock;
- ret = dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
- if (ret)
- goto err_disable_hw_ahb_clock;
-
return 0;
-err_disable_hw_ahb_clock:
- iris_disable_unprepare_clock(core, IRIS_HW_AHB_CLK);
err_disable_hw_clock:
iris_disable_unprepare_clock(core, IRIS_HW_CLK);
err_disable_power:
@@ -308,6 +302,16 @@ int iris_vpu_power_on_hw(struct iris_core *core)
return ret;
}
+int iris_vpu_set_hwmode(struct iris_core *core)
+{
+ return dev_pm_genpd_set_hwmode(core->pmdomain_tbl->pd_devs[IRIS_HW_POWER_DOMAIN], true);
+}
+
+int iris_vpu_switch_to_hwmode(struct iris_core *core)
+{
+ return core->iris_platform_data->vpu_ops->set_hwmode(core);
+}
+
int iris_vpu35_vpu4x_power_off_controller(struct iris_core *core)
{
u32 clk_rst_tbl_size = core->iris_platform_data->clk_rst_tbl_size;
diff --git a/drivers/media/platform/qcom/iris/iris_vpu_common.h b/drivers/media/platform/qcom/iris/iris_vpu_common.h
index 07728c4c72b64bd15f4e4fdfdce90a5d4d6e9d3e..09799a375c1426d808ab5ea4fdfcac3a203e15b3 100644
--- a/drivers/media/platform/qcom/iris/iris_vpu_common.h
+++ b/drivers/media/platform/qcom/iris/iris_vpu_common.h
@@ -21,6 +21,7 @@ struct vpu_ops {
int (*power_on_controller)(struct iris_core *core);
void (*program_bootup_registers)(struct iris_core *core);
u64 (*calc_freq)(struct iris_inst *inst, size_t data_size);
+ int (*set_hwmode)(struct iris_core *core);
};
int iris_vpu_boot_firmware(struct iris_core *core);
@@ -30,6 +31,8 @@ int iris_vpu_watchdog(struct iris_core *core, u32 intr_status);
int iris_vpu_prepare_pc(struct iris_core *core);
int iris_vpu_power_on_controller(struct iris_core *core);
int iris_vpu_power_on_hw(struct iris_core *core);
+int iris_vpu_set_hwmode(struct iris_core *core);
+int iris_vpu_switch_to_hwmode(struct iris_core *core);
int iris_vpu_power_on(struct iris_core *core);
int iris_vpu_power_off_controller(struct iris_core *core);
void iris_vpu_power_off_hw(struct iris_core *core);
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 2/3] media: iris: Initialize HFI ops after firmware load in core init
2026-04-29 12:09 [PATCH v4 0/3] media: qcom: iris: Add generic Gen2 firmware detection and loading Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 1/3] media: iris: Switch to hardware mode after firmware boot Dikshita Agarwal
@ 2026-04-29 12:09 ` Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback Dikshita Agarwal
2 siblings, 0 replies; 10+ messages in thread
From: Dikshita Agarwal @ 2026-04-29 12:09 UTC (permalink / raw)
To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil
Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal,
Bryan O'Donoghue, Dmitry Baryshkov
The HFI sys ops were previously initialized in probe() but, we don't
have firmware loaded at probe time. Since HFI is tightly coupled to
firmware, initialize the HFI sys ops after firmware has been successfully
loaded and booted.
Reviewed-by: Bryan O'Donoghue <bryan.odonoghue@linaro.org>
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_core.c | 2 ++
drivers/media/platform/qcom/iris/iris_probe.c | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/qcom/iris/iris_core.c b/drivers/media/platform/qcom/iris/iris_core.c
index 1f326f696d08014f5ebfeb0b99cfed70665fd6ab..52bf56e517f91e98569ee02986183971266e1c76 100644
--- a/drivers/media/platform/qcom/iris/iris_core.c
+++ b/drivers/media/platform/qcom/iris/iris_core.c
@@ -78,6 +78,8 @@ int iris_core_init(struct iris_core *core)
if (ret)
goto error_unload_fw;
+ core->iris_firmware_data->init_hfi_ops(core);
+
ret = iris_hfi_core_init(core);
if (ret)
goto error_unload_fw;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index d36f0c0e785b7de0e3527e0a824942db0fb79133..dbc15edc602b72fdec8bb2d8d3623676afee728c 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -266,7 +266,6 @@ static int iris_probe(struct platform_device *pdev)
disable_irq_nosync(core->irq);
iris_init_ops(core);
- core->iris_firmware_data->init_hfi_ops(core);
ret = iris_init_resources(core);
if (ret)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-04-29 12:09 [PATCH v4 0/3] media: qcom: iris: Add generic Gen2 firmware detection and loading Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 1/3] media: iris: Switch to hardware mode after firmware boot Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 2/3] media: iris: Initialize HFI ops after firmware load in core init Dikshita Agarwal
@ 2026-04-29 12:09 ` Dikshita Agarwal
2026-05-07 19:04 ` Vikash Garodia
2026-05-30 23:43 ` bod
2 siblings, 2 replies; 10+ messages in thread
From: Dikshita Agarwal @ 2026-04-29 12:09 UTC (permalink / raw)
To: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil
Cc: linux-media, linux-arm-msm, linux-kernel, Dikshita Agarwal,
Dmitry Baryshkov
Some Iris platforms support both Gen1 and Gen2 HFI firmware images.
Update the firmware loading logic to handle this generically by
preferring Gen2 when available, while safely falling back to Gen1
when required.
The firmware loading logic is updated with the following priority:
1. Device Tree (`firmware-name`): If specified, load unconditionally.
2. Gen2 default : If no DT override exists, select the Gen2 firmware
descriptor when present and attempt to load the corresponding
firmware image.
3. Gen1 Fallback: If loading the Gen2 firmware fails and a Gen1
descriptor is available, retry with the Gen1 firmware image.
When a platform provides both Gen1 and Gen2 firmware descriptors and the
firmware is loaded via a DT override, the driver detects the
firmware generation at runtime before authentication by inspecting
the firmware data. The firmware is classified as Gen2 if the
QC_IMAGE_VERSION_STRING starts with "vfw" or matches the
"video-firmware.N.M" format with N >= 2.
If a Gen1 firmware image is detected in this case, the driver switches
to the Gen1 firmware descriptor and associated platform data so that
the correct HFI implementation is used.
This change makes firmware generation detection platform‑agnostic,
preserves DT overrides, prefers newer Gen2 firmware when available,
and maintains compatibility with platforms that only support Gen1.
Co-developed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
---
drivers/media/platform/qcom/iris/iris_firmware.c | 105 +++++++++++++++++----
.../platform/qcom/iris/iris_platform_common.h | 6 +-
.../media/platform/qcom/iris/iris_platform_vpu2.c | 11 ++-
.../media/platform/qcom/iris/iris_platform_vpu3x.c | 8 +-
drivers/media/platform/qcom/iris/iris_probe.c | 4 -
drivers/media/platform/qcom/iris/iris_vidc.c | 3 +
6 files changed, 105 insertions(+), 32 deletions(-)
diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
index 1a476146d7580849d7b68c7c15dd7f82f89a680b..64a2170bf538a6d291b3d909f5563408a3a75e50 100644
--- a/drivers/media/platform/qcom/iris/iris_firmware.c
+++ b/drivers/media/platform/qcom/iris/iris_firmware.c
@@ -16,20 +16,95 @@
#define MAX_FIRMWARE_NAME_SIZE 128
-static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
+/* Detect Gen2 firmware by scanning the blob for:
+ * QC_IMAGE_VERSION_STRING=<version>
+ * and then checking:
+ * - version starts with "vfw", OR
+ * - version matches "video-firmware.N.M" with N >= 2
+ */
+
+static bool iris_detect_gen2_from_fwdata(const u8 *data, size_t size)
+{
+ const char *marker = "QC_IMAGE_VERSION_STRING=";
+ const size_t mlen = strlen(marker);
+ int major = 0, minor = 0;
+ char version_buf[64];
+ size_t max;
+
+ max = (size > mlen) ? size - mlen : 0;
+ for (size_t i = 0; i < max; i++) {
+ if (!memcmp(data + i, marker, mlen)) {
+ const char *found = (const char *)(data + i + mlen);
+
+ strscpy(version_buf, found, sizeof(version_buf));
+ if (!strncmp(version_buf, "vfw", 3))
+ return true;
+ if (sscanf(version_buf, "video-firmware.%d.%d", &major, &minor) == 2 &&
+ major >= 2)
+ return true;
+ break;
+ }
+ }
+
+ return false;
+}
+
+static const struct firmware *iris_detect_firmware(struct iris_core *core,
+ const char **fw_name)
+{
+ const struct firmware *firmware;
+ bool has_both_gens;
+ int ret;
+
+ *fw_name = NULL;
+ if (core->iris_platform_data->firmware_desc_gen2)
+ core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen2;
+ else if (core->iris_platform_data->firmware_desc_gen1)
+ core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
+ else
+ return ERR_PTR(-EINVAL);
+
+ has_both_gens = core->iris_platform_data->firmware_desc_gen2 &&
+ core->iris_platform_data->firmware_desc_gen1;
+
+ ret = of_property_read_string_index(dev_of_node(core->dev), "firmware-name", 0, fw_name);
+ if (ret) {
+ *fw_name = core->iris_firmware_desc->fwname;
+ ret = request_firmware(&firmware, *fw_name, core->dev);
+ if (ret && has_both_gens) {
+ core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
+ *fw_name = core->iris_firmware_desc->fwname;
+ ret = request_firmware(&firmware, *fw_name, core->dev);
+ }
+
+ return ret ? ERR_PTR(ret) : firmware;
+ }
+
+ ret = request_firmware(&firmware, *fw_name, core->dev);
+ if (ret)
+ return ERR_PTR(ret);
+
+ if (has_both_gens &&
+ !iris_detect_gen2_from_fwdata((const u8 *)firmware->data, firmware->size)) {
+ dev_info(core->dev, "Gen1 FW detected in %s\n", *fw_name);
+ core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
+ }
+
+ return firmware;
+}
+
+static int iris_load_fw_to_memory(struct iris_core *core)
{
const struct firmware *firmware = NULL;
struct device *dev = core->dev;
struct resource res;
phys_addr_t mem_phys;
+ const char *fw_name;
size_t res_size;
ssize_t fw_size;
void *mem_virt;
int ret;
- if (strlen(fw_name) >= MAX_FIRMWARE_NAME_SIZE - 4)
- return -EINVAL;
-
ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
if (ret)
return ret;
@@ -37,9 +112,11 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
mem_phys = res.start;
res_size = resource_size(&res);
- ret = request_firmware(&firmware, fw_name, dev);
- if (ret)
- return ret;
+ firmware = iris_detect_firmware(core, &fw_name);
+ if (IS_ERR(firmware))
+ return PTR_ERR(firmware);
+
+ core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
fw_size = qcom_mdt_get_size(firmware);
if (fw_size < 0 || res_size < (size_t)fw_size) {
@@ -66,18 +143,12 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
int iris_fw_load(struct iris_core *core)
{
const struct tz_cp_config *cp_config;
- const char *fwpath = NULL;
int i, ret;
- ret = of_property_read_string_index(core->dev->of_node, "firmware-name", 0,
- &fwpath);
- if (ret)
- fwpath = core->iris_firmware_desc->fwname;
-
- ret = iris_load_fw_to_memory(core, fwpath);
+ ret = iris_load_fw_to_memory(core);
if (ret) {
- dev_err(core->dev, "firmware download failed\n");
- return -ENOMEM;
+ dev_err(core->dev, "firmware download failed %d\n", ret);
+ return ret;
}
ret = qcom_scm_pas_auth_and_reset(IRIS_PAS_ID);
@@ -99,7 +170,7 @@ int iris_fw_load(struct iris_core *core)
}
}
- return ret;
+ return 0;
}
int iris_fw_unload(struct iris_core *core)
diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 0408d51188b27251986780de6b4672b155ab1005..7acb073f719746f57ebaa2afd9061db9239f860e 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -257,11 +257,7 @@ struct iris_firmware_desc {
};
struct iris_platform_data {
- /*
- * XXX: replace with gen1 / gen2 pointers once we have platforms
- * supporting both firmware kinds.
- */
- const struct iris_firmware_desc *firmware_desc;
+ const struct iris_firmware_desc *firmware_desc_gen1, *firmware_desc_gen2;
const struct vpu_ops *vpu_ops;
const struct icc_info *icc_tbl;
diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
index 00d6244bc92fd9216bd7c0e6153689e7d8982a67..8259709ba203eac2230da3048166b33892b337b2 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
@@ -22,6 +22,12 @@ const struct iris_firmware_desc iris_vpu20_p1_gen1_desc = {
.fwname = "qcom/vpu/vpu20_p1.mbn",
};
+const struct iris_firmware_desc iris_vpu20_p1_gen2_s6_desc = {
+ .firmware_data = &iris_hfi_gen2_data,
+ .get_vpu_buffer_size = iris_vpu33_buf_size,
+ .fwname = "qcom/vpu/vpu20_p1_gen2_s6.mbn",
+};
+
const struct iris_firmware_desc iris_vpu20_p4_gen1_desc = {
.firmware_data = &iris_hfi_gen1_data,
.get_vpu_buffer_size = iris_vpu_buf_size,
@@ -65,7 +71,8 @@ static const struct tz_cp_config tz_cp_config_vpu2[] = {
};
const struct iris_platform_data sc7280_data = {
- .firmware_desc = &iris_vpu20_p1_gen1_desc,
+ .firmware_desc_gen1 = &iris_vpu20_p1_gen1_desc,
+ .firmware_desc_gen2 = &iris_vpu20_p1_gen2_s6_desc,
.vpu_ops = &iris_vpu2_ops,
.icc_tbl = iris_icc_info_vpu2,
.icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu2),
@@ -94,7 +101,7 @@ const struct iris_platform_data sc7280_data = {
};
const struct iris_platform_data sm8250_data = {
- .firmware_desc = &iris_vpu20_p4_gen1_desc,
+ .firmware_desc_gen1 = &iris_vpu20_p4_gen1_desc,
.vpu_ops = &iris_vpu2_ops,
.icc_tbl = iris_icc_info_vpu2,
.icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu2),
diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
index 6180104f3b94bf0d5e3206481816802fbd09849d..829dc37b4058101e7dddd484533724272b502560 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
+++ b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
@@ -83,7 +83,7 @@ static const struct tz_cp_config tz_cp_config_vpu3[] = {
* - inst_caps to platform_inst_cap_qcs8300
*/
const struct iris_platform_data qcs8300_data = {
- .firmware_desc = &iris_vpu30_p4_s6_gen2_desc,
+ .firmware_desc_gen2 = &iris_vpu30_p4_s6_gen2_desc,
.vpu_ops = &iris_vpu3_ops,
.icc_tbl = iris_icc_info_vpu3x,
.icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
@@ -112,7 +112,7 @@ const struct iris_platform_data qcs8300_data = {
};
const struct iris_platform_data sm8550_data = {
- .firmware_desc = &iris_vpu30_p4_gen2_desc,
+ .firmware_desc_gen2 = &iris_vpu30_p4_gen2_desc,
.vpu_ops = &iris_vpu3_ops,
.icc_tbl = iris_icc_info_vpu3x,
.icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
@@ -147,7 +147,7 @@ const struct iris_platform_data sm8550_data = {
* - controller_rst_tbl to sm8650_controller_reset_table
*/
const struct iris_platform_data sm8650_data = {
- .firmware_desc = &iris_vpu33_p4_gen2_desc,
+ .firmware_desc_gen2 = &iris_vpu33_p4_gen2_desc,
.vpu_ops = &iris_vpu33_ops,
.icc_tbl = iris_icc_info_vpu3x,
.icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
@@ -178,7 +178,7 @@ const struct iris_platform_data sm8650_data = {
};
const struct iris_platform_data sm8750_data = {
- .firmware_desc = &iris_vpu35_p4_gen2_desc,
+ .firmware_desc_gen2 = &iris_vpu35_p4_gen2_desc,
.vpu_ops = &iris_vpu35_ops,
.icc_tbl = iris_icc_info_vpu3x,
.icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index dbc15edc602b72fdec8bb2d8d3623676afee728c..89426ed42facca7729c987c5b283d11e862e4fe1 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -251,8 +251,6 @@ static int iris_probe(struct platform_device *pdev)
return core->irq;
core->iris_platform_data = of_device_get_match_data(core->dev);
- core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
- core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
core->ubwc_cfg = qcom_ubwc_config_get_data();
if (IS_ERR(core->ubwc_cfg))
@@ -271,8 +269,6 @@ static int iris_probe(struct platform_device *pdev)
if (ret)
return ret;
- iris_session_init_caps(core);
-
ret = v4l2_device_register(dev, &core->v4l2_dev);
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 807c9a20b6ba17fdda8e7e91956bdf19e83a3ad8..6fbc20366f5fd3a80468d90d813851ecf54e4cef 100644
--- a/drivers/media/platform/qcom/iris/iris_vidc.c
+++ b/drivers/media/platform/qcom/iris/iris_vidc.c
@@ -9,6 +9,7 @@
#include <media/v4l2-mem2mem.h>
#include <media/videobuf2-dma-contig.h>
+#include "iris_ctrls.h"
#include "iris_vidc.h"
#include "iris_instance.h"
#include "iris_vdec.h"
@@ -196,6 +197,8 @@ int iris_open(struct file *filp)
goto fail_m2m_release;
}
+ iris_session_init_caps(core);
+
if (inst->domain == DECODER)
ret = iris_vdec_inst_init(inst);
else if (inst->domain == ENCODER)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-04-29 12:09 ` [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback Dikshita Agarwal
@ 2026-05-07 19:04 ` Vikash Garodia
2026-05-13 15:07 ` Dmitry Baryshkov
2026-05-30 23:43 ` bod
1 sibling, 1 reply; 10+ messages in thread
From: Vikash Garodia @ 2026-05-07 19:04 UTC (permalink / raw)
To: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil,
Dmitry Baryshkov
Cc: linux-media, linux-arm-msm, linux-kernel
On 4/29/2026 5:39 PM, Dikshita Agarwal wrote:
> Some Iris platforms support both Gen1 and Gen2 HFI firmware images.
> Update the firmware loading logic to handle this generically by
> preferring Gen2 when available, while safely falling back to Gen1
> when required.
>
> The firmware loading logic is updated with the following priority:
> 1. Device Tree (`firmware-name`): If specified, load unconditionally.
> 2. Gen2 default : If no DT override exists, select the Gen2 firmware
> descriptor when present and attempt to load the corresponding
> firmware image.
> 3. Gen1 Fallback: If loading the Gen2 firmware fails and a Gen1
> descriptor is available, retry with the Gen1 firmware image.
>
> When a platform provides both Gen1 and Gen2 firmware descriptors and the
> firmware is loaded via a DT override, the driver detects the
> firmware generation at runtime before authentication by inspecting
> the firmware data. The firmware is classified as Gen2 if the
> QC_IMAGE_VERSION_STRING starts with "vfw" or matches the
> "video-firmware.N.M" format with N >= 2.
>
> If a Gen1 firmware image is detected in this case, the driver switches
> to the Gen1 firmware descriptor and associated platform data so that
> the correct HFI implementation is used.
>
> This change makes firmware generation detection platform‑agnostic,
> preserves DT overrides, prefers newer Gen2 firmware when available,
> and maintains compatibility with platforms that only support Gen1.
>
> Co-developed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/iris_firmware.c | 105 +++++++++++++++++----
> .../platform/qcom/iris/iris_platform_common.h | 6 +-
> .../media/platform/qcom/iris/iris_platform_vpu2.c | 11 ++-
> .../media/platform/qcom/iris/iris_platform_vpu3x.c | 8 +-
> drivers/media/platform/qcom/iris/iris_probe.c | 4 -
> drivers/media/platform/qcom/iris/iris_vidc.c | 3 +
> 6 files changed, 105 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> index 1a476146d7580849d7b68c7c15dd7f82f89a680b..64a2170bf538a6d291b3d909f5563408a3a75e50 100644
> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> @@ -16,20 +16,95 @@
>
> #define MAX_FIRMWARE_NAME_SIZE 128
>
> -static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> +/* Detect Gen2 firmware by scanning the blob for:
> + * QC_IMAGE_VERSION_STRING=<version>
> + * and then checking:
> + * - version starts with "vfw", OR
> + * - version matches "video-firmware.N.M" with N >= 2
> + */
> +
> +static bool iris_detect_gen2_from_fwdata(const u8 *data, size_t size)
> +{
> + const char *marker = "QC_IMAGE_VERSION_STRING=";
> + const size_t mlen = strlen(marker);
> + int major = 0, minor = 0;
> + char version_buf[64];
> + size_t max;
> +
> + max = (size > mlen) ? size - mlen : 0;
better to limit the size of the blob to be parsed to 4K ? version
strings should be in the initial part of the firmware image.
A bad (and big enough) firmware blob might slow down the system with the
current logic
something like
size = min(size, (size_t)SZ_4K);
> + for (size_t i = 0; i < max; i++) {
> + if (!memcmp(data + i, marker, mlen)) {
> + const char *found = (const char *)(data + i + mlen);
> +
> + strscpy(version_buf, found, sizeof(version_buf));
> + if (!strncmp(version_buf, "vfw", 3))
> + return true;
> + if (sscanf(version_buf, "video-firmware.%d.%d", &major, &minor) == 2 &&
> + major >= 2)
> + return true;
> + break;
> + }
> + }
> +
> + return false;
> +}
> +
> +static const struct firmware *iris_detect_firmware(struct iris_core *core,
> + const char **fw_name)
> +{
> + const struct firmware *firmware;
> + bool has_both_gens;
> + int ret;
> +
> + *fw_name = NULL;
> + if (core->iris_platform_data->firmware_desc_gen2)
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen2;
> + else if (core->iris_platform_data->firmware_desc_gen1)
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
> + else
> + return ERR_PTR(-EINVAL);
> +
> + has_both_gens = core->iris_platform_data->firmware_desc_gen2 &&
> + core->iris_platform_data->firmware_desc_gen1;
> +
> + ret = of_property_read_string_index(dev_of_node(core->dev), "firmware-name", 0, fw_name);
> + if (ret) {
> + *fw_name = core->iris_firmware_desc->fwname;
> + ret = request_firmware(&firmware, *fw_name, core->dev);
> + if (ret && has_both_gens) {
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
> + *fw_name = core->iris_firmware_desc->fwname;
> + ret = request_firmware(&firmware, *fw_name, core->dev);
> + }
> +
> + return ret ? ERR_PTR(ret) : firmware;
> + }
> +
> + ret = request_firmware(&firmware, *fw_name, core->dev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (has_both_gens &&
> + !iris_detect_gen2_from_fwdata((const u8 *)firmware->data, firmware->size)) {
> + dev_info(core->dev, "Gen1 FW detected in %s\n", *fw_name);
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
> + }
> +
> + return firmware;
> +}
> +
> +static int iris_load_fw_to_memory(struct iris_core *core)
> {
> const struct firmware *firmware = NULL;
> struct device *dev = core->dev;
> struct resource res;
> phys_addr_t mem_phys;
> + const char *fw_name;
> size_t res_size;
> ssize_t fw_size;
> void *mem_virt;
> int ret;
>
> - if (strlen(fw_name) >= MAX_FIRMWARE_NAME_SIZE - 4)
> - return -EINVAL;
> -
> ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
> if (ret)
> return ret;
> @@ -37,9 +112,11 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> mem_phys = res.start;
> res_size = resource_size(&res);
>
> - ret = request_firmware(&firmware, fw_name, dev);
> - if (ret)
> - return ret;
> + firmware = iris_detect_firmware(core, &fw_name);
> + if (IS_ERR(firmware))
> + return PTR_ERR(firmware);
> +
> + core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
>
> fw_size = qcom_mdt_get_size(firmware);
> if (fw_size < 0 || res_size < (size_t)fw_size) {
> @@ -66,18 +143,12 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> int iris_fw_load(struct iris_core *core)
> {
> const struct tz_cp_config *cp_config;
> - const char *fwpath = NULL;
> int i, ret;
>
> - ret = of_property_read_string_index(core->dev->of_node, "firmware-name", 0,
> - &fwpath);
> - if (ret)
> - fwpath = core->iris_firmware_desc->fwname;
> -
> - ret = iris_load_fw_to_memory(core, fwpath);
> + ret = iris_load_fw_to_memory(core);
> if (ret) {
> - dev_err(core->dev, "firmware download failed\n");
> - return -ENOMEM;
> + dev_err(core->dev, "firmware download failed %d\n", ret);
> + return ret;
> }
>
> ret = qcom_scm_pas_auth_and_reset(IRIS_PAS_ID);
> @@ -99,7 +170,7 @@ int iris_fw_load(struct iris_core *core)
> }
> }
>
> - return ret;
> + return 0;
> }
>
> int iris_fw_unload(struct iris_core *core)
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index 0408d51188b27251986780de6b4672b155ab1005..7acb073f719746f57ebaa2afd9061db9239f860e 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -257,11 +257,7 @@ struct iris_firmware_desc {
> };
>
> struct iris_platform_data {
> - /*
> - * XXX: replace with gen1 / gen2 pointers once we have platforms
> - * supporting both firmware kinds.
> - */
> - const struct iris_firmware_desc *firmware_desc;
> + const struct iris_firmware_desc *firmware_desc_gen1, *firmware_desc_gen2;
>
> const struct vpu_ops *vpu_ops;
> const struct icc_info *icc_tbl;
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> index 00d6244bc92fd9216bd7c0e6153689e7d8982a67..8259709ba203eac2230da3048166b33892b337b2 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> @@ -22,6 +22,12 @@ const struct iris_firmware_desc iris_vpu20_p1_gen1_desc = {
> .fwname = "qcom/vpu/vpu20_p1.mbn",
> };
>
> +const struct iris_firmware_desc iris_vpu20_p1_gen2_s6_desc = {
> + .firmware_data = &iris_hfi_gen2_data,
> + .get_vpu_buffer_size = iris_vpu33_buf_size,
> + .fwname = "qcom/vpu/vpu20_p1_gen2_s6.mbn",
> +};
> +
> const struct iris_firmware_desc iris_vpu20_p4_gen1_desc = {
> .firmware_data = &iris_hfi_gen1_data,
> .get_vpu_buffer_size = iris_vpu_buf_size,
> @@ -65,7 +71,8 @@ static const struct tz_cp_config tz_cp_config_vpu2[] = {
> };
>
> const struct iris_platform_data sc7280_data = {
> - .firmware_desc = &iris_vpu20_p1_gen1_desc,
> + .firmware_desc_gen1 = &iris_vpu20_p1_gen1_desc,
> + .firmware_desc_gen2 = &iris_vpu20_p1_gen2_s6_desc,
> .vpu_ops = &iris_vpu2_ops,
> .icc_tbl = iris_icc_info_vpu2,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu2),
> @@ -94,7 +101,7 @@ const struct iris_platform_data sc7280_data = {
> };
>
> const struct iris_platform_data sm8250_data = {
> - .firmware_desc = &iris_vpu20_p4_gen1_desc,
> + .firmware_desc_gen1 = &iris_vpu20_p4_gen1_desc,
> .vpu_ops = &iris_vpu2_ops,
> .icc_tbl = iris_icc_info_vpu2,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu2),
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> index 6180104f3b94bf0d5e3206481816802fbd09849d..829dc37b4058101e7dddd484533724272b502560 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> @@ -83,7 +83,7 @@ static const struct tz_cp_config tz_cp_config_vpu3[] = {
> * - inst_caps to platform_inst_cap_qcs8300
> */
> const struct iris_platform_data qcs8300_data = {
> - .firmware_desc = &iris_vpu30_p4_s6_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu30_p4_s6_gen2_desc,
> .vpu_ops = &iris_vpu3_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> @@ -112,7 +112,7 @@ const struct iris_platform_data qcs8300_data = {
> };
>
> const struct iris_platform_data sm8550_data = {
> - .firmware_desc = &iris_vpu30_p4_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu30_p4_gen2_desc,
> .vpu_ops = &iris_vpu3_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> @@ -147,7 +147,7 @@ const struct iris_platform_data sm8550_data = {
> * - controller_rst_tbl to sm8650_controller_reset_table
> */
> const struct iris_platform_data sm8650_data = {
> - .firmware_desc = &iris_vpu33_p4_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu33_p4_gen2_desc,
> .vpu_ops = &iris_vpu33_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> @@ -178,7 +178,7 @@ const struct iris_platform_data sm8650_data = {
> };
>
> const struct iris_platform_data sm8750_data = {
> - .firmware_desc = &iris_vpu35_p4_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu35_p4_gen2_desc,
> .vpu_ops = &iris_vpu35_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index dbc15edc602b72fdec8bb2d8d3623676afee728c..89426ed42facca7729c987c5b283d11e862e4fe1 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -251,8 +251,6 @@ static int iris_probe(struct platform_device *pdev)
> return core->irq;
>
> core->iris_platform_data = of_device_get_match_data(core->dev);
> - core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
> - core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
>
> core->ubwc_cfg = qcom_ubwc_config_get_data();
> if (IS_ERR(core->ubwc_cfg))
> @@ -271,8 +269,6 @@ static int iris_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - iris_session_init_caps(core);
> -
> ret = v4l2_device_register(dev, &core->v4l2_dev);
> 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 807c9a20b6ba17fdda8e7e91956bdf19e83a3ad8..6fbc20366f5fd3a80468d90d813851ecf54e4cef 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -9,6 +9,7 @@
> #include <media/v4l2-mem2mem.h>
> #include <media/videobuf2-dma-contig.h>
>
> +#include "iris_ctrls.h"
> #include "iris_vidc.h"
> #include "iris_instance.h"
> #include "iris_vdec.h"
> @@ -196,6 +197,8 @@ int iris_open(struct file *filp)
> goto fail_m2m_release;
> }
>
> + iris_session_init_caps(core);
> +
> if (inst->domain == DECODER)
> ret = iris_vdec_inst_init(inst);
> else if (inst->domain == ENCODER)
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-05-07 19:04 ` Vikash Garodia
@ 2026-05-13 15:07 ` Dmitry Baryshkov
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Baryshkov @ 2026-05-13 15:07 UTC (permalink / raw)
To: Vikash Garodia
Cc: Dikshita Agarwal, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
On Fri, May 08, 2026 at 12:34:04AM +0530, Vikash Garodia wrote:
> On 4/29/2026 5:39 PM, Dikshita Agarwal wrote:
> > Some Iris platforms support both Gen1 and Gen2 HFI firmware images.
> > Update the firmware loading logic to handle this generically by
> > preferring Gen2 when available, while safely falling back to Gen1
> > when required.
> >
> > The firmware loading logic is updated with the following priority:
> > 1. Device Tree (`firmware-name`): If specified, load unconditionally.
> > 2. Gen2 default : If no DT override exists, select the Gen2 firmware
> > descriptor when present and attempt to load the corresponding
> > firmware image.
> > 3. Gen1 Fallback: If loading the Gen2 firmware fails and a Gen1
> > descriptor is available, retry with the Gen1 firmware image.
> >
> > When a platform provides both Gen1 and Gen2 firmware descriptors and the
> > firmware is loaded via a DT override, the driver detects the
> > firmware generation at runtime before authentication by inspecting
> > the firmware data. The firmware is classified as Gen2 if the
> > QC_IMAGE_VERSION_STRING starts with "vfw" or matches the
> > "video-firmware.N.M" format with N >= 2.
> >
> > If a Gen1 firmware image is detected in this case, the driver switches
> > to the Gen1 firmware descriptor and associated platform data so that
> > the correct HFI implementation is used.
> >
> > This change makes firmware generation detection platform‑agnostic,
> > preserves DT overrides, prefers newer Gen2 firmware when available,
> > and maintains compatibility with platforms that only support Gen1.
> >
> > Co-developed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> > ---
> > drivers/media/platform/qcom/iris/iris_firmware.c | 105 +++++++++++++++++----
> > .../platform/qcom/iris/iris_platform_common.h | 6 +-
> > .../media/platform/qcom/iris/iris_platform_vpu2.c | 11 ++-
> > .../media/platform/qcom/iris/iris_platform_vpu3x.c | 8 +-
> > drivers/media/platform/qcom/iris/iris_probe.c | 4 -
> > drivers/media/platform/qcom/iris/iris_vidc.c | 3 +
> > 6 files changed, 105 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> > index 1a476146d7580849d7b68c7c15dd7f82f89a680b..64a2170bf538a6d291b3d909f5563408a3a75e50 100644
> > --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> > +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> > @@ -16,20 +16,95 @@
> > #define MAX_FIRMWARE_NAME_SIZE 128
> > -static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> > +/* Detect Gen2 firmware by scanning the blob for:
> > + * QC_IMAGE_VERSION_STRING=<version>
> > + * and then checking:
> > + * - version starts with "vfw", OR
> > + * - version matches "video-firmware.N.M" with N >= 2
> > + */
> > +
> > +static bool iris_detect_gen2_from_fwdata(const u8 *data, size_t size)
> > +{
> > + const char *marker = "QC_IMAGE_VERSION_STRING=";
> > + const size_t mlen = strlen(marker);
> > + int major = 0, minor = 0;
> > + char version_buf[64];
> > + size_t max;
> > +
> > + max = (size > mlen) ? size - mlen : 0;
>
> better to limit the size of the blob to be parsed to 4K ? version strings
> should be in the initial part of the firmware image.
>
> A bad (and big enough) firmware blob might slow down the system with the
> current logic
>
> something like
> size = min(size, (size_t)SZ_4K);
And if you've checked the actual firmware, you'd have seen that the
version strings can't be a part of the first 4k of it. The actual
offsets (and file sizes) for existing files in linux-firmware:
venus-1.8/venus.mbn 856876 992976
venus-4.2/venus.mbn 846796 925432
venus-5.2/venus.mbn 812224 883264
venus-5.4/venus.mbn 846904 922312
venus-6.0/venus.mbn 1159212 1794924
ar50lt_p1_gen2_s6.mbn 1167572 1861732
vpu20_p1_gen2_s6.mbn 1193056 2030620
vpu20_p1.mbn: 1188900 2026452
vpu20_p4.mbn: 1178644 1980084
vpu20_p4_sm8450_s7.mbn 1199172 2024040
vpu30_p4_s6_16mb.mbn 1356852 2315172
vpu30_p4_s6.mbn 1356852 2315156
vpu30_p4_s7.mbn 1357204 2323048
vpu33_p4_s7.mbn 1345968 2343624
vpu35_p4_s7.mbn 1339952 2409160
vpu36_p4_s7.mbn 1364016 2503368
vpu40_p2_s7.mbn 1350400 2474696
I don't think we can easily limit file area to scan for the version
string.
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-04-29 12:09 ` [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback Dikshita Agarwal
2026-05-07 19:04 ` Vikash Garodia
@ 2026-05-30 23:43 ` bod
2026-06-06 10:51 ` Dmitry Baryshkov
1 sibling, 1 reply; 10+ messages in thread
From: bod @ 2026-05-30 23:43 UTC (permalink / raw)
To: Dikshita Agarwal
Cc: Vikash Garodia, Abhinav Kumar, Bryan O'Donoghue,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel, Dmitry Baryshkov
On 2026-04-29 17:39 +0530, Dikshita Agarwal wrote:
> Some Iris platforms support both Gen1 and Gen2 HFI firmware images.
> Update the firmware loading logic to handle this generically by
> preferring Gen2 when available, while safely falling back to Gen1
> when required.
>
> The firmware loading logic is updated with the following priority:
> 1. Device Tree (`firmware-name`): If specified, load unconditionally.
> 2. Gen2 default : If no DT override exists, select the Gen2 firmware
> descriptor when present and attempt to load the corresponding
> firmware image.
> 3. Gen1 Fallback: If loading the Gen2 firmware fails and a Gen1
> descriptor is available, retry with the Gen1 firmware image.
>
> When a platform provides both Gen1 and Gen2 firmware descriptors and the
> firmware is loaded via a DT override, the driver detects the
> firmware generation at runtime before authentication by inspecting
> the firmware data. The firmware is classified as Gen2 if the
> QC_IMAGE_VERSION_STRING starts with "vfw" or matches the
> "video-firmware.N.M" format with N >= 2.
>
> If a Gen1 firmware image is detected in this case, the driver switches
> to the Gen1 firmware descriptor and associated platform data so that
> the correct HFI implementation is used.
>
> This change makes firmware generation detection platform‑agnostic,
> preserves DT overrides, prefers newer Gen2 firmware when available,
> and maintains compatibility with platforms that only support Gen1.
>
> Co-developed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> ---
> drivers/media/platform/qcom/iris/iris_firmware.c | 105 +++++++++++++++++----
> .../platform/qcom/iris/iris_platform_common.h | 6 +-
> .../media/platform/qcom/iris/iris_platform_vpu2.c | 11 ++-
> .../media/platform/qcom/iris/iris_platform_vpu3x.c | 8 +-
> drivers/media/platform/qcom/iris/iris_probe.c | 4 -
> drivers/media/platform/qcom/iris/iris_vidc.c | 3 +
> 6 files changed, 105 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> index 1a476146d7580849d7b68c7c15dd7f82f89a680b..64a2170bf538a6d291b3d909f5563408a3a75e50 100644
> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> @@ -16,20 +16,95 @@
>
> #define MAX_FIRMWARE_NAME_SIZE 128
>
> -static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> +/* Detect Gen2 firmware by scanning the blob for:
> + * QC_IMAGE_VERSION_STRING=<version>
> + * and then checking:
> + * - version starts with "vfw", OR
> + * - version matches "video-firmware.N.M" with N >= 2
> + */
> +
> +static bool iris_detect_gen2_from_fwdata(const u8 *data, size_t size)
> +{
> + const char *marker = "QC_IMAGE_VERSION_STRING=";
> + const size_t mlen = strlen(marker);
> + int major = 0, minor = 0;
> + char version_buf[64];
> + size_t max;
> +
> + max = (size > mlen) ? size - mlen : 0;
> + for (size_t i = 0; i < max; i++) {
Iterating character by character through the string is boilerplate you
don't need.
size = 27;
mlen = 24;
max = 3;
for (i = 0; i < max = 3; i++)
i = 2;
=>
> + if (!memcmp(data + i, marker, mlen)) {
> + const char *found = (const char *)(data + i + mlen);
> +
> + strscpy(version_buf, found, sizeof(version_buf));
This strscpy can exceed the extent of the data buffer here because the
bounds check is the sizeof(version_buf) and all you've bounds checked is
the start of the string not its extent by this point.
found = data + 2 + mlen - strscpy copies from data+2 to data+64 which is an
overflow.
> + if (!strncmp(version_buf, "vfw", 3))
> + return true;
> + if (sscanf(version_buf, "video-firmware.%d.%d", &major, &minor) == 2 &&
> + major >= 2)
> + return true;
> + break;
Right so it is valid to find maker only once with this break
> + }
> + }
> +
> + return false;
> +}
const char * const marker = "QC_IMAGE_VERSION_STRING=";
const char * const terminator = data + size;
size_t marker_len = strlen(marker);
size_t marker_off;
char *fat_buf;
char *version;
bool ret = false;
version = strnstr(data, marker, size);
if (version) {
marker_off = version - data;
version += marker_len;
size -= marker_off + marker_len;
if (version < terminator-3) {
/* This is safe because we bounds check */
if (strncmp("vfw", version, size) == 0)
return true;
}
/* To do your sscanf() you need to create a zeorised buffer */
fat_buf = kzalloc(size+1, GFP_KERNEL);
if (!fat_buf)
return false;
memcpy(fat_buf, version, size);
/* sscanf is now guaranteed to terminate on NULL */
if (sscanf(fat_buf, "video-firmware.%d.%d", &major, &minor) == 2) {
if (major >= 2) {
ret = true;
goto free_mem;
}
}
}
free_mem:
if (fat_buf)
kfree(fat_buf);
return ret;
> +
> +static const struct firmware *iris_detect_firmware(struct iris_core *core,
> + const char **fw_name)
> +{
> + const struct firmware *firmware;
> + bool has_both_gens;
> + int ret;
> +
> + *fw_name = NULL;
> + if (core->iris_platform_data->firmware_desc_gen2)
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen2;
> + else if (core->iris_platform_data->firmware_desc_gen1)
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
> + else
> + return ERR_PTR(-EINVAL);
> +
> + has_both_gens = core->iris_platform_data->firmware_desc_gen2 &&
> + core->iris_platform_data->firmware_desc_gen1;
> +
> + ret = of_property_read_string_index(dev_of_node(core->dev), "firmware-name", 0, fw_name);
> + if (ret) {
> + *fw_name = core->iris_firmware_desc->fwname;
> + ret = request_firmware(&firmware, *fw_name, core->dev);
> + if (ret && has_both_gens) {
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
> + *fw_name = core->iris_firmware_desc->fwname;
> + ret = request_firmware(&firmware, *fw_name, core->dev);
> + }
> +
> + return ret ? ERR_PTR(ret) : firmware;
> + }
> +
> + ret = request_firmware(&firmware, *fw_name, core->dev);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + if (has_both_gens &&
> + !iris_detect_gen2_from_fwdata((const u8 *)firmware->data, firmware->size)) {
> + dev_info(core->dev, "Gen1 FW detected in %s\n", *fw_name);
> + core->iris_firmware_desc = core->iris_platform_data->firmware_desc_gen1;
> + }
> +
> + return firmware;
> +}
> +
> +static int iris_load_fw_to_memory(struct iris_core *core)
> {
> const struct firmware *firmware = NULL;
> struct device *dev = core->dev;
> struct resource res;
> phys_addr_t mem_phys;
> + const char *fw_name;
> size_t res_size;
> ssize_t fw_size;
> void *mem_virt;
> int ret;
>
> - if (strlen(fw_name) >= MAX_FIRMWARE_NAME_SIZE - 4)
> - return -EINVAL;
> -
> ret = of_reserved_mem_region_to_resource(dev->of_node, 0, &res);
> if (ret)
> return ret;
> @@ -37,9 +112,11 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> mem_phys = res.start;
> res_size = resource_size(&res);
>
> - ret = request_firmware(&firmware, fw_name, dev);
> - if (ret)
> - return ret;
> + firmware = iris_detect_firmware(core, &fw_name);
> + if (IS_ERR(firmware))
> + return PTR_ERR(firmware);
> +
> + core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
>
> fw_size = qcom_mdt_get_size(firmware);
> if (fw_size < 0 || res_size < (size_t)fw_size) {
> @@ -66,18 +143,12 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> int iris_fw_load(struct iris_core *core)
> {
> const struct tz_cp_config *cp_config;
> - const char *fwpath = NULL;
> int i, ret;
>
> - ret = of_property_read_string_index(core->dev->of_node, "firmware-name", 0,
> - &fwpath);
> - if (ret)
> - fwpath = core->iris_firmware_desc->fwname;
> -
> - ret = iris_load_fw_to_memory(core, fwpath);
> + ret = iris_load_fw_to_memory(core);
> if (ret) {
> - dev_err(core->dev, "firmware download failed\n");
> - return -ENOMEM;
> + dev_err(core->dev, "firmware download failed %d\n", ret);
> + return ret;
> }
>
> ret = qcom_scm_pas_auth_and_reset(IRIS_PAS_ID);
> @@ -99,7 +170,7 @@ int iris_fw_load(struct iris_core *core)
> }
> }
>
> - return ret;
> + return 0;
> }
>
> int iris_fw_unload(struct iris_core *core)
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index 0408d51188b27251986780de6b4672b155ab1005..7acb073f719746f57ebaa2afd9061db9239f860e 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -257,11 +257,7 @@ struct iris_firmware_desc {
> };
>
> struct iris_platform_data {
> - /*
> - * XXX: replace with gen1 / gen2 pointers once we have platforms
> - * supporting both firmware kinds.
> - */
> - const struct iris_firmware_desc *firmware_desc;
> + const struct iris_firmware_desc *firmware_desc_gen1, *firmware_desc_gen2;
>
> const struct vpu_ops *vpu_ops;
> const struct icc_info *icc_tbl;
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> index 00d6244bc92fd9216bd7c0e6153689e7d8982a67..8259709ba203eac2230da3048166b33892b337b2 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> @@ -22,6 +22,12 @@ const struct iris_firmware_desc iris_vpu20_p1_gen1_desc = {
> .fwname = "qcom/vpu/vpu20_p1.mbn",
> };
>
> +const struct iris_firmware_desc iris_vpu20_p1_gen2_s6_desc = {
> + .firmware_data = &iris_hfi_gen2_data,
> + .get_vpu_buffer_size = iris_vpu33_buf_size,
> + .fwname = "qcom/vpu/vpu20_p1_gen2_s6.mbn",
> +};
> +
> const struct iris_firmware_desc iris_vpu20_p4_gen1_desc = {
> .firmware_data = &iris_hfi_gen1_data,
> .get_vpu_buffer_size = iris_vpu_buf_size,
> @@ -65,7 +71,8 @@ static const struct tz_cp_config tz_cp_config_vpu2[] = {
> };
>
> const struct iris_platform_data sc7280_data = {
> - .firmware_desc = &iris_vpu20_p1_gen1_desc,
> + .firmware_desc_gen1 = &iris_vpu20_p1_gen1_desc,
> + .firmware_desc_gen2 = &iris_vpu20_p1_gen2_s6_desc,
> .vpu_ops = &iris_vpu2_ops,
> .icc_tbl = iris_icc_info_vpu2,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu2),
> @@ -94,7 +101,7 @@ const struct iris_platform_data sc7280_data = {
> };
>
> const struct iris_platform_data sm8250_data = {
> - .firmware_desc = &iris_vpu20_p4_gen1_desc,
> + .firmware_desc_gen1 = &iris_vpu20_p4_gen1_desc,
> .vpu_ops = &iris_vpu2_ops,
> .icc_tbl = iris_icc_info_vpu2,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu2),
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> index 6180104f3b94bf0d5e3206481816802fbd09849d..829dc37b4058101e7dddd484533724272b502560 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu3x.c
> @@ -83,7 +83,7 @@ static const struct tz_cp_config tz_cp_config_vpu3[] = {
> * - inst_caps to platform_inst_cap_qcs8300
> */
> const struct iris_platform_data qcs8300_data = {
> - .firmware_desc = &iris_vpu30_p4_s6_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu30_p4_s6_gen2_desc,
> .vpu_ops = &iris_vpu3_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> @@ -112,7 +112,7 @@ const struct iris_platform_data qcs8300_data = {
> };
>
> const struct iris_platform_data sm8550_data = {
> - .firmware_desc = &iris_vpu30_p4_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu30_p4_gen2_desc,
> .vpu_ops = &iris_vpu3_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> @@ -147,7 +147,7 @@ const struct iris_platform_data sm8550_data = {
> * - controller_rst_tbl to sm8650_controller_reset_table
> */
> const struct iris_platform_data sm8650_data = {
> - .firmware_desc = &iris_vpu33_p4_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu33_p4_gen2_desc,
> .vpu_ops = &iris_vpu33_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> @@ -178,7 +178,7 @@ const struct iris_platform_data sm8650_data = {
> };
>
> const struct iris_platform_data sm8750_data = {
> - .firmware_desc = &iris_vpu35_p4_gen2_desc,
> + .firmware_desc_gen2 = &iris_vpu35_p4_gen2_desc,
> .vpu_ops = &iris_vpu35_ops,
> .icc_tbl = iris_icc_info_vpu3x,
> .icc_tbl_size = ARRAY_SIZE(iris_icc_info_vpu3x),
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index dbc15edc602b72fdec8bb2d8d3623676afee728c..89426ed42facca7729c987c5b283d11e862e4fe1 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -251,8 +251,6 @@ static int iris_probe(struct platform_device *pdev)
> return core->irq;
>
> core->iris_platform_data = of_device_get_match_data(core->dev);
> - core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
> - core->iris_firmware_data = core->iris_firmware_desc->firmware_data;
>
> core->ubwc_cfg = qcom_ubwc_config_get_data();
> if (IS_ERR(core->ubwc_cfg))
> @@ -271,8 +269,6 @@ static int iris_probe(struct platform_device *pdev)
> if (ret)
> return ret;
>
> - iris_session_init_caps(core);
> -
> ret = v4l2_device_register(dev, &core->v4l2_dev);
> 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 807c9a20b6ba17fdda8e7e91956bdf19e83a3ad8..6fbc20366f5fd3a80468d90d813851ecf54e4cef 100644
> --- a/drivers/media/platform/qcom/iris/iris_vidc.c
> +++ b/drivers/media/platform/qcom/iris/iris_vidc.c
> @@ -9,6 +9,7 @@
> #include <media/v4l2-mem2mem.h>
> #include <media/videobuf2-dma-contig.h>
>
> +#include "iris_ctrls.h"
> #include "iris_vidc.h"
> #include "iris_instance.h"
> #include "iris_vdec.h"
> @@ -196,6 +197,8 @@ int iris_open(struct file *filp)
> goto fail_m2m_release;
> }
>
> + iris_session_init_caps(core);
> +
> if (inst->domain == DECODER)
> ret = iris_vdec_inst_init(inst);
> else if (inst->domain == ENCODER)
>
> --
> 2.34.1
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-05-30 23:43 ` bod
@ 2026-06-06 10:51 ` Dmitry Baryshkov
2026-06-06 11:13 ` Bryan O'Donoghue
0 siblings, 1 reply; 10+ messages in thread
From: Dmitry Baryshkov @ 2026-06-06 10:51 UTC (permalink / raw)
To: bod
Cc: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
On Sun, May 31, 2026 at 12:43:18AM +0100, bod@kernel.org wrote:
> On 2026-04-29 17:39 +0530, Dikshita Agarwal wrote:
> > Some Iris platforms support both Gen1 and Gen2 HFI firmware images.
> > Update the firmware loading logic to handle this generically by
> > preferring Gen2 when available, while safely falling back to Gen1
> > when required.
> >
> > The firmware loading logic is updated with the following priority:
> > 1. Device Tree (`firmware-name`): If specified, load unconditionally.
> > 2. Gen2 default : If no DT override exists, select the Gen2 firmware
> > descriptor when present and attempt to load the corresponding
> > firmware image.
> > 3. Gen1 Fallback: If loading the Gen2 firmware fails and a Gen1
> > descriptor is available, retry with the Gen1 firmware image.
> >
> > When a platform provides both Gen1 and Gen2 firmware descriptors and the
> > firmware is loaded via a DT override, the driver detects the
> > firmware generation at runtime before authentication by inspecting
> > the firmware data. The firmware is classified as Gen2 if the
> > QC_IMAGE_VERSION_STRING starts with "vfw" or matches the
> > "video-firmware.N.M" format with N >= 2.
> >
> > If a Gen1 firmware image is detected in this case, the driver switches
> > to the Gen1 firmware descriptor and associated platform data so that
> > the correct HFI implementation is used.
> >
> > This change makes firmware generation detection platform‑agnostic,
> > preserves DT overrides, prefers newer Gen2 firmware when available,
> > and maintains compatibility with platforms that only support Gen1.
> >
> > Co-developed-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > Signed-off-by: Dikshita Agarwal <dikshita.agarwal@oss.qualcomm.com>
> > ---
> > drivers/media/platform/qcom/iris/iris_firmware.c | 105 +++++++++++++++++----
> > .../platform/qcom/iris/iris_platform_common.h | 6 +-
> > .../media/platform/qcom/iris/iris_platform_vpu2.c | 11 ++-
> > .../media/platform/qcom/iris/iris_platform_vpu3x.c | 8 +-
> > drivers/media/platform/qcom/iris/iris_probe.c | 4 -
> > drivers/media/platform/qcom/iris/iris_vidc.c | 3 +
> > 6 files changed, 105 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/media/platform/qcom/iris/iris_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
> > index 1a476146d7580849d7b68c7c15dd7f82f89a680b..64a2170bf538a6d291b3d909f5563408a3a75e50 100644
> > --- a/drivers/media/platform/qcom/iris/iris_firmware.c
> > +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
> > @@ -16,20 +16,95 @@
> >
> > #define MAX_FIRMWARE_NAME_SIZE 128
> >
> > -static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
> > +/* Detect Gen2 firmware by scanning the blob for:
> > + * QC_IMAGE_VERSION_STRING=<version>
> > + * and then checking:
> > + * - version starts with "vfw", OR
> > + * - version matches "video-firmware.N.M" with N >= 2
> > + */
> > +
> > +static bool iris_detect_gen2_from_fwdata(const u8 *data, size_t size)
> > +{
> > + const char *marker = "QC_IMAGE_VERSION_STRING=";
> > + const size_t mlen = strlen(marker);
> > + int major = 0, minor = 0;
> > + char version_buf[64];
> > + size_t max;
> > +
> > + max = (size > mlen) ? size - mlen : 0;
> > + for (size_t i = 0; i < max; i++) {
>
> Iterating character by character through the string is boilerplate you
> don't need.
>
> size = 27;
> mlen = 24;
> max = 3;
>
> for (i = 0; i < max = 3; i++)
>
> i = 2;
>
> =>
>
>
> > + if (!memcmp(data + i, marker, mlen)) {
> > + const char *found = (const char *)(data + i + mlen);
> > +
> > + strscpy(version_buf, found, sizeof(version_buf));
>
> This strscpy can exceed the extent of the data buffer here because the
> bounds check is the sizeof(version_buf) and all you've bounds checked is
> the start of the string not its extent by this point.
>
> found = data + 2 + mlen - strscpy copies from data+2 to data+64 which is an
> overflow.
This can be made more robust, but I think it would be enough to stop
scanning sizeof(version_buf) + strlen(marker) bytes before the end.
>
> > + if (!strncmp(version_buf, "vfw", 3))
> > + return true;
> > + if (sscanf(version_buf, "video-firmware.%d.%d", &major, &minor) == 2 &&
> > + major >= 2)
> > + return true;
> > + break;
>
> Right so it is valid to find maker only once with this break
Yes, there is just one version per firmware file.
>
> > + }
> > + }
> > +
> > + return false;
> > +}
>
> const char * const marker = "QC_IMAGE_VERSION_STRING=";
> const char * const terminator = data + size;
> size_t marker_len = strlen(marker);
> size_t marker_off;
> char *fat_buf;
> char *version;
> bool ret = false;
>
> version = strnstr(data, marker, size);
strnstr is defined to search for the substring in another string. There
is no promise that it will work if data contains \0 chars (which would
terminate the string). And the memmem is not defined for the Linux
kernel.
> if (version) {
> marker_off = version - data;
> version += marker_len;
> size -= marker_off + marker_len;
>
> if (version < terminator-3) {
> /* This is safe because we bounds check */
> if (strncmp("vfw", version, size) == 0)
> return true;
> }
>
> /* To do your sscanf() you need to create a zeorised buffer */
> fat_buf = kzalloc(size+1, GFP_KERNEL);
> if (!fat_buf)
> return false;
>
> memcpy(fat_buf, version, size);
Creating a copy of about the half of the image is definitely an
overkill.
>
> /* sscanf is now guaranteed to terminate on NULL */
> if (sscanf(fat_buf, "video-firmware.%d.%d", &major, &minor) == 2) {
I think we can replace this with string comparisons too. No need to
sscanf it.
WDYT?
> if (major >= 2) {
> ret = true;
> goto free_mem;
> }
> }
> }
>
> free_mem:
> if (fat_buf)
> kfree(fat_buf);
> return ret;
>
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-06-06 10:51 ` Dmitry Baryshkov
@ 2026-06-06 11:13 ` Bryan O'Donoghue
2026-06-06 11:46 ` Dmitry Baryshkov
0 siblings, 1 reply; 10+ messages in thread
From: Bryan O'Donoghue @ 2026-06-06 11:13 UTC (permalink / raw)
To: Dmitry Baryshkov
Cc: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
On 06/06/2026 11:51, Dmitry Baryshkov wrote:
>> version = strnstr(data, marker, size);
> strnstr is defined to search for the substring in another string. There
> is no promise that it will work if data contains \0 chars (which would
> terminate the string).
I mean... we'd want to terminate on a NULL here I'd have thought. The
subsequent strscpy, strncmp and sscanf in this routine would imply NULL
termination.
No wait I see - the strscpy() in the original creates a putatively NULL
terminated string from potentially non-NULL terminated data.
The key question is if this is a NULL terminated string or not. If not
we would expect a header somewhere telling us the field length.
>> if (version) {
>> marker_off = version - data;
>> version += marker_len;
>> size -= marker_off + marker_len;
>>
>> if (version < terminator-3) {
>> /* This is safe because we bounds check */
>> if (strncmp("vfw", version, size) == 0)
>> return true;
>> }
>>
>> /* To do your sscanf() you need to create a zeorised buffer */
>> fat_buf = kzalloc(size+1, GFP_KERNEL);
>> if (!fat_buf)
>> return false;
>>
>> memcpy(fat_buf, version, size);
> Creating a copy of about the half of the image is definitely an
> overkill.
The image size part I wasn't sure about - were we dealing with a defined
header or the _entire_ image with the given size.
That said - why are we scanning the entire image if size == sizeof(fw)
anyway ?
There must be a maximum header size and if not a maximum we would be
prepared to parse in-kernel - say the first 1k or 4k at max.
...
>> /* sscanf is now guaranteed to terminate on NULL */
>> if (sscanf(fat_buf, "video-firmware.%d.%d", &major, &minor) == 2) {
> I think we can replace this with string comparisons too. No need to
> sscanf it.
>
> WDYT?
I'm just as happy with that. It was really this code "looked wrong" and
so I dug into it a little bit.
The overflow is real. The size you pointed out is true. Take the
suggested changes with a pinch of salt.
This is the part that really pinged me
+ for (size_t i = 0; i < max; i++) {
+ if (!memcmp(data + i, marker, mlen)) {
Iterating a string for a memcmp() instead of using standard string libs,
only then when I looked did I see the overflow.
So long as that gets fixed I'm sanguine about the rest.
---
bod
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback
2026-06-06 11:13 ` Bryan O'Donoghue
@ 2026-06-06 11:46 ` Dmitry Baryshkov
0 siblings, 0 replies; 10+ messages in thread
From: Dmitry Baryshkov @ 2026-06-06 11:46 UTC (permalink / raw)
To: Bryan O'Donoghue
Cc: Dikshita Agarwal, Vikash Garodia, Abhinav Kumar,
Mauro Carvalho Chehab, Vishnu Reddy, Hans Verkuil, linux-media,
linux-arm-msm, linux-kernel
On Sat, Jun 06, 2026 at 12:13:46PM +0100, Bryan O'Donoghue wrote:
> On 06/06/2026 11:51, Dmitry Baryshkov wrote:
> > > version = strnstr(data, marker, size);
> > strnstr is defined to search for the substring in another string. There
> > is no promise that it will work if data contains \0 chars (which would
> > terminate the string).
>
> I mean... we'd want to terminate on a NULL here I'd have thought. The
> subsequent strscpy, strncmp and sscanf in this routine would imply NULL
> termination.
>
> No wait I see - the strscpy() in the original creates a putatively NULL
> terminated string from potentially non-NULL terminated data.
No, it is documented to create a NULL-terminated string from a
NULL-terminated string.
>
> The key question is if this is a NULL terminated string or not. If not we
> would expect a header somewhere telling us the field length.
ELF file definitely is not a NULL-terminated string.
>
> > > if (version) {
> > > marker_off = version - data;
> > > version += marker_len;
> > > size -= marker_off + marker_len;
> > >
> > > if (version < terminator-3) {
> > > /* This is safe because we bounds check */
> > > if (strncmp("vfw", version, size) == 0)
> > > return true;
> > > }
> > >
> > > /* To do your sscanf() you need to create a zeorised buffer */
> > > fat_buf = kzalloc(size+1, GFP_KERNEL);
> > > if (!fat_buf)
> > > return false;
> > >
> > > memcpy(fat_buf, version, size);
> > Creating a copy of about the half of the image is definitely an
> > overkill.
>
> The image size part I wasn't sure about - were we dealing with a defined
> header or the _entire_ image with the given size.
>
> That said - why are we scanning the entire image if size == sizeof(fw)
> anyway ?
>
> There must be a maximum header size and if not a maximum we would be
> prepared to parse in-kernel - say the first 1k or 4k at max.
Because nobody has expected that we'd need to parse firmware version
from it. So it's not a header. It is a string in the middle of the
firmware blob.
>
> ...
> > > /* sscanf is now guaranteed to terminate on NULL */
> > > if (sscanf(fat_buf, "video-firmware.%d.%d", &major, &minor) == 2) {
> > I think we can replace this with string comparisons too. No need to
> > sscanf it.
> >
> > WDYT?
>
> I'm just as happy with that. It was really this code "looked wrong" and so I
> dug into it a little bit.
>
> The overflow is real. The size you pointed out is true. Take the suggested
> changes with a pinch of salt.
>
> This is the part that really pinged me
>
> + for (size_t i = 0; i < max; i++) {
> + if (!memcmp(data + i, marker, mlen)) {
>
> Iterating a string for a memcmp() instead of using standard string libs,
> only then when I looked did I see the overflow.
>
> So long as that gets fixed I'm sanguine about the rest.
Ack
--
With best wishes
Dmitry
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2026-06-06 11:46 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-04-29 12:09 [PATCH v4 0/3] media: qcom: iris: Add generic Gen2 firmware detection and loading Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 1/3] media: iris: Switch to hardware mode after firmware boot Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 2/3] media: iris: Initialize HFI ops after firmware load in core init Dikshita Agarwal
2026-04-29 12:09 ` [PATCH v4 3/3] media: iris: Add Gen2 firmware autodetect and fallback Dikshita Agarwal
2026-05-07 19:04 ` Vikash Garodia
2026-05-13 15:07 ` Dmitry Baryshkov
2026-05-30 23:43 ` bod
2026-06-06 10:51 ` Dmitry Baryshkov
2026-06-06 11:13 ` Bryan O'Donoghue
2026-06-06 11:46 ` Dmitry Baryshkov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®