Re: [PATCH v7 7/8] media: qcom: iris: split firmware_data from raw platform data

[Dmitry Baryshkov]

On Wed, Mar 18, 2026 at 04:12:25PM +0530, Dikshita Agarwal wrote:
> 
> 
> On 3/18/2026 4:10 PM, Dmitry Baryshkov wrote:
> > On Wed, Mar 18, 2026 at 04:03:07PM +0530, Dikshita Agarwal wrote:
> >>
> >>
> >> On 3/18/2026 3:59 PM, Dmitry Baryshkov wrote:
> >>> On Wed, Mar 18, 2026 at 02:35:30PM +0530, Dikshita Agarwal wrote:
> >>>>
> >>>>
> >>>> On 3/17/2026 12:24 AM, Dmitry Baryshkov wrote:
> >>>>> Having firmware-related fields in platform data results in the tying
> >>>>> platform data to the HFI firmware data rather than the actual hardware.
> >>>>> For example, SM8450 uses Gen2 firmware, so currently its platform data
> >>>>> should be placed next to the other gen2 platforms, although it has the
> >>>>> VPU2.0 core, similar to the one found on SM8250 and SC7280 and so the
> >>>>> hardware-specific platform data is also close to those devices.
> >>>>>
> >>>>> Split firmware data to a separate struct, separating hardware-related
> >>>>> data from the firmware interfaces.
> >>>>>
> >>>>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> >>>>> ---
> >>>>>  drivers/media/platform/qcom/iris/iris_buffer.c     |  82 +++----
> >>>>>  drivers/media/platform/qcom/iris/iris_core.h       |   2 +
> >>>>>  drivers/media/platform/qcom/iris/iris_ctrls.c      |   8 +-
> >>>>>  .../platform/qcom/iris/iris_hfi_gen1_command.c     |   8 +-
> >>>>>  .../platform/qcom/iris/iris_hfi_gen2_command.c     |  66 +++---
> >>>>>  .../platform/qcom/iris/iris_platform_common.h      |  82 ++++---
> >>>>>  .../media/platform/qcom/iris/iris_platform_gen1.c  |  67 +++---
> >>>>>  .../media/platform/qcom/iris/iris_platform_gen2.c  | 246 ++++-----------------
> >>>>>  drivers/media/platform/qcom/iris/iris_probe.c      |   3 +-
> >>>>>  drivers/media/platform/qcom/iris/iris_vidc.c       |  10 +-
> >>>>>  10 files changed, 205 insertions(+), 369 deletions(-)
> >>>>>
> >>>>
> >>>> <snip>
> >>>>
> >>>>> -struct iris_platform_data {
> >>>>> +struct iris_firmware_data {
> >>>>>  	void (*init_hfi_ops)(struct iris_core *core);
> >>>>> -	u32 (*get_vpu_buffer_size)(struct iris_inst *inst, enum iris_buffer_type buffer_type);
> >>>>> -	const struct vpu_ops *vpu_ops;
> >>>>> -	const struct icc_info *icc_tbl;
> >>>>> -	unsigned int icc_tbl_size;
> >>>>> -	const struct bw_info *bw_tbl_dec;
> >>>>> -	unsigned int bw_tbl_dec_size;
> >>>>> -	const char * const *pmdomain_tbl;
> >>>>> -	unsigned int pmdomain_tbl_size;
> >>>>> -	const char * const *opp_pd_tbl;
> >>>>> -	unsigned int opp_pd_tbl_size;
> >>>>> -	const struct platform_clk_data *clk_tbl;
> >>>>> -	const char * const *opp_clk_tbl;
> >>>>> -	unsigned int clk_tbl_size;
> >>>>> -	const char * const *clk_rst_tbl;
> >>>>> -	unsigned int clk_rst_tbl_size;
> >>>>> -	const char * const *controller_rst_tbl;
> >>>>> -	unsigned int controller_rst_tbl_size;
> >>>>> -	u64 dma_mask;
> >>>>> -	const char *fwname;
> >>>>> -	struct iris_fmt *inst_iris_fmts;
> >>>>> -	u32 inst_iris_fmts_size;
> >>>>> -	struct platform_inst_caps *inst_caps;
> >>>>> +
> >>>>>  	const struct platform_inst_fw_cap *inst_fw_caps_dec;
> >>>>>  	u32 inst_fw_caps_dec_size;
> >>>>>  	const struct platform_inst_fw_cap *inst_fw_caps_enc;
> >>>>>  	u32 inst_fw_caps_enc_size;
> >>>>> -	const struct tz_cp_config *tz_cp_config_data;
> >>>>> -	u32 tz_cp_config_data_size;
> >>>>> -	u32 core_arch;
> >>>>
> >>>> This can also be tied to firmware data as this is required to be set for
> >>>> Gen2 only and is 0 for Gen1.
> >>>
> >>> Will it differ between platforms?
> >>
> >> Nope, same for all platforms with HFI Gen2.
> > 
> > Then it should be removed from the data at all.
> 
> Yes, I was also thinking the same but this should be set for HFI Gen2 and
> not for Gen1, how do we handle this then?
> 
> That's why asking to keep in firmware data.

Ack, I will move it to firmware_data.

-- 
With best wishes
Dmitry