Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <8140a64b-3f3e-b8c7-7edc-03f5cc7d14e7@quicinc.com>
Date:   Mon, 2 Jan 2023 15:26:39 +0530
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Thunderbird/91.3.2
Subject: Re: [PATCH v4 1/5] PM: domains: Allow a genpd consumer to require a
 synced power off
Content-Language: en-US
To:     Bjorn Andersson <andersson@kernel.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>
CC:     freedreno <freedreno@lists.freedesktop.org>,
        <dri-devel@lists.freedesktop.org>, <linux-arm-msm@vger.kernel.org>,
        Rob Clark <robdclark@gmail.com>,
        Ulf Hansson <ulf.hansson@linaro.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Philipp Zabel <p.zabel@pengutronix.de>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Kevin Hilman <khilman@kernel.org>,
        Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
        <linux-kernel@vger.kernel.org>, <linux-pm@vger.kernel.org>
References: <1671642843-5244-1-git-send-email-quic_akhilpo@quicinc.com>
 <20221221224338.v4.1.I3e6b1f078ad0f1ca9358c573daa7b70ec132cdbe@changeid>
 <20221228184304.4lldh7dxp2pnplzc@builder.lan>
From:   Akhil P Oommen <quic_akhilpo@quicinc.com>
In-Reply-To: <20221228184304.4lldh7dxp2pnplzc@builder.lan>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 12/29/2022 12:13 AM, Bjorn Andersson wrote:
> On Wed, Dec 21, 2022 at 10:43:59PM +0530, Akhil P Oommen wrote:
>> From: Ulf Hansson <ulf.hansson@linaro.org>
>>
>> Some genpd providers doesn't ensure that it has turned off at hardware.
>> This is fine until the consumer really requires during some special
>> scenarios that the power domain collapse at hardware before it is
>> turned ON again.
>>
>> An example is the reset sequence of Adreno GPU which requires that the
>> 'gpucc cx gdsc' power domain should move to OFF state in hardware at
>> least once before turning in ON again to clear the internal state.
>>
>> Signed-off-by: Ulf Hansson <ulf.hansson@linaro.org>
>> Signed-off-by: Akhil P Oommen <quic_akhilpo@quicinc.com>
> Reviewed-by: Bjorn Andersson <andersson@kernel.org>
>
> @Rafael, would you be willing to share an immutable branch with this
> change? Or would you be okay with me doing so from the qcom clock tree?
>
> Regards,
> Bjorn
Rafael, gentle ping. Could you please check Bjorn's question here?

-Akhil.
>
>> ---
>>
>> Changes in v4:
>> - Update genpd function documentation (Ulf)
>>
>> Changes in v2:
>> - Minor formatting fix
>>
>>  drivers/base/power/domain.c | 26 ++++++++++++++++++++++++++
>>  include/linux/pm_domain.h   |  5 +++++
>>  2 files changed, 31 insertions(+)
>>
>> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
>> index 967bcf9d415e..84662d338188 100644
>> --- a/drivers/base/power/domain.c
>> +++ b/drivers/base/power/domain.c
>> @@ -519,6 +519,31 @@ ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
>>  }
>>  EXPORT_SYMBOL_GPL(dev_pm_genpd_get_next_hrtimer);
>>  
>> +/*
>> + * dev_pm_genpd_synced_poweroff - Next power off should be synchronous
>> + *
>> + * @dev: A device that is attached to the genpd.
>> + *
>> + * Allows a consumer of the genpd to notify the provider that the next power off
>> + * should be synchronous.
>> + *
>> + * It is assumed that the users guarantee that the genpd wouldn't be detached
>> + * while this routine is getting called.
>> + */
>> +void dev_pm_genpd_synced_poweroff(struct device *dev)
>> +{
>> +	struct generic_pm_domain *genpd;
>> +
>> +	genpd = dev_to_genpd_safe(dev);
>> +	if (!genpd)
>> +		return;
>> +
>> +	genpd_lock(genpd);
>> +	genpd->synced_poweroff = true;
>> +	genpd_unlock(genpd);
>> +}
>> +EXPORT_SYMBOL_GPL(dev_pm_genpd_synced_poweroff);
>> +
>>  static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
>>  {
>>  	unsigned int state_idx = genpd->state_idx;
>> @@ -562,6 +587,7 @@ static int _genpd_power_on(struct generic_pm_domain *genpd, bool timed)
>>  
>>  out:
>>  	raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_ON, NULL);
>> +	genpd->synced_poweroff = false;
>>  	return 0;
>>  err:
>>  	raw_notifier_call_chain(&genpd->power_notifiers, GENPD_NOTIFY_OFF,
>> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
>> index 1cd41bdf73cf..f776fb93eaa0 100644
>> --- a/include/linux/pm_domain.h
>> +++ b/include/linux/pm_domain.h
>> @@ -136,6 +136,7 @@ struct generic_pm_domain {
>>  	unsigned int prepared_count;	/* Suspend counter of prepared devices */
>>  	unsigned int performance_state;	/* Aggregated max performance state */
>>  	cpumask_var_t cpus;		/* A cpumask of the attached CPUs */
>> +	bool synced_poweroff;		/* A consumer needs a synced poweroff */
>>  	int (*power_off)(struct generic_pm_domain *domain);
>>  	int (*power_on)(struct generic_pm_domain *domain);
>>  	struct raw_notifier_head power_notifiers; /* Power on/off notifiers */
>> @@ -235,6 +236,7 @@ int dev_pm_genpd_add_notifier(struct device *dev, struct notifier_block *nb);
>>  int dev_pm_genpd_remove_notifier(struct device *dev);
>>  void dev_pm_genpd_set_next_wakeup(struct device *dev, ktime_t next);
>>  ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev);
>> +void dev_pm_genpd_synced_poweroff(struct device *dev);
>>  
>>  extern struct dev_power_governor simple_qos_governor;
>>  extern struct dev_power_governor pm_domain_always_on_gov;
>> @@ -300,6 +302,9 @@ static inline ktime_t dev_pm_genpd_get_next_hrtimer(struct device *dev)
>>  {
>>  	return KTIME_MAX;
>>  }
>> +static inline void dev_pm_genpd_synced_poweroff(struct device *dev)
>> +{ }
>> +
>>  #define simple_qos_governor		(*(struct dev_power_governor *)(NULL))
>>  #define pm_domain_always_on_gov		(*(struct dev_power_governor *)(NULL))
>>  #endif
>> -- 
>> 2.7.4
>>