mbox series

[v3,00/12] OPP API fixes and improvements

Message ID 20210118005524.27787-1-digetx@gmail.com
Headers show
Series OPP API fixes and improvements | expand

Message

Dmitry Osipenko Jan. 18, 2021, 12:55 a.m. UTC
Hi,

This series fixes problems and adds features to OPP API that are required
for implementation of a power domain driver for NVIDIA Tegra SoCs.

It is a continuation of [1], where Viresh Kumar asked to factor OPP
patches into a separate series. I factored out the patches into this
series, addressed the previous review comments and re-based patches
on top of [2], which replaced some of my patches that added resource-managed
helpers.

[1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130
[2] https://lore.kernel.org/linux-pm/20210101165507.19486-1-tiny.windzz@gmail.com/

Changelog:

v3: - Reordered patches by importance.

    - Added locking to dev_pm_opp_set_voltage().

    - Reworked "Fix adding OPP entries in a wrong order if rate is unavailable"
      patch, like it was suggested by Viresh Kumar.

    - Reworked "Support set_opp() customization without requiring to use
      regulators" patch, like it was suggested by Viresh Kumar.

      The opp_table->set_opp_data is now allocated by dev_pm_opp_register_set_opp_helper().

      The set_opp_data is refcounted now and can be allocated by any other
      OPP functions if this will become needed in the future for other OPP API
      changes.

Dmitry Osipenko (12):
  opp: Fix adding OPP entries in a wrong order if rate is unavailable
  opp: Filter out OPPs based on availability of a required-OPP
  opp: Correct debug message in _opp_add_static_v2()
  opp: Add dev_pm_opp_sync_regulators()
  opp: Add dev_pm_opp_set_voltage()
  opp: Add dev_pm_opp_find_level_ceil()
  opp: Add dev_pm_opp_get_required_pstate()
  opp: Add devm_pm_opp_register_set_opp_helper
  opp: Add devm_pm_opp_attach_genpd
  opp: Support set_opp() customization without requiring to use
    regulators
  opp: Handle missing OPP table in dev_pm_opp_xlate_performance_state()
  opp: Print OPP level in debug message of _opp_add_static_v2()

 drivers/opp/core.c     | 309 +++++++++++++++++++++++++++++++++++++++--
 drivers/opp/of.c       |   9 +-
 include/linux/pm_opp.h |  49 +++++++
 3 files changed, 349 insertions(+), 18 deletions(-)

Comments

Viresh Kumar Jan. 18, 2021, 8:14 a.m. UTC | #1
On 18-01-21, 03:55, Dmitry Osipenko wrote:
> The debug message always prints rate=0 instead of a proper value, fix it.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Nicolas Chauvet <kwizart@gmail.com>
> Tested-by: Matt Merhar <mattmerhar@protonmail.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/opp/of.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/opp/of.c b/drivers/opp/of.c
> index 50df483c7dc3..63b16cdba5ea 100644
> --- a/drivers/opp/of.c
> +++ b/drivers/opp/of.c
> @@ -755,7 +755,6 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
>  		struct device *dev, struct device_node *np)
>  {
>  	struct dev_pm_opp *new_opp;
> -	u64 rate = 0;
>  	u32 val;
>  	int ret;
>  	bool rate_not_available = false;
> @@ -772,7 +771,8 @@ static struct dev_pm_opp *_opp_add_static_v2(struct opp_table *opp_table,
>  
>  	/* Check if the OPP supports hardware's hierarchy of versions or not */
>  	if (!_opp_is_supported(dev, opp_table, np)) {
> -		dev_dbg(dev, "OPP not supported by hardware: %llu\n", rate);
> +		dev_dbg(dev, "OPP not supported by hardware: %lu\n",
> +			new_opp->rate);
>  		goto free_opp;
>  	}
>  

Applied and added Fixes tag. Thanks.
Viresh Kumar Jan. 18, 2021, 10:50 a.m. UTC | #2
On 18-01-21, 03:55, Dmitry Osipenko wrote:
> Add dev_pm_opp_get_required_pstate() which allows OPP users to retrieve
> required performance state of a given OPP.
> 
> Tested-by: Peter Geis <pgwipeout@gmail.com>
> Tested-by: Nicolas Chauvet <kwizart@gmail.com>
> Tested-by: Matt Merhar <mattmerhar@protonmail.com>
> Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
> ---
>  drivers/opp/core.c     | 22 ++++++++++++++++++++++
>  include/linux/pm_opp.h | 10 ++++++++++
>  2 files changed, 32 insertions(+)
> 
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index df0969002555..fde2ec00ab0e 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -145,6 +145,28 @@ unsigned int dev_pm_opp_get_level(struct dev_pm_opp *opp)
>  }
>  EXPORT_SYMBOL_GPL(dev_pm_opp_get_level);
>  
> +/**
> + * dev_pm_opp_get_required_pstate() - Gets the required performance state
> + *                                    corresponding to an available opp
> + * @opp:	opp for which performance state has to be returned for
> + * @index:	index of the required opp
> + *
> + * Return: performance state read from device tree corresponding to the
> + * required opp, else return 0.
> + */
> +unsigned int dev_pm_opp_get_required_pstate(struct dev_pm_opp *opp,
> +					    unsigned int index)
> +{
> +	if (IS_ERR_OR_NULL(opp) || !opp->available ||
> +	    index >= opp->opp_table->required_opp_count) {
> +		pr_err("%s: Invalid parameters\n", __func__);
> +		return 0;
> +	}
> +
> +	return opp->required_opps[index]->pstate;
> +}
> +EXPORT_SYMBOL_GPL(dev_pm_opp_get_required_pstate);
> +
>  /**
>   * dev_pm_opp_is_turbo() - Returns if opp is turbo OPP or not
>   * @opp: opp for which turbo mode is being verified
> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h
> index b7dc993487c7..e072148ae0e1 100644
> --- a/include/linux/pm_opp.h
> +++ b/include/linux/pm_opp.h
> @@ -98,6 +98,9 @@ unsigned long dev_pm_opp_get_freq(struct dev_pm_opp *opp);
>  
>  unsigned int dev_pm_opp_get_level(struct dev_pm_opp *opp);
>  
> +unsigned int dev_pm_opp_get_required_pstate(struct dev_pm_opp *opp,
> +					    unsigned int index);
> +
>  bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp);
>  
>  int dev_pm_opp_get_opp_count(struct device *dev);
> @@ -194,6 +197,13 @@ static inline unsigned int dev_pm_opp_get_level(struct dev_pm_opp *opp)
>  	return 0;
>  }
>  
> +static inline
> +unsigned int dev_pm_opp_get_required_pstate(struct dev_pm_opp *opp,
> +					    unsigned int index)
> +{
> +	return 0;
> +}
> +
>  static inline bool dev_pm_opp_is_turbo(struct dev_pm_opp *opp)
>  {
>  	return false;

Applied. Thanks.
Viresh Kumar Jan. 19, 2021, 6:36 a.m. UTC | #3
On 18-01-21, 21:48, Dmitry Osipenko wrote:
> 18.01.2021 14:44, Viresh Kumar пишет:

> > On 18-01-21, 03:55, Dmitry Osipenko wrote:

> >> diff --git a/include/linux/pm_opp.h b/include/linux/pm_opp.h

> >> index eefd0b15890c..c98fd2add563 100644

> >> --- a/include/linux/pm_opp.h

> >> +++ b/include/linux/pm_opp.h

> >> @@ -13,6 +13,7 @@

> >>  

> >>  #include <linux/energy_model.h>

> >>  #include <linux/err.h>

> >> +#include <linux/kref.h>

> >>  #include <linux/notifier.h>

> >>  

> >>  struct clk;

> >> @@ -74,6 +75,7 @@ struct dev_pm_opp_info {

> >>   * @regulator_count: Number of regulators

> >>   * @clk:	Pointer to clk

> >>   * @dev:	Pointer to the struct device

> >> + * @kref:	Reference counter

> >>   *

> >>   * This structure contains all information required for setting an OPP.

> >>   */

> >> @@ -85,6 +87,7 @@ struct dev_pm_set_opp_data {

> >>  	unsigned int regulator_count;

> >>  	struct clk *clk;

> >>  	struct device *dev;

> >> +	struct kref kref;

> >>  };

> > 

> > Instead of kref thing, allocate the memory for supplies from

> > dev_pm_opp_set_regulators() and store it in new entries in opp-table

> > and for rest of the data from dev_pm_opp_register_set_opp_helper(), to

> > which you can copy supplies pointers then.

> > 

> 

> Could you please show a code sample?


Sent a patch to you.

-- 
viresh
Dmitry Osipenko Jan. 19, 2021, 5:35 p.m. UTC | #4
18.01.2021 14:46, Viresh Kumar пишет:
> On 18-01-21, 03:55, Dmitry Osipenko wrote:

>> Hi,

>>

>> This series fixes problems and adds features to OPP API that are required

>> for implementation of a power domain driver for NVIDIA Tegra SoCs.

>>

>> It is a continuation of [1], where Viresh Kumar asked to factor OPP

>> patches into a separate series. I factored out the patches into this

>> series, addressed the previous review comments and re-based patches

>> on top of [2], which replaced some of my patches that added resource-managed

>> helpers.

>>

>> [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130

>> [2] https://lore.kernel.org/linux-pm/20210101165507.19486-1-tiny.windzz@gmail.com/

> 

> Hi Dmitry,

> 

> I have applied 9 out of 12 patches already. Thanks.

> 


Thanks, I checked that everything is applied properly using today's
linux-next.
Dmitry Osipenko Jan. 20, 2021, 3:41 p.m. UTC | #5
19.01.2021 20:35, Dmitry Osipenko пишет:
> 18.01.2021 14:46, Viresh Kumar пишет:

>> On 18-01-21, 03:55, Dmitry Osipenko wrote:

>>> Hi,

>>>

>>> This series fixes problems and adds features to OPP API that are required

>>> for implementation of a power domain driver for NVIDIA Tegra SoCs.

>>>

>>> It is a continuation of [1], where Viresh Kumar asked to factor OPP

>>> patches into a separate series. I factored out the patches into this

>>> series, addressed the previous review comments and re-based patches

>>> on top of [2], which replaced some of my patches that added resource-managed

>>> helpers.

>>>

>>> [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130

>>> [2] https://lore.kernel.org/linux-pm/20210101165507.19486-1-tiny.windzz@gmail.com/

>>

>> Hi Dmitry,

>>

>> I have applied 9 out of 12 patches already. Thanks.

>>

> 

> Thanks, I checked that everything is applied properly using today's

> linux-next.

> 


Turned out that one minor issue was actually introduced, the
devm_pm_opp_attach_genpd() lost the export. I'll make a patch to fix this.
Viresh Kumar Jan. 21, 2021, 7:51 a.m. UTC | #6
On 20-01-21, 18:41, Dmitry Osipenko wrote:
> 19.01.2021 20:35, Dmitry Osipenko пишет:

> > 18.01.2021 14:46, Viresh Kumar пишет:

> >> On 18-01-21, 03:55, Dmitry Osipenko wrote:

> >>> Hi,

> >>>

> >>> This series fixes problems and adds features to OPP API that are required

> >>> for implementation of a power domain driver for NVIDIA Tegra SoCs.

> >>>

> >>> It is a continuation of [1], where Viresh Kumar asked to factor OPP

> >>> patches into a separate series. I factored out the patches into this

> >>> series, addressed the previous review comments and re-based patches

> >>> on top of [2], which replaced some of my patches that added resource-managed

> >>> helpers.

> >>>

> >>> [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130

> >>> [2] https://lore.kernel.org/linux-pm/20210101165507.19486-1-tiny.windzz@gmail.com/

> >>

> >> Hi Dmitry,

> >>

> >> I have applied 9 out of 12 patches already. Thanks.

> >>

> > 

> > Thanks, I checked that everything is applied properly using today's

> > linux-next.

> > 

> 

> Turned out that one minor issue was actually introduced, the

> devm_pm_opp_attach_genpd() lost the export. I'll make a patch to fix this.


I have fixed the original patch for that.

-- 
viresh
Dmitry Osipenko Jan. 21, 2021, 8:30 p.m. UTC | #7
21.01.2021 10:51, Viresh Kumar пишет:
> On 20-01-21, 18:41, Dmitry Osipenko wrote:

>> 19.01.2021 20:35, Dmitry Osipenko пишет:

>>> 18.01.2021 14:46, Viresh Kumar пишет:

>>>> On 18-01-21, 03:55, Dmitry Osipenko wrote:

>>>>> Hi,

>>>>>

>>>>> This series fixes problems and adds features to OPP API that are required

>>>>> for implementation of a power domain driver for NVIDIA Tegra SoCs.

>>>>>

>>>>> It is a continuation of [1], where Viresh Kumar asked to factor OPP

>>>>> patches into a separate series. I factored out the patches into this

>>>>> series, addressed the previous review comments and re-based patches

>>>>> on top of [2], which replaced some of my patches that added resource-managed

>>>>> helpers.

>>>>>

>>>>> [1] https://patchwork.ozlabs.org/project/linux-tegra/list/?series=221130

>>>>> [2] https://lore.kernel.org/linux-pm/20210101165507.19486-1-tiny.windzz@gmail.com/

>>>>

>>>> Hi Dmitry,

>>>>

>>>> I have applied 9 out of 12 patches already. Thanks.

>>>>

>>>

>>> Thanks, I checked that everything is applied properly using today's

>>> linux-next.

>>>

>>

>> Turned out that one minor issue was actually introduced, the

>> devm_pm_opp_attach_genpd() lost the export. I'll make a patch to fix this.

> 

> I have fixed the original patch for that.

> 


Okay, thank you.