Re: [PATCH v8 11/11] media: qcom: iris: extract firmware description data

[Dmitry Baryshkov]

On Mon, Mar 23, 2026 at 12:50:32PM +0530, Dikshita Agarwal wrote:
> 
> 
> On 3/19/2026 11:29 AM, Dmitry Baryshkov wrote:
> > In preparation to adding support for several firmware revisions to be
> > used for a platform, extract the firmware description data. It
> > incorporates firmware name, HFI ops and buffer requirements of the
> > particular firmware build.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/media/platform/qcom/iris/iris_buffer.c     |  2 +-
> >  drivers/media/platform/qcom/iris/iris_core.h       |  1 +
> >  drivers/media/platform/qcom/iris/iris_firmware.c   |  2 +-
> >  .../platform/qcom/iris/iris_hfi_gen1_command.c     |  2 +-
> >  .../platform/qcom/iris/iris_platform_common.h      | 15 ++++----
> >  .../media/platform/qcom/iris/iris_platform_vpu2.c  | 20 +++++++----
> >  .../media/platform/qcom/iris/iris_platform_vpu3x.c | 41 +++++++++++++++-------
> >  drivers/media/platform/qcom/iris/iris_probe.c      |  3 +-
> >  8 files changed, 57 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/media/platform/qcom/iris/iris_buffer.c b/drivers/media/platform/qcom/iris/iris_buffer.c
> > index fbe136360aa1..ef7f6f931557 100644
> > --- a/drivers/media/platform/qcom/iris/iris_buffer.c
> > +++ b/drivers/media/platform/qcom/iris/iris_buffer.c
> > @@ -295,7 +295,7 @@ static void iris_fill_internal_buf_info(struct iris_inst *inst,
> >  {
> >  	struct iris_buffers *buffers = &inst->buffers[buffer_type];
> >  
> > -	buffers->size = inst->core->iris_platform_data->get_vpu_buffer_size(inst, buffer_type);
> > +	buffers->size = inst->core->iris_firmware_desc->get_vpu_buffer_size(inst, buffer_type);
> >  	buffers->min_count = iris_vpu_buf_count(inst, buffer_type);
> >  }
> >  
> > diff --git a/drivers/media/platform/qcom/iris/iris_core.h b/drivers/media/platform/qcom/iris/iris_core.h
> > index e0ca245c8c63..7f36eb65dcbf 100644
> > --- a/drivers/media/platform/qcom/iris/iris_core.h
> > +++ b/drivers/media/platform/qcom/iris/iris_core.h
> > @@ -99,6 +99,7 @@ struct iris_core {
> >  	struct reset_control_bulk_data		*controller_resets;
> >  	const struct iris_platform_data		*iris_platform_data;
> >  	const struct iris_firmware_data		*iris_firmware_data;
> > +	const struct iris_firmware_desc		*iris_firmware_desc;
> 
> Missing documentation.

Will fix in v10.

> 
> >  	const struct qcom_ubwc_cfg_data		*ubwc_cfg;
> >  	enum iris_core_state			state;
> >  	dma_addr_t				iface_q_table_daddr;
> 
> <snip>
> 
> > index dd87504c2e67..d36f0c0e785b 100644
> > --- a/drivers/media/platform/qcom/iris/iris_probe.c
> > +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> > @@ -251,7 +251,8 @@ static int iris_probe(struct platform_device *pdev)
> >  		return core->irq;
> >  
> >  	core->iris_platform_data = of_device_get_match_data(core->dev);
> > -	core->iris_firmware_data = core->iris_platform_data->firmware_data;
> > +	core->iris_firmware_desc = core->iris_platform_data->firmware_desc;
> 
> How will iris_firmware_desc be selected once a SoC supports both Gen1 and
> Gen2 firmware?
> Today it’s fixed in platform_data, but eventually probe would need to
> choose between firmware_desc_gen1 / firmware_desc_gen2 based on the generation.

Yes. Looking at the past firmware releases, we might need to implement
more than one "description", e.g. when upgrading between firmware
branches would also cause extra side effects (additional codecs, changes
to the buffer sizes, etc.).  But... I'd really prefer to do that in
steps and only when required.

I'd leave concurrent gen1 / gen2 support that to be implemented in your
series, if you consider this interface to be worthwhile. Currently every
platform supports only one firmware "description".

-- 
With best wishes
Dmitry