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

[Dikshita Agarwal]

On 3/23/2026 1:03 PM, Dmitry Baryshkov wrote:
> 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".

Ack, Thanks.

Regards,
Dikshita
>