Re: [PATCH v2 5/7] media: iris: add helper to select context bank device

[Vikash Garodia <vikash.garodia@oss.qualcomm.com>]

On 3/4/2026 3:57 AM, Dmitry Baryshkov wrote:
> On Wed, Mar 04, 2026 at 12:46:27AM +0530, Vikash Garodia wrote:
>>
>>
>> On 2/28/2026 1:57 AM, Dmitry Baryshkov wrote:
>>> On Fri, Feb 27, 2026 at 07:41:21PM +0530, Vikash Garodia wrote:
>>>> Depending on the buffer type (input, output, internal and interface
>>>> queues), associated context bank is selected, if available. Fallback to
>>>> parent device for backward compatibility.
>>>>
>>>> Co-developed-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>>>> Signed-off-by: Vishnu Reddy <busanna.reddy@oss.qualcomm.com>
>>>> Signed-off-by: Vikash Garodia <vikash.garodia@oss.qualcomm.com>
>>>> ---
>>>>    drivers/media/platform/qcom/iris/iris_buffer.c    |  7 +--
>>>>    drivers/media/platform/qcom/iris/iris_buffer.h    |  2 +
>>>>    drivers/media/platform/qcom/iris/iris_hfi_queue.c | 16 +++---
>>>>    drivers/media/platform/qcom/iris/iris_resources.c | 60 +++++++++++++++++++++++
>>>>    drivers/media/platform/qcom/iris/iris_resources.h |  2 +
>>>>    drivers/media/platform/qcom/iris/iris_vidc.c      |  4 +-
>>>>    6 files changed, 79 insertions(+), 12 deletions(-)
>>>>
>>>> @@ -177,3 +178,62 @@ int iris_create_child_device_and_map(struct iris_core *core, struct iris_context
>>>>    	return 0;
>>>>    }
>>>> +
>>>> +static enum iris_buffer_region iris_get_region(struct iris_inst *inst,
>>>> +					       enum iris_buffer_type buffer_type)
>>>> +{
>>>> +	switch (buffer_type) {
>>>> +	case BUF_INPUT:
>>>> +		if (inst && inst->domain == ENCODER)
>>>
>>> Can inst be NULL here?
>>
>> during queues init/deinit, instances are not created.
> 
> Is this function being called during queues init?

yes, via iris_get_cb_dev()

> 
>>
>>>
>>>> +			return IRIS_PIXEL_REGION;
>>>> +		else if (inst && inst->domain == DECODER)
>>>> +			return IRIS_BITSTREAM_REGION;
>>>
>>> Are there any other possibilities than encoder and decoder?
>>
>> will simplify it as
>>
>> if (inst) {
>>      if (inst->domain == ENCODER)
>>          return IRIS_PIXEL_REGION;
>>      else
>>          return IRIS_BITSTREAM_REGION;
>> }
>>>
>>>> +		break;
>>>> +	case BUF_OUTPUT:
>>>> +		if (inst && inst->domain == ENCODER)
>>>> +			return IRIS_BITSTREAM_REGION;
>>>> +		else if (inst && inst->domain == DECODER)
>>>> +			return IRIS_PIXEL_REGION;
>>>> +		break;
>>>> +	case BUF_BIN:
>>>> +		return IRIS_BITSTREAM_REGION;
>>>> +	case BUF_DPB:
>>>> +	case BUF_PARTIAL:
>>>> +	case BUF_SCRATCH_2:
>>>> +	case BUF_VPSS:
>>>> +		return IRIS_PIXEL_REGION;
>>>> +	case BUF_ARP:
>>>> +	case BUF_COMV:
>>>> +	case BUF_HFI_QUEUE:
>>>> +	case BUF_LINE:
>>>> +	case BUF_NON_COMV:
>>>> +	case BUF_PERSIST:
>>>> +		return IRIS_NON_PIXEL_REGION;
>>>> +	default:
>>>> +		return 0;
>>>
>>> dev_err(dev, "unsupported buffer type %x\n", buffer_type)
>>> return -EINVAL;
>>
>> these are bit fields, returning -EINVAL would still match some bits and can
>> make the logic as true. 0 can be defined as IRIS_UNKNOWN_REGION
> 
> Yes, sounds good.
> 
>>
>>>
>>>> +	}
>>>> +
>>>> +	return 0;
>>>
>>> Drop
>>>
>>
>> Ack
>>
>>>> +}
>>>> +
>>>> +struct device *iris_get_cb_dev(struct iris_core *core, struct iris_inst *inst,
>>>> +			       enum iris_buffer_type buffer_type)
>>>> +{
>>>> +	enum iris_buffer_region region;
>>>> +	struct device *dev = NULL;
>>>> +	int i;
>>>> +
>>>> +	region = iris_get_region(inst, buffer_type);
>>>> +
>>>> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
>>>> +		if (core->iris_platform_data->cb_data[i].region & region) {
>>>> +			dev = core->iris_platform_data->cb_data[i].dev;
>>>> +			break;
>>>> +		}
>>>
>>> You really seem to overcomplicate things. Replace array search with the
>>> indexed array access. Much easier and much better.
>>>
>>> enum iris_buffer_region {
>>> 	IRIS_PIXEL_REGION,
>>> 	IRIS_BITSTREAM_REGION,
>>> 	IRIS_NON_PIXEL_REGION,
>>> 	// add more when necessary
>>> 	IRIS_NUM_REGIONS,
>>> };
>>>
>>> struct iris_core {
>>> 	struct iris_cb_device cb_devices[IRIS_NUM_REGIONS];
>>> };
>>>
>>> region = iris_get_region(inst, buffer_type);
>>> dev = core->cb_devices[region];
>>
>> all the regions may/may not be present in all SOC
> 
> You can check for dev != NULL afterwards.

with one CB to multiple region mapping, this logic would not work.

Regards,
Vikash

> 
>