Message-ID: <ec4348fc-b27c-9b56-2967-222e4e280c8a@oss.qualcomm.com>
Date: Thu, 5 Feb 2026 14:40:39 +0530
From: Dikshita Agarwal <dikshita.agarwal@....qualcomm.com>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Vikash Garodia <vikash.garodia@....qualcomm.com>,
        Bryan O'Donoghue <bod@...nel.org>,
        Mauro Carvalho Chehab
 <mchehab@...nel.org>,
        Rob Herring <robh@...nel.org>,
        Krzysztof Kozlowski <krzk+dt@...nel.org>,
        Conor Dooley
 <conor+dt@...nel.org>,
        Konrad Dybcio <konradybcio@...nel.org>,
        Abhinav Kumar <abhinav.kumar@...ux.dev>,
        Bjorn Andersson <andersson@...nel.org>,
        David Heidelberg <david@...t.cz>, linux-media@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org,
        Konrad Dybcio <konrad.dybcio@....qualcomm.com>,
        Johan Hovold <johan+linaro@...nel.org>
Subject: Re: [PATCH v3 2/7] media: iris: introduce SM8350 and SC8280XP support



On 1/31/2026 12:58 PM, Dmitry Baryshkov wrote:
> On Fri, Jan 30, 2026 at 06:46:04PM +0530, Dikshita Agarwal wrote:
>>
>>
>> On 1/25/2026 9:02 PM, Dmitry Baryshkov wrote:
>>> SM8350 and SC8280XP have an updated version of the Iris2 core also
>>> present on the SM8250 and SC7280 platforms. Add necessary platform data
>>> to utilize the core on those two platforms.
>>>
>>> The iris_platform_gen1.c is now compiled unconditionally, even if Venus
>>> driver is enabled, but SM8250 and SC7280 are still disabled in
>>> iris_dt_match.
>>>
>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
>>> ---
>>>  drivers/media/platform/qcom/iris/Makefile          |   5 +-
>>>  .../platform/qcom/iris/iris_platform_common.h      |   2 +
>>>  .../media/platform/qcom/iris/iris_platform_gen1.c  | 111 +++++++++++++++++++++
>>>  .../platform/qcom/iris/iris_platform_sm8350.h      |  20 ++++
>>>  drivers/media/platform/qcom/iris/iris_probe.c      |  10 ++
>>>  5 files changed, 144 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/drivers/media/platform/qcom/iris/Makefile b/drivers/media/platform/qcom/iris/Makefile
>>> index 2abbd3aeb4af..2fde45f81727 100644
>>> --- a/drivers/media/platform/qcom/iris/Makefile
>>> +++ b/drivers/media/platform/qcom/iris/Makefile
>>> @@ -10,6 +10,7 @@ qcom-iris-objs += iris_buffer.o \
>>>               iris_hfi_gen2_packet.o \
>>>               iris_hfi_gen2_response.o \
>>>               iris_hfi_queue.o \
>>> +             iris_platform_gen1.o \
>>>               iris_platform_gen2.o \
>>>               iris_power.o \
>>>               iris_probe.o \
>>> @@ -26,8 +27,4 @@ qcom-iris-objs += iris_buffer.o \
>>>               iris_vpu_buffer.o \
>>>               iris_vpu_common.o \
>>>  
>>> -ifeq ($(CONFIG_VIDEO_QCOM_VENUS),)
>>> -qcom-iris-objs += iris_platform_gen1.o
>>> -endif
>>> -
>>>  obj-$(CONFIG_VIDEO_QCOM_IRIS) += qcom-iris.o
>>
>> This change is not needed in this patch, pls remove.
> 
> It is necessary in this patch. We enable gen1 platforms which are not a
> part of the venus->iris transition (they have never been supported by
> the venus driver). As such, iris_platform_gen1.c now needs to be
> compiled unconditionally.
> 

Ack, I assumed you have this series dependent on the other series "flip the
switch between Venus and Iris drivers" which has this change already so
won't be needed here.

>>
>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> index 5a489917580e..49dba0f50988 100644
>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>> @@ -43,7 +43,9 @@ 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 sc8280xp_data;
>>>  extern const struct iris_platform_data sm8250_data;
>>> +extern const struct iris_platform_data sm8350_data;
>>>  extern const struct iris_platform_data sm8550_data;
>>>  extern const struct iris_platform_data sm8650_data;
>>>  extern const struct iris_platform_data sm8750_data;
>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_gen1.c b/drivers/media/platform/qcom/iris/iris_platform_gen1.c
>>> index df8e6bf9430e..c99ff4d4644d 100644
>>> --- a/drivers/media/platform/qcom/iris/iris_platform_gen1.c
>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_gen1.c
>>> @@ -14,6 +14,7 @@
>>>  #include "iris_instance.h"
>>>  
>>>  #include "iris_platform_sc7280.h"
>>> +#include "iris_platform_sm8350.h"
>>>  
>>>  #define BITRATE_MIN		32000
>>>  #define BITRATE_MAX		160000000
>>> @@ -392,6 +393,61 @@ const struct iris_platform_data sm8250_data = {
>>>  	.enc_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_enc_ip_int_buf_tbl),
>>>  };
>>>  
>>> +const struct iris_platform_data sm8350_data = {
>>> +	.get_instance = iris_hfi_gen1_get_instance,
>>> +	.init_hfi_command_ops = &iris_hfi_gen1_command_ops_init,
>>> +	.init_hfi_response_ops = iris_hfi_gen1_response_ops_init,
>>> +	.get_vpu_buffer_size = iris_vpu_buf_size,
>>> +	.vpu_ops = &iris_vpu2_ops,
>>> +	.set_preset_registers = iris_set_sm8350_preset_registers,
>>> +	.icc_tbl = sm8250_icc_table,
>>> +	.icc_tbl_size = ARRAY_SIZE(sm8250_icc_table),
>>> +	.clk_rst_tbl = sm8350_clk_reset_table,
>>> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8350_clk_reset_table),
>>> +	.bw_tbl_dec = sm8250_bw_table_dec,
>>> +	.bw_tbl_dec_size = ARRAY_SIZE(sm8250_bw_table_dec),
>>> +	.pmdomain_tbl = sm8250_pmdomain_table,
>>> +	.pmdomain_tbl_size = ARRAY_SIZE(sm8250_pmdomain_table),
>>> +	.opp_pd_tbl = sm8250_opp_pd_table,
>>> +	.opp_pd_tbl_size = ARRAY_SIZE(sm8250_opp_pd_table),
>>> +	.clk_tbl = sm8250_clk_table,
>>> +	.clk_tbl_size = ARRAY_SIZE(sm8250_clk_table),
>>> +	.opp_clk_tbl = sm8250_opp_clk_table,
>>> +	/* Upper bound of DMA address range */
>>> +	.dma_mask = 0xe0000000 - 1,
>>> +	.fwname = "qcom/vpu/vpu20_p4.mbn",
>>
>> This firmware is not compatible with SM8350.
>> SM8350 firmware is not released to linux-firmware yet.
> 
> What would be the name for the firmware? The downstream uses vpu20_4v
> here, so, I guess, in upstream we should be using vpu20_p4, but a newer
> version?
> 

Using a newer version won't work as the firmware for SM8250 and SM8350 are
different binaries generated from different firmware source branch.
You can give it a try, but AFAIK it won't work.

>>
>>> +	.pas_id = IRIS_PAS_ID,
>>> +	.inst_iris_fmts = platform_fmts_sm8250_dec,
>>> +	.inst_iris_fmts_size = ARRAY_SIZE(platform_fmts_sm8250_dec),
>>> +	.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),
>>> +	.inst_fw_caps_enc = inst_fw_cap_sm8250_enc,
>>> +	.inst_fw_caps_enc_size = ARRAY_SIZE(inst_fw_cap_sm8250_enc),
>>> +	.tz_cp_config_data = tz_cp_config_sm8250,
>>> +	.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8250),
>>> +	.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
>>> +	.num_vpp_pipe = 4,
>>> +	.max_session_count = 16,
>>> +	.max_core_mbpf = NUM_MBS_8K,
>>> +	.max_core_mbps = ((7680 * 4320) / 256) * 60,
>>> +	.dec_input_config_params_default =
>>> +		sm8250_vdec_input_config_param_default,
>>> +	.dec_input_config_params_default_size =
>>> +		ARRAY_SIZE(sm8250_vdec_input_config_param_default),
>>> +	.enc_input_config_params = sm8250_venc_input_config_param,
>>> +	.enc_input_config_params_size =
>>> +		ARRAY_SIZE(sm8250_venc_input_config_param),
>>> +
>>> +	.dec_ip_int_buf_tbl = sm8250_dec_ip_int_buf_tbl,
>>> +	.dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_dec_ip_int_buf_tbl),
>>> +	.dec_op_int_buf_tbl = sm8250_dec_op_int_buf_tbl,
>>> +	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8250_dec_op_int_buf_tbl),
>>> +
>>> +	.enc_ip_int_buf_tbl = sm8250_enc_ip_int_buf_tbl,
>>> +	.enc_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_enc_ip_int_buf_tbl),
>>> +};
>>> +
>>>  const struct iris_platform_data sc7280_data = {
>>>  	.get_instance = iris_hfi_gen1_get_instance,
>>>  	.init_hfi_command_ops = &iris_hfi_gen1_command_ops_init,
>>> @@ -446,3 +502,58 @@ const struct iris_platform_data sc7280_data = {
>>>  	.enc_ip_int_buf_tbl = sm8250_enc_ip_int_buf_tbl,
>>>  	.enc_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_enc_ip_int_buf_tbl),
>>>  };
>>> +
>>> +const struct iris_platform_data sc8280xp_data = {
>>> +	.get_instance = iris_hfi_gen1_get_instance,
>>> +	.init_hfi_command_ops = &iris_hfi_gen1_command_ops_init,
>>> +	.init_hfi_response_ops = iris_hfi_gen1_response_ops_init,
>>> +	.get_vpu_buffer_size = iris_vpu_buf_size,
>>> +	.vpu_ops = &iris_vpu2_ops,
>>> +	.set_preset_registers = iris_set_sm8350_preset_registers,
>>> +	.icc_tbl = sm8250_icc_table,
>>> +	.icc_tbl_size = ARRAY_SIZE(sm8250_icc_table),
>>> +	.clk_rst_tbl = sm8350_clk_reset_table,
>>> +	.clk_rst_tbl_size = ARRAY_SIZE(sm8350_clk_reset_table),
>>> +	.bw_tbl_dec = sm8250_bw_table_dec,
>>> +	.bw_tbl_dec_size = ARRAY_SIZE(sm8250_bw_table_dec),
>>> +	.pmdomain_tbl = sm8250_pmdomain_table,
>>> +	.pmdomain_tbl_size = ARRAY_SIZE(sm8250_pmdomain_table),
>>> +	.opp_pd_tbl = sm8250_opp_pd_table,
>>> +	.opp_pd_tbl_size = ARRAY_SIZE(sm8250_opp_pd_table),
>>> +	.clk_tbl = sm8250_clk_table,
>>> +	.clk_tbl_size = ARRAY_SIZE(sm8250_clk_table),
>>> +	.opp_clk_tbl = sm8250_opp_clk_table,
>>> +	/* Upper bound of DMA address range */
>>> +	.dma_mask = 0xe0000000 - 1,
>>> +	.fwname = "qcom/vpu/vpu20_p2.mbn",
>>
>> this firmware doesn't exist on linux-firmware.
> 
> It was based on the assumption of having 2 pipes. If Iris here has 2
> pipes, then probably we should still point to vpu20_p4.mbn?
> 

SC8280XP also uses the Iris2 4‑pipe configuration, though its firmware
comes from a different source branch compared to SM8250 and SM8350. This
means we have multiple firmwares with identical VPU and pipe configurations
but different origins. Could you propose a suitable naming scheme that can
differentiate such firmware?

>>
>>> +	.pas_id = IRIS_PAS_ID,
>>> +	.inst_iris_fmts = platform_fmts_sm8250_dec,
>>> +	.inst_iris_fmts_size = ARRAY_SIZE(platform_fmts_sm8250_dec),
>>> +	.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),
>>> +	.inst_fw_caps_enc = inst_fw_cap_sm8250_enc,
>>> +	.inst_fw_caps_enc_size = ARRAY_SIZE(inst_fw_cap_sm8250_enc),
>>> +	.tz_cp_config_data = tz_cp_config_sm8250,
>>> +	.tz_cp_config_data_size = ARRAY_SIZE(tz_cp_config_sm8250),
>>> +	.hw_response_timeout = HW_RESPONSE_TIMEOUT_VALUE,
>>> +	.num_vpp_pipe = 2,
>>
>> sc8280xp is IRIS2 4 Pipe.
> 
> Ack
> 
>>
>>> +	.max_session_count = 16,
>>> +	.max_core_mbpf = NUM_MBS_8K,
>>> +	.max_core_mbps = ((7680 * 4320) / 256) * 60,
>>> +	.dec_input_config_params_default =
>>> +		sm8250_vdec_input_config_param_default,
>>> +	.dec_input_config_params_default_size =
>>> +		ARRAY_SIZE(sm8250_vdec_input_config_param_default),
>>> +	.enc_input_config_params = sm8250_venc_input_config_param,
>>> +	.enc_input_config_params_size =
>>> +		ARRAY_SIZE(sm8250_venc_input_config_param),
>>> +
>>> +	.dec_ip_int_buf_tbl = sm8250_dec_ip_int_buf_tbl,
>>> +	.dec_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_dec_ip_int_buf_tbl),
>>> +	.dec_op_int_buf_tbl = sm8250_dec_op_int_buf_tbl,
>>> +	.dec_op_int_buf_tbl_size = ARRAY_SIZE(sm8250_dec_op_int_buf_tbl),
>>> +
>>> +	.enc_ip_int_buf_tbl = sm8250_enc_ip_int_buf_tbl,
>>> +	.enc_ip_int_buf_tbl_size = ARRAY_SIZE(sm8250_enc_ip_int_buf_tbl),
>>> +};
>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_sm8350.h b/drivers/media/platform/qcom/iris/iris_platform_sm8350.h
>>> new file mode 100644
>>> index 000000000000..74cf5ea2359a
>>> --- /dev/null
>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_sm8350.h
>>> @@ -0,0 +1,20 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (c) Qualcomm Technologies, Inc. and/or its subsidiaries.
>>> + */
>>> +
>>> +#ifndef __IRIS_PLATFORM_SM8350_H__
>>> +#define __IRIS_PLATFORM_SM8350_H__
>>> +
>>> +static void iris_set_sm8350_preset_registers(struct iris_core *core)
>>> +{
>>> +	u32 val;
>>> +
>>> +	val = readl(core->reg_base + 0xb0088);
>>> +	val &= ~0x11;
>>> +	writel(val, core->reg_base + 0xb0088);
>>> +}
>>
>> you can reuse this from SM8250. That would work.
> 
> Hmm, downstream driver was explicit about clearing only these two bits.
> Is it really fine to clear all the bits?
> 

Yes it is. We are doing the same for other SOCs as well.

Thanks,
Dikshita

>>
>> Thanks,
>> Dikshita
>>
>>> +
>>> +static const char * const sm8350_clk_reset_table[] = { "core" };
>>> +
>>> +#endif
> 

Powered by blists - more mailing lists