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

[Dmitry Baryshkov]

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?


-- 
With best wishes
Dmitry