* [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state
@ 2023-06-21 14:40 Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 1/4] driver core: Add dev_set_drv_sync_state() Abel Vesa
` (4 more replies)
0 siblings, 5 replies; 10+ messages in thread
From: Abel Vesa @ 2023-06-21 14:40 UTC (permalink / raw)
To: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Greg Kroah-Hartman, Saravana Kannan
Cc: Bjorn Andersson, linux-pm, Linux Kernel Mailing List,
linux-arm-msm, Dmitry Baryshkov
This new approach drops the is_off change that was part of v4. That was
kind of beyond the scope of this patchset. This new approach changes the
boot_keep_on in such a way that we won't need any kind of new locking
for a PD. This involves using the patch [1] for adding dev_set_drv_sync_state
from Saravana for allowing the genpd core to set a default sync state
callback for a provider that doesn't register one by itself. While at it,
we can add another such API but this time to query a device's sync state.
Then, we filter out each power off request in such a way that if a boot
powered on power domain is not attached to its consumer device and
the provider has not state synced yet, the power off request is skipped.
[1] https://lore.kernel.org/all/20210407034456.516204-2-saravanak@google.com/
No worth mentioning what changed since v4 as this version is almost
entirely reworked.
Abel Vesa (3):
driver core: Add dev_is_drv_state_synced()
PM: domains: Ignore power off request for enabled unused domains
PM: domains: Add and set generic sync state callback
Saravana Kannan (1):
driver core: Add dev_set_drv_sync_state()
drivers/base/power/domain.c | 72 +++++++++++++++++++++++++++++++++++++
include/linux/device.h | 26 ++++++++++++++
include/linux/pm_domain.h | 4 +++
3 files changed, 102 insertions(+)
--
2.34.1
^ permalink raw reply [flat|nested] 10+ messages in thread
* [RFC PATCH v5 1/4] driver core: Add dev_set_drv_sync_state()
2023-06-21 14:40 [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state Abel Vesa
@ 2023-06-21 14:40 ` Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced() Abel Vesa
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Abel Vesa @ 2023-06-21 14:40 UTC (permalink / raw)
To: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Greg Kroah-Hartman, Saravana Kannan
Cc: Bjorn Andersson, linux-pm, Linux Kernel Mailing List,
linux-arm-msm, Dmitry Baryshkov
From: Saravana Kannan <saravanak@google.com>
This can be used by frameworks to set the sync_state() helper functions
for drivers that don't already have them set.
Signed-off-by: Saravana Kannan <saravanak@google.com>
Acked-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
include/linux/device.h | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index 66c13965153d..bae11928ef7e 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -995,6 +995,18 @@ static inline bool dev_has_sync_state(struct device *dev)
return false;
}
+static inline int dev_set_drv_sync_state(struct device *dev,
+ void (*fn)(struct device *dev))
+{
+ if (!dev || !dev->driver)
+ return 0;
+ if (dev->driver->sync_state && dev->driver->sync_state != fn)
+ return -EBUSY;
+ if (!dev->driver->sync_state)
+ dev->driver->sync_state = fn;
+ return 0;
+}
+
static inline void dev_set_removable(struct device *dev,
enum device_removable removable)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced()
2023-06-21 14:40 [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 1/4] driver core: Add dev_set_drv_sync_state() Abel Vesa
@ 2023-06-21 14:40 ` Abel Vesa
2023-06-21 15:22 ` Greg Kroah-Hartman
2023-06-21 14:40 ` [RFC PATCH v5 3/4] PM: domains: Ignore power off request for enabled unused domains Abel Vesa
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Abel Vesa @ 2023-06-21 14:40 UTC (permalink / raw)
To: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Greg Kroah-Hartman, Saravana Kannan
Cc: Bjorn Andersson, linux-pm, Linux Kernel Mailing List,
linux-arm-msm, Dmitry Baryshkov
This can be used by drivers to figure out if a different device
driver has state synced or not for a specific device.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
include/linux/device.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/linux/device.h b/include/linux/device.h
index bae11928ef7e..8f042f04b5d9 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -1007,6 +1007,20 @@ static inline int dev_set_drv_sync_state(struct device *dev,
return 0;
}
+static inline bool dev_is_drv_state_synced(struct device *dev)
+{
+ bool ret = false;
+
+ if (!dev)
+ return ret;
+
+ device_lock(dev);
+ ret = dev->state_synced;
+ device_unlock(dev);
+
+ return ret;
+}
+
static inline void dev_set_removable(struct device *dev,
enum device_removable removable)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v5 3/4] PM: domains: Ignore power off request for enabled unused domains
2023-06-21 14:40 [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 1/4] driver core: Add dev_set_drv_sync_state() Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced() Abel Vesa
@ 2023-06-21 14:40 ` Abel Vesa
2023-07-04 13:24 ` Ulf Hansson
2023-06-21 14:40 ` [RFC PATCH v5 4/4] PM: domains: Add and set generic sync state callback Abel Vesa
[not found] ` <2786d9ff-0579-429b-b431-a8547cbf6fb6@ti>
4 siblings, 1 reply; 10+ messages in thread
From: Abel Vesa @ 2023-06-21 14:40 UTC (permalink / raw)
To: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Greg Kroah-Hartman, Saravana Kannan
Cc: Bjorn Andersson, linux-pm, Linux Kernel Mailing List,
linux-arm-msm, Dmitry Baryshkov
First of, safekeep the boot state that is provided on init, then use this
boot state to make decisions whether a power off request should be
ignored or not. In case a domain was left enabled before boot, most
likely such domain is needed and should not be disabled on the 'disable
unused' late initcall, but rather needs to stay powered on until the
consumer driver gets a chance to probe. In order to keep such domain
powered on until the consumer handles it correctly, the domain needs to
be registered by a provider that has a sync_state callback registered
and said provider has state synced.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
drivers/base/power/domain.c | 49 +++++++++++++++++++++++++++++++++++++
include/linux/pm_domain.h | 1 +
2 files changed, 50 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 51b9d4eaab5e..5967ade160e2 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -654,6 +654,43 @@ static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
queue_work(pm_wq, &genpd->power_off_work);
}
+/**
+ * genpd_keep_on - Tells if the domain should skip the power 'off' request
+ * @genpd: PM domain to be checked.
+ *
+ * If the domain's current state meets the following conditions:
+ * - marked for being kept as enabled
+ * - has a provider with a sync state callback registered
+ * - the provider hasn't state synced yet
+ * then the power 'off' request should be skipped.
+ *
+ * This function should only be called from genpd_power_off and with
+ * the lock held.
+ */
+static inline bool genpd_keep_on(struct generic_pm_domain *genpd)
+{
+ bool ret = false;
+
+ if (!(genpd->boot_keep_on))
+ return false;
+
+ if (!genpd->has_provider)
+ goto out;
+
+ if (!dev_has_sync_state(genpd->provider->dev))
+ goto out;
+
+ if (dev_is_drv_state_synced(genpd->provider->dev))
+ goto out;
+
+ return true;
+
+out:
+ genpd->boot_keep_on = false;
+
+ return ret;
+}
+
/**
* genpd_power_off - Remove power from a given PM domain.
* @genpd: PM domain to power down.
@@ -682,6 +719,13 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
if (!genpd_status_on(genpd) || genpd->prepared_count > 0)
return 0;
+ /*
+ * If the domain is enabled and unused, bail out and ignore
+ * the 'off' request until the provider has state synced.
+ */
+ if (genpd_keep_on(genpd))
+ return -EBUSY;
+
/*
* Abort power off for the PM domain in the following situations:
* (1) The domain is configured as always on.
@@ -2065,6 +2109,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
atomic_set(&genpd->sd_count, 0);
genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON;
genpd->device_count = 0;
+ genpd->boot_keep_on = !is_off;
genpd->provider = NULL;
genpd->has_provider = false;
genpd->accounting_time = ktime_get_mono_fast_ns();
@@ -2718,6 +2763,10 @@ static void genpd_dev_pm_sync(struct device *dev)
if (IS_ERR(pd))
return;
+ genpd_lock(pd);
+ pd->boot_keep_on = false;
+ genpd_unlock(pd);
+
genpd_queue_power_off_work(pd);
}
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index f776fb93eaa0..3eb32c4b6d4f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -131,6 +131,7 @@ struct generic_pm_domain {
const char *name;
atomic_t sd_count; /* Number of subdomains with power "on" */
enum gpd_status status; /* Current state of the domain */
+ bool boot_keep_on; /* Keep enabled during 'disable unused' late initcall */
unsigned int device_count; /* Number of devices */
unsigned int suspended_count; /* System suspend device counter */
unsigned int prepared_count; /* Suspend counter of prepared devices */
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [RFC PATCH v5 4/4] PM: domains: Add and set generic sync state callback
2023-06-21 14:40 [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state Abel Vesa
` (2 preceding siblings ...)
2023-06-21 14:40 ` [RFC PATCH v5 3/4] PM: domains: Ignore power off request for enabled unused domains Abel Vesa
@ 2023-06-21 14:40 ` Abel Vesa
2023-07-04 13:54 ` Ulf Hansson
[not found] ` <2786d9ff-0579-429b-b431-a8547cbf6fb6@ti>
4 siblings, 1 reply; 10+ messages in thread
From: Abel Vesa @ 2023-06-21 14:40 UTC (permalink / raw)
To: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Greg Kroah-Hartman, Saravana Kannan
Cc: Bjorn Andersson, linux-pm, Linux Kernel Mailing List,
linux-arm-msm, Dmitry Baryshkov
For every provider that doesn't register a sync_state callback,
register a generic one. This new generic sync_state callback queues up a
power off request.
Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
---
drivers/base/power/domain.c | 23 +++++++++++++++++++++++
include/linux/pm_domain.h | 3 +++
2 files changed, 26 insertions(+)
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
index 5967ade160e2..ec16db0599ac 100644
--- a/drivers/base/power/domain.c
+++ b/drivers/base/power/domain.c
@@ -654,6 +654,27 @@ static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
queue_work(pm_wq, &genpd->power_off_work);
}
+/**
+ * pm_genpd_power_off_unused_sync_state - Power off all domains for provider.
+ * @dev: Provider's device.
+ *
+ * Request power off for all unused domains of the provider.
+ * This should be used exclusively as sync state callback for genpd providers.
+ */
+void pm_genpd_power_off_unused_sync_state(struct device *dev)
+{
+ struct generic_pm_domain *genpd;
+
+ mutex_lock(&gpd_list_lock);
+
+ list_for_each_entry(genpd, &gpd_list, gpd_list_node)
+ if (genpd->provider && genpd->provider->dev == dev)
+ genpd_queue_power_off_work(genpd);
+
+ mutex_unlock(&gpd_list_lock);
+}
+EXPORT_SYMBOL_GPL(pm_genpd_power_off_unused_sync_state);
+
/**
* genpd_keep_on - Tells if the domain should skip the power 'off' request
* @genpd: PM domain to be checked.
@@ -2329,6 +2350,8 @@ static int genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
cp->xlate = xlate;
fwnode_dev_initialized(&np->fwnode, true);
+ dev_set_drv_sync_state(np->fwnode.dev, pm_genpd_power_off_unused_sync_state);
+
mutex_lock(&of_genpd_mutex);
list_add(&cp->link, &of_genpd_providers);
mutex_unlock(&of_genpd_mutex);
diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
index 3eb32c4b6d4f..78164244b89f 100644
--- a/include/linux/pm_domain.h
+++ b/include/linux/pm_domain.h
@@ -279,6 +279,9 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
return -EOPNOTSUPP;
}
+static inline void pm_genpd_power_off_unused_sync_state(struct device *dev)
+{ }
+
static inline int dev_pm_genpd_set_performance_state(struct device *dev,
unsigned int state)
{
--
2.34.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced()
2023-06-21 14:40 ` [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced() Abel Vesa
@ 2023-06-21 15:22 ` Greg Kroah-Hartman
2023-06-28 11:50 ` Abel Vesa
0 siblings, 1 reply; 10+ messages in thread
From: Greg Kroah-Hartman @ 2023-06-21 15:22 UTC (permalink / raw)
To: Abel Vesa
Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Saravana Kannan, Bjorn Andersson, linux-pm,
Linux Kernel Mailing List, linux-arm-msm, Dmitry Baryshkov
On Wed, Jun 21, 2023 at 05:40:17PM +0300, Abel Vesa wrote:
> This can be used by drivers to figure out if a different device
> driver has state synced or not for a specific device.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> include/linux/device.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/linux/device.h b/include/linux/device.h
> index bae11928ef7e..8f042f04b5d9 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -1007,6 +1007,20 @@ static inline int dev_set_drv_sync_state(struct device *dev,
> return 0;
> }
>
> +static inline bool dev_is_drv_state_synced(struct device *dev)
> +{
> + bool ret = false;
> +
> + if (!dev)
> + return ret;
> +
> + device_lock(dev);
> + ret = dev->state_synced;
> + device_unlock(dev);
This lock is "protecting" nothing, given that the value can instantly
change after it is read.
Because it can change, how will this function actually show anything
relevant?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced()
2023-06-21 15:22 ` Greg Kroah-Hartman
@ 2023-06-28 11:50 ` Abel Vesa
0 siblings, 0 replies; 10+ messages in thread
From: Abel Vesa @ 2023-06-28 11:50 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Rafael J . Wysocki, Kevin Hilman, Ulf Hansson, Len Brown,
Pavel Machek, Saravana Kannan, Bjorn Andersson, linux-pm,
Linux Kernel Mailing List, linux-arm-msm, Dmitry Baryshkov
On 23-06-21 17:22:54, Greg Kroah-Hartman wrote:
> On Wed, Jun 21, 2023 at 05:40:17PM +0300, Abel Vesa wrote:
> > This can be used by drivers to figure out if a different device
> > driver has state synced or not for a specific device.
> >
> > Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> > ---
> > include/linux/device.h | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/include/linux/device.h b/include/linux/device.h
> > index bae11928ef7e..8f042f04b5d9 100644
> > --- a/include/linux/device.h
> > +++ b/include/linux/device.h
> > @@ -1007,6 +1007,20 @@ static inline int dev_set_drv_sync_state(struct device *dev,
> > return 0;
> > }
> >
> > +static inline bool dev_is_drv_state_synced(struct device *dev)
> > +{
> > + bool ret = false;
> > +
> > + if (!dev)
> > + return ret;
> > +
> > + device_lock(dev);
> > + ret = dev->state_synced;
> > + device_unlock(dev);
>
> This lock is "protecting" nothing, given that the value can instantly
> change after it is read.
Hmm, for some reason I thought it needs to be synchronized with the
sync state callback being called already. But I just noticed that call
to the sync state callback is independently locked after state_synced is
set. So I guess the lock can go away here.
>
> Because it can change, how will this function actually show anything
> relevant?
The only usecase I can think of for this new API is for some driver
to delay an action until ultimately the driver for a specific device
gets state synced. So even if the value can change after it has
been checked, such consumer driver will most likely retry later on.
Hope that makes sense.
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v5 3/4] PM: domains: Ignore power off request for enabled unused domains
2023-06-21 14:40 ` [RFC PATCH v5 3/4] PM: domains: Ignore power off request for enabled unused domains Abel Vesa
@ 2023-07-04 13:24 ` Ulf Hansson
0 siblings, 0 replies; 10+ messages in thread
From: Ulf Hansson @ 2023-07-04 13:24 UTC (permalink / raw)
To: Abel Vesa
Cc: Rafael J . Wysocki, Kevin Hilman, Len Brown, Pavel Machek,
Greg Kroah-Hartman, Saravana Kannan, Bjorn Andersson, linux-pm,
Linux Kernel Mailing List, linux-arm-msm, Dmitry Baryshkov
On Wed, 21 Jun 2023 at 16:40, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> First of, safekeep the boot state that is provided on init, then use this
> boot state to make decisions whether a power off request should be
> ignored or not. In case a domain was left enabled before boot, most
> likely such domain is needed and should not be disabled on the 'disable
> unused' late initcall, but rather needs to stay powered on until the
> consumer driver gets a chance to probe. In order to keep such domain
> powered on until the consumer handles it correctly, the domain needs to
> be registered by a provider that has a sync_state callback registered
> and said provider has state synced.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/base/power/domain.c | 49 +++++++++++++++++++++++++++++++++++++
> include/linux/pm_domain.h | 1 +
> 2 files changed, 50 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 51b9d4eaab5e..5967ade160e2 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -654,6 +654,43 @@ static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
> queue_work(pm_wq, &genpd->power_off_work);
> }
>
> +/**
> + * genpd_keep_on - Tells if the domain should skip the power 'off' request
> + * @genpd: PM domain to be checked.
> + *
> + * If the domain's current state meets the following conditions:
> + * - marked for being kept as enabled
> + * - has a provider with a sync state callback registered
> + * - the provider hasn't state synced yet
> + * then the power 'off' request should be skipped.
> + *
> + * This function should only be called from genpd_power_off and with
> + * the lock held.
> + */
> +static inline bool genpd_keep_on(struct generic_pm_domain *genpd)
> +{
> + bool ret = false;
> +
> + if (!(genpd->boot_keep_on))
> + return false;
> +
> + if (!genpd->has_provider)
> + goto out;
Hmm, resetting the boot_keep_on flag based on the above condition
isn't really working, I think.
genpd_power_off() may be called before/after there is an OF provider
assigned/removed for the genpd. With the current genpd APIs
(pm_genpd_init() and of_genpd_add_provider_*()), we have at least two
separate function calls to complete the initialization of the genpd
provider(s). Theoretically, we can't know when genpd_power_off() may
be called, especially if there are child-domains being used too.
It looks to me that we should not clear the boot_keep_on flag at all
in this path. Instead, we should rather bail out and return false, to
prevent the genpd from being powered off. Although this should be fine
for most cases, we have some genpd providers, which don't use OF
providers at all (pm-s3c64xx, amdgpu_acp). To deal with these cases,
we seem to need an opt-out solution (maybe a new genpd configuration
bit) that they can set, before calling pm_genpd_init().
That said, it looks like the genpd->has_provider seems not to be
entirely protected by the genpd lock (not in this path, but in other
paths in genpd). I think we need to fix that too, in some way or the
other.
> +
> + if (!dev_has_sync_state(genpd->provider->dev))
> + goto out;
> +
> + if (dev_is_drv_state_synced(genpd->provider->dev))
> + goto out;
> +
> + return true;
> +
> +out:
> + genpd->boot_keep_on = false;
> +
> + return ret;
> +}
> +
> /**
> * genpd_power_off - Remove power from a given PM domain.
> * @genpd: PM domain to power down.
> @@ -682,6 +719,13 @@ static int genpd_power_off(struct generic_pm_domain *genpd, bool one_dev_on,
> if (!genpd_status_on(genpd) || genpd->prepared_count > 0)
> return 0;
>
> + /*
> + * If the domain is enabled and unused, bail out and ignore
> + * the 'off' request until the provider has state synced.
> + */
> + if (genpd_keep_on(genpd))
> + return -EBUSY;
> +
> /*
> * Abort power off for the PM domain in the following situations:
> * (1) The domain is configured as always on.
> @@ -2065,6 +2109,7 @@ int pm_genpd_init(struct generic_pm_domain *genpd,
> atomic_set(&genpd->sd_count, 0);
> genpd->status = is_off ? GENPD_STATE_OFF : GENPD_STATE_ON;
> genpd->device_count = 0;
> + genpd->boot_keep_on = !is_off;
> genpd->provider = NULL;
> genpd->has_provider = false;
> genpd->accounting_time = ktime_get_mono_fast_ns();
> @@ -2718,6 +2763,10 @@ static void genpd_dev_pm_sync(struct device *dev)
> if (IS_ERR(pd))
> return;
>
> + genpd_lock(pd);
> + pd->boot_keep_on = false;
This should not be needed. I think you can drop this.
> + genpd_unlock(pd);
> +
> genpd_queue_power_off_work(pd);
> }
>
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index f776fb93eaa0..3eb32c4b6d4f 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -131,6 +131,7 @@ struct generic_pm_domain {
> const char *name;
> atomic_t sd_count; /* Number of subdomains with power "on" */
> enum gpd_status status; /* Current state of the domain */
> + bool boot_keep_on; /* Keep enabled during 'disable unused' late initcall */
> unsigned int device_count; /* Number of devices */
> unsigned int suspended_count; /* System suspend device counter */
> unsigned int prepared_count; /* Suspend counter of prepared devices */
Kind regards
Uffe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v5 4/4] PM: domains: Add and set generic sync state callback
2023-06-21 14:40 ` [RFC PATCH v5 4/4] PM: domains: Add and set generic sync state callback Abel Vesa
@ 2023-07-04 13:54 ` Ulf Hansson
0 siblings, 0 replies; 10+ messages in thread
From: Ulf Hansson @ 2023-07-04 13:54 UTC (permalink / raw)
To: Abel Vesa
Cc: Rafael J . Wysocki, Kevin Hilman, Len Brown, Pavel Machek,
Greg Kroah-Hartman, Saravana Kannan, Bjorn Andersson, linux-pm,
Linux Kernel Mailing List, linux-arm-msm, Dmitry Baryshkov
On Wed, 21 Jun 2023 at 16:40, Abel Vesa <abel.vesa@linaro.org> wrote:
>
> For every provider that doesn't register a sync_state callback,
> register a generic one. This new generic sync_state callback queues up a
> power off request.
>
> Signed-off-by: Abel Vesa <abel.vesa@linaro.org>
> ---
> drivers/base/power/domain.c | 23 +++++++++++++++++++++++
> include/linux/pm_domain.h | 3 +++
> 2 files changed, 26 insertions(+)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 5967ade160e2..ec16db0599ac 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -654,6 +654,27 @@ static void genpd_queue_power_off_work(struct generic_pm_domain *genpd)
> queue_work(pm_wq, &genpd->power_off_work);
> }
>
> +/**
> + * pm_genpd_power_off_unused_sync_state - Power off all domains for provider.
> + * @dev: Provider's device.
> + *
> + * Request power off for all unused domains of the provider.
> + * This should be used exclusively as sync state callback for genpd providers.
> + */
> +void pm_genpd_power_off_unused_sync_state(struct device *dev)
> +{
> + struct generic_pm_domain *genpd;
> +
> + mutex_lock(&gpd_list_lock);
> +
> + list_for_each_entry(genpd, &gpd_list, gpd_list_node)
> + if (genpd->provider && genpd->provider->dev == dev)
Not all genpds have the ->provider assigned. Moreover, the
of_genpd_mutex is protecting the list of providers, we should use that
instead.
> + genpd_queue_power_off_work(genpd);
> +
> + mutex_unlock(&gpd_list_lock);
> +}
> +EXPORT_SYMBOL_GPL(pm_genpd_power_off_unused_sync_state);
Why does this need to be exported? Is there a provider that assigns
it's own sync state callback that needs to call this? If that is the
case, I would prefer to see a user of this API as a part of the series
too.
> +
> /**
> * genpd_keep_on - Tells if the domain should skip the power 'off' request
> * @genpd: PM domain to be checked.
> @@ -2329,6 +2350,8 @@ static int genpd_add_provider(struct device_node *np, genpd_xlate_t xlate,
> cp->xlate = xlate;
> fwnode_dev_initialized(&np->fwnode, true);
>
> + dev_set_drv_sync_state(np->fwnode.dev, pm_genpd_power_off_unused_sync_state);
> +
> mutex_lock(&of_genpd_mutex);
> list_add(&cp->link, &of_genpd_providers);
> mutex_unlock(&of_genpd_mutex);
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 3eb32c4b6d4f..78164244b89f 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -279,6 +279,9 @@ static inline int pm_genpd_remove(struct generic_pm_domain *genpd)
> return -EOPNOTSUPP;
> }
>
> +static inline void pm_genpd_power_off_unused_sync_state(struct device *dev)
> +{ }
> +
> static inline int dev_pm_genpd_set_performance_state(struct device *dev,
> unsigned int state)
> {
Kind regards
Uffe
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state
[not found] ` <2786d9ff-0579-429b-b431-a8547cbf6fb6@ti>
@ 2025-04-07 16:13 ` Ulf Hansson
0 siblings, 0 replies; 10+ messages in thread
From: Ulf Hansson @ 2025-04-07 16:13 UTC (permalink / raw)
To: Devarsh Thakkar
Cc: Abel Vesa, Rafael J . Wysocki, Kevin Hilman, Len Brown,
Pavel Machek, Greg Kroah-Hartman, Saravana Kannan,
Bjorn Andersson, linux-pm, Linux Kernel Mailing List,
linux-arm-msm, Dmitry Baryshkov
On Mon, 7 Apr 2025 at 16:13, Devarsh Thakkar
<devarsht@lewv0571a.ent.ti.com> wrote:
>
> Hi Abel,
>
> On 21/06/23 20:10, Abel Vesa wrote:
>
> Thanks a lot for working on this.
>
> > This new approach drops the is_off change that was part of v4. That was
> > kind of beyond the scope of this patchset. This new approach changes the
> > boot_keep_on in such a way that we won't need any kind of new locking
> > for a PD. This involves using the patch [1] for adding dev_set_drv_sync_state
> > from Saravana for allowing the genpd core to set a default sync state
> > callback for a provider that doesn't register one by itself. While at it,
> > we can add another such API but this time to query a device's sync state.
> > Then, we filter out each power off request in such a way that if a boot
> > powered on power domain is not attached to its consumer device and
> > the provider has not state synced yet, the power off request is skipped.
> >
> > [1] https://lore.kernel.org/all/20210407034456.516204-2-saravanak@google.com/
> >
> > No worth mentioning what changed since v4 as this version is almost
> > entirely reworked.
> >
> > Abel Vesa (3):
> > driver core: Add dev_is_drv_state_synced()
> > PM: domains: Ignore power off request for enabled unused domains
> > PM: domains: Add and set generic sync state callback
> >
> > Saravana Kannan (1):
> > driver core: Add dev_set_drv_sync_state()
> >
>
> Could you please share if you are planning to re-spin this series as
> non-RFC in near future ?
>
> We think that these patches would be useful to enable smooth display
> transition from boot-loader to kernel space, something which our team is
> working on, so just wanted to know your plans for this series.
I am working on it as we speak, but I need a few more days for
testing. You should expect something to hit LKML later this week or at
least early next week.
I will keep you posted!
Kind regards
Uffe
>
>
> Regards
> Devarsh
>
> > drivers/base/power/domain.c | 72 +++++++++++++++++++++++++++++++++++++
> > include/linux/device.h | 26 ++++++++++++++
> > include/linux/pm_domain.h | 4 +++
> > 3 files changed, 102 insertions(+)
> >
>
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2025-04-07 16:14 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-21 14:40 [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 1/4] driver core: Add dev_set_drv_sync_state() Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 2/4] driver core: Add dev_is_drv_state_synced() Abel Vesa
2023-06-21 15:22 ` Greg Kroah-Hartman
2023-06-28 11:50 ` Abel Vesa
2023-06-21 14:40 ` [RFC PATCH v5 3/4] PM: domains: Ignore power off request for enabled unused domains Abel Vesa
2023-07-04 13:24 ` Ulf Hansson
2023-06-21 14:40 ` [RFC PATCH v5 4/4] PM: domains: Add and set generic sync state callback Abel Vesa
2023-07-04 13:54 ` Ulf Hansson
[not found] ` <2786d9ff-0579-429b-b431-a8547cbf6fb6@ti>
2025-04-07 16:13 ` [RFC PATCH v5 0/4] PM: domain: Support skiping disabling unused domains until sync state Ulf Hansson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).