public inbox for linux-pm@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
@ 2025-01-15 11:39 Sudeep Holla
  2025-01-16 15:54 ` Ulf Hansson
  0 siblings, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2025-01-15 11:39 UTC (permalink / raw)
  To: arm-scmi, linux-pm, Ulf Hansson
  Cc: Sudeep Holla, Cristian Marussi, Ranjani Vaidyanathan, Peng Fan

On a system with multiple active SCMI agents, one agent(other than OSPM/
Linux or bootloader) would request to turn on a shared power domain
before the Linux boots/initialise the genpds. So when the Linux boots
and gets the power state as already ON, it just registers the genpd with
a default ON state.

However, when the driver that needs this shared power domain is probed
genpd sees that the power domain status is ON and never makes any SCMI
call to power it up which is correct. But, since Linux didn't make an
explicit request to turn on the shared power domain, the SCMI platform
firmware will not know if the OSPM agent is actively using it.

Suppose the other agent that requested the shared power domain to be
powered ON requests to power it OFF as it no longer needs it, the SCMI
platform firmware needs to turn it off if there are no active users of
it which in the above scenaro is the case.

As a result of SCMI platform firmware turning off the resource, OSPM/
Linux will crash the moment as it expects the shared power domain to be
powered ON.

Send an explicit request to set the current state when setting up the
genpd power domains so that OSPM registers its vote in the power domain
state with the SCMI platform firmware.

The other option is to not read the state and set the genpds as default
OFF, but it can't handle the scenario on certain platforms where SCMI
platform keeps all the power domains turned ON by default for faster boot
(or any other such variations) and expect the OSPM to turn off the unused
domains if power saving is required.

Cc: Ulf Hansson <ulf.hansson@linaro.org>
Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
Reported-by: Peng Fan <peng.fan@nxp.com>
Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
---
 drivers/pmdomain/arm/scmi_pm_domain.c | 8 ++++++++
 1 file changed, 8 insertions(+)

Hi Ulf,

We need your feedback or suggestions for better solution on this matter.
Let us know if we can do something better with the genpd's help to resolve
this issue.

Regards,
Sudeep

diff --git a/drivers/pmdomain/arm/scmi_pm_domain.c b/drivers/pmdomain/arm/scmi_pm_domain.c
index a7784a8bb5db..86b531e15b85 100644
--- a/drivers/pmdomain/arm/scmi_pm_domain.c
+++ b/drivers/pmdomain/arm/scmi_pm_domain.c
@@ -96,6 +96,14 @@ static int scmi_pm_domain_probe(struct scmi_device *sdev)
 			continue;
 		}
 
+		/*
+		 * Register the explicit power on request to the firmware so
+		 * that it is tracked as used by OSPM agent and not
+		 * accidentally turned off with OSPM's knowledge
+		 */
+		if (state == SCMI_POWER_STATE_GENERIC_ON)
+			power_ops->state_set(ph, i, state);
+
 		scmi_pd->domain = i;
 		scmi_pd->ph = ph;
 		scmi_pd->name = power_ops->name_get(ph, i);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
  2025-01-15 11:39 [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state Sudeep Holla
@ 2025-01-16 15:54 ` Ulf Hansson
  2025-01-16 16:27   ` Cristian Marussi
  2025-01-16 16:29   ` Sudeep Holla
  0 siblings, 2 replies; 7+ messages in thread
From: Ulf Hansson @ 2025-01-16 15:54 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: arm-scmi, linux-pm, Cristian Marussi, Ranjani Vaidyanathan,
	Peng Fan

On Wed, 15 Jan 2025 at 12:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On a system with multiple active SCMI agents, one agent(other than OSPM/
> Linux or bootloader) would request to turn on a shared power domain
> before the Linux boots/initialise the genpds. So when the Linux boots
> and gets the power state as already ON, it just registers the genpd with
> a default ON state.
>
> However, when the driver that needs this shared power domain is probed
> genpd sees that the power domain status is ON and never makes any SCMI
> call to power it up which is correct. But, since Linux didn't make an
> explicit request to turn on the shared power domain, the SCMI platform
> firmware will not know if the OSPM agent is actively using it.
>
> Suppose the other agent that requested the shared power domain to be
> powered ON requests to power it OFF as it no longer needs it, the SCMI
> platform firmware needs to turn it off if there are no active users of
> it which in the above scenaro is the case.
>
> As a result of SCMI platform firmware turning off the resource, OSPM/
> Linux will crash the moment as it expects the shared power domain to be
> powered ON.
>
> Send an explicit request to set the current state when setting up the
> genpd power domains so that OSPM registers its vote in the power domain
> state with the SCMI platform firmware.
>
> The other option is to not read the state and set the genpds as default
> OFF, but it can't handle the scenario on certain platforms where SCMI
> platform keeps all the power domains turned ON by default for faster boot
> (or any other such variations) and expect the OSPM to turn off the unused
> domains if power saving is required.
>
> Cc: Ulf Hansson <ulf.hansson@linaro.org>
> Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
> Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
> Reported-by: Peng Fan <peng.fan@nxp.com>
> Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>

I read up on the discussion and it looks like there is not really a
simple solution here.

In principle if a boot-loader wants to do a handover and leave the
power-domain powered-on for the kernel, the additional call to
->state_set() *could* bump the usagecount in the SCMI FW, forever
leaving the power-domain on.

I guess this problem only exists for power-domains being shared across
scmi agents. Perhaps some kind of configuration flag can help us to
determine what to do?

> ---
>  drivers/pmdomain/arm/scmi_pm_domain.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> Hi Ulf,
>
> We need your feedback or suggestions for better solution on this matter.
> Let us know if we can do something better with the genpd's help to resolve
> this issue.

At initialization, genpd tries to get the correct state of the HW. If
the power-domain is on, genpd believes that it will stay on until it
requests it to be powered-off.

That said, I know we have FW's that aren't capable of informing us
about the power-domain's current state. For this, I (and Abel Vesa)
have been exploring how to introduce an "unknown-default" power-state,
which would mean that genpd would request on or off the first time
there is a transition of the genpd's power-state. This is on my TODO
list, perhaps that is something that can help here too?

Kind regards
Uffe

>
> Regards,
> Sudeep
>
> diff --git a/drivers/pmdomain/arm/scmi_pm_domain.c b/drivers/pmdomain/arm/scmi_pm_domain.c
> index a7784a8bb5db..86b531e15b85 100644
> --- a/drivers/pmdomain/arm/scmi_pm_domain.c
> +++ b/drivers/pmdomain/arm/scmi_pm_domain.c
> @@ -96,6 +96,14 @@ static int scmi_pm_domain_probe(struct scmi_device *sdev)
>                         continue;
>                 }
>
> +               /*
> +                * Register the explicit power on request to the firmware so
> +                * that it is tracked as used by OSPM agent and not
> +                * accidentally turned off with OSPM's knowledge
> +                */
> +               if (state == SCMI_POWER_STATE_GENERIC_ON)
> +                       power_ops->state_set(ph, i, state);
> +
>                 scmi_pd->domain = i;
>                 scmi_pd->ph = ph;
>                 scmi_pd->name = power_ops->name_get(ph, i);
> --
> 2.34.1
>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
  2025-01-16 15:54 ` Ulf Hansson
@ 2025-01-16 16:27   ` Cristian Marussi
  2025-01-17 11:11     ` Ulf Hansson
  2025-01-16 16:29   ` Sudeep Holla
  1 sibling, 1 reply; 7+ messages in thread
From: Cristian Marussi @ 2025-01-16 16:27 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: Sudeep Holla, arm-scmi, linux-pm, Cristian Marussi,
	Ranjani Vaidyanathan, Peng Fan

On Thu, Jan 16, 2025 at 04:54:44PM +0100, Ulf Hansson wrote:
> On Wed, 15 Jan 2025 at 12:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On a system with multiple active SCMI agents, one agent(other than OSPM/
> > Linux or bootloader) would request to turn on a shared power domain
> > before the Linux boots/initialise the genpds. So when the Linux boots
> > and gets the power state as already ON, it just registers the genpd with
> > a default ON state.
> >
> > However, when the driver that needs this shared power domain is probed
> > genpd sees that the power domain status is ON and never makes any SCMI
> > call to power it up which is correct. But, since Linux didn't make an
> > explicit request to turn on the shared power domain, the SCMI platform
> > firmware will not know if the OSPM agent is actively using it.
> >
> > Suppose the other agent that requested the shared power domain to be
> > powered ON requests to power it OFF as it no longer needs it, the SCMI
> > platform firmware needs to turn it off if there are no active users of
> > it which in the above scenaro is the case.
> >
> > As a result of SCMI platform firmware turning off the resource, OSPM/
> > Linux will crash the moment as it expects the shared power domain to be
> > powered ON.
> >
> > Send an explicit request to set the current state when setting up the
> > genpd power domains so that OSPM registers its vote in the power domain
> > state with the SCMI platform firmware.
> >
> > The other option is to not read the state and set the genpds as default
> > OFF, but it can't handle the scenario on certain platforms where SCMI
> > platform keeps all the power domains turned ON by default for faster boot
> > (or any other such variations) and expect the OSPM to turn off the unused
> > domains if power saving is required.
> >
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
> > Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
> > Reported-by: Peng Fan <peng.fan@nxp.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 
HI Ulf,

> I read up on the discussion and it looks like there is not really a
> simple solution here.
> 
> In principle if a boot-loader wants to do a handover and leave the
> power-domain powered-on for the kernel, the additional call to
> ->state_set() *could* bump the usagecount in the SCMI FW, forever
> leaving the power-domain on.
> 

TWell, in theory, this should NOT be the case in the SCMI world since
the resource accounting is supposed to happen per-agent...so multiple
ONs from the same agent should be only accounted as ONE request from
agent_A, because the assumption is that multiple requests from the same
agent wouldl have already been filtered out from within the agent
frameowrks itself, like genPD of CLock...so the platform really takes
care tro harmonize access between shared resources from multiple
different agents.

Now, I fear also, though, that all of the above is IMPDEF and only a
reccomendation, even though the (few) systems I aware of follow this
per-agent-accounting approach.

Thanks,
Cristian

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
  2025-01-16 15:54 ` Ulf Hansson
  2025-01-16 16:27   ` Cristian Marussi
@ 2025-01-16 16:29   ` Sudeep Holla
  2025-01-17 11:17     ` Ulf Hansson
  1 sibling, 1 reply; 7+ messages in thread
From: Sudeep Holla @ 2025-01-16 16:29 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: arm-scmi, linux-pm, Cristian Marussi, Ranjani Vaidyanathan,
	Peng Fan

On Thu, Jan 16, 2025 at 04:54:44PM +0100, Ulf Hansson wrote:
> On Wed, 15 Jan 2025 at 12:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On a system with multiple active SCMI agents, one agent(other than OSPM/
> > Linux or bootloader) would request to turn on a shared power domain
> > before the Linux boots/initialise the genpds. So when the Linux boots
> > and gets the power state as already ON, it just registers the genpd with
> > a default ON state.
> >
> > However, when the driver that needs this shared power domain is probed
> > genpd sees that the power domain status is ON and never makes any SCMI
> > call to power it up which is correct. But, since Linux didn't make an
> > explicit request to turn on the shared power domain, the SCMI platform
> > firmware will not know if the OSPM agent is actively using it.
> >
> > Suppose the other agent that requested the shared power domain to be
> > powered ON requests to power it OFF as it no longer needs it, the SCMI
> > platform firmware needs to turn it off if there are no active users of
> > it which in the above scenaro is the case.
> >
> > As a result of SCMI platform firmware turning off the resource, OSPM/
> > Linux will crash the moment as it expects the shared power domain to be
> > powered ON.
> >
> > Send an explicit request to set the current state when setting up the
> > genpd power domains so that OSPM registers its vote in the power domain
> > state with the SCMI platform firmware.
> >
> > The other option is to not read the state and set the genpds as default
> > OFF, but it can't handle the scenario on certain platforms where SCMI
> > platform keeps all the power domains turned ON by default for faster boot
> > (or any other such variations) and expect the OSPM to turn off the unused
> > domains if power saving is required.
> >
> > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
> > Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
> > Reported-by: Peng Fan <peng.fan@nxp.com>
> > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> 
> I read up on the discussion and it looks like there is not really a
> simple solution here.
> 
> In principle if a boot-loader wants to do a handover and leave the
> power-domain powered-on for the kernel, the additional call to
> ->state_set() *could* bump the usagecount in the SCMI FW, forever
> leaving the power-domain on.
>

IIUC, the refcount in firmware differs from the one in the kernel. It is
refcount per agent i.e. it is really just a kind of boolean to indicate if
the agent is active user of the resource. So if the bootloader and the Linux
being the same agent request to be turned on without a request to turn off
doesn't mean the refcount is set to 2 and Linux needs to turn off twice.
This is just my opinion and understanding.

> I guess this problem only exists for power-domains being shared across
> scmi agents. Perhaps some kind of configuration flag can help us to
> determine what to do?
>

While I can't disagree, there is also a thought that OS shouldn't be aware
of that detail for equally valid reasons. I am not sure if we can get that
added in the spec.

> > ---
> >  drivers/pmdomain/arm/scmi_pm_domain.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > Hi Ulf,
> >
> > We need your feedback or suggestions for better solution on this matter.
> > Let us know if we can do something better with the genpd's help to resolve
> > this issue.
> 
> At initialization, genpd tries to get the correct state of the HW. If
> the power-domain is on, genpd believes that it will stay on until it
> requests it to be powered-off.
>

Agreed and it is right. I don't think that should change.

> That said, I know we have FW's that aren't capable of informing us
> about the power-domain's current state. For this, I (and Abel Vesa)
> have been exploring how to introduce an "unknown-default" power-state,
> which would mean that genpd would request on or off the first time
> there is a transition of the genpd's power-state. This is on my TODO
> list, perhaps that is something that can help here too?
>

Ah interesting. But I still can't get my head around how this fits with
in with the existing 2 extreme use cases. One where f/w leaves everything
on to speed up boot(which is fair requirement) and one where OS is expected
to turn on when it needs a resource(which is ideal but may impact boot time
loads of power domains needs to be turned on.

IIUC if we initialise it to unknown state, we will issue ON request when
we use the resource in the kernel and OFF when we are trying to power off
unused on. It may defer the request to the firmware and is move this hack
away from the genpd provider driver into core to deal with this "unknown"
state. Sounds not bad to me if it is feasible to implement.

--
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
  2025-01-16 16:27   ` Cristian Marussi
@ 2025-01-17 11:11     ` Ulf Hansson
  0 siblings, 0 replies; 7+ messages in thread
From: Ulf Hansson @ 2025-01-17 11:11 UTC (permalink / raw)
  To: Cristian Marussi
  Cc: Sudeep Holla, arm-scmi, linux-pm, Ranjani Vaidyanathan, Peng Fan

On Thu, 16 Jan 2025 at 17:27, Cristian Marussi <cristian.marussi@arm.com> wrote:
>
> On Thu, Jan 16, 2025 at 04:54:44PM +0100, Ulf Hansson wrote:
> > On Wed, 15 Jan 2025 at 12:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On a system with multiple active SCMI agents, one agent(other than OSPM/
> > > Linux or bootloader) would request to turn on a shared power domain
> > > before the Linux boots/initialise the genpds. So when the Linux boots
> > > and gets the power state as already ON, it just registers the genpd with
> > > a default ON state.
> > >
> > > However, when the driver that needs this shared power domain is probed
> > > genpd sees that the power domain status is ON and never makes any SCMI
> > > call to power it up which is correct. But, since Linux didn't make an
> > > explicit request to turn on the shared power domain, the SCMI platform
> > > firmware will not know if the OSPM agent is actively using it.
> > >
> > > Suppose the other agent that requested the shared power domain to be
> > > powered ON requests to power it OFF as it no longer needs it, the SCMI
> > > platform firmware needs to turn it off if there are no active users of
> > > it which in the above scenaro is the case.
> > >
> > > As a result of SCMI platform firmware turning off the resource, OSPM/
> > > Linux will crash the moment as it expects the shared power domain to be
> > > powered ON.
> > >
> > > Send an explicit request to set the current state when setting up the
> > > genpd power domains so that OSPM registers its vote in the power domain
> > > state with the SCMI platform firmware.
> > >
> > > The other option is to not read the state and set the genpds as default
> > > OFF, but it can't handle the scenario on certain platforms where SCMI
> > > platform keeps all the power domains turned ON by default for faster boot
> > > (or any other such variations) and expect the OSPM to turn off the unused
> > > domains if power saving is required.
> > >
> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > > Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
> > > Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
> > > Reported-by: Peng Fan <peng.fan@nxp.com>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> HI Ulf,
>
> > I read up on the discussion and it looks like there is not really a
> > simple solution here.
> >
> > In principle if a boot-loader wants to do a handover and leave the
> > power-domain powered-on for the kernel, the additional call to
> > ->state_set() *could* bump the usagecount in the SCMI FW, forever
> > leaving the power-domain on.
> >
>
> TWell, in theory, this should NOT be the case in the SCMI world since
> the resource accounting is supposed to happen per-agent...so multiple
> ONs from the same agent should be only accounted as ONE request from
> agent_A, because the assumption is that multiple requests from the same
> agent wouldl have already been filtered out from within the agent
> frameowrks itself, like genPD of CLock...so the platform really takes
> care tro harmonize access between shared resources from multiple
> different agents.
>
> Now, I fear also, though, that all of the above is IMPDEF and only a
> reccomendation, even though the (few) systems I aware of follow this
> per-agent-accounting approach.

Thanks for clarifying! I assume I can consider this as a reviewed-by
for $subject patch, if not please let me know.

>
> Thanks,
> Cristian

Kind regards
Uffe

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
  2025-01-16 16:29   ` Sudeep Holla
@ 2025-01-17 11:17     ` Ulf Hansson
  2025-01-17 14:24       ` Sudeep Holla
  0 siblings, 1 reply; 7+ messages in thread
From: Ulf Hansson @ 2025-01-17 11:17 UTC (permalink / raw)
  To: Sudeep Holla
  Cc: arm-scmi, linux-pm, Cristian Marussi, Ranjani Vaidyanathan,
	Peng Fan

On Thu, 16 Jan 2025 at 17:29, Sudeep Holla <sudeep.holla@arm.com> wrote:
>
> On Thu, Jan 16, 2025 at 04:54:44PM +0100, Ulf Hansson wrote:
> > On Wed, 15 Jan 2025 at 12:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > >
> > > On a system with multiple active SCMI agents, one agent(other than OSPM/
> > > Linux or bootloader) would request to turn on a shared power domain
> > > before the Linux boots/initialise the genpds. So when the Linux boots
> > > and gets the power state as already ON, it just registers the genpd with
> > > a default ON state.
> > >
> > > However, when the driver that needs this shared power domain is probed
> > > genpd sees that the power domain status is ON and never makes any SCMI
> > > call to power it up which is correct. But, since Linux didn't make an
> > > explicit request to turn on the shared power domain, the SCMI platform
> > > firmware will not know if the OSPM agent is actively using it.
> > >
> > > Suppose the other agent that requested the shared power domain to be
> > > powered ON requests to power it OFF as it no longer needs it, the SCMI
> > > platform firmware needs to turn it off if there are no active users of
> > > it which in the above scenaro is the case.
> > >
> > > As a result of SCMI platform firmware turning off the resource, OSPM/
> > > Linux will crash the moment as it expects the shared power domain to be
> > > powered ON.
> > >
> > > Send an explicit request to set the current state when setting up the
> > > genpd power domains so that OSPM registers its vote in the power domain
> > > state with the SCMI platform firmware.
> > >
> > > The other option is to not read the state and set the genpds as default
> > > OFF, but it can't handle the scenario on certain platforms where SCMI
> > > platform keeps all the power domains turned ON by default for faster boot
> > > (or any other such variations) and expect the OSPM to turn off the unused
> > > domains if power saving is required.
> > >
> > > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > > Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
> > > Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
> > > Reported-by: Peng Fan <peng.fan@nxp.com>
> > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> >
> > I read up on the discussion and it looks like there is not really a
> > simple solution here.
> >
> > In principle if a boot-loader wants to do a handover and leave the
> > power-domain powered-on for the kernel, the additional call to
> > ->state_set() *could* bump the usagecount in the SCMI FW, forever
> > leaving the power-domain on.
> >
>
> IIUC, the refcount in firmware differs from the one in the kernel. It is
> refcount per agent i.e. it is really just a kind of boolean to indicate if
> the agent is active user of the resource. So if the bootloader and the Linux
> being the same agent request to be turned on without a request to turn off
> doesn't mean the refcount is set to 2 and Linux needs to turn off twice.
> This is just my opinion and understanding.
>
> > I guess this problem only exists for power-domains being shared across
> > scmi agents. Perhaps some kind of configuration flag can help us to
> > determine what to do?
> >
>
> While I can't disagree, there is also a thought that OS shouldn't be aware
> of that detail for equally valid reasons. I am not sure if we can get that
> added in the spec.

Okay, it seems like $subject patch is the way forward at this moment.

I have applied it for *next* to allow it to be a bit more tested
before we decide if this is material for stable kernels too. That
means we may have to send the patch to stable maintainers manually to
get it applied.

Kind regards
Uffe

>
> > > ---
> > >  drivers/pmdomain/arm/scmi_pm_domain.c | 8 ++++++++
> > >  1 file changed, 8 insertions(+)
> > >
> > > Hi Ulf,
> > >
> > > We need your feedback or suggestions for better solution on this matter.
> > > Let us know if we can do something better with the genpd's help to resolve
> > > this issue.
> >
> > At initialization, genpd tries to get the correct state of the HW. If
> > the power-domain is on, genpd believes that it will stay on until it
> > requests it to be powered-off.
> >
>
> Agreed and it is right. I don't think that should change.
>
> > That said, I know we have FW's that aren't capable of informing us
> > about the power-domain's current state. For this, I (and Abel Vesa)
> > have been exploring how to introduce an "unknown-default" power-state,
> > which would mean that genpd would request on or off the first time
> > there is a transition of the genpd's power-state. This is on my TODO
> > list, perhaps that is something that can help here too?
> >
>
> Ah interesting. But I still can't get my head around how this fits with
> in with the existing 2 extreme use cases. One where f/w leaves everything
> on to speed up boot(which is fair requirement) and one where OS is expected
> to turn on when it needs a resource(which is ideal but may impact boot time
> loads of power domains needs to be turned on.
>
> IIUC if we initialise it to unknown state, we will issue ON request when
> we use the resource in the kernel and OFF when we are trying to power off
> unused on. It may defer the request to the firmware and is move this hack
> away from the genpd provider driver into core to deal with this "unknown"
> state. Sounds not bad to me if it is feasible to implement.
>
> --
> Regards,
> Sudeep

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state
  2025-01-17 11:17     ` Ulf Hansson
@ 2025-01-17 14:24       ` Sudeep Holla
  0 siblings, 0 replies; 7+ messages in thread
From: Sudeep Holla @ 2025-01-17 14:24 UTC (permalink / raw)
  To: Ulf Hansson
  Cc: arm-scmi, linux-pm, Sudeep Holla, Cristian Marussi,
	Ranjani Vaidyanathan, Peng Fan

On Fri, Jan 17, 2025 at 12:17:39PM +0100, Ulf Hansson wrote:
> On Thu, 16 Jan 2025 at 17:29, Sudeep Holla <sudeep.holla@arm.com> wrote:
> >
> > On Thu, Jan 16, 2025 at 04:54:44PM +0100, Ulf Hansson wrote:
> > > On Wed, 15 Jan 2025 at 12:39, Sudeep Holla <sudeep.holla@arm.com> wrote:
> > > >
> > > > On a system with multiple active SCMI agents, one agent(other than OSPM/
> > > > Linux or bootloader) would request to turn on a shared power domain
> > > > before the Linux boots/initialise the genpds. So when the Linux boots
> > > > and gets the power state as already ON, it just registers the genpd with
> > > > a default ON state.
> > > >
> > > > However, when the driver that needs this shared power domain is probed
> > > > genpd sees that the power domain status is ON and never makes any SCMI
> > > > call to power it up which is correct. But, since Linux didn't make an
> > > > explicit request to turn on the shared power domain, the SCMI platform
> > > > firmware will not know if the OSPM agent is actively using it.
> > > >
> > > > Suppose the other agent that requested the shared power domain to be
> > > > powered ON requests to power it OFF as it no longer needs it, the SCMI
> > > > platform firmware needs to turn it off if there are no active users of
> > > > it which in the above scenaro is the case.
> > > >
> > > > As a result of SCMI platform firmware turning off the resource, OSPM/
> > > > Linux will crash the moment as it expects the shared power domain to be
> > > > powered ON.
> > > >
> > > > Send an explicit request to set the current state when setting up the
> > > > genpd power domains so that OSPM registers its vote in the power domain
> > > > state with the SCMI platform firmware.
> > > >
> > > > The other option is to not read the state and set the genpds as default
> > > > OFF, but it can't handle the scenario on certain platforms where SCMI
> > > > platform keeps all the power domains turned ON by default for faster boot
> > > > (or any other such variations) and expect the OSPM to turn off the unused
> > > > domains if power saving is required.
> > > >
> > > > Cc: Ulf Hansson <ulf.hansson@linaro.org>
> > > > Link: https://lore.kernel.org/all/Z4aBkezSWOPCXcUh@bogus
> > > > Reported-by: Ranjani Vaidyanathan <ranjani.vaidyanathan@nxp.com>
> > > > Reported-by: Peng Fan <peng.fan@nxp.com>
> > > > Signed-off-by: Sudeep Holla <sudeep.holla@arm.com>
> > >
> > > I read up on the discussion and it looks like there is not really a
> > > simple solution here.
> > >
> > > In principle if a boot-loader wants to do a handover and leave the
> > > power-domain powered-on for the kernel, the additional call to
> > > ->state_set() *could* bump the usagecount in the SCMI FW, forever
> > > leaving the power-domain on.
> > >
> >
> > IIUC, the refcount in firmware differs from the one in the kernel. It is
> > refcount per agent i.e. it is really just a kind of boolean to indicate if
> > the agent is active user of the resource. So if the bootloader and the Linux
> > being the same agent request to be turned on without a request to turn off
> > doesn't mean the refcount is set to 2 and Linux needs to turn off twice.
> > This is just my opinion and understanding.
> >
> > > I guess this problem only exists for power-domains being shared across
> > > scmi agents. Perhaps some kind of configuration flag can help us to
> > > determine what to do?
> > >
> >
> > While I can't disagree, there is also a thought that OS shouldn't be aware
> > of that detail for equally valid reasons. I am not sure if we can get that
> > added in the spec.
>
> Okay, it seems like $subject patch is the way forward at this moment.
>
> I have applied it for *next* to allow it to be a bit more tested
> before we decide if this is material for stable kernels too. That
> means we may have to send the patch to stable maintainers manually to
> get it applied.

Thanks. We should remember to drop this if we manage to get "unknown"
default/initial state supported in core genpd. I liked the proposal
you made, it is just that I feel we invariably end up sending the ON/OFF
request to the firmware anyways(can't think of any other way), it is just
deferred to a point where the domain is used and at late initcall when
unused genpds are powered off.

--
Regards,
Sudeep

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-01-17 14:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-15 11:39 [PATCH] pmdomain: arm: scmi_pm_domain: Send an explicit request to set the current state Sudeep Holla
2025-01-16 15:54 ` Ulf Hansson
2025-01-16 16:27   ` Cristian Marussi
2025-01-17 11:11     ` Ulf Hansson
2025-01-16 16:29   ` Sudeep Holla
2025-01-17 11:17     ` Ulf Hansson
2025-01-17 14:24       ` Sudeep Holla

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox