| Message ID | 20210926224058.1252-3-digetx@gmail.com |
|---|---|
| State | Not Applicable |
| Headers | show |
| Series | NVIDIA Tegra power management patches for 5.16 | expand |
On Mon, 27 Sept 2021 at 00:42, Dmitry Osipenko <digetx@gmail.com> wrote: > > Only couple drivers need to get the -ENODEV error code and majority of > drivers need to explicitly initialize the performance state. Add new > common helper which sets up OPP table for these drivers. > > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > include/soc/tegra/common.h | 24 ++++++++++++++++++++++++ > 1 file changed, 24 insertions(+) > > diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h > index af41ad80ec21..5b4a042f60fb 100644 > --- a/include/soc/tegra/common.h > +++ b/include/soc/tegra/common.h > @@ -39,4 +39,28 @@ devm_tegra_core_dev_init_opp_table(struct device *dev, > } > #endif > > +/* > + * This function should be invoked with the enabled runtime PM of the device > + * in order to initialize performance state properly. Most of Tegra devices > + * are assumed to be suspended at a probe time and GENPD require RPM to be > + * enabled to set up the rpm-resume state, otherwise device is active and > + * performance state is applied immediately. Note that it will initialize > + * OPP bandwidth if it's wired in a device-tree for this device, which is > + * undesirable for a suspended device. > + */ > +static inline int > +devm_tegra_core_dev_init_opp_table_common(struct device *dev) > +{ > + struct tegra_core_opp_params opp_params = {}; > + int err; > + > + opp_params.init_state = true; > + > + err = devm_tegra_core_dev_init_opp_table(dev, &opp_params); > + if (err != -ENODEV) > + return err; > + > + return 0; > +} Just want to share a few thoughts around these functions. So, I assume it's fine to call devm_tegra_core_dev_init_opp_table_common() or devm_tegra_core_dev_init_opp_table() from consumer drivers during ->probe(), as long as those drivers are tegra specific, which I assume all are in the series!? My point is, a cross SoC consumer driver that needs to initiate OPP tables can get rather messy, if it would need to make one specific function call per SoC. That said, I hope we can tackle this as a separate/future problem, so the series can get merged as is. > + > #endif /* __SOC_TEGRA_COMMON_H__ */ > -- > 2.32.0 > Kind regards Uffe
01.10.2021 15:50, Ulf Hansson пишет: > On Mon, 27 Sept 2021 at 00:42, Dmitry Osipenko <digetx@gmail.com> wrote: >> >> Only couple drivers need to get the -ENODEV error code and majority of >> drivers need to explicitly initialize the performance state. Add new >> common helper which sets up OPP table for these drivers. >> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> include/soc/tegra/common.h | 24 ++++++++++++++++++++++++ >> 1 file changed, 24 insertions(+) >> >> diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h >> index af41ad80ec21..5b4a042f60fb 100644 >> --- a/include/soc/tegra/common.h >> +++ b/include/soc/tegra/common.h >> @@ -39,4 +39,28 @@ devm_tegra_core_dev_init_opp_table(struct device *dev, >> } >> #endif >> >> +/* >> + * This function should be invoked with the enabled runtime PM of the device >> + * in order to initialize performance state properly. Most of Tegra devices >> + * are assumed to be suspended at a probe time and GENPD require RPM to be >> + * enabled to set up the rpm-resume state, otherwise device is active and >> + * performance state is applied immediately. Note that it will initialize >> + * OPP bandwidth if it's wired in a device-tree for this device, which is >> + * undesirable for a suspended device. >> + */ >> +static inline int >> +devm_tegra_core_dev_init_opp_table_common(struct device *dev) >> +{ >> + struct tegra_core_opp_params opp_params = {}; >> + int err; >> + >> + opp_params.init_state = true; >> + >> + err = devm_tegra_core_dev_init_opp_table(dev, &opp_params); >> + if (err != -ENODEV) >> + return err; >> + >> + return 0; >> +} > > Just want to share a few thoughts around these functions. > > So, I assume it's fine to call > devm_tegra_core_dev_init_opp_table_common() or > devm_tegra_core_dev_init_opp_table() from consumer drivers during > ->probe(), as long as those drivers are tegra specific, which I assume > all are in the series!? That is correct, all drivers are tegra-specific in this series. External devices are attached to the internal SoC devices and this series is about the SoC power management. > My point is, a cross SoC consumer driver that needs to initiate OPP > tables can get rather messy, if it would need to make one specific > function call per SoC. > > That said, I hope we can tackle this as a separate/future problem, so > the series can get merged as is. Yes, as we already have seen, it's not an easy problem to make PD core to handle it in a generic way. If there will be a similar demand from other SoCs, then we may try to solve that problem again.
diff --git a/include/soc/tegra/common.h b/include/soc/tegra/common.h index af41ad80ec21..5b4a042f60fb 100644 --- a/include/soc/tegra/common.h +++ b/include/soc/tegra/common.h @@ -39,4 +39,28 @@ devm_tegra_core_dev_init_opp_table(struct device *dev, } #endif +/* + * This function should be invoked with the enabled runtime PM of the device + * in order to initialize performance state properly. Most of Tegra devices + * are assumed to be suspended at a probe time and GENPD require RPM to be + * enabled to set up the rpm-resume state, otherwise device is active and + * performance state is applied immediately. Note that it will initialize + * OPP bandwidth if it's wired in a device-tree for this device, which is + * undesirable for a suspended device. + */ +static inline int +devm_tegra_core_dev_init_opp_table_common(struct device *dev) +{ + struct tegra_core_opp_params opp_params = {}; + int err; + + opp_params.init_state = true; + + err = devm_tegra_core_dev_init_opp_table(dev, &opp_params); + if (err != -ENODEV) + return err; + + return 0; +} + #endif /* __SOC_TEGRA_COMMON_H__ */
Only couple drivers need to get the -ENODEV error code and majority of drivers need to explicitly initialize the performance state. Add new common helper which sets up OPP table for these drivers. Signed-off-by: Dmitry Osipenko <digetx@gmail.com> --- include/soc/tegra/common.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+)