Re: [PATCH v3 2/7] media: iris: introduce SM8350 and SC8280XP support

[Dikshita Agarwal]

On 2/5/2026 4:24 PM, Dmitry Baryshkov wrote:
> On Thu, Feb 05, 2026 at 02:40:39PM +0530, Dikshita Agarwal wrote:
>>
>>
>> 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@xxxxxxxxxxxxxxxx>
>>>>> ---
>>>>>  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(-)
>>>>>
>>>>> @@ -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.
> 
> Ugh...
> 
>>>>> +	.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?
> 
> Can we have a single binary that works on all Iris2 4-pipe cores?

That is not possible as it is older VPU.
For newer ones, the plan is have single source branch for same VPU to avoid
such issues in future.

> Or are there any differences between Iris2 on SM8250 / SM8350 /
> SC8280XP? Are they stil vpu20_something or should we use different VPU
> versions in the firmware name?

I think we can keep vpu20_4p_* with some way to different among SM8250,
SM8350, SC8280XP.

> 
>>>>> 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.
> 
> Wouldn't this also ungate / start the second core?
>