[v2] media: iris: add support for kaanapali platform

[PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Vikash Garodia 1 month, 1 week ago

Introduce different context banks(CB) and the associated buffer region.
Different stream IDs from VPU would be associated to one of these CB.
Multiple CBs are needed to increase the IOVA for the video usecases like
higher concurrent sessions.

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>
---
 .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
 drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
 drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
 drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
 4 files changed, 111 insertions(+), 4 deletions(-)

diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
--- a/drivers/media/platform/qcom/iris/iris_platform_common.h
+++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
@@ -204,6 +204,22 @@ struct icc_vote_data {
 	u32 fps;
 };
 
+enum iris_buffer_region {
+	IRIS_BITSTREAM_REGION		= BIT(0),
+	IRIS_NON_PIXEL_REGION		= BIT(1),
+	IRIS_PIXEL_REGION		= BIT(2),
+	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
+	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
+	IRIS_SECURE_PIXEL_REGION	= BIT(5),
+};
+
+struct iris_context_bank {
+	struct device *dev;
+	const char *name;
+	const u32 f_id;
+	const enum iris_buffer_region region;
+};
+
 enum platform_pm_domain_type {
 	IRIS_CTRL_POWER_DOMAIN,
 	IRIS_HW_POWER_DOMAIN,
@@ -246,6 +262,8 @@ struct iris_platform_data {
 	u32 inst_fw_caps_enc_size;
 	const struct tz_cp_config *tz_cp_config_data;
 	u32 tz_cp_config_data_size;
+	struct iris_context_bank *cb_data;
+	u32 cb_data_size;
 	u32 core_arch;
 	u32 hw_response_timeout;
 	struct ubwc_config_data *ubwc_config;
diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
index ddaacda523ecb9990af0dd0640196223fbcc2cab..557adb038328a75510591d91569819abc0b7b1c9 100644
--- a/drivers/media/platform/qcom/iris/iris_probe.c
+++ b/drivers/media/platform/qcom/iris/iris_probe.c
@@ -123,6 +123,49 @@ static int iris_init_resets(struct iris_core *core)
 				     core->iris_platform_data->controller_rst_tbl_size);
 }
 
+static void iris_destroy_child_device(struct iris_context_bank *cb)
+{
+	struct device *dev = cb->dev;
+
+	if (dev)
+		device_unregister(dev);
+
+	cb->dev = NULL;
+}
+
+static void iris_deinit_context_bank_devices(struct iris_core *core)
+{
+	struct iris_context_bank *cb;
+	int i;
+
+	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
+		cb = &core->iris_platform_data->cb_data[i];
+		iris_destroy_child_device(cb);
+	}
+}
+
+static int iris_init_context_bank_devices(struct iris_core *core)
+{
+	struct iris_context_bank *cb;
+	int ret, i;
+
+	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
+		cb = &core->iris_platform_data->cb_data[i];
+
+		ret = iris_create_child_device_and_map(core, cb);
+		if (ret)
+			goto err_deinit_cb;
+	}
+
+	return 0;
+
+err_deinit_cb:
+	while (i-- > 0)
+		iris_destroy_child_device(&core->iris_platform_data->cb_data[i]);
+
+	return ret;
+}
+
 static int iris_init_resources(struct iris_core *core)
 {
 	int ret;
@@ -193,6 +236,7 @@ static void iris_remove(struct platform_device *pdev)
 		return;
 
 	iris_core_deinit(core);
+	iris_deinit_context_bank_devices(core);
 
 	video_unregister_device(core->vdev_dec);
 	video_unregister_device(core->vdev_enc);
@@ -275,12 +319,18 @@ static int iris_probe(struct platform_device *pdev)
 
 	platform_set_drvdata(pdev, core);
 
-	dma_mask = core->iris_platform_data->dma_mask;
-
-	ret = dma_set_mask_and_coherent(dev, dma_mask);
+	ret = iris_init_context_bank_devices(core);
 	if (ret)
 		goto err_vdev_unreg_enc;
 
+	dma_mask = core->iris_platform_data->dma_mask;
+
+	if (device_iommu_mapped(core->dev)) {
+		ret = dma_set_mask_and_coherent(core->dev, dma_mask);
+		if (ret)
+			goto err_deinit_cb;
+	}
+
 	dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
 	dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
 
@@ -288,10 +338,12 @@ static int iris_probe(struct platform_device *pdev)
 	pm_runtime_use_autosuspend(core->dev);
 	ret = devm_pm_runtime_enable(core->dev);
 	if (ret)
-		goto err_vdev_unreg_enc;
+		goto err_deinit_cb;
 
 	return 0;
 
+err_deinit_cb:
+	iris_deinit_context_bank_devices(core);
 err_vdev_unreg_enc:
 	video_unregister_device(core->vdev_enc);
 err_vdev_unreg_dec:
diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
index 773f6548370a257b8ae7332242544266cbbd61a9..be58e8620086d0f82c2c2bda29247483f5c56d79 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.c
+++ b/drivers/media/platform/qcom/iris/iris_resources.c
@@ -6,6 +6,7 @@
 #include <linux/clk.h>
 #include <linux/devfreq.h>
 #include <linux/interconnect.h>
+#include <linux/iris_vpu_bus.h>
 #include <linux/pm_domain.h>
 #include <linux/pm_opp.h>
 #include <linux/pm_runtime.h>
@@ -141,3 +142,38 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
 
 	return 0;
 }
+
+static void iris_device_release(struct device *dev)
+{
+	dev_set_drvdata(dev, NULL);
+	kfree(dev);
+}
+
+int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb)
+{
+	struct device *dev;
+	int ret;
+
+	dev = kzalloc_obj(*dev);
+	if (!dev)
+		return -ENOMEM;
+
+	dev->release = iris_device_release;
+	dev->bus = &iris_vpu_bus_type;
+	dev->parent = core->dev;
+	dev->coherent_dma_mask = core->iris_platform_data->dma_mask;
+	dev->dma_mask = &dev->coherent_dma_mask;
+
+	dev_set_name(dev, "%s", cb->name);
+	dev_set_drvdata(dev, cb);
+
+	ret = device_register(dev);
+	if (ret) {
+		put_device(dev);
+		return ret;
+	}
+
+	cb->dev = dev;
+
+	return 0;
+}
diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
index 6bfbd2dc6db095ec05e53c894e048285f82446c6..b7efe15facb203eea9ae13d5f0abdcc2ea718b4d 100644
--- a/drivers/media/platform/qcom/iris/iris_resources.h
+++ b/drivers/media/platform/qcom/iris/iris_resources.h
@@ -15,5 +15,6 @@ int iris_unset_icc_bw(struct iris_core *core);
 int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw);
 int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type clk_type);
 int iris_prepare_enable_clock(struct iris_core *core, enum platform_clk_type clk_type);
+int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb);
 
 #endif

-- 
2.34.1

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Dmitry Baryshkov 1 month, 1 week ago

On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
> Introduce different context banks(CB) and the associated buffer region.
> Different stream IDs from VPU would be associated to one of these CB.
> Multiple CBs are needed to increase the IOVA for the video usecases like
> higher concurrent sessions.
> 
> 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>
> ---
>  .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
>  drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
>  drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
>  drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
>  4 files changed, 111 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -204,6 +204,22 @@ struct icc_vote_data {
>  	u32 fps;
>  };
>  
> +enum iris_buffer_region {
> +	IRIS_BITSTREAM_REGION		= BIT(0),
> +	IRIS_NON_PIXEL_REGION		= BIT(1),
> +	IRIS_PIXEL_REGION		= BIT(2),
> +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
> +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
> +	IRIS_SECURE_PIXEL_REGION	= BIT(5),

Can a context bank belong to multiple regions at the same time?

> +};
> +
> +struct iris_context_bank {
> +	struct device *dev;

Separate data and the actual device. Define a wrapper around struct
device for the actual runtime usage.

> +	const char *name;
> +	const u32 f_id;
> +	const enum iris_buffer_region region;
> +};
> +
>  enum platform_pm_domain_type {
>  	IRIS_CTRL_POWER_DOMAIN,
>  	IRIS_HW_POWER_DOMAIN,
> @@ -246,6 +262,8 @@ struct iris_platform_data {
>  	u32 inst_fw_caps_enc_size;
>  	const struct tz_cp_config *tz_cp_config_data;
>  	u32 tz_cp_config_data_size;
> +	struct iris_context_bank *cb_data;
> +	u32 cb_data_size;

Do they differ from platform to platform? Mark them as const, it should
be data only.

>  	u32 core_arch;
>  	u32 hw_response_timeout;
>  	struct ubwc_config_data *ubwc_config;
> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> index ddaacda523ecb9990af0dd0640196223fbcc2cab..557adb038328a75510591d91569819abc0b7b1c9 100644
> --- a/drivers/media/platform/qcom/iris/iris_probe.c
> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> @@ -123,6 +123,49 @@ static int iris_init_resets(struct iris_core *core)
>  				     core->iris_platform_data->controller_rst_tbl_size);
>  }
>  
> +static void iris_destroy_child_device(struct iris_context_bank *cb)
> +{
> +	struct device *dev = cb->dev;
> +
> +	if (dev)
> +		device_unregister(dev);
> +
> +	cb->dev = NULL;
> +}
> +
> +static void iris_deinit_context_bank_devices(struct iris_core *core)
> +{
> +	struct iris_context_bank *cb;
> +	int i;
> +
> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
> +		cb = &core->iris_platform_data->cb_data[i];
> +		iris_destroy_child_device(cb);
> +	}
> +}
> +
> +static int iris_init_context_bank_devices(struct iris_core *core)
> +{
> +	struct iris_context_bank *cb;
> +	int ret, i;
> +
> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
> +		cb = &core->iris_platform_data->cb_data[i];
> +
> +		ret = iris_create_child_device_and_map(core, cb);
> +		if (ret)
> +			goto err_deinit_cb;
> +	}
> +
> +	return 0;
> +
> +err_deinit_cb:
> +	while (i-- > 0)
> +		iris_destroy_child_device(&core->iris_platform_data->cb_data[i]);
> +
> +	return ret;
> +}
> +
>  static int iris_init_resources(struct iris_core *core)
>  {
>  	int ret;
> @@ -193,6 +236,7 @@ static void iris_remove(struct platform_device *pdev)
>  		return;
>  
>  	iris_core_deinit(core);
> +	iris_deinit_context_bank_devices(core);
>  
>  	video_unregister_device(core->vdev_dec);
>  	video_unregister_device(core->vdev_enc);
> @@ -275,12 +319,18 @@ static int iris_probe(struct platform_device *pdev)
>  
>  	platform_set_drvdata(pdev, core);
>  
> -	dma_mask = core->iris_platform_data->dma_mask;
> -
> -	ret = dma_set_mask_and_coherent(dev, dma_mask);
> +	ret = iris_init_context_bank_devices(core);
>  	if (ret)
>  		goto err_vdev_unreg_enc;
>  
> +	dma_mask = core->iris_platform_data->dma_mask;
> +
> +	if (device_iommu_mapped(core->dev)) {
> +		ret = dma_set_mask_and_coherent(core->dev, dma_mask);
> +		if (ret)
> +			goto err_deinit_cb;
> +	}
> +
>  	dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>  	dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
>  
> @@ -288,10 +338,12 @@ static int iris_probe(struct platform_device *pdev)
>  	pm_runtime_use_autosuspend(core->dev);
>  	ret = devm_pm_runtime_enable(core->dev);
>  	if (ret)
> -		goto err_vdev_unreg_enc;
> +		goto err_deinit_cb;
>  
>  	return 0;
>  
> +err_deinit_cb:
> +	iris_deinit_context_bank_devices(core);
>  err_vdev_unreg_enc:
>  	video_unregister_device(core->vdev_enc);
>  err_vdev_unreg_dec:
> diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
> index 773f6548370a257b8ae7332242544266cbbd61a9..be58e8620086d0f82c2c2bda29247483f5c56d79 100644
> --- a/drivers/media/platform/qcom/iris/iris_resources.c
> +++ b/drivers/media/platform/qcom/iris/iris_resources.c
> @@ -6,6 +6,7 @@
>  #include <linux/clk.h>
>  #include <linux/devfreq.h>
>  #include <linux/interconnect.h>
> +#include <linux/iris_vpu_bus.h>
>  #include <linux/pm_domain.h>
>  #include <linux/pm_opp.h>
>  #include <linux/pm_runtime.h>
> @@ -141,3 +142,38 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
>  
>  	return 0;
>  }
> +
> +static void iris_device_release(struct device *dev)
> +{
> +	dev_set_drvdata(dev, NULL);
> +	kfree(dev);
> +}
> +
> +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb)
> +{
> +	struct device *dev;
> +	int ret;
> +
> +	dev = kzalloc_obj(*dev);
> +	if (!dev)
> +		return -ENOMEM;
> +
> +	dev->release = iris_device_release;
> +	dev->bus = &iris_vpu_bus_type;
> +	dev->parent = core->dev;
> +	dev->coherent_dma_mask = core->iris_platform_data->dma_mask;
> +	dev->dma_mask = &dev->coherent_dma_mask;
> +
> +	dev_set_name(dev, "%s", cb->name);
> +	dev_set_drvdata(dev, cb);
> +
> +	ret = device_register(dev);
> +	if (ret) {
> +		put_device(dev);
> +		return ret;
> +	}
> +
> +	cb->dev = dev;
> +
> +	return 0;
> +}
> diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
> index 6bfbd2dc6db095ec05e53c894e048285f82446c6..b7efe15facb203eea9ae13d5f0abdcc2ea718b4d 100644
> --- a/drivers/media/platform/qcom/iris/iris_resources.h
> +++ b/drivers/media/platform/qcom/iris/iris_resources.h
> @@ -15,5 +15,6 @@ int iris_unset_icc_bw(struct iris_core *core);
>  int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw);
>  int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type clk_type);
>  int iris_prepare_enable_clock(struct iris_core *core, enum platform_clk_type clk_type);
> +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb);
>  
>  #endif
> 
> -- 
> 2.34.1
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Vikash Garodia 1 month ago

On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
> On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
>> Introduce different context banks(CB) and the associated buffer region.
>> Different stream IDs from VPU would be associated to one of these CB.
>> Multiple CBs are needed to increase the IOVA for the video usecases like
>> higher concurrent sessions.
>>
>> 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>
>> ---
>>   .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
>>   drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
>>   drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
>>   drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
>>   4 files changed, 111 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>> @@ -204,6 +204,22 @@ struct icc_vote_data {
>>   	u32 fps;
>>   };
>>   
>> +enum iris_buffer_region {
>> +	IRIS_BITSTREAM_REGION		= BIT(0),
>> +	IRIS_NON_PIXEL_REGION		= BIT(1),
>> +	IRIS_PIXEL_REGION		= BIT(2),
>> +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
>> +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
>> +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
> 
> Can a context bank belong to multiple regions at the same time?

yes, they would.

> 
>> +};
>> +
>> +struct iris_context_bank {
>> +	struct device *dev;
> 
> Separate data and the actual device. Define a wrapper around struct
> device for the actual runtime usage.

we still have to store the list of dynamically created device. Name can 
be used to fetch the device from the list, i think the existing approach 
is simpler ?

> 
>> +	const char *name;
>> +	const u32 f_id;
>> +	const enum iris_buffer_region region;
>> +};
>> +
>>   enum platform_pm_domain_type {
>>   	IRIS_CTRL_POWER_DOMAIN,
>>   	IRIS_HW_POWER_DOMAIN,
>> @@ -246,6 +262,8 @@ struct iris_platform_data {
>>   	u32 inst_fw_caps_enc_size;
>>   	const struct tz_cp_config *tz_cp_config_data;
>>   	u32 tz_cp_config_data_size;
>> +	struct iris_context_bank *cb_data;
>> +	u32 cb_data_size;
> 
> Do they differ from platform to platform?
Yes

> Mark them as const, it should be data only.

cb_data_size can be marked as const

> 
>>   	u32 core_arch;
>>   	u32 hw_response_timeout;
>>   	struct ubwc_config_data *ubwc_config;
>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
>> index ddaacda523ecb9990af0dd0640196223fbcc2cab..557adb038328a75510591d91569819abc0b7b1c9 100644
>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>> @@ -123,6 +123,49 @@ static int iris_init_resets(struct iris_core *core)
>>   				     core->iris_platform_data->controller_rst_tbl_size);
>>   }
>>   
>> +static void iris_destroy_child_device(struct iris_context_bank *cb)
>> +{
>> +	struct device *dev = cb->dev;
>> +
>> +	if (dev)
>> +		device_unregister(dev);
>> +
>> +	cb->dev = NULL;
>> +}
>> +
>> +static void iris_deinit_context_bank_devices(struct iris_core *core)
>> +{
>> +	struct iris_context_bank *cb;
>> +	int i;
>> +
>> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
>> +		cb = &core->iris_platform_data->cb_data[i];
>> +		iris_destroy_child_device(cb);
>> +	}
>> +}
>> +
>> +static int iris_init_context_bank_devices(struct iris_core *core)
>> +{
>> +	struct iris_context_bank *cb;
>> +	int ret, i;
>> +
>> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
>> +		cb = &core->iris_platform_data->cb_data[i];
>> +
>> +		ret = iris_create_child_device_and_map(core, cb);
>> +		if (ret)
>> +			goto err_deinit_cb;
>> +	}
>> +
>> +	return 0;
>> +
>> +err_deinit_cb:
>> +	while (i-- > 0)
>> +		iris_destroy_child_device(&core->iris_platform_data->cb_data[i]);
>> +
>> +	return ret;
>> +}
>> +
>>   static int iris_init_resources(struct iris_core *core)
>>   {
>>   	int ret;
>> @@ -193,6 +236,7 @@ static void iris_remove(struct platform_device *pdev)
>>   		return;
>>   
>>   	iris_core_deinit(core);
>> +	iris_deinit_context_bank_devices(core);
>>   
>>   	video_unregister_device(core->vdev_dec);
>>   	video_unregister_device(core->vdev_enc);
>> @@ -275,12 +319,18 @@ static int iris_probe(struct platform_device *pdev)
>>   
>>   	platform_set_drvdata(pdev, core);
>>   
>> -	dma_mask = core->iris_platform_data->dma_mask;
>> -
>> -	ret = dma_set_mask_and_coherent(dev, dma_mask);
>> +	ret = iris_init_context_bank_devices(core);
>>   	if (ret)
>>   		goto err_vdev_unreg_enc;
>>   
>> +	dma_mask = core->iris_platform_data->dma_mask;
>> +
>> +	if (device_iommu_mapped(core->dev)) {
>> +		ret = dma_set_mask_and_coherent(core->dev, dma_mask);
>> +		if (ret)
>> +			goto err_deinit_cb;
>> +	}
>> +
>>   	dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>>   	dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
>>   
>> @@ -288,10 +338,12 @@ static int iris_probe(struct platform_device *pdev)
>>   	pm_runtime_use_autosuspend(core->dev);
>>   	ret = devm_pm_runtime_enable(core->dev);
>>   	if (ret)
>> -		goto err_vdev_unreg_enc;
>> +		goto err_deinit_cb;
>>   
>>   	return 0;
>>   
>> +err_deinit_cb:
>> +	iris_deinit_context_bank_devices(core);
>>   err_vdev_unreg_enc:
>>   	video_unregister_device(core->vdev_enc);
>>   err_vdev_unreg_dec:
>> diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
>> index 773f6548370a257b8ae7332242544266cbbd61a9..be58e8620086d0f82c2c2bda29247483f5c56d79 100644
>> --- a/drivers/media/platform/qcom/iris/iris_resources.c
>> +++ b/drivers/media/platform/qcom/iris/iris_resources.c
>> @@ -6,6 +6,7 @@
>>   #include <linux/clk.h>
>>   #include <linux/devfreq.h>
>>   #include <linux/interconnect.h>
>> +#include <linux/iris_vpu_bus.h>
>>   #include <linux/pm_domain.h>
>>   #include <linux/pm_opp.h>
>>   #include <linux/pm_runtime.h>
>> @@ -141,3 +142,38 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
>>   
>>   	return 0;
>>   }
>> +
>> +static void iris_device_release(struct device *dev)
>> +{
>> +	dev_set_drvdata(dev, NULL);
>> +	kfree(dev);
>> +}
>> +
>> +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb)
>> +{
>> +	struct device *dev;
>> +	int ret;
>> +
>> +	dev = kzalloc_obj(*dev);
>> +	if (!dev)
>> +		return -ENOMEM;
>> +
>> +	dev->release = iris_device_release;
>> +	dev->bus = &iris_vpu_bus_type;
>> +	dev->parent = core->dev;
>> +	dev->coherent_dma_mask = core->iris_platform_data->dma_mask;
>> +	dev->dma_mask = &dev->coherent_dma_mask;
>> +
>> +	dev_set_name(dev, "%s", cb->name);
>> +	dev_set_drvdata(dev, cb);
>> +
>> +	ret = device_register(dev);
>> +	if (ret) {
>> +		put_device(dev);
>> +		return ret;
>> +	}
>> +
>> +	cb->dev = dev;
>> +
>> +	return 0;
>> +}
>> diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
>> index 6bfbd2dc6db095ec05e53c894e048285f82446c6..b7efe15facb203eea9ae13d5f0abdcc2ea718b4d 100644
>> --- a/drivers/media/platform/qcom/iris/iris_resources.h
>> +++ b/drivers/media/platform/qcom/iris/iris_resources.h
>> @@ -15,5 +15,6 @@ int iris_unset_icc_bw(struct iris_core *core);
>>   int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw);
>>   int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type clk_type);
>>   int iris_prepare_enable_clock(struct iris_core *core, enum platform_clk_type clk_type);
>> +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb);
>>   
>>   #endif
>>
>> -- 
>> 2.34.1
>>
>

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Dmitry Baryshkov 1 month ago

On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
> 
> On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
> > On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
> > > Introduce different context banks(CB) and the associated buffer region.
> > > Different stream IDs from VPU would be associated to one of these CB.
> > > Multiple CBs are needed to increase the IOVA for the video usecases like
> > > higher concurrent sessions.
> > > 
> > > 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>
> > > ---
> > >   .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
> > >   drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
> > >   drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
> > >   drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
> > >   4 files changed, 111 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > @@ -204,6 +204,22 @@ struct icc_vote_data {
> > >   	u32 fps;
> > >   };
> > > +enum iris_buffer_region {
> > > +	IRIS_BITSTREAM_REGION		= BIT(0),
> > > +	IRIS_NON_PIXEL_REGION		= BIT(1),
> > > +	IRIS_PIXEL_REGION		= BIT(2),
> > > +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
> > > +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
> > > +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
> > 
> > Can a context bank belong to multiple regions at the same time?
> 
> yes, they would.

How? Each set of CBs is defined by a separate function in the DT. How
can CB belong to multiple regions? Could you please provide an example?

> 
> > 
> > > +};
> > > +
> > > +struct iris_context_bank {
> > > +	struct device *dev;
> > 
> > Separate data and the actual device. Define a wrapper around struct
> > device for the actual runtime usage.
> 
> we still have to store the list of dynamically created device. Name can be
> used to fetch the device from the list, i think the existing approach is
> simpler ?

You don't need a list. You have an array of the size, which is known and
fixed. You have at most 9 functions, which means less than 9 devices.

> 
> > 
> > > +	const char *name;
> > > +	const u32 f_id;
> > > +	const enum iris_buffer_region region;
> > > +};
> > > +
> > >   enum platform_pm_domain_type {
> > >   	IRIS_CTRL_POWER_DOMAIN,
> > >   	IRIS_HW_POWER_DOMAIN,
> > > @@ -246,6 +262,8 @@ struct iris_platform_data {
> > >   	u32 inst_fw_caps_enc_size;
> > >   	const struct tz_cp_config *tz_cp_config_data;
> > >   	u32 tz_cp_config_data_size;
> > > +	struct iris_context_bank *cb_data;
> > > +	u32 cb_data_size;
> > 
> > Do they differ from platform to platform?
> Yes
> 
> > Mark them as const, it should be data only.
> 
> cb_data_size can be marked as const

Why is cb_data non-const?

> 
> > 
> > >   	u32 core_arch;
> > >   	u32 hw_response_timeout;
> > >   	struct ubwc_config_data *ubwc_config;
> > > diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
> > > index ddaacda523ecb9990af0dd0640196223fbcc2cab..557adb038328a75510591d91569819abc0b7b1c9 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_probe.c
> > > +++ b/drivers/media/platform/qcom/iris/iris_probe.c
> > > @@ -123,6 +123,49 @@ static int iris_init_resets(struct iris_core *core)
> > >   				     core->iris_platform_data->controller_rst_tbl_size);
> > >   }
> > > +static void iris_destroy_child_device(struct iris_context_bank *cb)
> > > +{
> > > +	struct device *dev = cb->dev;
> > > +
> > > +	if (dev)
> > > +		device_unregister(dev);
> > > +
> > > +	cb->dev = NULL;
> > > +}
> > > +
> > > +static void iris_deinit_context_bank_devices(struct iris_core *core)
> > > +{
> > > +	struct iris_context_bank *cb;
> > > +	int i;
> > > +
> > > +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
> > > +		cb = &core->iris_platform_data->cb_data[i];
> > > +		iris_destroy_child_device(cb);
> > > +	}
> > > +}
> > > +
> > > +static int iris_init_context_bank_devices(struct iris_core *core)
> > > +{
> > > +	struct iris_context_bank *cb;
> > > +	int ret, i;
> > > +
> > > +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
> > > +		cb = &core->iris_platform_data->cb_data[i];
> > > +
> > > +		ret = iris_create_child_device_and_map(core, cb);
> > > +		if (ret)
> > > +			goto err_deinit_cb;
> > > +	}
> > > +
> > > +	return 0;
> > > +
> > > +err_deinit_cb:
> > > +	while (i-- > 0)
> > > +		iris_destroy_child_device(&core->iris_platform_data->cb_data[i]);
> > > +
> > > +	return ret;
> > > +}
> > > +
> > >   static int iris_init_resources(struct iris_core *core)
> > >   {
> > >   	int ret;
> > > @@ -193,6 +236,7 @@ static void iris_remove(struct platform_device *pdev)
> > >   		return;
> > >   	iris_core_deinit(core);
> > > +	iris_deinit_context_bank_devices(core);
> > >   	video_unregister_device(core->vdev_dec);
> > >   	video_unregister_device(core->vdev_enc);
> > > @@ -275,12 +319,18 @@ static int iris_probe(struct platform_device *pdev)
> > >   	platform_set_drvdata(pdev, core);
> > > -	dma_mask = core->iris_platform_data->dma_mask;
> > > -
> > > -	ret = dma_set_mask_and_coherent(dev, dma_mask);
> > > +	ret = iris_init_context_bank_devices(core);
> > >   	if (ret)
> > >   		goto err_vdev_unreg_enc;
> > > +	dma_mask = core->iris_platform_data->dma_mask;
> > > +
> > > +	if (device_iommu_mapped(core->dev)) {
> > > +		ret = dma_set_mask_and_coherent(core->dev, dma_mask);
> > > +		if (ret)
> > > +			goto err_deinit_cb;
> > > +	}
> > > +
> > >   	dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
> > >   	dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
> > > @@ -288,10 +338,12 @@ static int iris_probe(struct platform_device *pdev)
> > >   	pm_runtime_use_autosuspend(core->dev);
> > >   	ret = devm_pm_runtime_enable(core->dev);
> > >   	if (ret)
> > > -		goto err_vdev_unreg_enc;
> > > +		goto err_deinit_cb;
> > >   	return 0;
> > > +err_deinit_cb:
> > > +	iris_deinit_context_bank_devices(core);
> > >   err_vdev_unreg_enc:
> > >   	video_unregister_device(core->vdev_enc);
> > >   err_vdev_unreg_dec:
> > > diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
> > > index 773f6548370a257b8ae7332242544266cbbd61a9..be58e8620086d0f82c2c2bda29247483f5c56d79 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_resources.c
> > > +++ b/drivers/media/platform/qcom/iris/iris_resources.c
> > > @@ -6,6 +6,7 @@
> > >   #include <linux/clk.h>
> > >   #include <linux/devfreq.h>
> > >   #include <linux/interconnect.h>
> > > +#include <linux/iris_vpu_bus.h>
> > >   #include <linux/pm_domain.h>
> > >   #include <linux/pm_opp.h>
> > >   #include <linux/pm_runtime.h>
> > > @@ -141,3 +142,38 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
> > >   	return 0;
> > >   }
> > > +
> > > +static void iris_device_release(struct device *dev)
> > > +{
> > > +	dev_set_drvdata(dev, NULL);
> > > +	kfree(dev);
> > > +}
> > > +
> > > +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb)
> > > +{
> > > +	struct device *dev;
> > > +	int ret;
> > > +
> > > +	dev = kzalloc_obj(*dev);
> > > +	if (!dev)
> > > +		return -ENOMEM;
> > > +
> > > +	dev->release = iris_device_release;
> > > +	dev->bus = &iris_vpu_bus_type;
> > > +	dev->parent = core->dev;
> > > +	dev->coherent_dma_mask = core->iris_platform_data->dma_mask;
> > > +	dev->dma_mask = &dev->coherent_dma_mask;
> > > +
> > > +	dev_set_name(dev, "%s", cb->name);
> > > +	dev_set_drvdata(dev, cb);
> > > +
> > > +	ret = device_register(dev);
> > > +	if (ret) {
> > > +		put_device(dev);
> > > +		return ret;
> > > +	}
> > > +
> > > +	cb->dev = dev;
> > > +
> > > +	return 0;
> > > +}
> > > diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
> > > index 6bfbd2dc6db095ec05e53c894e048285f82446c6..b7efe15facb203eea9ae13d5f0abdcc2ea718b4d 100644
> > > --- a/drivers/media/platform/qcom/iris/iris_resources.h
> > > +++ b/drivers/media/platform/qcom/iris/iris_resources.h
> > > @@ -15,5 +15,6 @@ int iris_unset_icc_bw(struct iris_core *core);
> > >   int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw);
> > >   int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type clk_type);
> > >   int iris_prepare_enable_clock(struct iris_core *core, enum platform_clk_type clk_type);
> > > +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb);
> > >   #endif
> > > 
> > > -- 
> > > 2.34.1
> > > 
> > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Vikash Garodia 1 month ago

On 3/4/2026 3:55 AM, Dmitry Baryshkov wrote:
> On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
>>
>> On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
>>> On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
>>>> Introduce different context banks(CB) and the associated buffer region.
>>>> Different stream IDs from VPU would be associated to one of these CB.
>>>> Multiple CBs are needed to increase the IOVA for the video usecases like
>>>> higher concurrent sessions.
>>>>
>>>> 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>
>>>> ---
>>>>    .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
>>>>    drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
>>>>    drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
>>>>    drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
>>>>    4 files changed, 111 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>> @@ -204,6 +204,22 @@ struct icc_vote_data {
>>>>    	u32 fps;
>>>>    };
>>>> +enum iris_buffer_region {
>>>> +	IRIS_BITSTREAM_REGION		= BIT(0),
>>>> +	IRIS_NON_PIXEL_REGION		= BIT(1),
>>>> +	IRIS_PIXEL_REGION		= BIT(2),
>>>> +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
>>>> +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
>>>> +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
>>>
>>> Can a context bank belong to multiple regions at the same time?
>>
>> yes, they would.
> 
> How? Each set of CBs is defined by a separate function in the DT. How
> can CB belong to multiple regions? Could you please provide an example?

SM8550 would have same stream id for VPU hardwares (tensilica and 
vcodec) accessing bitstream and non pixel regions. Thereby non_pixel and 
bitstream regions would map to one CB.
While kaanapali would have different stream id for tensilica accessing 
non pixel region and vcodec accessing bitstream region, thereby having 
different CB.

> 
>>
>>>
>>>> +};
>>>> +
>>>> +struct iris_context_bank {
>>>> +	struct device *dev;
>>>
>>> Separate data and the actual device. Define a wrapper around struct
>>> device for the actual runtime usage.
>>
>> we still have to store the list of dynamically created device. Name can be
>> used to fetch the device from the list, i think the existing approach is
>> simpler ?
> 
> You don't need a list. You have an array of the size, which is known and
> fixed. You have at most 9 functions, which means less than 9 devices.
> 

as mentioned above, its not the same for all platforms to have one to 
one mapping between CBs and buffer region. Thereby indexing based on 
array would be an issue here
It would end up something like this, considering [dev region] array,

SM8550
non_pixel_device  non_pixel_region
non_pixel_device  bitstream_region
pixel_device      pixel_region

kaanapali
non_pixel_device  non_pixel_region
bitstream_device  bitstream_region
pixel_device      pixel_region


>>
>>>
>>>> +	const char *name;
>>>> +	const u32 f_id;
>>>> +	const enum iris_buffer_region region;
>>>> +};
>>>> +
>>>>    enum platform_pm_domain_type {
>>>>    	IRIS_CTRL_POWER_DOMAIN,
>>>>    	IRIS_HW_POWER_DOMAIN,
>>>> @@ -246,6 +262,8 @@ struct iris_platform_data {
>>>>    	u32 inst_fw_caps_enc_size;
>>>>    	const struct tz_cp_config *tz_cp_config_data;
>>>>    	u32 tz_cp_config_data_size;
>>>> +	struct iris_context_bank *cb_data;
>>>> +	u32 cb_data_size;
>>>
>>> Do they differ from platform to platform?
>> Yes
>>
>>> Mark them as const, it should be data only.
>>
>> cb_data_size can be marked as const
> 
> Why is cb_data non-const?

dev is being updated once created dynamically.

> 
>>
>>>
>>>>    	u32 core_arch;
>>>>    	u32 hw_response_timeout;
>>>>    	struct ubwc_config_data *ubwc_config;
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_probe.c b/drivers/media/platform/qcom/iris/iris_probe.c
>>>> index ddaacda523ecb9990af0dd0640196223fbcc2cab..557adb038328a75510591d91569819abc0b7b1c9 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_probe.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_probe.c
>>>> @@ -123,6 +123,49 @@ static int iris_init_resets(struct iris_core *core)
>>>>    				     core->iris_platform_data->controller_rst_tbl_size);
>>>>    }
>>>> +static void iris_destroy_child_device(struct iris_context_bank *cb)
>>>> +{
>>>> +	struct device *dev = cb->dev;
>>>> +
>>>> +	if (dev)
>>>> +		device_unregister(dev);
>>>> +
>>>> +	cb->dev = NULL;
>>>> +}
>>>> +
>>>> +static void iris_deinit_context_bank_devices(struct iris_core *core)
>>>> +{
>>>> +	struct iris_context_bank *cb;
>>>> +	int i;
>>>> +
>>>> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
>>>> +		cb = &core->iris_platform_data->cb_data[i];
>>>> +		iris_destroy_child_device(cb);
>>>> +	}
>>>> +}
>>>> +
>>>> +static int iris_init_context_bank_devices(struct iris_core *core)
>>>> +{
>>>> +	struct iris_context_bank *cb;
>>>> +	int ret, i;
>>>> +
>>>> +	for (i = 0; i < core->iris_platform_data->cb_data_size; i++) {
>>>> +		cb = &core->iris_platform_data->cb_data[i];
>>>> +
>>>> +		ret = iris_create_child_device_and_map(core, cb);
>>>> +		if (ret)
>>>> +			goto err_deinit_cb;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +
>>>> +err_deinit_cb:
>>>> +	while (i-- > 0)
>>>> +		iris_destroy_child_device(&core->iris_platform_data->cb_data[i]);
>>>> +
>>>> +	return ret;
>>>> +}
>>>> +
>>>>    static int iris_init_resources(struct iris_core *core)
>>>>    {
>>>>    	int ret;
>>>> @@ -193,6 +236,7 @@ static void iris_remove(struct platform_device *pdev)
>>>>    		return;
>>>>    	iris_core_deinit(core);
>>>> +	iris_deinit_context_bank_devices(core);
>>>>    	video_unregister_device(core->vdev_dec);
>>>>    	video_unregister_device(core->vdev_enc);
>>>> @@ -275,12 +319,18 @@ static int iris_probe(struct platform_device *pdev)
>>>>    	platform_set_drvdata(pdev, core);
>>>> -	dma_mask = core->iris_platform_data->dma_mask;
>>>> -
>>>> -	ret = dma_set_mask_and_coherent(dev, dma_mask);
>>>> +	ret = iris_init_context_bank_devices(core);
>>>>    	if (ret)
>>>>    		goto err_vdev_unreg_enc;
>>>> +	dma_mask = core->iris_platform_data->dma_mask;
>>>> +
>>>> +	if (device_iommu_mapped(core->dev)) {
>>>> +		ret = dma_set_mask_and_coherent(core->dev, dma_mask);
>>>> +		if (ret)
>>>> +			goto err_deinit_cb;
>>>> +	}
>>>> +
>>>>    	dma_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
>>>>    	dma_set_seg_boundary(&pdev->dev, DMA_BIT_MASK(32));
>>>> @@ -288,10 +338,12 @@ static int iris_probe(struct platform_device *pdev)
>>>>    	pm_runtime_use_autosuspend(core->dev);
>>>>    	ret = devm_pm_runtime_enable(core->dev);
>>>>    	if (ret)
>>>> -		goto err_vdev_unreg_enc;
>>>> +		goto err_deinit_cb;
>>>>    	return 0;
>>>> +err_deinit_cb:
>>>> +	iris_deinit_context_bank_devices(core);
>>>>    err_vdev_unreg_enc:
>>>>    	video_unregister_device(core->vdev_enc);
>>>>    err_vdev_unreg_dec:
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_resources.c b/drivers/media/platform/qcom/iris/iris_resources.c
>>>> index 773f6548370a257b8ae7332242544266cbbd61a9..be58e8620086d0f82c2c2bda29247483f5c56d79 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_resources.c
>>>> +++ b/drivers/media/platform/qcom/iris/iris_resources.c
>>>> @@ -6,6 +6,7 @@
>>>>    #include <linux/clk.h>
>>>>    #include <linux/devfreq.h>
>>>>    #include <linux/interconnect.h>
>>>> +#include <linux/iris_vpu_bus.h>
>>>>    #include <linux/pm_domain.h>
>>>>    #include <linux/pm_opp.h>
>>>>    #include <linux/pm_runtime.h>
>>>> @@ -141,3 +142,38 @@ int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type
>>>>    	return 0;
>>>>    }
>>>> +
>>>> +static void iris_device_release(struct device *dev)
>>>> +{
>>>> +	dev_set_drvdata(dev, NULL);
>>>> +	kfree(dev);
>>>> +}
>>>> +
>>>> +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb)
>>>> +{
>>>> +	struct device *dev;
>>>> +	int ret;
>>>> +
>>>> +	dev = kzalloc_obj(*dev);
>>>> +	if (!dev)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	dev->release = iris_device_release;
>>>> +	dev->bus = &iris_vpu_bus_type;
>>>> +	dev->parent = core->dev;
>>>> +	dev->coherent_dma_mask = core->iris_platform_data->dma_mask;
>>>> +	dev->dma_mask = &dev->coherent_dma_mask;
>>>> +
>>>> +	dev_set_name(dev, "%s", cb->name);
>>>> +	dev_set_drvdata(dev, cb);
>>>> +
>>>> +	ret = device_register(dev);
>>>> +	if (ret) {
>>>> +		put_device(dev);
>>>> +		return ret;
>>>> +	}
>>>> +
>>>> +	cb->dev = dev;
>>>> +
>>>> +	return 0;
>>>> +}
>>>> diff --git a/drivers/media/platform/qcom/iris/iris_resources.h b/drivers/media/platform/qcom/iris/iris_resources.h
>>>> index 6bfbd2dc6db095ec05e53c894e048285f82446c6..b7efe15facb203eea9ae13d5f0abdcc2ea718b4d 100644
>>>> --- a/drivers/media/platform/qcom/iris/iris_resources.h
>>>> +++ b/drivers/media/platform/qcom/iris/iris_resources.h
>>>> @@ -15,5 +15,6 @@ int iris_unset_icc_bw(struct iris_core *core);
>>>>    int iris_set_icc_bw(struct iris_core *core, unsigned long icc_bw);
>>>>    int iris_disable_unprepare_clock(struct iris_core *core, enum platform_clk_type clk_type);
>>>>    int iris_prepare_enable_clock(struct iris_core *core, enum platform_clk_type clk_type);
>>>> +int iris_create_child_device_and_map(struct iris_core *core, struct iris_context_bank *cb);
>>>>    #endif
>>>>
>>>> -- 
>>>> 2.34.1
>>>>
>>>
>>
>

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Dmitry Baryshkov 1 month ago

On Thu, Mar 05, 2026 at 06:19:52PM +0530, Vikash Garodia wrote:
> 
> On 3/4/2026 3:55 AM, Dmitry Baryshkov wrote:
> > On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
> > > 
> > > On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
> > > > On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
> > > > > Introduce different context banks(CB) and the associated buffer region.
> > > > > Different stream IDs from VPU would be associated to one of these CB.
> > > > > Multiple CBs are needed to increase the IOVA for the video usecases like
> > > > > higher concurrent sessions.
> > > > > 
> > > > > 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>
> > > > > ---
> > > > >    .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
> > > > >    drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
> > > > >    drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
> > > > >    drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
> > > > >    4 files changed, 111 insertions(+), 4 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
> > > > > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > @@ -204,6 +204,22 @@ struct icc_vote_data {
> > > > >    	u32 fps;
> > > > >    };
> > > > > +enum iris_buffer_region {
> > > > > +	IRIS_BITSTREAM_REGION		= BIT(0),
> > > > > +	IRIS_NON_PIXEL_REGION		= BIT(1),
> > > > > +	IRIS_PIXEL_REGION		= BIT(2),
> > > > > +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
> > > > > +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
> > > > > +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
> > > > 
> > > > Can a context bank belong to multiple regions at the same time?
> > > 
> > > yes, they would.
> > 
> > How? Each set of CBs is defined by a separate function in the DT. How
> > can CB belong to multiple regions? Could you please provide an example?
> 
> SM8550 would have same stream id for VPU hardwares (tensilica and vcodec)
> accessing bitstream and non pixel regions. Thereby non_pixel and bitstream
> regions would map to one CB.

In my opinion it means only one thing: you will have two CBs (one for
non_pixel and one for bitstream) having the same SIDs. An alternative
would be to define fallback rules (if CB foo doesn't exist, use CB bar).

> While kaanapali would have different stream id for tensilica accessing non
> pixel region and vcodec accessing bitstream region, thereby having different
> CB.
> 
> > 
> > > 
> > > > 
> > > > > +};
> > > > > +
> > > > > +struct iris_context_bank {
> > > > > +	struct device *dev;
> > > > 
> > > > Separate data and the actual device. Define a wrapper around struct
> > > > device for the actual runtime usage.
> > > 
> > > we still have to store the list of dynamically created device. Name can be
> > > used to fetch the device from the list, i think the existing approach is
> > > simpler ?
> > 
> > You don't need a list. You have an array of the size, which is known and
> > fixed. You have at most 9 functions, which means less than 9 devices.
> > 
> 
> as mentioned above, its not the same for all platforms to have one to one
> mapping between CBs and buffer region. Thereby indexing based on array would
> be an issue here
> It would end up something like this, considering [dev region] array,
> 
> SM8550
> non_pixel_device  non_pixel_region
> non_pixel_device  bitstream_region
> pixel_device      pixel_region
> 
> kaanapali
> non_pixel_device  non_pixel_region
> bitstream_device  bitstream_region
> pixel_device      pixel_region

I'm sorry, I'm not sure I follow here. Could you please explain? Maybe
by explititly mapping DT function values to iris_buffer_region values?

> 
> 
> > > 
> > > > 
> > > > > +	const char *name;
> > > > > +	const u32 f_id;
> > > > > +	const enum iris_buffer_region region;
> > > > > +};
> > > > > +
> > > > >    enum platform_pm_domain_type {
> > > > >    	IRIS_CTRL_POWER_DOMAIN,
> > > > >    	IRIS_HW_POWER_DOMAIN,
> > > > > @@ -246,6 +262,8 @@ struct iris_platform_data {
> > > > >    	u32 inst_fw_caps_enc_size;
> > > > >    	const struct tz_cp_config *tz_cp_config_data;
> > > > >    	u32 tz_cp_config_data_size;
> > > > > +	struct iris_context_bank *cb_data;
> > > > > +	u32 cb_data_size;
> > > > 
> > > > Do they differ from platform to platform?
> > > Yes
> > > 
> > > > Mark them as const, it should be data only.
> > > 
> > > cb_data_size can be marked as const
> > 
> > Why is cb_data non-const?
> 
> dev is being updated once created dynamically.

That's a bad idea. Please make the platform description constant.

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Vikash Garodia 1 month ago

On 3/5/2026 7:51 PM, Dmitry Baryshkov wrote:
> On Thu, Mar 05, 2026 at 06:19:52PM +0530, Vikash Garodia wrote:
>>
>> On 3/4/2026 3:55 AM, Dmitry Baryshkov wrote:
>>> On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
>>>>
>>>> On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
>>>>> On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
>>>>>> Introduce different context banks(CB) and the associated buffer region.
>>>>>> Different stream IDs from VPU would be associated to one of these CB.
>>>>>> Multiple CBs are needed to increase the IOVA for the video usecases like
>>>>>> higher concurrent sessions.
>>>>>>
>>>>>> 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>
>>>>>> ---
>>>>>>     .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
>>>>>>     drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
>>>>>>     drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
>>>>>>     drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
>>>>>>     4 files changed, 111 insertions(+), 4 deletions(-)
>>>>>>
>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
>>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>> @@ -204,6 +204,22 @@ struct icc_vote_data {
>>>>>>     	u32 fps;
>>>>>>     };
>>>>>> +enum iris_buffer_region {
>>>>>> +	IRIS_BITSTREAM_REGION		= BIT(0),
>>>>>> +	IRIS_NON_PIXEL_REGION		= BIT(1),
>>>>>> +	IRIS_PIXEL_REGION		= BIT(2),
>>>>>> +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
>>>>>> +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
>>>>>> +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
>>>>>
>>>>> Can a context bank belong to multiple regions at the same time?
>>>>
>>>> yes, they would.
>>>
>>> How? Each set of CBs is defined by a separate function in the DT. How
>>> can CB belong to multiple regions? Could you please provide an example?
>>
>> SM8550 would have same stream id for VPU hardwares (tensilica and vcodec)
>> accessing bitstream and non pixel regions. Thereby non_pixel and bitstream
>> regions would map to one CB.
> 
> In my opinion it means only one thing: you will have two CBs (one for
> non_pixel and one for bitstream) having the same SIDs. An alternative
> would be to define fallback rules (if CB foo doesn't exist, use CB bar).
> 
>> While kaanapali would have different stream id for tensilica accessing non
>> pixel region and vcodec accessing bitstream region, thereby having different
>> CB.
>>
>>>
>>>>
>>>>>
>>>>>> +};
>>>>>> +
>>>>>> +struct iris_context_bank {
>>>>>> +	struct device *dev;
>>>>>
>>>>> Separate data and the actual device. Define a wrapper around struct
>>>>> device for the actual runtime usage.
>>>>
>>>> we still have to store the list of dynamically created device. Name can be
>>>> used to fetch the device from the list, i think the existing approach is
>>>> simpler ?
>>>
>>> You don't need a list. You have an array of the size, which is known and
>>> fixed. You have at most 9 functions, which means less than 9 devices.
>>>
>>
>> as mentioned above, its not the same for all platforms to have one to one
>> mapping between CBs and buffer region. Thereby indexing based on array would
>> be an issue here
>> It would end up something like this, considering [dev region] array,
>>
>> SM8550
>> non_pixel_device  non_pixel_region
>> non_pixel_device  bitstream_region
>> pixel_device      pixel_region
>>
>> kaanapali
>> non_pixel_device  non_pixel_region
>> bitstream_device  bitstream_region
>> pixel_device      pixel_region
> 
> I'm sorry, I'm not sure I follow here. Could you please explain? Maybe
> by explititly mapping DT function values to iris_buffer_region values?
> 

Kaanapali
IRIS_BITSTREAM IRIS_BITSTREAM_REGION
IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION	
IRIS_PIXEL     IRIS_PIXEL_REGION

SM8550
IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION | IRIS_BITSTREAM_REGION
IRIS_PIXEL     IRIS_PIXEL_REGION

>>
>>
>>>>
>>>>>
>>>>>> +	const char *name;
>>>>>> +	const u32 f_id;
>>>>>> +	const enum iris_buffer_region region;
>>>>>> +};
>>>>>> +
>>>>>>     enum platform_pm_domain_type {
>>>>>>     	IRIS_CTRL_POWER_DOMAIN,
>>>>>>     	IRIS_HW_POWER_DOMAIN,
>>>>>> @@ -246,6 +262,8 @@ struct iris_platform_data {
>>>>>>     	u32 inst_fw_caps_enc_size;
>>>>>>     	const struct tz_cp_config *tz_cp_config_data;
>>>>>>     	u32 tz_cp_config_data_size;
>>>>>> +	struct iris_context_bank *cb_data;
>>>>>> +	u32 cb_data_size;
>>>>>
>>>>> Do they differ from platform to platform?
>>>> Yes
>>>>
>>>>> Mark them as const, it should be data only.
>>>>
>>>> cb_data_size can be marked as const
>>>
>>> Why is cb_data non-const?
>>
>> dev is being updated once created dynamically.
> 
> That's a bad idea. Please make the platform description constant.
> 

I can give it a try to move CBs in core struct out of platform data and 
have a buffer region based lookup array to fetch the device.

Regards,
Vikash

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Dmitry Baryshkov 1 month ago

On Thu, Mar 05, 2026 at 10:56:40PM +0530, Vikash Garodia wrote:
> 
> On 3/5/2026 7:51 PM, Dmitry Baryshkov wrote:
> > On Thu, Mar 05, 2026 at 06:19:52PM +0530, Vikash Garodia wrote:
> > > 
> > > On 3/4/2026 3:55 AM, Dmitry Baryshkov wrote:
> > > > On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
> > > > > 
> > > > > On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
> > > > > > On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
> > > > > > > Introduce different context banks(CB) and the associated buffer region.
> > > > > > > Different stream IDs from VPU would be associated to one of these CB.
> > > > > > > Multiple CBs are needed to increase the IOVA for the video usecases like
> > > > > > > higher concurrent sessions.
> > > > > > > 
> > > > > > > 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>
> > > > > > > ---
> > > > > > >     .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
> > > > > > >     drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
> > > > > > >     drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
> > > > > > >     drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
> > > > > > >     4 files changed, 111 insertions(+), 4 deletions(-)
> > > > > > > 
> > > > > > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > > > index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
> > > > > > > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > > > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > > > @@ -204,6 +204,22 @@ struct icc_vote_data {
> > > > > > >     	u32 fps;
> > > > > > >     };
> > > > > > > +enum iris_buffer_region {
> > > > > > > +	IRIS_BITSTREAM_REGION		= BIT(0),
> > > > > > > +	IRIS_NON_PIXEL_REGION		= BIT(1),
> > > > > > > +	IRIS_PIXEL_REGION		= BIT(2),
> > > > > > > +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
> > > > > > > +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
> > > > > > > +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
> > > > > > 
> > > > > > Can a context bank belong to multiple regions at the same time?
> > > > > 
> > > > > yes, they would.
> > > > 
> > > > How? Each set of CBs is defined by a separate function in the DT. How
> > > > can CB belong to multiple regions? Could you please provide an example?
> > > 
> > > SM8550 would have same stream id for VPU hardwares (tensilica and vcodec)
> > > accessing bitstream and non pixel regions. Thereby non_pixel and bitstream
> > > regions would map to one CB.
> > 
> > In my opinion it means only one thing: you will have two CBs (one for
> > non_pixel and one for bitstream) having the same SIDs. An alternative
> > would be to define fallback rules (if CB foo doesn't exist, use CB bar).
> > 
> > > While kaanapali would have different stream id for tensilica accessing non
> > > pixel region and vcodec accessing bitstream region, thereby having different
> > > CB.
> > > 
> > > > 
> > > > > 
> > > > > > 
> > > > > > > +};
> > > > > > > +
> > > > > > > +struct iris_context_bank {
> > > > > > > +	struct device *dev;
> > > > > > 
> > > > > > Separate data and the actual device. Define a wrapper around struct
> > > > > > device for the actual runtime usage.
> > > > > 
> > > > > we still have to store the list of dynamically created device. Name can be
> > > > > used to fetch the device from the list, i think the existing approach is
> > > > > simpler ?
> > > > 
> > > > You don't need a list. You have an array of the size, which is known and
> > > > fixed. You have at most 9 functions, which means less than 9 devices.
> > > > 
> > > 
> > > as mentioned above, its not the same for all platforms to have one to one
> > > mapping between CBs and buffer region. Thereby indexing based on array would
> > > be an issue here
> > > It would end up something like this, considering [dev region] array,
> > > 
> > > SM8550
> > > non_pixel_device  non_pixel_region
> > > non_pixel_device  bitstream_region
> > > pixel_device      pixel_region
> > > 
> > > kaanapali
> > > non_pixel_device  non_pixel_region
> > > bitstream_device  bitstream_region
> > > pixel_device      pixel_region
> > 
> > I'm sorry, I'm not sure I follow here. Could you please explain? Maybe
> > by explititly mapping DT function values to iris_buffer_region values?
> > 
> 
> Kaanapali
> IRIS_BITSTREAM IRIS_BITSTREAM_REGION
> IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION	
> IRIS_PIXEL     IRIS_PIXEL_REGION
> 
> SM8550
> IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION | IRIS_BITSTREAM_REGION
> IRIS_PIXEL     IRIS_PIXEL_REGION

So, why not:

Kaanapali:

iris_cb_dev_bs = iris_cb_dev_alloc(IRIS_BITSTREAM];
iris_cb_dev_np = iris_cb_dev_alloc(IRIS_NON_PIXEL];
iris_cb_dev_px = iris_cb_dev_alloc(IRIS_PIXEL];

core->cb_devs = {
	[IRIS_BITSTREAM_REGION] = iris_cb_dev_bs,
	[IRIS_NON_PIXEL_REGION] = iris_cb_dev_np,
	[IRIS_PIXEL_REGION] = iris_cb_dev_px;
};

SM8550:

iris_cb_dev_np = iris_cb_dev_alloc(IRIS_NON_PIXEL];
iris_cb_dev_px = iris_cb_dev_alloc(IRIS_PIXEL];

core->cb_devs = {
	[IRIS_BITSTREAM_REGION] = iris_cb_dev_np,
	[IRIS_NON_PIXEL_REGION] = iris_cb_dev_np,
	[IRIS_PIXEL_REGION] = iris_cb_dev_px;
};


Yes, it would require coding of those functions, however afterwards you
can access necessary CB device simply by doing core->cb_devs[region].

I think current code is overcomplicated for the sake of having the
platform flexibility expressed as data.

> 
> > > 
> > > 
> > > > > 
> > > > > > 
> > > > > > > +	const char *name;
> > > > > > > +	const u32 f_id;
> > > > > > > +	const enum iris_buffer_region region;
> > > > > > > +};
> > > > > > > +
> > > > > > >     enum platform_pm_domain_type {
> > > > > > >     	IRIS_CTRL_POWER_DOMAIN,
> > > > > > >     	IRIS_HW_POWER_DOMAIN,
> > > > > > > @@ -246,6 +262,8 @@ struct iris_platform_data {
> > > > > > >     	u32 inst_fw_caps_enc_size;
> > > > > > >     	const struct tz_cp_config *tz_cp_config_data;
> > > > > > >     	u32 tz_cp_config_data_size;
> > > > > > > +	struct iris_context_bank *cb_data;
> > > > > > > +	u32 cb_data_size;
> > > > > > 
> > > > > > Do they differ from platform to platform?
> > > > > Yes
> > > > > 
> > > > > > Mark them as const, it should be data only.
> > > > > 
> > > > > cb_data_size can be marked as const
> > > > 
> > > > Why is cb_data non-const?
> > > 
> > > dev is being updated once created dynamically.
> > 
> > That's a bad idea. Please make the platform description constant.
> > 
> 
> I can give it a try to move CBs in core struct out of platform data and have
> a buffer region based lookup array to fetch the device.

It might be easier to express that as a callback, filling core->cb_devs
with struct device pointers, as I wrote above.

> 
> Regards,
> Vikash

-- 
With best wishes
Dmitry

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Vikash Garodia 1 month ago

On 3/6/2026 12:02 AM, Dmitry Baryshkov wrote:
> On Thu, Mar 05, 2026 at 10:56:40PM +0530, Vikash Garodia wrote:
>>
>> On 3/5/2026 7:51 PM, Dmitry Baryshkov wrote:
>>> On Thu, Mar 05, 2026 at 06:19:52PM +0530, Vikash Garodia wrote:
>>>>
>>>> On 3/4/2026 3:55 AM, Dmitry Baryshkov wrote:
>>>>> On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
>>>>>>
>>>>>> On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
>>>>>>> On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
>>>>>>>> Introduce different context banks(CB) and the associated buffer region.
>>>>>>>> Different stream IDs from VPU would be associated to one of these CB.
>>>>>>>> Multiple CBs are needed to increase the IOVA for the video usecases like
>>>>>>>> higher concurrent sessions.
>>>>>>>>
>>>>>>>> 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>
>>>>>>>> ---
>>>>>>>>      .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
>>>>>>>>      drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
>>>>>>>>      drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
>>>>>>>>      drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
>>>>>>>>      4 files changed, 111 insertions(+), 4 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>>> index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
>>>>>>>> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>>> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
>>>>>>>> @@ -204,6 +204,22 @@ struct icc_vote_data {
>>>>>>>>      	u32 fps;
>>>>>>>>      };
>>>>>>>> +enum iris_buffer_region {
>>>>>>>> +	IRIS_BITSTREAM_REGION		= BIT(0),
>>>>>>>> +	IRIS_NON_PIXEL_REGION		= BIT(1),
>>>>>>>> +	IRIS_PIXEL_REGION		= BIT(2),
>>>>>>>> +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
>>>>>>>> +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
>>>>>>>> +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
>>>>>>>
>>>>>>> Can a context bank belong to multiple regions at the same time?
>>>>>>
>>>>>> yes, they would.
>>>>>
>>>>> How? Each set of CBs is defined by a separate function in the DT. How
>>>>> can CB belong to multiple regions? Could you please provide an example?
>>>>
>>>> SM8550 would have same stream id for VPU hardwares (tensilica and vcodec)
>>>> accessing bitstream and non pixel regions. Thereby non_pixel and bitstream
>>>> regions would map to one CB.
>>>
>>> In my opinion it means only one thing: you will have two CBs (one for
>>> non_pixel and one for bitstream) having the same SIDs. An alternative
>>> would be to define fallback rules (if CB foo doesn't exist, use CB bar).
>>>
>>>> While kaanapali would have different stream id for tensilica accessing non
>>>> pixel region and vcodec accessing bitstream region, thereby having different
>>>> CB.
>>>>
>>>>>
>>>>>>
>>>>>>>
>>>>>>>> +};
>>>>>>>> +
>>>>>>>> +struct iris_context_bank {
>>>>>>>> +	struct device *dev;
>>>>>>>
>>>>>>> Separate data and the actual device. Define a wrapper around struct
>>>>>>> device for the actual runtime usage.
>>>>>>
>>>>>> we still have to store the list of dynamically created device. Name can be
>>>>>> used to fetch the device from the list, i think the existing approach is
>>>>>> simpler ?
>>>>>
>>>>> You don't need a list. You have an array of the size, which is known and
>>>>> fixed. You have at most 9 functions, which means less than 9 devices.
>>>>>
>>>>
>>>> as mentioned above, its not the same for all platforms to have one to one
>>>> mapping between CBs and buffer region. Thereby indexing based on array would
>>>> be an issue here
>>>> It would end up something like this, considering [dev region] array,
>>>>
>>>> SM8550
>>>> non_pixel_device  non_pixel_region
>>>> non_pixel_device  bitstream_region
>>>> pixel_device      pixel_region
>>>>
>>>> kaanapali
>>>> non_pixel_device  non_pixel_region
>>>> bitstream_device  bitstream_region
>>>> pixel_device      pixel_region
>>>
>>> I'm sorry, I'm not sure I follow here. Could you please explain? Maybe
>>> by explititly mapping DT function values to iris_buffer_region values?
>>>
>>
>> Kaanapali
>> IRIS_BITSTREAM IRIS_BITSTREAM_REGION
>> IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION	
>> IRIS_PIXEL     IRIS_PIXEL_REGION
>>
>> SM8550
>> IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION | IRIS_BITSTREAM_REGION
>> IRIS_PIXEL     IRIS_PIXEL_REGION
> 
> So, why not:
> 
> Kaanapali:
> 
> iris_cb_dev_bs = iris_cb_dev_alloc(IRIS_BITSTREAM];
> iris_cb_dev_np = iris_cb_dev_alloc(IRIS_NON_PIXEL];
> iris_cb_dev_px = iris_cb_dev_alloc(IRIS_PIXEL];
> 
> core->cb_devs = {
> 	[IRIS_BITSTREAM_REGION] = iris_cb_dev_bs,
> 	[IRIS_NON_PIXEL_REGION] = iris_cb_dev_np,
> 	[IRIS_PIXEL_REGION] = iris_cb_dev_px;
> };
> 
> SM8550:
> 
> iris_cb_dev_np = iris_cb_dev_alloc(IRIS_NON_PIXEL];
> iris_cb_dev_px = iris_cb_dev_alloc(IRIS_PIXEL];
> 
> core->cb_devs = {
> 	[IRIS_BITSTREAM_REGION] = iris_cb_dev_np,
> 	[IRIS_NON_PIXEL_REGION] = iris_cb_dev_np,
> 	[IRIS_PIXEL_REGION] = iris_cb_dev_px;
> };
> 
> 
> Yes, it would require coding of those functions, however afterwards you
> can access necessary CB device simply by doing core->cb_devs[region].
> 
> I think current code is overcomplicated for the sake of having the
> platform flexibility expressed as data.
> 
>>
>>>>
>>>>
>>>>>>
>>>>>>>
>>>>>>>> +	const char *name;
>>>>>>>> +	const u32 f_id;
>>>>>>>> +	const enum iris_buffer_region region;
>>>>>>>> +};
>>>>>>>> +
>>>>>>>>      enum platform_pm_domain_type {
>>>>>>>>      	IRIS_CTRL_POWER_DOMAIN,
>>>>>>>>      	IRIS_HW_POWER_DOMAIN,
>>>>>>>> @@ -246,6 +262,8 @@ struct iris_platform_data {
>>>>>>>>      	u32 inst_fw_caps_enc_size;
>>>>>>>>      	const struct tz_cp_config *tz_cp_config_data;
>>>>>>>>      	u32 tz_cp_config_data_size;
>>>>>>>> +	struct iris_context_bank *cb_data;
>>>>>>>> +	u32 cb_data_size;
>>>>>>>
>>>>>>> Do they differ from platform to platform?
>>>>>> Yes
>>>>>>
>>>>>>> Mark them as const, it should be data only.
>>>>>>
>>>>>> cb_data_size can be marked as const
>>>>>
>>>>> Why is cb_data non-const?
>>>>
>>>> dev is being updated once created dynamically.
>>>
>>> That's a bad idea. Please make the platform description constant.
>>>
>>
>> I can give it a try to move CBs in core struct out of platform data and have
>> a buffer region based lookup array to fetch the device.
> 
> It might be easier to express that as a callback, filling core->cb_devs
> with struct device pointers, as I wrote above.

IIUC, introduce an ops like below and fill ore->cb_devs[] in SOC 
specific data i.e sm8550_data ?

struct iris_platform_ops {
     int (*populate_cb_devs)(struct iris_core *core);
};
struct iris_platform_data {
     const struct iris_platform_ops *ops;
     /* ... other pdata fields ... */
};

At present lemans shares the platform data of sm8550, with the above ops 
approach, that commonality leverage would be lost ?

Please correct if your proposal is different than what i interpreted.

> 
>>
>> Regards,
>> Vikash
>

Re: [PATCH v2 4/7] media: iris: add context bank devices using iommu-map

Posted by Dmitry Baryshkov 3 weeks, 3 days ago

On Fri, Mar 06, 2026 at 03:04:04AM +0530, Vikash Garodia wrote:
> 
> On 3/6/2026 12:02 AM, Dmitry Baryshkov wrote:
> > On Thu, Mar 05, 2026 at 10:56:40PM +0530, Vikash Garodia wrote:
> > > 
> > > On 3/5/2026 7:51 PM, Dmitry Baryshkov wrote:
> > > > On Thu, Mar 05, 2026 at 06:19:52PM +0530, Vikash Garodia wrote:
> > > > > 
> > > > > On 3/4/2026 3:55 AM, Dmitry Baryshkov wrote:
> > > > > > On Wed, Mar 04, 2026 at 12:16:50AM +0530, Vikash Garodia wrote:
> > > > > > > 
> > > > > > > On 2/28/2026 1:50 AM, Dmitry Baryshkov wrote:
> > > > > > > > On Fri, Feb 27, 2026 at 07:41:20PM +0530, Vikash Garodia wrote:
> > > > > > > > > Introduce different context banks(CB) and the associated buffer region.
> > > > > > > > > Different stream IDs from VPU would be associated to one of these CB.
> > > > > > > > > Multiple CBs are needed to increase the IOVA for the video usecases like
> > > > > > > > > higher concurrent sessions.
> > > > > > > > > 
> > > > > > > > > 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>
> > > > > > > > > ---
> > > > > > > > >      .../platform/qcom/iris/iris_platform_common.h      | 18 +++++++
> > > > > > > > >      drivers/media/platform/qcom/iris/iris_probe.c      | 60 ++++++++++++++++++++--
> > > > > > > > >      drivers/media/platform/qcom/iris/iris_resources.c  | 36 +++++++++++++
> > > > > > > > >      drivers/media/platform/qcom/iris/iris_resources.h  |  1 +
> > > > > > > > >      4 files changed, 111 insertions(+), 4 deletions(-)
> > > > > > > > > 
> > > > > > > > > diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > > > > > index 5a489917580eb10022fdcb52f7321a915e8b239d..03c50d6e54853fca34d7d32f65d09eb80945fcdd 100644
> > > > > > > > > --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > > > > > +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> > > > > > > > > @@ -204,6 +204,22 @@ struct icc_vote_data {
> > > > > > > > >      	u32 fps;
> > > > > > > > >      };
> > > > > > > > > +enum iris_buffer_region {
> > > > > > > > > +	IRIS_BITSTREAM_REGION		= BIT(0),
> > > > > > > > > +	IRIS_NON_PIXEL_REGION		= BIT(1),
> > > > > > > > > +	IRIS_PIXEL_REGION		= BIT(2),
> > > > > > > > > +	IRIS_SECURE_BITSTREAM_REGION	= BIT(3),
> > > > > > > > > +	IRIS_SECURE_NON_PIXEL_REGION	= BIT(4),
> > > > > > > > > +	IRIS_SECURE_PIXEL_REGION	= BIT(5),
> > > > > > > > 
> > > > > > > > Can a context bank belong to multiple regions at the same time?
> > > > > > > 
> > > > > > > yes, they would.
> > > > > > 
> > > > > > How? Each set of CBs is defined by a separate function in the DT. How
> > > > > > can CB belong to multiple regions? Could you please provide an example?
> > > > > 
> > > > > SM8550 would have same stream id for VPU hardwares (tensilica and vcodec)
> > > > > accessing bitstream and non pixel regions. Thereby non_pixel and bitstream
> > > > > regions would map to one CB.
> > > > 
> > > > In my opinion it means only one thing: you will have two CBs (one for
> > > > non_pixel and one for bitstream) having the same SIDs. An alternative
> > > > would be to define fallback rules (if CB foo doesn't exist, use CB bar).
> > > > 
> > > > > While kaanapali would have different stream id for tensilica accessing non
> > > > > pixel region and vcodec accessing bitstream region, thereby having different
> > > > > CB.
> > > > > 
> > > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > > +};
> > > > > > > > > +
> > > > > > > > > +struct iris_context_bank {
> > > > > > > > > +	struct device *dev;
> > > > > > > > 
> > > > > > > > Separate data and the actual device. Define a wrapper around struct
> > > > > > > > device for the actual runtime usage.
> > > > > > > 
> > > > > > > we still have to store the list of dynamically created device. Name can be
> > > > > > > used to fetch the device from the list, i think the existing approach is
> > > > > > > simpler ?
> > > > > > 
> > > > > > You don't need a list. You have an array of the size, which is known and
> > > > > > fixed. You have at most 9 functions, which means less than 9 devices.
> > > > > > 
> > > > > 
> > > > > as mentioned above, its not the same for all platforms to have one to one
> > > > > mapping between CBs and buffer region. Thereby indexing based on array would
> > > > > be an issue here
> > > > > It would end up something like this, considering [dev region] array,
> > > > > 
> > > > > SM8550
> > > > > non_pixel_device  non_pixel_region
> > > > > non_pixel_device  bitstream_region
> > > > > pixel_device      pixel_region
> > > > > 
> > > > > kaanapali
> > > > > non_pixel_device  non_pixel_region
> > > > > bitstream_device  bitstream_region
> > > > > pixel_device      pixel_region
> > > > 
> > > > I'm sorry, I'm not sure I follow here. Could you please explain? Maybe
> > > > by explititly mapping DT function values to iris_buffer_region values?
> > > > 
> > > 
> > > Kaanapali
> > > IRIS_BITSTREAM IRIS_BITSTREAM_REGION
> > > IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION	
> > > IRIS_PIXEL     IRIS_PIXEL_REGION
> > > 
> > > SM8550
> > > IRIS_NON_PIXEL IRIS_NON_PIXEL_REGION | IRIS_BITSTREAM_REGION
> > > IRIS_PIXEL     IRIS_PIXEL_REGION
> > 
> > So, why not:
> > 
> > Kaanapali:
> > 
> > iris_cb_dev_bs = iris_cb_dev_alloc(IRIS_BITSTREAM];
> > iris_cb_dev_np = iris_cb_dev_alloc(IRIS_NON_PIXEL];
> > iris_cb_dev_px = iris_cb_dev_alloc(IRIS_PIXEL];
> > 
> > core->cb_devs = {
> > 	[IRIS_BITSTREAM_REGION] = iris_cb_dev_bs,
> > 	[IRIS_NON_PIXEL_REGION] = iris_cb_dev_np,
> > 	[IRIS_PIXEL_REGION] = iris_cb_dev_px;
> > };
> > 
> > SM8550:
> > 
> > iris_cb_dev_np = iris_cb_dev_alloc(IRIS_NON_PIXEL];
> > iris_cb_dev_px = iris_cb_dev_alloc(IRIS_PIXEL];
> > 
> > core->cb_devs = {
> > 	[IRIS_BITSTREAM_REGION] = iris_cb_dev_np,
> > 	[IRIS_NON_PIXEL_REGION] = iris_cb_dev_np,
> > 	[IRIS_PIXEL_REGION] = iris_cb_dev_px;
> > };
> > 
> > 
> > Yes, it would require coding of those functions, however afterwards you
> > can access necessary CB device simply by doing core->cb_devs[region].
> > 
> > I think current code is overcomplicated for the sake of having the
> > platform flexibility expressed as data.
> > 
> > > 
> > > > > 
> > > > > 
> > > > > > > 
> > > > > > > > 
> > > > > > > > > +	const char *name;
> > > > > > > > > +	const u32 f_id;
> > > > > > > > > +	const enum iris_buffer_region region;
> > > > > > > > > +};
> > > > > > > > > +
> > > > > > > > >      enum platform_pm_domain_type {
> > > > > > > > >      	IRIS_CTRL_POWER_DOMAIN,
> > > > > > > > >      	IRIS_HW_POWER_DOMAIN,
> > > > > > > > > @@ -246,6 +262,8 @@ struct iris_platform_data {
> > > > > > > > >      	u32 inst_fw_caps_enc_size;
> > > > > > > > >      	const struct tz_cp_config *tz_cp_config_data;
> > > > > > > > >      	u32 tz_cp_config_data_size;
> > > > > > > > > +	struct iris_context_bank *cb_data;
> > > > > > > > > +	u32 cb_data_size;
> > > > > > > > 
> > > > > > > > Do they differ from platform to platform?
> > > > > > > Yes
> > > > > > > 
> > > > > > > > Mark them as const, it should be data only.
> > > > > > > 
> > > > > > > cb_data_size can be marked as const
> > > > > > 
> > > > > > Why is cb_data non-const?
> > > > > 
> > > > > dev is being updated once created dynamically.
> > > > 
> > > > That's a bad idea. Please make the platform description constant.
> > > > 
> > > 
> > > I can give it a try to move CBs in core struct out of platform data and have
> > > a buffer region based lookup array to fetch the device.
> > 
> > It might be easier to express that as a callback, filling core->cb_devs
> > with struct device pointers, as I wrote above.
> 
> IIUC, introduce an ops like below and fill ore->cb_devs[] in SOC specific
> data i.e sm8550_data ?
> 
> struct iris_platform_ops {
>     int (*populate_cb_devs)(struct iris_core *core);
> };
> struct iris_platform_data {
>     const struct iris_platform_ops *ops;
>     /* ... other pdata fields ... */
> };
> 
> At present lemans shares the platform data of sm8550, with the above ops
> approach, that commonality leverage would be lost ?
> 
> Please correct if your proposal is different than what i interpreted.

Yes, it was just about the callback. You've coded it again as a loop of
bitmasks. My opinion would be to just have platform-specific easy
callbacks without extra generalization.

> 
> > 
> > > 
> > > Regards,
> > > Vikash
> > 
> 

-- 
With best wishes
Dmitry

[PATCH v2 1/7] media: dt-bindings: qcom-kaanapali-iris: Add kaanapali video codec binding
[PATCH v2 2/7] media: iris: switch to hardware mode after firmware boot
[PATCH v2 3/7] media: iris: add iris vpu bus support and register it with iommu_buses
[PATCH v2 4/7] media: iris: add context bank devices using iommu-map
[PATCH v2 5/7] media: iris: add helper to select context bank device
[PATCH v2 6/7] media: iris: add iris4 specific H265 line buffer calculation
[PATCH v2 7/7] media: iris: add platform data for kaanapali