Re: [PATCH v2 3/3] media: iris: Add support for Gen2 firmware detection and loading on SC7280

[Dikshita Agarwal]

On 3/5/2026 10:27 AM, Dmitry Baryshkov wrote:
> On Mon, Mar 02, 2026 at 02:56:16PM +0530, Dikshita Agarwal wrote:
>>
>>
>> On 2/27/2026 5:48 PM, Dmitry Baryshkov wrote:
>>> On Fri, Feb 27, 2026 at 12:21:03PM +0530, Dikshita Agarwal wrote:
>>>> SC7280 supports both Gen1 and Gen2 HFI firmware. To support both
>>>> dynamically, update the firmware loading mechanism to prioritize
>>>> Gen2 availability and detect the loaded firmware version at runtime.
>>>>
>>>> The firmware loading logic is updated with the following priority:
>>>> 1. Device Tree (`firmware-name`): If specified, load unconditionally.
>>>> 2. Gen2 Autodetect (SC7280 only): If no DT property exists, attempt to
>>>>    load the specific Gen2 firmware image (`vpu20_p1_gen2_s6.mbn`).
>>>> 3. Default Fallback: If Gen2 loading fails or is not applicable, use
>>>>    the default firmware name defined in the default platform data.
>>>>
>>>> Additionally, introduce `iris_update_platform_data` to inspect the
>>>> loaded firmware memory before authentication. This function scans for
>>>> `QC_IMAGE_VERSION_STRING`. If the version string starts with "vfw" or
>>>> matches "video-firmware.N.M" (where N >= 2), it identifies the
>>>> firmware as Gen2.
>>>>
>>>> If Gen2 firmware is detected on SC7280, the driver switches the
>>>> internal platform data pointer to the Gen2 configuration.
>>>>
>>>> Signed-off-by: Dikshita Agarwal <dikshita.agarwal@xxxxxxxxxxxxxxxx>
>>>> ---
>>>>  drivers/media/platform/qcom/iris/iris_firmware.c   | 70 +++++++++++++++++-
>>>>  .../platform/qcom/iris/iris_platform_common.h      |  1 +
>>>>  .../media/platform/qcom/iris/iris_platform_gen1.c  |  4 +-
>>>>  .../media/platform/qcom/iris/iris_platform_gen2.c  | 83 ++++++++++++++++++++++
>>>>  .../platform/qcom/iris/iris_platform_sc7280.h      | 15 ++++
>>>>  drivers/media/platform/qcom/iris/iris_probe.c      |  3 -
>>>>  drivers/media/platform/qcom/iris/iris_vidc.c       |  3 +
>>>>  7 files changed, 171 insertions(+), 8 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen1.c b/drivers/media/platform/qcom/iris/iris_platform_gen1.c
>>>> index df8e6bf9430ed2a070e092edae9ef998d092cb5e..6dbdd0833dcdc7dfac6d7b35f99837c883e188e7 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen1.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen1.c
>>>> @@ -414,8 +414,8 @@ const struct iris_platform_data sc7280_data = {
>>>>  	.dma_mask = 0xe0000000 - 1,
>>>>  	.fwname = "qcom/vpu/vpu20_p1.mbn",
>>>>  	.pas_id = IRIS_PAS_ID,
>>>> -	.inst_iris_fmts = platform_fmts_sm8250_dec,
>>>> -	.inst_iris_fmts_size = ARRAY_SIZE(platform_fmts_sm8250_dec),
>>>> +	.inst_iris_fmts = platform_fmts_sc7280_dec,
>>>> +	.inst_iris_fmts_size = ARRAY_SIZE(platform_fmts_sc7280_dec),
>>>
>>> Why?
>>>
>>
>> SC7280 Gen2 platform data relies heavily on SM8550 data structures.
>> However, unlike SM8550, SC7280 does not support AV1. To address this, I am
>> defining a dedicated platform_fmts_sc7280_dec array that correctly lists
>> the supported codecs (H264, HEVC, VP9) excluding AV1 and using for both
>> gen1 and gen2 platform data for SC7280.
> 
> Why can't we continue using SM8250 data? Also please see the series I
> posted few days ago, it might simplify this piece for you.
> 
>>
>>>>  	.inst_caps = &platform_inst_cap_sm8250,
>>>>  	.inst_fw_caps_dec = inst_fw_cap_sm8250_dec,
>>>>  	.inst_fw_caps_dec_size = ARRAY_SIZE(inst_fw_cap_sm8250_dec),
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen2.c b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>>>> index 5da90d47f9c6eab4a7e6b17841fdc0e599397bf7..5f3be22a003fe5d80b683b43a1b2386497785fb1 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen2.c
>>>> @@ -15,6 +15,7 @@
>>>>  #include "iris_platform_qcs8300.h"
>>>>  #include "iris_platform_sm8650.h"
>>>>  #include "iris_platform_sm8750.h"
>>>> +#include "iris_platform_sc7280.h"
>>>
>>> Don't you end up with two copies of 7280 data in the object files?
>>>
>>
>> You are right, there is a duplication.
>> The header is needed majorly for above reason to exclude AV1, I can have
>> only platform_fmts_sc7280_dec defined in gen1 file and extern and use in
>> gen2 file, that will deviate from the design we are currently following for
>> platform specific caps though.
> 
> Then the design needs to be changed. I've posted a proposal.
> 
>>
>>>>  
>>>>  #define VIDEO_ARCH_LX 1
>>>>  #define BITRATE_MAX				245000000
> 
>>>> @@ -257,8 +256,6 @@ static int iris_probe(struct platform_device *pdev)
>>>>  	if (ret)
>>>>  		return ret;
>>>>  
>>>> -	iris_session_init_caps(core);
>>>> -
>>>
>>> Why?
>>
>> Movin iris_session_init_caps to iris_open because platform data this
>> capabilities may change after firmware loading, which happens after probe.
>> Initializing caps in probe would result in stale Gen1 capabilities if the
>> driver later switches to Gen2.
> 
> Is there a window where devices already exist, but the params are not
> yet initialized?

The device nodes exist after probe, but session caps are only consumed
through ioctls on an open fd, so there’s no user-visible window where caps
can be queried before open().

Thanks,
Dikshita
> 
>