Re: [PATCH v4 2/2] remoteproc: qcom_q6v5_pas: add per-PD proxy performance states for Hawi CDSP

[Mukesh Ojha]

On Fri, Sep 11, 2026 at 06:54:34AM +0200, Manivannan Sadhasivam wrote:
> On Thu, Sep 10, 2026 at 08:36:38PM +0530, Mukesh Ojha wrote:
> > The proxy power domain enable path requests INT_MAX performance
> > state for every proxy PD. On Hawi, the NSP proxy power domain's
> > RPMH power domain has no OPP at INT_MAX, while CX and MXC accept
> > INT_MAX, mapping to their maximum supported level.
> > 
> > Introduce a proxy_pd_performance_states array in qcom_pas_data
> > to allow per-PD RPMH levels to be declared explicitly. Platforms
> > that omit this field retain the existing INT_MAX behaviour.
> > 
> > Add Hawi CDSP remoteproc support with the following proxy PD
> > performance states:
> > 
> >   CX:  RPMH_REGULATOR_LEVEL_TURBO
> >   MXC: RPMH_REGULATOR_LEVEL_TURBO
> >   NSP: RPMH_REGULATOR_LEVEL_NOM
> > 
> > Reviewed-by: Abel Vesa <abel.vesa@xxxxxxxxxxxxxxxx>
> > Tested-by: Yijie Yang <yijie.yang@xxxxxxxxxxxxxxxx>
> > Signed-off-by: Mukesh Ojha <mukesh.ojha@xxxxxxxxxxxxxxxx>
> > ---
> >  drivers/remoteproc/qcom_q6v5_pas.c | 48 +++++++++++++++++++++++++++++-
> >  1 file changed, 47 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/remoteproc/qcom_q6v5_pas.c b/drivers/remoteproc/qcom_q6v5_pas.c
> > index a005546c265d..96a2436b777d 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,8 @@ struct qcom_pas_data {
> >  	bool decrypt_shutdown;
> >  
> >  	char **proxy_pd_names;
> > +	const unsigned int *proxy_pd_performance_states;
> > +	unsigned int num_proxy_pd_performance_states;
> >  
> >  	const char *load_state;
> >  	const char *ssr_name;
> > @@ -79,6 +82,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 +171,12 @@ 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];
> > +
> > +		dev_pm_genpd_set_performance_state(pds[i], state);
> >  		ret = pm_runtime_get_sync(pds[i]);
> >  		if (ret < 0) {
> >  			pm_runtime_put_noidle(pds[i]);
> > @@ -873,6 +882,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;
> > @@ -908,6 +918,14 @@ static int qcom_pas_probe(struct platform_device *pdev)
> >  		goto unassign_mem;
> >  	pas->proxy_pd_count = ret;
> >  
> > +	if (WARN(desc->proxy_pd_performance_states &&
> 
> Do you really need WARN() here, and not dev_warn()? WARN() may cause panic on
> systems that use 'panic_on_warn' cmdline param. In mobile/IoT world, use of this
> cmdline params is not common, but I strongly suggest we scrutinize the use of
> WARN*() macros in new code.

Is it not panic_on_warn for the development environment?  However, I
don't see a problem in replacing it with dev_warn().

-- 
-Mukesh Ojha