| Message ID | 20201115212922.4390-1-digetx@gmail.com |
|---|---|
| Headers | show |
| Series | Introduce memory interconnect for NVIDIA Tegra SoCs | expand |
On 16-11-20, 00:29, Dmitry Osipenko wrote: > This patch moves ACTMON driver away from generating OPP table by itself, > transitioning it to use the table which comes from device-tree. This > change breaks compatibility with older device-trees in order to bring > support for the interconnect framework to the driver. This is a mandatory > change which needs to be done in order to implement interconnect-based > memory DVFS. Users of legacy device-trees will get a message telling that > theirs DT needs to be upgraded. Now ACTMON issues memory bandwidth request > using dev_pm_opp_set_bw(), instead of driving EMC clock rate directly. > > Tested-by: Peter Geis <pgwipeout@gmail.com> > Tested-by: Nicolas Chauvet <kwizart@gmail.com> > Acked-by: Chanwoo Choi <cw00.choi@samsung.com> > Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > --- > drivers/devfreq/tegra30-devfreq.c | 86 ++++++++++++++++--------------- > 1 file changed, 44 insertions(+), 42 deletions(-) > > diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c > index 38cc0d014738..ed6d4469c8c7 100644 > --- a/drivers/devfreq/tegra30-devfreq.c > +++ b/drivers/devfreq/tegra30-devfreq.c > @@ -19,6 +19,8 @@ > #include <linux/reset.h> > #include <linux/workqueue.h> > > +#include <soc/tegra/fuse.h> > + > #include "governor.h" > > #define ACTMON_GLB_STATUS 0x0 > @@ -155,6 +157,7 @@ struct tegra_devfreq_device { > > struct tegra_devfreq { > struct devfreq *devfreq; > + struct opp_table *opp_table; > > struct reset_control *reset; > struct clk *clock; > @@ -612,34 +615,19 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra) > static int tegra_devfreq_target(struct device *dev, unsigned long *freq, > u32 flags) > { > - struct tegra_devfreq *tegra = dev_get_drvdata(dev); > - struct devfreq *devfreq = tegra->devfreq; > struct dev_pm_opp *opp; > - unsigned long rate; > - int err; > + int ret; > > opp = devfreq_recommended_opp(dev, freq, flags); > if (IS_ERR(opp)) { > dev_err(dev, "Failed to find opp for %lu Hz\n", *freq); > return PTR_ERR(opp); > } > - rate = dev_pm_opp_get_freq(opp); > - dev_pm_opp_put(opp); > - > - err = clk_set_min_rate(tegra->emc_clock, rate * KHZ); > - if (err) > - return err; > - > - err = clk_set_rate(tegra->emc_clock, 0); > - if (err) > - goto restore_min_rate; > > - return 0; > - > -restore_min_rate: > - clk_set_min_rate(tegra->emc_clock, devfreq->previous_freq); > + ret = dev_pm_opp_set_bw(dev, opp); > + dev_pm_opp_put(opp); > > - return err; > + return ret; > } > > static int tegra_devfreq_get_dev_status(struct device *dev, > @@ -655,7 +643,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev, > stat->private_data = tegra; > > /* The below are to be used by the other governors */ > - stat->current_frequency = cur_freq; > + stat->current_frequency = cur_freq * KHZ; > > actmon_dev = &tegra->devices[MCALL]; > > @@ -705,7 +693,12 @@ static int tegra_governor_get_target(struct devfreq *devfreq, > target_freq = max(target_freq, dev->target_freq); > } > > - *freq = target_freq; > + /* > + * tegra-devfreq driver operates with KHz units, while OPP table > + * entries use Hz units. Hence we need to convert the units for the > + * devfreq core. > + */ > + *freq = target_freq * KHZ; > > return 0; > } > @@ -774,6 +767,7 @@ static struct devfreq_governor tegra_devfreq_governor = { > > static int tegra_devfreq_probe(struct platform_device *pdev) > { > + u32 hw_version = BIT(tegra_sku_info.soc_speedo_id); > struct tegra_devfreq_device *dev; > struct tegra_devfreq *tegra; > struct devfreq *devfreq; > @@ -781,6 +775,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > long rate; > int err; > > + /* legacy device-trees don't have OPP table and must be updated */ > + if (!device_property_present(&pdev->dev, "operating-points-v2")) { > + dev_err(&pdev->dev, > + "OPP table not found, please update your device tree\n"); > + return -ENODEV; > + } > + You forgot to remove this ? > tegra = devm_kzalloc(&pdev->dev, sizeof(*tegra), GFP_KERNEL); > if (!tegra) > return -ENOMEM; > @@ -822,11 +823,25 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > return err; > } > > + tegra->opp_table = dev_pm_opp_set_supported_hw(&pdev->dev, > + &hw_version, 1); > + err = PTR_ERR_OR_ZERO(tegra->opp_table); > + if (err) { > + dev_err(&pdev->dev, "Failed to set supported HW: %d\n", err); > + return err; > + } > + > + err = dev_pm_opp_of_add_table(&pdev->dev); > + if (err) { > + dev_err(&pdev->dev, "Failed to add OPP table: %d\n", err); > + goto put_hw; > + } > + > err = clk_prepare_enable(tegra->clock); > if (err) { > dev_err(&pdev->dev, > "Failed to prepare and enable ACTMON clock\n"); > - return err; > + goto remove_table; > } > > err = reset_control_reset(tegra->reset); > @@ -850,23 +865,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > dev->regs = tegra->regs + dev->config->offset; > } > > - for (rate = 0; rate <= tegra->max_freq * KHZ; rate++) { > - rate = clk_round_rate(tegra->emc_clock, rate); > - > - if (rate < 0) { > - dev_err(&pdev->dev, > - "Failed to round clock rate: %ld\n", rate); > - err = rate; > - goto remove_opps; > - } > - > - err = dev_pm_opp_add(&pdev->dev, rate / KHZ, 0); > - if (err) { > - dev_err(&pdev->dev, "Failed to add OPP: %d\n", err); > - goto remove_opps; > - } > - } > - > platform_set_drvdata(pdev, tegra); > > tegra->clk_rate_change_nb.notifier_call = tegra_actmon_clk_notify_cb; > @@ -882,7 +880,6 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > } > > tegra_devfreq_profile.initial_freq = clk_get_rate(tegra->emc_clock); > - tegra_devfreq_profile.initial_freq /= KHZ; > > devfreq = devfreq_add_device(&pdev->dev, &tegra_devfreq_profile, > "tegra_actmon", NULL); > @@ -902,6 +899,10 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > reset_control_reset(tegra->reset); > disable_clk: > clk_disable_unprepare(tegra->clock); > +remove_table: > + dev_pm_opp_of_remove_table(&pdev->dev); > +put_hw: > + dev_pm_opp_put_supported_hw(tegra->opp_table); > > return err; > } > @@ -913,11 +914,12 @@ static int tegra_devfreq_remove(struct platform_device *pdev) > devfreq_remove_device(tegra->devfreq); > devfreq_remove_governor(&tegra_devfreq_governor); > > - dev_pm_opp_remove_all_dynamic(&pdev->dev); > - > reset_control_reset(tegra->reset); > clk_disable_unprepare(tegra->clock); > > + dev_pm_opp_of_remove_table(&pdev->dev); > + dev_pm_opp_put_supported_hw(tegra->opp_table); > + > return 0; > } > > -- > 2.29.2
17.11.2020 13:07, Viresh Kumar пишет: > On 16-11-20, 00:29, Dmitry Osipenko wrote: >> This patch moves ACTMON driver away from generating OPP table by itself, >> transitioning it to use the table which comes from device-tree. This >> change breaks compatibility with older device-trees in order to bring >> support for the interconnect framework to the driver. This is a mandatory >> change which needs to be done in order to implement interconnect-based >> memory DVFS. Users of legacy device-trees will get a message telling that >> theirs DT needs to be upgraded. Now ACTMON issues memory bandwidth request >> using dev_pm_opp_set_bw(), instead of driving EMC clock rate directly. >> >> Tested-by: Peter Geis <pgwipeout@gmail.com> >> Tested-by: Nicolas Chauvet <kwizart@gmail.com> >> Acked-by: Chanwoo Choi <cw00.choi@samsung.com> >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> >> --- >> drivers/devfreq/tegra30-devfreq.c | 86 ++++++++++++++++--------------- >> 1 file changed, 44 insertions(+), 42 deletions(-) >> >> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c >> index 38cc0d014738..ed6d4469c8c7 100644 >> --- a/drivers/devfreq/tegra30-devfreq.c >> +++ b/drivers/devfreq/tegra30-devfreq.c >> @@ -19,6 +19,8 @@ >> #include <linux/reset.h> >> #include <linux/workqueue.h> >> >> +#include <soc/tegra/fuse.h> >> + >> #include "governor.h" >> >> #define ACTMON_GLB_STATUS 0x0 >> @@ -155,6 +157,7 @@ struct tegra_devfreq_device { >> >> struct tegra_devfreq { >> struct devfreq *devfreq; >> + struct opp_table *opp_table; >> >> struct reset_control *reset; >> struct clk *clock; >> @@ -612,34 +615,19 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra) >> static int tegra_devfreq_target(struct device *dev, unsigned long *freq, >> u32 flags) >> { >> - struct tegra_devfreq *tegra = dev_get_drvdata(dev); >> - struct devfreq *devfreq = tegra->devfreq; >> struct dev_pm_opp *opp; >> - unsigned long rate; >> - int err; >> + int ret; >> >> opp = devfreq_recommended_opp(dev, freq, flags); >> if (IS_ERR(opp)) { >> dev_err(dev, "Failed to find opp for %lu Hz\n", *freq); >> return PTR_ERR(opp); >> } >> - rate = dev_pm_opp_get_freq(opp); >> - dev_pm_opp_put(opp); >> - >> - err = clk_set_min_rate(tegra->emc_clock, rate * KHZ); >> - if (err) >> - return err; >> - >> - err = clk_set_rate(tegra->emc_clock, 0); >> - if (err) >> - goto restore_min_rate; >> >> - return 0; >> - >> -restore_min_rate: >> - clk_set_min_rate(tegra->emc_clock, devfreq->previous_freq); >> + ret = dev_pm_opp_set_bw(dev, opp); >> + dev_pm_opp_put(opp); >> >> - return err; >> + return ret; >> } >> >> static int tegra_devfreq_get_dev_status(struct device *dev, >> @@ -655,7 +643,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev, >> stat->private_data = tegra; >> >> /* The below are to be used by the other governors */ >> - stat->current_frequency = cur_freq; >> + stat->current_frequency = cur_freq * KHZ; >> >> actmon_dev = &tegra->devices[MCALL]; >> >> @@ -705,7 +693,12 @@ static int tegra_governor_get_target(struct devfreq *devfreq, >> target_freq = max(target_freq, dev->target_freq); >> } >> >> - *freq = target_freq; >> + /* >> + * tegra-devfreq driver operates with KHz units, while OPP table >> + * entries use Hz units. Hence we need to convert the units for the >> + * devfreq core. >> + */ >> + *freq = target_freq * KHZ; >> >> return 0; >> } >> @@ -774,6 +767,7 @@ static struct devfreq_governor tegra_devfreq_governor = { >> >> static int tegra_devfreq_probe(struct platform_device *pdev) >> { >> + u32 hw_version = BIT(tegra_sku_info.soc_speedo_id); >> struct tegra_devfreq_device *dev; >> struct tegra_devfreq *tegra; >> struct devfreq *devfreq; >> @@ -781,6 +775,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) >> long rate; >> int err; >> >> + /* legacy device-trees don't have OPP table and must be updated */ >> + if (!device_property_present(&pdev->dev, "operating-points-v2")) { >> + dev_err(&pdev->dev, >> + "OPP table not found, please update your device tree\n"); >> + return -ENODEV; >> + } >> + > > You forgot to remove this ? Yes, good catch. I'm planning to replace this code with a common helper sometime soon, so if there won't be another reasons to make a new revision, then I'd prefer to keep it as-is for now.
On 17-11-20, 17:17, Dmitry Osipenko wrote: > 17.11.2020 13:07, Viresh Kumar пишет: > > On 16-11-20, 00:29, Dmitry Osipenko wrote: > >> This patch moves ACTMON driver away from generating OPP table by itself, > >> transitioning it to use the table which comes from device-tree. This > >> change breaks compatibility with older device-trees in order to bring > >> support for the interconnect framework to the driver. This is a mandatory > >> change which needs to be done in order to implement interconnect-based > >> memory DVFS. Users of legacy device-trees will get a message telling that > >> theirs DT needs to be upgraded. Now ACTMON issues memory bandwidth request > >> using dev_pm_opp_set_bw(), instead of driving EMC clock rate directly. > >> > >> Tested-by: Peter Geis <pgwipeout@gmail.com> > >> Tested-by: Nicolas Chauvet <kwizart@gmail.com> > >> Acked-by: Chanwoo Choi <cw00.choi@samsung.com> > >> Signed-off-by: Dmitry Osipenko <digetx@gmail.com> > >> --- > >> drivers/devfreq/tegra30-devfreq.c | 86 ++++++++++++++++--------------- > >> 1 file changed, 44 insertions(+), 42 deletions(-) > >> > >> diff --git a/drivers/devfreq/tegra30-devfreq.c b/drivers/devfreq/tegra30-devfreq.c > >> index 38cc0d014738..ed6d4469c8c7 100644 > >> --- a/drivers/devfreq/tegra30-devfreq.c > >> +++ b/drivers/devfreq/tegra30-devfreq.c > >> @@ -19,6 +19,8 @@ > >> #include <linux/reset.h> > >> #include <linux/workqueue.h> > >> > >> +#include <soc/tegra/fuse.h> > >> + > >> #include "governor.h" > >> > >> #define ACTMON_GLB_STATUS 0x0 > >> @@ -155,6 +157,7 @@ struct tegra_devfreq_device { > >> > >> struct tegra_devfreq { > >> struct devfreq *devfreq; > >> + struct opp_table *opp_table; > >> > >> struct reset_control *reset; > >> struct clk *clock; > >> @@ -612,34 +615,19 @@ static void tegra_actmon_stop(struct tegra_devfreq *tegra) > >> static int tegra_devfreq_target(struct device *dev, unsigned long *freq, > >> u32 flags) > >> { > >> - struct tegra_devfreq *tegra = dev_get_drvdata(dev); > >> - struct devfreq *devfreq = tegra->devfreq; > >> struct dev_pm_opp *opp; > >> - unsigned long rate; > >> - int err; > >> + int ret; > >> > >> opp = devfreq_recommended_opp(dev, freq, flags); > >> if (IS_ERR(opp)) { > >> dev_err(dev, "Failed to find opp for %lu Hz\n", *freq); > >> return PTR_ERR(opp); > >> } > >> - rate = dev_pm_opp_get_freq(opp); > >> - dev_pm_opp_put(opp); > >> - > >> - err = clk_set_min_rate(tegra->emc_clock, rate * KHZ); > >> - if (err) > >> - return err; > >> - > >> - err = clk_set_rate(tegra->emc_clock, 0); > >> - if (err) > >> - goto restore_min_rate; > >> > >> - return 0; > >> - > >> -restore_min_rate: > >> - clk_set_min_rate(tegra->emc_clock, devfreq->previous_freq); > >> + ret = dev_pm_opp_set_bw(dev, opp); > >> + dev_pm_opp_put(opp); > >> > >> - return err; > >> + return ret; > >> } > >> > >> static int tegra_devfreq_get_dev_status(struct device *dev, > >> @@ -655,7 +643,7 @@ static int tegra_devfreq_get_dev_status(struct device *dev, > >> stat->private_data = tegra; > >> > >> /* The below are to be used by the other governors */ > >> - stat->current_frequency = cur_freq; > >> + stat->current_frequency = cur_freq * KHZ; > >> > >> actmon_dev = &tegra->devices[MCALL]; > >> > >> @@ -705,7 +693,12 @@ static int tegra_governor_get_target(struct devfreq *devfreq, > >> target_freq = max(target_freq, dev->target_freq); > >> } > >> > >> - *freq = target_freq; > >> + /* > >> + * tegra-devfreq driver operates with KHz units, while OPP table > >> + * entries use Hz units. Hence we need to convert the units for the > >> + * devfreq core. > >> + */ > >> + *freq = target_freq * KHZ; > >> > >> return 0; > >> } > >> @@ -774,6 +767,7 @@ static struct devfreq_governor tegra_devfreq_governor = { > >> > >> static int tegra_devfreq_probe(struct platform_device *pdev) > >> { > >> + u32 hw_version = BIT(tegra_sku_info.soc_speedo_id); > >> struct tegra_devfreq_device *dev; > >> struct tegra_devfreq *tegra; > >> struct devfreq *devfreq; > >> @@ -781,6 +775,13 @@ static int tegra_devfreq_probe(struct platform_device *pdev) > >> long rate; > >> int err; > >> > >> + /* legacy device-trees don't have OPP table and must be updated */ > >> + if (!device_property_present(&pdev->dev, "operating-points-v2")) { > >> + dev_err(&pdev->dev, > >> + "OPP table not found, please update your device tree\n"); > >> + return -ENODEV; > >> + } > >> + > > > > You forgot to remove this ? > > Yes, good catch. I'm planning to replace this code with a common helper > sometime soon, so if there won't be another reasons to make a new > revision, then I'd prefer to keep it as-is for now. You should just replace this patch only with a version of V9.1 and you aren't really required to resend the whole series. And you should fix it before it gets merged. This isn't a formatting issue which we just let through. I trust you when you say that you will fix it, but this must be fixed now. -- viresh
18.11.2020 07:21, Viresh Kumar пишет: ... >>>> + /* legacy device-trees don't have OPP table and must be updated */ >>>> + if (!device_property_present(&pdev->dev, "operating-points-v2")) { >>>> + dev_err(&pdev->dev, >>>> + "OPP table not found, please update your device tree\n"); >>>> + return -ENODEV; >>>> + } >>>> + >>> >>> You forgot to remove this ? >> >> Yes, good catch. I'm planning to replace this code with a common helper >> sometime soon, so if there won't be another reasons to make a new >> revision, then I'd prefer to keep it as-is for now. > > You should just replace this patch only with a version of V9.1 and you > aren't really required to resend the whole series. And you should fix > it before it gets merged. This isn't a formatting issue which we just > let through. I trust you when you say that you will fix it, but this > must be fixed now. > Should be easier to resend it all. I'll update it over the weekend, thanks.
This series brings initial support for memory interconnect to Tegra20, Tegra30 and Tegra124 SoCs. For the starter only display controllers and devfreq devices are getting interconnect API support, others could be supported later on. The display controllers have the biggest demand for interconnect API right now because dynamic memory frequency scaling can't be done safely without taking into account bandwidth requirement from the displays. In particular this series fixes distorted display output on T30 Ouya and T124 TK1 devices. Changelog: v9: - Squashed "memory: tegra30-emc: Factor out clk initialization" into patch "tegra30: Support interconnect framework". Suggested by Krzysztof Kozlowski. - Improved Kconfig in the patch "memory: tegra124-emc: Make driver modular" by adding CONFIG_TEGRA124_CLK_EMC entry, which makes clk-driver changes to look a bit more cleaner. Suggested by Krzysztof Kozlowski. - Dropped voltage regulator support from ICC and DT patches for now because there is a new discussion about using a power domain abstraction for controlling the regulator, which is likely to happen. - Replaced direct "operating-points-v2" property checking in EMC drivers with checking of a returned error code from dev_pm_opp_of_add_table(). Note that I haven't touched T20 EMC driver because it's very likely that we'll replace that code with a common helper soon anyways. Suggested by Viresh Kumar. - The T30 DT patches now include EMC OPP changes for Ouya board, which is available now in linux-next. Dmitry Osipenko (17): memory: tegra30: Support interconnect framework memory: tegra124-emc: Make driver modular memory: tegra124-emc: Continue probing if timings are missing in device-tree memory: tegra124: Support interconnect framework drm/tegra: dc: Support memory bandwidth management drm/tegra: dc: Extend debug stats with total number of events PM / devfreq: tegra30: Support interconnect and OPPs from device-tree PM / devfreq: tegra30: Separate configurations per-SoC generation PM / devfreq: tegra20: Deprecate in a favor of emc-stat based driver ARM: tegra: Correct EMC registers size in Tegra20 device-tree ARM: tegra: Add interconnect properties to Tegra20 device-tree ARM: tegra: Add interconnect properties to Tegra30 device-tree ARM: tegra: Add interconnect properties to Tegra124 device-tree ARM: tegra: Add nvidia,memory-controller phandle to Tegra20 EMC device-tree ARM: tegra: Add EMC OPP properties to Tegra20 device-trees ARM: tegra: Add EMC OPP and ICC properties to Tegra30 EMC and ACTMON device-tree nodes ARM: tegra: Add EMC OPP and ICC properties to Tegra124 EMC and ACTMON device-tree nodes MAINTAINERS | 1 - arch/arm/boot/dts/tegra124-apalis-emc.dtsi | 8 + .../arm/boot/dts/tegra124-jetson-tk1-emc.dtsi | 8 + arch/arm/boot/dts/tegra124-nyan-big-emc.dtsi | 10 + .../arm/boot/dts/tegra124-nyan-blaze-emc.dtsi | 10 + .../boot/dts/tegra124-peripherals-opp.dtsi | 419 ++++++++++++++++++ arch/arm/boot/dts/tegra124.dtsi | 31 ++ .../boot/dts/tegra20-acer-a500-picasso.dts | 5 + arch/arm/boot/dts/tegra20-colibri.dtsi | 4 + arch/arm/boot/dts/tegra20-paz00.dts | 4 + .../arm/boot/dts/tegra20-peripherals-opp.dtsi | 92 ++++ arch/arm/boot/dts/tegra20.dtsi | 33 +- ...30-asus-nexus7-grouper-memory-timings.dtsi | 12 + arch/arm/boot/dts/tegra30-ouya.dts | 8 + .../arm/boot/dts/tegra30-peripherals-opp.dtsi | 383 ++++++++++++++++ arch/arm/boot/dts/tegra30.dtsi | 33 +- drivers/clk/tegra/Kconfig | 3 + drivers/clk/tegra/Makefile | 2 +- drivers/clk/tegra/clk-tegra124-emc.c | 41 +- drivers/clk/tegra/clk-tegra124.c | 26 +- drivers/clk/tegra/clk.h | 18 +- drivers/devfreq/Kconfig | 10 - drivers/devfreq/Makefile | 1 - drivers/devfreq/tegra20-devfreq.c | 210 --------- drivers/devfreq/tegra30-devfreq.c | 154 ++++--- drivers/gpu/drm/tegra/Kconfig | 1 + drivers/gpu/drm/tegra/dc.c | 359 +++++++++++++++ drivers/gpu/drm/tegra/dc.h | 19 + drivers/gpu/drm/tegra/drm.c | 14 + drivers/gpu/drm/tegra/hub.c | 3 + drivers/gpu/drm/tegra/plane.c | 121 +++++ drivers/gpu/drm/tegra/plane.h | 15 + drivers/memory/tegra/Kconfig | 5 +- drivers/memory/tegra/tegra124-emc.c | 382 ++++++++++++++-- drivers/memory/tegra/tegra124.c | 82 +++- drivers/memory/tegra/tegra30-emc.c | 349 ++++++++++++++- drivers/memory/tegra/tegra30.c | 173 +++++++- include/linux/clk/tegra.h | 8 + include/soc/tegra/emc.h | 16 - 39 files changed, 2699 insertions(+), 374 deletions(-) create mode 100644 arch/arm/boot/dts/tegra124-peripherals-opp.dtsi create mode 100644 arch/arm/boot/dts/tegra20-peripherals-opp.dtsi create mode 100644 arch/arm/boot/dts/tegra30-peripherals-opp.dtsi delete mode 100644 drivers/devfreq/tegra20-devfreq.c delete mode 100644 include/soc/tegra/emc.h