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

[Dikshita Agarwal]

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_firmware.c b/drivers/media/platform/qcom/iris/iris_firmware.c
>> index 5f408024e967fd21ade66cc3fa377d8507f9002e..f6ee7f58d4ce215ad9f7fb5fdcadec17f99c8848 100644
>> --- a/drivers/media/platform/qcom/iris/iris_firmware.c
>> +++ b/drivers/media/platform/qcom/iris/iris_firmware.c
>> @@ -14,6 +14,53 @@
>>  
>>  #define MAX_FIRMWARE_NAME_SIZE	128
>>  
>> +static void iris_update_platform_data(struct iris_core *core)
>> +{
>> +	const char *marker = "QC_IMAGE_VERSION_STRING=";
>> +	struct device_node *node = core->dev->of_node;
>> +	const char *found = NULL;
>> +	int major = 0, minor = 0;
>> +	char version_buf[64];
>> +	struct resource res;
>> +	void *mem_virt;
>> +	size_t i;
>> +
>> +	if (!of_device_is_compatible(node, "qcom,sc7280-venus"))
>> +		return;
>> +
>> +	if (of_reserved_mem_region_to_resource(node, 0, &res)) {
>> +		dev_err(core->dev, "Failed to get reserved memory for version check\n");
>> +		return;
>> +	}
>> +
>> +	mem_virt = memremap(res.start, resource_size(&res), MEMREMAP_WC);
>> +	if (!mem_virt) {
>> +		dev_err(core->dev, "Failed to remap memory for version check\n");
>> +		return;
>> +	}
>> +
>> +	for (i = 0; i < resource_size(&res) - strlen(marker); i++) {
>> +		if (memcmp(mem_virt + i, marker, strlen(marker)) == 0) {
>> +			found = (const char *)(mem_virt + i + strlen(marker));
>> +			break;
>> +		}
>> +	}
> 
> This should be done in iris_load_fw_to_memory(). Saves you from extra
> memremap() / memunmap() and also from scanning extra data beyond the
> loaded image area.
> 

Agree, I will move the version detection logic inside iris_load_fw_to_memory.

>> +
>> +	if (found) {
>> +		strscpy(version_buf, found, sizeof(version_buf));
>> +
>> +		/* Check for gen2 version string: "vfw..." OR "video-firmware.N..." (N>=2) */
>> +		if (strncmp(version_buf, "vfw", 3) == 0 ||
>> +		    (sscanf(version_buf, "video-firmware.%d.%d", &major, &minor) == 2 &&
>> +			    major >= 2)) {
>> +			dev_info(core->dev, "Gen2 FW Detected: %s\n", version_buf);
>> +			core->iris_platform_data = &sc7280_gen2_data;
>> +		}
>> +	}
>> +
>> +	memunmap(mem_virt);
>> +}
>> +
>>  static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
>>  {
>>  	u32 pas_id = core->iris_platform_data->pas_id;
>> @@ -64,21 +111,38 @@ static int iris_load_fw_to_memory(struct iris_core *core, const char *fw_name)
>>  
>>  int iris_fw_load(struct iris_core *core)
>>  {
>> +	struct device_node *node = core->dev->of_node;
>>  	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_platform_data->fwname;
>> +	if (!ret) {
>> +		ret = iris_load_fw_to_memory(core, fwpath);
>> +	} else {
>> +		bool fw_loaded = false;
>> +
>> +		if (of_device_is_compatible(node, "qcom,sc7280-venus")) {
>> +			ret = iris_load_fw_to_memory(core, "qcom/vpu/vpu20_p1_gen2_s6.mbn");
>> +			if (!ret)
>> +				fw_loaded = true;
>> +		}
>> +
>> +		if (!fw_loaded) {
>> +			fwpath = core->iris_platform_data->fwname;
>> +			dev_dbg(core->dev, "loading default fw: %s\n", fwpath);
>> +			ret = iris_load_fw_to_memory(core, fwpath);
>> +		}
> 
> Make SC7280 default to Gen2 firmware. Then:
> 
> 	ret = iris_load_fw_to_memory(core, fwpath);
> 	if (ret == -ENOENT &&
> 	    fwpath == core->iris_platform_data->fwname &&
> 	    of_device_is_compatible(node, "qcom,sc7280-venus"))
> 		ret = iris_load_fw_to_memory(core, sc7280_data.fwname);
> 

Ack. I would rework iris_fw_load() so that SC7280 defaults to the Gen2
firmware when no DT firmware-name is provided, and falls back to the Gen1
name only when the Gen2 image is missing.

>> +	}
>>  
>> -	ret = iris_load_fw_to_memory(core, fwpath);
>>  	if (ret) {
>>  		dev_err(core->dev, "firmware download failed\n");
>>  		return -ENOMEM;
>>  	}
>>  
>> +	iris_update_platform_data(core);
>> +
>>  	ret = qcom_scm_pas_auth_and_reset(core->iris_platform_data->pas_id);
>>  	if (ret)  {
>>  		dev_err(core->dev, "auth and reset failed: %d\n", ret);
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> index 5a489917580eb10022fdcb52f7321a915e8b239d..f1bbbe043e3a3ccc5eebf67091162678eb83bf45 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> @@ -43,6 +43,7 @@ enum pipe_type {
>>  
>>  extern const struct iris_platform_data qcs8300_data;
>>  extern const struct iris_platform_data sc7280_data;
>> +extern const struct iris_platform_data sc7280_gen2_data;
>>  extern const struct iris_platform_data sm8250_data;
>>  extern const struct iris_platform_data sm8550_data;
>>  extern const struct iris_platform_data sm8650_data;
>> 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.

>>  	.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.

>>  
>>  #define VIDEO_ARCH_LX 1
>>  #define BITRATE_MAX				245000000
>> @@ -1317,3 +1318,85 @@ const struct iris_platform_data qcs8300_data = {
>>  	.enc_op_int_buf_tbl = sm8550_enc_op_int_buf_tbl,
>>  	.enc_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_enc_op_int_buf_tbl),
>>  };
>> +
>> +const struct iris_platform_data sc7280_gen2_data = {
>> +	.get_instance = iris_hfi_gen2_get_instance,
>> +	.init_hfi_command_ops = iris_hfi_gen2_command_ops_init,
>> +	.init_hfi_response_ops = iris_hfi_gen2_response_ops_init,
>> +	/* Gen2 FW for SC7280 requires bigger size for line buffer for encoder */
>> +	.get_vpu_buffer_size = iris_vpu33_buf_size,
>> +	.vpu_ops = &iris_vpu2_ops,
>> +	.set_preset_registers = iris_set_sm8550_preset_registers,
>> +	.icc_tbl = sm8550_icc_table,
>> +	.icc_tbl_size = ARRAY_SIZE(sm8550_icc_table),
>> +	.bw_tbl_dec = sc7280_bw_table_dec,
>> +	.bw_tbl_dec_size = ARRAY_SIZE(sc7280_bw_table_dec),
>> +	.pmdomain_tbl = sm8550_pmdomain_table,
>> +	.pmdomain_tbl_size = ARRAY_SIZE(sm8550_pmdomain_table),
>> +	.opp_pd_tbl = sc7280_opp_pd_table,
>> +	.opp_pd_tbl_size = ARRAY_SIZE(sc7280_opp_pd_table),
>> +	.clk_tbl = sc7280_clk_table,
>> +	.clk_tbl_size = ARRAY_SIZE(sc7280_clk_table),
>> +	.opp_clk_tbl = sc7280_opp_clk_table,
>> +	/* Upper bound of DMA address range */
>> +	.dma_mask = 0xe0000000 - 1,
>> +	.fwname = "qcom/vpu/vpu20_p1_gen2_s6.mbn",
>> +	.pas_id = IRIS_PAS_ID,
>> +	.inst_iris_fmts = platform_fmts_sc7280_dec,
>> +	.inst_iris_fmts_size = ARRAY_SIZE(platform_fmts_sc7280_dec),
>> +	.inst_caps = &platform_inst_cap_sm8550,
>> +	.inst_fw_caps_dec = inst_fw_cap_sm8550_dec,
>> +	.inst_fw_caps_dec_size = ARRAY_SIZE(inst_fw_cap_sm8550_dec),
>> +	.inst_fw_caps_enc = inst_fw_cap_sm8550_enc,
>> +	.inst_fw_caps_enc_size = ARRAY_SIZE(inst_fw_cap_sm8550_enc),
>> +	.tz_cp_config_data = tz_cp_config_sm8550,
>> +	.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8550),
>> +	.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
>> +	.ubwc_config = &ubwc_config_sm8550,
>> +	.core_arch = VIDEO_ARCH_LX,
>> +	.num_vpp_pipe = 1,
>> +	.no_aon = true,
>> +	.max_session_count = 16,
>> +	.max_core_mbpf = 4096 * 2176 / 256 * 2 + 1920 * 1088 / 256,
>> +	/* max spec for SC7280 is 4096x2176@60fps */
>> +	.max_core_mbps = 4096 * 2176 / 256 * 60,
>> +	.dec_input_config_params_default =
>> +		sm8550_vdec_input_config_params_default,
>> +	.dec_input_config_params_default_size =
>> +		ARRAY_SIZE(sm8550_vdec_input_config_params_default),
>> +	.dec_input_config_params_hevc =
>> +		sm8550_vdec_input_config_param_hevc,
>> +	.dec_input_config_params_hevc_size =
>> +		ARRAY_SIZE(sm8550_vdec_input_config_param_hevc),
>> +	.dec_input_config_params_vp9 =
>> +		sm8550_vdec_input_config_param_vp9,
>> +	.dec_input_config_params_vp9_size =
>> +		ARRAY_SIZE(sm8550_vdec_input_config_param_vp9),
>> +	.enc_input_config_params = sm8550_venc_input_config_params,
>> +	.enc_input_config_params_size =
>> +		ARRAY_SIZE(sm8550_venc_input_config_params),
>> +	.dec_output_config_params = sm8550_vdec_output_config_params,
>> +	.dec_output_config_params_size = ARRAY_SIZE(sm8550_vdec_output_config_params),
>> +	.enc_output_config_params = sm8550_venc_output_config_params,
>> +	.enc_output_config_params_size = ARRAY_SIZE(sm8550_venc_output_config_params),
>> +
>> +	.dec_ip_int_buf_tbl = sm8550_dec_ip_int_buf_tbl,
>> +	.dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_ip_int_buf_tbl),
>> +	.dec_op_int_buf_tbl = sm8550_dec_op_int_buf_tbl,
>> +	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_dec_op_int_buf_tbl),
>> +
>> +	.enc_op_int_buf_tbl = sm8550_enc_op_int_buf_tbl,
>> +	.enc_op_int_buf_tbl_size = ARRAY_SIZE(sm8550_enc_op_int_buf_tbl),
>> +
>> +	.dec_input_prop = sm8550_vdec_subscribe_input_properties,
>> +	.dec_input_prop_size = ARRAY_SIZE(sm8550_vdec_subscribe_input_properties),
>> +	.dec_output_prop_avc = sm8550_vdec_subscribe_output_properties_avc,
>> +	.dec_output_prop_avc_size =
>> +		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_avc),
>> +	.dec_output_prop_hevc = sm8550_vdec_subscribe_output_properties_hevc,
>> +	.dec_output_prop_hevc_size =
>> +		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_hevc),
>> +	.dec_output_prop_vp9 = sm8550_vdec_subscribe_output_properties_vp9,
>> +	.dec_output_prop_vp9_size =
>> +		ARRAY_SIZE(sm8550_vdec_subscribe_output_properties_vp9),
>> +};
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sc7280.h b/drivers/media/platform/qcom/iris/iris_platform_sc7280.h
>> index 0ec8f334df670c3c1548a5ee3b8907b333e34db3..6e05f2542a5457bd0b3b6acced3bd54d166b2023 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_sc7280.h
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sc7280.h
>> @@ -6,6 +6,21 @@
>>  #ifndef __IRIS_PLATFORM_SC7280_H__
>>  #define __IRIS_PLATFORM_SC7280_H__
>>  
>> +static struct iris_fmt platform_fmts_sc7280_dec[] = {
>> +	[IRIS_FMT_H264] = {
>> +		.pixfmt = V4L2_PIX_FMT_H264,
>> +		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> +	},
>> +	[IRIS_FMT_HEVC] = {
>> +		.pixfmt = V4L2_PIX_FMT_HEVC,
>> +		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> +	},
>> +	[IRIS_FMT_VP9] = {
>> +		.pixfmt = V4L2_PIX_FMT_VP9,
>> +		.type = V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE,
>> +	},
>> +};
>> +
>>  static const struct bw_info sc7280_bw_table_dec[] = {
>>  	{ ((3840 * 2160) / 256) * 60, 1896000, },
>>  	{ ((3840 * 2160) / 256) * 30,  968000, },
>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
>> index 22c7b3410710328b900fc49459cd399aa0e89b02..1f44d3ea337df63fbf5317b9b99139a0867267c3 100644
>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>> @@ -12,7 +12,6 @@
>>  #include <linux/reset.h>
>>  
>>  #include "iris_core.h"
>> -#include "iris_ctrls.h"
>>  #include "iris_vidc.h"
>>  
>>  static int iris_init_icc(struct iris_core *core)
>> @@ -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.

Thanks,
Dikshita
> 
>>  	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 bd38d84c9cc79d15585ed5dd5f905a37521cb6dc..0727d5d19cb9b7ed1f72ab840ae5dfda0162e23d 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
>>
>