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

[Dmitry Baryshkov]

On Wed, Mar 04, 2026 at 08:59:24PM +0530, Vikash Garodia wrote:
> 
> 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()

I think this is a part of overcomplication. queue init uses
BUF_HFI_QUEUE, which always maps to NON_PIXEL. If you remove all
indirection and device lists, you can access necessary device directly.
BUF_HFI_QUEUE looks like an extra entity created just to get the device.

> 
> > 
> > > 
> > > > 
> > > > > +}
> > > > > +
> > > > > +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.

I'm not sure I follow. We always need only one CB device and we can
always access it (or check that it's NULL).

-- 
With best wishes
Dmitry