mbox series

[RFC,v2,0/5] Allow genpd providers to power off domains on sync state

Message ID 20230320134217.1685781-1-abel.vesa@linaro.org (mailing list archive)
Headers show
Series Allow genpd providers to power off domains on sync state | expand

Message

Abel Vesa March 20, 2023, 1:42 p.m. UTC
There have been already a couple of tries to make the genpd "disable
unused" late initcall skip the powering off of domains that might be
needed until later on (i.e. until some consumer probes). The conclusion
was that the provider could return -EBUSY from the power_off callback
until the provider's sync state has been reached. This patch series tries
to provide a proof-of-concept that is working on Qualcomm platforms.

I've been doing extensive testing on SM8450, but I've also spinned this
on my X13s (SC8280XP). Both patches that add the sync state callback to
the SC8280XP and SM8450 are here to provide context. Once we agree on
the form, I intend to add the sync state callback to all gdsc providers.

Currently, some of the gdsc providers might not reach sync state due to
list of consumers not probing yet (or at all). The sync state can be
enforced by writing 1 to the state_synced sysfs attribute of the
provider, thanks to Saravana's commit [1] which has been already merged.

[1] https://lore.kernel.org/r/20230304005355.746421-3-saravanak@google.com

V1 of this patchset was here:
https://lore.kernel.org/all/20230315132330.450877-1-abel.vesa@linaro.org/

Changes since v1:
 * Added the qcom_cc sync state callback which calls in turn the gdsc one
 * dropped extra semicolon from pm_domain.h

Abel Vesa (5):
  PM: domains: Allow power off queuing from providers
  soc: qcom: rpmhpd: Do proper power off when state synced
  clk: qcom: gdsc: Avoid actual power off until sync state
  clk: qcom: Add sync state callback to all SC8280XP providers
  clk: qcom: Add sync state callback to all SM8450 providers

 drivers/base/power/domain.c        |  3 ++-
 drivers/clk/qcom/camcc-sm8450.c    |  1 +
 drivers/clk/qcom/common.c          | 19 +++++++++++++++++++
 drivers/clk/qcom/common.h          |  2 ++
 drivers/clk/qcom/dispcc-sc8280xp.c |  1 +
 drivers/clk/qcom/dispcc-sm8450.c   |  1 +
 drivers/clk/qcom/gcc-sc8280xp.c    |  1 +
 drivers/clk/qcom/gcc-sm8450.c      |  1 +
 drivers/clk/qcom/gdsc.c            | 26 ++++++++++++++++++++++++++
 drivers/clk/qcom/gdsc.h            |  6 ++++++
 drivers/clk/qcom/gpucc-sc8280xp.c  |  1 +
 drivers/soc/qcom/rpmhpd.c          | 19 +++++++------------
 include/linux/pm_domain.h          |  6 ++++++
 13 files changed, 74 insertions(+), 13 deletions(-)

Comments

Ulf Hansson March 21, 2023, 1:07 p.m. UTC | #1
On Mon, 20 Mar 2023 at 14:42, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> There have been already a couple of tries to make the genpd "disable
> unused" late initcall skip the powering off of domains that might be
> needed until later on (i.e. until some consumer probes). The conclusion
> was that the provider could return -EBUSY from the power_off callback
> until the provider's sync state has been reached. This patch series tries
> to provide a proof-of-concept that is working on Qualcomm platforms.
>
> I've been doing extensive testing on SM8450, but I've also spinned this
> on my X13s (SC8280XP). Both patches that add the sync state callback to
> the SC8280XP and SM8450 are here to provide context. Once we agree on
> the form, I intend to add the sync state callback to all gdsc providers.
>
> Currently, some of the gdsc providers might not reach sync state due to
> list of consumers not probing yet (or at all). The sync state can be
> enforced by writing 1 to the state_synced sysfs attribute of the
> provider, thanks to Saravana's commit [1] which has been already merged.
>
> [1] https://lore.kernel.org/r/20230304005355.746421-3-saravanak@google.com
>
> V1 of this patchset was here:
> https://lore.kernel.org/all/20230315132330.450877-1-abel.vesa@linaro.org/
>
> Changes since v1:
>  * Added the qcom_cc sync state callback which calls in turn the gdsc one
>  * dropped extra semicolon from pm_domain.h
>
> Abel Vesa (5):
>   PM: domains: Allow power off queuing from providers
>   soc: qcom: rpmhpd: Do proper power off when state synced
>   clk: qcom: gdsc: Avoid actual power off until sync state
>   clk: qcom: Add sync state callback to all SC8280XP providers
>   clk: qcom: Add sync state callback to all SM8450 providers
>
>  drivers/base/power/domain.c        |  3 ++-
>  drivers/clk/qcom/camcc-sm8450.c    |  1 +
>  drivers/clk/qcom/common.c          | 19 +++++++++++++++++++
>  drivers/clk/qcom/common.h          |  2 ++
>  drivers/clk/qcom/dispcc-sc8280xp.c |  1 +
>  drivers/clk/qcom/dispcc-sm8450.c   |  1 +
>  drivers/clk/qcom/gcc-sc8280xp.c    |  1 +
>  drivers/clk/qcom/gcc-sm8450.c      |  1 +
>  drivers/clk/qcom/gdsc.c            | 26 ++++++++++++++++++++++++++
>  drivers/clk/qcom/gdsc.h            |  6 ++++++
>  drivers/clk/qcom/gpucc-sc8280xp.c  |  1 +
>  drivers/soc/qcom/rpmhpd.c          | 19 +++++++------------
>  include/linux/pm_domain.h          |  6 ++++++
>  13 files changed, 74 insertions(+), 13 deletions(-)
>

Besides the minor comments on patch1, this looks good to me! So, feel
free to add:

Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org>

Kind regards
Uffe