Thread (56 messages) 56 messages, 3 authors, 2021-08-12

Re: [PATCH v7 02/37] soc/tegra: pmc: Implement attach_dev() of power domain drivers

From: Ulf Hansson <hidden>
Date: 2021-08-02 14:49:05
Also in: linux-pm, lkml

On Fri, 2 Jul 2021 at 01:28, Dmitry Osipenko [off-list ref] wrote:
quoted hunk ↗ jump to hunk
Implement attach_dev() callback of power domain drivers that initializes
the domain's performance state. GENPD core will apply the performance
state on the first runtime PM resume of the attached device.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
 drivers/soc/tegra/pmc.c | 147 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 147 insertions(+)
diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c
index f63dfb2ca3f9..ebafb818b08e 100644
--- a/drivers/soc/tegra/pmc.c
+++ b/drivers/soc/tegra/pmc.c
@@ -506,6 +506,151 @@ static void tegra_pmc_scratch_writel(struct tegra_pmc *pmc, u32 value,
                writel(value, pmc->scratch + offset);
 }

+static const char * const tegra_emc_compats[] = {
+       "nvidia,tegra20-emc",
+       "nvidia,tegra30-emc",
+       NULL,
+};
+
+/*
+ * This GENPD callback is used by both powergate and core domains.
+ *
+ * We retrieve clock rate of the attached device and initialize domain's
+ * performance state in accordance to the clock rate.
+ */
+static int tegra_pmc_pd_attach_dev(struct generic_pm_domain *genpd,
+                                  struct device *dev)
+{
+       struct generic_pm_domain_data *gpd_data = dev_gpd_data(dev);
+       struct opp_table *opp_table, *pd_opp_table;
+       struct generic_pm_domain *core_genpd;
+       struct dev_pm_opp *opp, *pd_opp;
+       unsigned long rate, state;
+       struct gpd_link *link;
+       struct clk *clk;
+       u32 hw_version;
+       int ret;
+
+       /*
+        * Tegra114+ SocS don't support OPP yet.  But if they will get OPP
+        * support, then we want to skip OPP for older kernels to preserve
+        * compatibility of newer DTBs with older kernels.
+        */
+       if (!pmc->soc->supports_core_domain)
+               return 0;
+
+       /*
+        * The EMC devices are a special case because we have a protection
+        * from non-EMC drivers getting clock handle before EMC driver is
+        * fully initialized.  The goal of the protection is to prevent
+        * devfreq driver from getting failures if it will try to change
+        * EMC clock rate until clock is fully initialized.  The EMC drivers
+        * will initialize the performance state by themselves.
+        */
+       if (of_device_compatible_match(dev->of_node, tegra_emc_compats))
+               return 0;
+
+       clk = clk_get(dev, NULL);
+       if (IS_ERR(clk)) {
+               dev_err(&genpd->dev, "failed to get clk of %s: %pe\n",
+                       dev_name(dev), clk);
+               return PTR_ERR(clk);
+       }
+
+       rate = clk_get_rate(clk);
+       if (!rate) {
+               dev_err(&genpd->dev, "failed to get clk rate of %s\n",
+                       dev_name(dev));
+               ret = -EINVAL;
+               goto put_clk;
+       }
+
+       if (of_machine_is_compatible("nvidia,tegra20"))
+               hw_version = BIT(tegra_sku_info.soc_process_id);
+       else
+               hw_version = BIT(tegra_sku_info.soc_speedo_id);
+
+       opp_table = dev_pm_opp_set_supported_hw(dev, &hw_version, 1);
+       if (IS_ERR(opp_table)) {
+               dev_err(&genpd->dev, "failed to set OPP supported HW for %s: %d\n",
+                       dev_name(dev), ret);
+               ret = PTR_ERR(opp_table);
+               goto put_clk;
+       }
+
+       ret = dev_pm_opp_of_add_table(dev);
+       if (ret) {
+               /* older DTBs that don't have OPPs will get -ENODEV here */
+               if (ret != -ENODEV)
+                       dev_err(&genpd->dev, "failed to get OPP table of %s: %d\n",
+                               dev_name(dev), ret);
+               else
+                       ret = 0;
+
+               goto put_supported_hw;
+       }
+
+       /* find suitable OPP for the rate */
+       opp = dev_pm_opp_find_freq_ceil(dev, &rate);
+
+       if (opp == ERR_PTR(-ERANGE))
+               opp = dev_pm_opp_find_freq_floor(dev, &rate);
+
+       if (IS_ERR(opp)) {
+               dev_err(&genpd->dev, "failed to find OPP for %luHz of %s: %pe\n",
+                       rate, dev_name(dev), opp);
+               ret = PTR_ERR(opp);
+               goto remove_dev_table;
+       }
+
+       if (!list_empty(&genpd->child_links)) {
+               link = list_first_entry(&genpd->child_links, struct gpd_link,
+                                       child_node);
+               core_genpd = link->parent;
+       } else {
+               core_genpd = genpd;
+       }
This looks a bit odd to me. A genpd provider shouldn't need to walk
these links as these are considered internals to genpd. Normally this
needs lockings, etc.

Why exactly do you need this?
+
+       pd_opp_table = dev_pm_opp_get_opp_table(&core_genpd->dev);
+       if (IS_ERR(pd_opp_table)) {
+               dev_err(&genpd->dev, "failed to get OPP table of %s: %pe\n",
+                       dev_name(&core_genpd->dev), pd_opp_table);
+               ret = PTR_ERR(pd_opp_table);
+               goto put_dev_opp;
+       }
+
+       pd_opp = dev_pm_opp_xlate_required_opp(opp_table, pd_opp_table, opp);
+       if (IS_ERR(pd_opp)) {
+               dev_err(&genpd->dev,
+                       "failed to xlate required OPP for %luHz of %s: %pe\n",
+                       rate, dev_name(dev), pd_opp);
+               ret = PTR_ERR(pd_opp);
+               goto put_pd_opp_table;
+       }
+
+       /*
+        * The initialized state will be applied by GENPD core on the first
+        * RPM-resume of the device.  This means that drivers don't need to
+        * explicitly initialize performance state.
+        */
+       state = pm_genpd_opp_to_performance_state(&core_genpd->dev, pd_opp);
+       gpd_data->rpm_pstate = state;
Could the above be replaced with Rajendra's suggestion [1], which
changes genpd to internally during attach, to set a default
performance state when there is a "required-opp" specified in the
device  node?

[...]

Kind regards
Uffe

[1]
https://lkml.org/lkml/2021/7/20/99
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help