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

[Vikash Garodia]

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@xxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Vishnu Reddy <busanna.reddy@xxxxxxxxxxxxxxxx>
> > > > Signed-off-by: Vikash Garodia <vikash.garodia@xxxxxxxxxxxxxxxx>
> > > > ---
> > > >    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

>