Re: [PATCH v6 09/14] media: iris: Use power domain type to look up pd_devs index

[Dmitry Baryshkov]

On Fri, May 15, 2026 at 04:51:24PM +0530, Vishnu Reddy wrote:
> The pmdomain_tbl was a array of strings holding only the power domain
> names. Callers had to pass a pd_devs[] pointer indexed directly by the
> platform_pm_domain_type enum value to iris_enable_power_domains() and
> iris_disable_power_domains().
> 
> A future platform may need to introduce a new enum value that aliases
> an existing one (e.g. IRIS_VCODEC1_POWER_DOMAIN aliasing the
> IRIS_VPP0_HW_POWER_DOMAIN on Glymur), which would break the assumption
> that enum values map 1:1 to pd_devs[] indices.
> 
> To fix this, replace the string array with a new struct platform_pd_data
> that pairs each power domain name with its platform_pm_domain_type. Add
> a helper iris_get_pd_index_by_type() that walks this table and returns
> the correct pd_devs[] index for a given type.
> 
> Update iris_enable_power_domains() and iris_disable_power_domains()
> to accept a platform_pm_domain_type instead of a struct device pointer.
> They now call the helper internally to resolve the index, removing the
> need for callers to do the index lookup themselves.
> 
> This prepares the driver for adding new platforms where power domain enum
> values cannot be used directly as pd_devs[] indices.
> 
> Reviewed-by: Vikash Garodia <vikash.garodia@xxxxxxxxxxxxxxxx>
> Signed-off-by: Vishnu Reddy <busanna.reddy@xxxxxxxxxxxxxxxx>
> ---
>  .../platform/qcom/iris/iris_platform_common.h      |  9 +++-
>  .../media/platform/qcom/iris/iris_platform_vpu2.c  | 18 +++++---
>  .../media/platform/qcom/iris/iris_platform_vpu3x.c | 24 ++++++----
>  drivers/media/platform/qcom/iris/iris_probe.c      |  4 +-
>  drivers/media/platform/qcom/iris/iris_resources.c  | 43 +++++++++++++++++-
>  drivers/media/platform/qcom/iris/iris_resources.h  |  6 ++-
>  drivers/media/platform/qcom/iris/iris_vpu3x.c      |  7 ++-
>  drivers/media/platform/qcom/iris/iris_vpu4x.c      | 52 ++++++++--------------
>  drivers/media/platform/qcom/iris/iris_vpu_common.c | 23 +++++-----
>  9 files changed, 115 insertions(+), 71 deletions(-)
> 
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_common.h b/drivers/media/platform/qcom/iris/iris_platform_common.h
> index 07cc0ce25b84..1d757cb8e9e1 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_common.h
> +++ b/drivers/media/platform/qcom/iris/iris_platform_common.h
> @@ -70,6 +70,12 @@ struct platform_clk_data {
>  	const char *clk_name;
>  };
>  
> +struct platform_pd_data {
> +	enum platform_pm_domain_type *pd_types;
> +	const char * const *pd_names;
> +	u32 pd_count;
> +};
> +
>  struct tz_cp_config {
>  	u32 cp_start;
>  	u32 cp_size;
> @@ -270,8 +276,7 @@ struct iris_platform_data {
>  	unsigned int icc_tbl_size;
>  	const struct bw_info *bw_tbl_dec;
>  	unsigned int bw_tbl_dec_size;
> -	const char * const *pmdomain_tbl;
> -	unsigned int pmdomain_tbl_size;
> +	const struct platform_pd_data *pmdomain_tbl;
>  	const char * const *opp_pd_tbl;
>  	unsigned int opp_pd_tbl_size;
>  	const struct platform_clk_data *clk_tbl;
> diff --git a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> index 41986af8313b..bcf873829fd3 100644
> --- a/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> +++ b/drivers/media/platform/qcom/iris/iris_platform_vpu2.c
> @@ -62,7 +62,17 @@ static const struct icc_info iris_icc_info_vpu2[] = {
>  
>  static const char * const iris_clk_reset_table_vpu2[] = { "bus", "core" };
>  
> -static const char * const iris_pmdomain_table_vpu2[] = { "venus", "vcodec0" };
> +static const struct platform_pd_data iris_pmdomain_table_vpu2 = {
> +	.pd_types = (enum platform_pm_domain_type []) {
> +		IRIS_CTRL_POWER_DOMAIN,
> +		IRIS_VCODEC_POWER_DOMAIN,
> +	},
> +	.pd_names = (const char *[]) {
> +		"venus",
> +		"vcodec0",
> +	},
> +	.pd_count = 2,
> +};

I still don't think it is a good idea. You are using these lookup and
indirection to make _platform_ code take care about _platform_
specifics by the price of complicating the core and the core structures.

Instead it looks like it's time to push power domain handling to
platform code and then hopefully drop all IRIS_foo_POWER_DOMAIN enums.

I'm pretty sure that the clocks would also need to follow that approach.
There should be no need to bother the core code to introduce the
platform-specific clock type. It all needs to be handled in the
platform callback and platform-specific data structures.

>  
>  static const struct tz_cp_config tz_cp_config_vpu2[] = {
>  	{
> 

-- 
With best wishes
Dmitry