Re: [PATCH 3/3] remoteproc: qcom_q6v5_pas: Add per-PD proxy performance states for Hawi CDSP

[Bjorn Andersson]

On Fri, Aug 28, 2026 at 11:43:11PM +0530, Mukesh Ojha wrote:
> The proxy power domain enable path currently requests INT_MAX performance
> state for every proxy PD.  While this serves as a "take highest available"
> hint, some SoCs require specific per-domain RPMH levels for correct
> operation during firmware load rather than a blanket maximum.
> 
> Introduce a proxy_pd_performance_states array in qcom_pas_data to allow
> each proxy PD to declare its required RPMH performance level explicitly.
> Platforms that do not populate this field retain the existing INT_MAX
> behaviour.
> 

Is it possible to encode this using an optional opp-table instead of
filling the driver with such details? (This is a question, not a direct
suggestion)

> Also propagate the return value of dev_pm_genpd_set_performance_state()
> and emit a warning on failure rather than silently ignoring it.

Also remember that whenever you start a paragraph in a commit message
with the word "also"; it's probably a good sign that it would be better
to have a separate commit.

> 
> Add Hawi CDSP remoteproc support using this infrastructure with the
> following proxy PD performance states:

That is quite weird, because you already stated that we added Hawi CDSP
support in
https://lore.kernel.org/r/20260427190614.3679937-2-mukesh.ojha@xxxxxxxxxxxxxxxx


Note that the line:

  compatible = "qcom,hawi-cdsp-pas", "qcom,sm8550-cdsp-pas";

is supposed to tell an OS that "if you have an implementation for
qcom,hawi-cdsp-pas use that, if not you can use the implementation for
qcom,sm8550-cdsp-pas".

This patch tells me that you need to also fix the binding to not say
that - because hawi-cdsp is no longer compatible with sm8550-cdsp.

I'm guessing that this issue might have been a late discovery, state
that in your commit message changing the binding.

> 
>   CX:  RPMH_REGULATOR_LEVEL_TURBO
>   MXC: RPMH_REGULATOR_LEVEL_TURBO
>   NSP: RPMH_REGULATOR_LEVEL_NOM

I'm guessing that what you describe above about INT_MAX being a problem
is only for NSP? Would be nice to not having to guess though.

Regards,
Bjorn

> 
> Signed-off-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
> ---
>  drivers/remoteproc/qcom_q6v5_pas.c | 43 +++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> index 25942200ba03..c270c81e8bb4 100644
> --- a/drivers/remoteproc/qcom_q6v5_pas.c
> +++ b/drivers/remoteproc/qcom_q6v5_pas.c
> @@ -28,6 +28,7 @@
>  #include <linux/soc/qcom/mdt_loader.h>
>  #include <linux/soc/qcom/smem.h>
>  #include <linux/soc/qcom/smem_state.h>
> +#include <dt-bindings/power/qcom,rpmhpd.h>
>  
>  #include "qcom_common.h"
>  #include "qcom_pil_info.h"
> @@ -51,6 +52,7 @@ struct qcom_pas_data {
>  	bool decrypt_shutdown;
>  
>  	char **proxy_pd_names;
> +	const unsigned int *proxy_pd_performance_states;
>  
>  	const char *load_state;
>  	const char *ssr_name;
> @@ -79,6 +81,7 @@ struct qcom_pas {
>  	struct regulator *px_supply;
>  
>  	struct device *proxy_pds[3];
> +	const unsigned int *proxy_pd_performance_states;
>  
>  	int proxy_pd_count;
>  
> @@ -167,7 +170,17 @@ static int qcom_pas_pds_enable(struct qcom_pas *pas, struct device **pds,
>  	int i;
>  
>  	for (i = 0; i < pd_count; i++) {
> -		dev_pm_genpd_set_performance_state(pds[i], INT_MAX);
> +		unsigned int state = INT_MAX;
> +
> +		if (pas->proxy_pd_performance_states)
> +			state = pas->proxy_pd_performance_states[i];
> +
> +		ret = dev_pm_genpd_set_performance_state(pds[i], state);
> +		if (ret)
> +			dev_warn(pas->dev,
> +				 "failed to set proxy PD %d state %u: %d\n",
> +				 i, state, ret);
> +
>  		ret = pm_runtime_get_sync(pds[i]);
>  		if (ret < 0) {
>  			pm_runtime_put_noidle(pds[i]);
> @@ -873,6 +886,7 @@ static int qcom_pas_probe(struct platform_device *pdev)
>  	pas->info_name = desc->sysmon_name;
>  	pas->smem_host_id = desc->smem_host_id;
>  	pas->decrypt_shutdown = desc->decrypt_shutdown;
> +	pas->proxy_pd_performance_states = desc->proxy_pd_performance_states;
>  	pas->region_assign_idx = desc->region_assign_idx;
>  	pas->region_assign_count = min_t(int, MAX_ASSIGN_COUNT, desc->region_assign_count);
>  	pas->region_assign_vmid = desc->region_assign_vmid;
> @@ -1798,6 +1812,32 @@ static const struct qcom_pas_data glymur_soccp_resource = {
>  	.needs_tzmem = true,
>  };
>  
> +static const struct qcom_pas_data hawi_cdsp_resource = {
> +	.crash_reason_smem = 601,
> +	.firmware_name = "cdsp.mdt",
> +	.dtb_firmware_name = "cdsp_dtb.mdt",
> +	.pas_id = 18,
> +	.dtb_pas_id = 0x25,
> +	.minidump_id = 7,
> +	.auto_boot = true,
> +	.proxy_pd_names = (char*[]){
> +		"cx",
> +		"mxc",
> +		"nsp",
> +		NULL
> +	},
> +	.proxy_pd_performance_states = (const unsigned int[]){
> +		RPMH_REGULATOR_LEVEL_TURBO,
> +		RPMH_REGULATOR_LEVEL_TURBO,
> +		RPMH_REGULATOR_LEVEL_NOM,
> +	},
> +	.load_state = "cdsp",
> +	.ssr_name = "cdsp",
> +	.sysmon_name = "cdsp",
> +	.ssctl_id = 0x17,
> +	.smem_host_id = 5,
> +};
> +
>  static const struct qcom_pas_data eliza_cdsp_resource = {
>  	.crash_reason_smem = 601,
>  	.firmware_name = "cdsp.mbn",
> @@ -1827,6 +1867,7 @@ static const struct of_device_id qcom_pas_of_match[] = {
>  	{ .compatible = "qcom,eliza-adsp-pas", .data = &sm8550_adsp_resource },
>  	{ .compatible = "qcom,eliza-cdsp-pas", .data = &eliza_cdsp_resource },
>  	{ .compatible = "qcom,glymur-soccp-pas", .data = &glymur_soccp_resource },
> +	{ .compatible = "qcom,hawi-cdsp-pas", .data = &hawi_cdsp_resource },
>  	{ .compatible = "qcom,kaanapali-soccp-pas", .data = &kaanapali_soccp_resource },
>  	{ .compatible = "qcom,milos-adsp-pas", .data = &sm8550_adsp_resource },
>  	{ .compatible = "qcom,milos-cdsp-pas", .data = &milos_cdsp_resource },
> -- 
> 2.55.0
>