This series adds runtime PM support to Tegra drivers and enables core
voltage scaling for Tegra20/30 SoCs, resolving overheating troubles.
All patches should go via Tegra tree because they are interdependent,
please review and ack.
If you haven't seen this series before, that's because I wanted to
finalize the GENPD part at first and didn't bother you previously.
Changelog:
v8: - Added new generic dev_pm_opp_sync() helper that syncs OPP state with
hardware. All drivers changed to use it. This replaces GENPD attach_dev
callback hacks that were used in v7.
- Added new patch patch "soc/tegra: regulators: Prepare for suspend"
that fixes dying Tegra20 SoC after enabling VENC power domain during
resume from suspend. It matches to what downstream kernel does on
suspend/resume.
- After a second thought, I dropped patches which added RPM to memory
drivers since hardware is always-on and RPM not needed.
- Replaced the "dummy host1x driver" patch with new "Disable unused
host1x hardware" patch, since it's a cleaner solution.
Dmitry Osipenko (34):
opp: Add dev_pm_opp_sync() helper
soc/tegra: pmc: Disable PMC state syncing
soc/tegra: Don't print error message when OPPs not available
soc/tegra: Add devm_tegra_core_dev_init_opp_table_simple()
soc/tegra: Use dev_pm_opp_sync()
dt-bindings: clock: tegra-car: Document new tegra-clocks sub-node
clk: tegra: Support runtime PM and power domain
dt-bindings: host1x: Document OPP and power domain properties
dt-bindings: host1x: Document Memory Client resets of Host1x, GR2D and
GR3D
gpu: host1x: Add host1x_channel_stop()
gpu: host1x: Add runtime PM and OPP support
drm/tegra: dc: Support OPP and SoC core voltage scaling
drm/tegra: hdmi: Add OPP support
drm/tegra: gr2d: Support power management
drm/tegra: gr3d: Support power management
drm/tegra: vic: Support system suspend
usb: chipidea: tegra: Add runtime PM and OPP support
bus: tegra-gmi: Add runtime PM and OPP support
pwm: tegra: Add runtime PM and OPP support
mmc: sdhci-tegra: Add runtime PM and OPP support
mtd: rawnand: tegra: Add runtime PM and OPP support
spi: tegra20-slink: Add OPP support
media: dt: bindings: tegra-vde: Convert to schema
media: dt: bindings: tegra-vde: Document OPP and power domain
media: staging: tegra-vde: Support generic power domain and OPP
soc/tegra: fuse: Add OPP support
soc/tegra: fuse: Reset hardware
soc/tegra: regulators: Prepare for suspend
soc/tegra: pmc: Enable core domain support for Tegra20 and Tegra30
ARM: tegra: Add OPP tables and power domains to Tegra20 device-trees
ARM: tegra: Add OPP tables and power domains to Tegra30 device-trees
ARM: tegra: Add Memory Client resets to Tegra20 GR2D, GR3D and Host1x
ARM: tegra: Add Memory Client resets to Tegra30 GR2D, GR3D and Host1x
ARM: tegra20/30: Disable unused host1x hardware
.../bindings/clock/nvidia,tegra20-car.yaml | 51 +
.../display/tegra/nvidia,tegra20-host1x.txt | 53 +
.../bindings/media/nvidia,tegra-vde.txt | 64 -
.../bindings/media/nvidia,tegra-vde.yaml | 119 ++
.../boot/dts/tegra20-acer-a500-picasso.dts | 1 +
arch/arm/boot/dts/tegra20-colibri.dtsi | 3 +-
arch/arm/boot/dts/tegra20-harmony.dts | 3 +-
arch/arm/boot/dts/tegra20-paz00.dts | 1 +
.../arm/boot/dts/tegra20-peripherals-opp.dtsi | 941 +++++++++++
arch/arm/boot/dts/tegra20-seaboard.dts | 3 +-
arch/arm/boot/dts/tegra20-tamonten.dtsi | 3 +-
arch/arm/boot/dts/tegra20-trimslice.dts | 9 +
arch/arm/boot/dts/tegra20-ventana.dts | 1 +
arch/arm/boot/dts/tegra20.dtsi | 119 +-
.../tegra30-asus-nexus7-grouper-common.dtsi | 1 +
arch/arm/boot/dts/tegra30-beaver.dts | 1 +
arch/arm/boot/dts/tegra30-cardhu.dtsi | 1 +
arch/arm/boot/dts/tegra30-colibri.dtsi | 17 +-
arch/arm/boot/dts/tegra30-ouya.dts | 1 +
.../arm/boot/dts/tegra30-peripherals-opp.dtsi | 1412 +++++++++++++++++
arch/arm/boot/dts/tegra30.dtsi | 181 ++-
drivers/bus/tegra-gmi.c | 92 +-
drivers/clk/tegra/Makefile | 1 +
drivers/clk/tegra/clk-device.c | 222 +++
drivers/clk/tegra/clk-pll.c | 2 +-
drivers/clk/tegra/clk-super.c | 2 +-
drivers/clk/tegra/clk-tegra20.c | 39 +-
drivers/clk/tegra/clk-tegra30.c | 70 +-
drivers/clk/tegra/clk.c | 64 +
drivers/clk/tegra/clk.h | 2 +
drivers/gpu/drm/tegra/dc.c | 74 +
drivers/gpu/drm/tegra/dc.h | 2 +
drivers/gpu/drm/tegra/gr2d.c | 154 +-
drivers/gpu/drm/tegra/gr3d.c | 393 ++++-
drivers/gpu/drm/tegra/hdmi.c | 15 +-
drivers/gpu/drm/tegra/vic.c | 4 +
drivers/gpu/host1x/channel.c | 8 +
drivers/gpu/host1x/debug.c | 15 +
drivers/gpu/host1x/dev.c | 157 +-
drivers/gpu/host1x/dev.h | 3 +-
drivers/gpu/host1x/hw/channel_hw.c | 44 +-
drivers/gpu/host1x/intr.c | 3 -
drivers/gpu/host1x/syncpt.c | 5 +-
drivers/mmc/host/sdhci-tegra.c | 146 +-
drivers/mtd/nand/raw/tegra_nand.c | 62 +-
drivers/opp/core.c | 42 +-
drivers/pwm/pwm-tegra.c | 104 +-
drivers/soc/tegra/common.c | 34 +-
drivers/soc/tegra/fuse/fuse-tegra.c | 36 +
drivers/soc/tegra/fuse/fuse.h | 1 +
drivers/soc/tegra/pmc.c | 17 +
drivers/soc/tegra/regulators-tegra20.c | 99 ++
drivers/soc/tegra/regulators-tegra30.c | 122 ++
drivers/spi/spi-tegra20-slink.c | 15 +-
drivers/staging/media/tegra-vde/vde.c | 65 +-
drivers/usb/chipidea/ci_hdrc_tegra.c | 61 +-
include/linux/host1x.h | 1 +
include/linux/pm_opp.h | 6 +
include/soc/tegra/common.h | 13 +
59 files changed, 4796 insertions(+), 384 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/media/nvidia,tegra-vde.txt
create mode 100644 Documentation/devicetree/bindings/media/nvidia,tegra-vde.yaml
create mode 100644 drivers/clk/tegra/clk-device.c
--
2.32.0
Disable PMC state syncing in order to ensure that we won't break older
kernels once device-trees will be updated with the addition of the power
domains. Previously this was unnecessary because the plan was to make clk
device that will attach to the domain for each clock, but the plan changed
and now we're going make a better GENPD implementation that will require
to update each device driver with the runtime PM and OPP support before
we could safely enable the state syncing.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/pmc.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
@@ -1003,7 +1004,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,/* Find the currently set OPP if we don't know already */if(unlikely(!opp_table->current_opp))-_find_current_opp(dev,opp_table);+opp_table->current_opp=_find_current_opp(dev,opp_table);old_opp=opp_table->current_opp;
@@ -2931,3 +2932,38 @@ int dev_pm_opp_sync_regulators(struct device *dev)returnret;}EXPORT_SYMBOL_GPL(dev_pm_opp_sync_regulators);++/**+*dev_pm_opp_sync()-SyncOPPstate+*@dev:deviceforwhichwedothisoperation+*+*InitializeOPPtableaccordinglytocurrentclockrateor+*firstavailableOPPifclocknotavailableforthisdevice.+*+*Return:0onsuccessoranegativeerrorvalue.+*/+intdev_pm_opp_sync(structdevice*dev)+{+structopp_table*opp_table;+structdev_pm_opp*opp;+intret=0;++/* Device may not have OPP table */+opp_table=_find_opp_table(dev);+if(IS_ERR(opp_table))+return0;++if(!_get_opp_count(opp_table))+gotoput_table;++opp=_find_current_opp(dev,opp_table);+ret=_set_opp(dev,opp_table,opp,opp->rate);+dev_pm_opp_put(opp);++put_table:+/* Drop reference taken by _find_opp_table() */+dev_pm_opp_put_opp_table(opp_table);++returnret;+}+EXPORT_SYMBOL_GPL(dev_pm_opp_sync);
Previously we assumed that devm_tegra_core_dev_init_opp_table() will
be used only by drivers that will always have device with OPP table,
but this is not true anymore. For example now Tegra30 will have OPP table
for PWM, but Tegra20 not and both use the same driver. Hence let's not
print the error message about missing OPP table in the common helper,
we can print it elsewhere.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/common.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
@@ -111,9 +111,7 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,*/err=devm_pm_opp_of_add_table(dev);if(err){-if(err==-ENODEV)-dev_err_once(dev,"OPP table not found, please update device-tree\n");-else+if(err!=-ENODEV)dev_err(dev,"failed to add OPP table: %d\n",err);returnerr;
Document tegra-clocks sub-node which describes Tegra SoC clocks that
require a higher voltage of the core power domain in order to operate
properly on a higher clock rates. Each node contains a phandle to OPP
table and power domain.
The root PLLs and system clocks don't have any specific device dedicated
to them, clock controller is in charge of managing power for them.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/clock/nvidia,tegra20-car.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
@@ -42,6 +42,48 @@ properties:"#reset-cells":const:1+tegra-clocks:+description:child nodes are the output clocks from the CAR+type:object++patternProperties:+"^[a-z]+[0-9]+$":+type:object+properties:+compatible:+allOf:+-items:+-enum:+-nvidia,tegra20-sclk+-nvidia,tegra30-sclk+-nvidia,tegra30-pllc+-nvidia,tegra30-plle+-nvidia,tegra30-pllm+-const:nvidia,tegra-clock++operating-points-v2:+$ref:/schemas/types.yaml#/definitions/phandle+description:+Phandle to OPP table that contains frequencies, voltages and+opp-supported-hw property, which is a bitfield indicating+SoC process or speedo ID mask.++clocks:+items:+-description:node's clock++power-domains:+maxItems:1+description:phandle to the core SoC power domain++required:+-compatible+-operating-points-v2+-clocks+-power-domains++additionalProperties:false+required:-compatible-reg
The Clock-and-Reset controller resides in a core power domain on NVIDIA
Tegra SoCs. In order to support voltage scaling of the core power domain,
we hook up DVFS-capable clocks to the core GENPD for managing of the
GENPD's performance state based on the clock changes.
Some clocks don't have any specific physical hardware unit that backs
them, like root PLLs and system clock and they have theirs own voltage
requirements. This patch adds new clk-device driver that backs the clocks
and provides runtime PM functionality for them. A virtual clk-device is
created for each such DVFS-capable clock at the clock's registration time
by the new tegra_clk_register() helper. Driver changes clock's device
GENPD performance state based on clk-rate notifications.
In result we have this sequence of events:
1. Clock driver creates virtual device for selective clocks, enables
runtime PM for the created device and registers the clock.
2. Clk-device driver starts to listen to clock rate changes.
3. Something changes clk rate or enables/disables clk.
4. CCF core propagates the change through the clk tree.
5. Clk-device driver gets clock rate-change notification or GENPD core
handles prepare/unprepare of the clock.
6. Clk-device driver changes GENPD performance state on clock rate
change.
7. GENPD driver changes voltage regulator state change.
8. The regulator state is committed to hardware via I2C.
We rely on fact that DVFS is not needed for Tegra I2C. Hence I2C
subsystem stays independent from the clk power management and there
are no deadlock spots in the sequence.
Currently all clocks are registered very early during kernel boot when the
device driver core isn't available yet. The clk-device can't be created
at that time. This patch splits the registration of the clocks in two
phases:
1. Register all essential clocks which don't use RPM and are needed
during early boot.
2. Register at a later boot time the rest of clocks.
This patch adds power management support for Tegra20 and Tegra30 clocks.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/clk/tegra/Makefile | 1 +
drivers/clk/tegra/clk-device.c | 222 ++++++++++++++++++++++++++++++++
drivers/clk/tegra/clk-pll.c | 2 +-
drivers/clk/tegra/clk-super.c | 2 +-
drivers/clk/tegra/clk-tegra20.c | 39 ++++--
drivers/clk/tegra/clk-tegra30.c | 70 ++++++----
drivers/clk/tegra/clk.c | 64 +++++++++
drivers/clk/tegra/clk.h | 2 +
8 files changed, 366 insertions(+), 36 deletions(-)
create mode 100644 drivers/clk/tegra/clk-device.c
@@ -0,0 +1,222 @@+// SPDX-License-Identifier: GPL-2.0-only++#include<linux/clk.h>+#include<linux/clk-provider.h>+#include<linux/mutex.h>+#include<linux/of_device.h>+#include<linux/platform_device.h>+#include<linux/pm_domain.h>+#include<linux/pm_opp.h>+#include<linux/pm_runtime.h>+#include<linux/slab.h>++#include<soc/tegra/common.h>++#include"clk.h"++/*+*Thisdrivermanagesperformancestateofthecorepowerdomainforthe+*independentPLLsandsystemclocks.Wecreatedavirtualclockdevice+*forsuchclocks,seetegra_clk_register().+*/++structtegra_clk_device{+structnotifier_blockclk_nb;+structdevice*dev;+structclk_hw*hw;+structmutexlock;+};++staticinttegra_clock_set_pd_state(structtegra_clk_device*clk_dev,+unsignedlongrate)+{+structdevice*dev=clk_dev->dev;+structdev_pm_opp*opp;+unsignedintpstate;++opp=dev_pm_opp_find_freq_ceil(dev,&rate);+if(opp==ERR_PTR(-ERANGE)){+dev_dbg(dev,"failed to find ceil OPP for %luHz\n",rate);+opp=dev_pm_opp_find_freq_floor(dev,&rate);+}++if(IS_ERR(opp)){+dev_err(dev,"failed to find OPP for %luHz: %pe\n",rate,opp);+returnPTR_ERR(opp);+}++pstate=dev_pm_opp_get_required_pstate(opp,0);+dev_pm_opp_put(opp);++returndev_pm_genpd_set_performance_state(dev,pstate);+}++staticinttegra_clock_change_notify(structnotifier_block*nb,+unsignedlongmsg,void*data)+{+structclk_notifier_data*cnd=data;+structtegra_clk_device*clk_dev;+interr=0;++clk_dev=container_of(nb,structtegra_clk_device,clk_nb);++mutex_lock(&clk_dev->lock);+switch(msg){+casePRE_RATE_CHANGE:+if(cnd->new_rate>cnd->old_rate)+err=tegra_clock_set_pd_state(clk_dev,cnd->new_rate);+break;++caseABORT_RATE_CHANGE:+err=tegra_clock_set_pd_state(clk_dev,cnd->old_rate);+break;++casePOST_RATE_CHANGE:+if(cnd->new_rate<cnd->old_rate)+err=tegra_clock_set_pd_state(clk_dev,cnd->new_rate);+break;++default:+break;+}+mutex_unlock(&clk_dev->lock);++returnnotifier_from_errno(err);+}++staticinttegra_clock_sync_pd_state(structtegra_clk_device*clk_dev)+{+unsignedlongrate;+intret=0;++mutex_lock(&clk_dev->lock);++if(!pm_runtime_status_suspended(clk_dev->dev)){+rate=clk_hw_get_rate(clk_dev->hw);+ret=tegra_clock_set_pd_state(clk_dev,rate);+}++mutex_unlock(&clk_dev->lock);++returnret;+}++staticinttegra_clock_probe(structplatform_device*pdev)+{+structtegra_clk_device*clk_dev;+structdevice*dev=&pdev->dev;+structclk*clk;+interr;++if(!dev->pm_domain)+return-EINVAL;++clk_dev=devm_kzalloc(dev,sizeof(*clk_dev),GFP_KERNEL);+if(!clk_dev)+return-ENOMEM;++clk=devm_clk_get(dev,NULL);+if(IS_ERR(clk))+returnPTR_ERR(clk);++clk_dev->dev=dev;+clk_dev->hw=__clk_get_hw(clk);+clk_dev->clk_nb.notifier_call=tegra_clock_change_notify;+mutex_init(&clk_dev->lock);++platform_set_drvdata(pdev,clk_dev);++err=devm_tegra_core_dev_init_opp_table_simple(dev);+if(err)+returnerr;++err=clk_notifier_register(clk,&clk_dev->clk_nb);+if(err){+dev_err(dev,"failed to register clk notifier: %d\n",err);+returnerr;+}++/*+*Thedriverisattachingtoapotentiallyactive/resumedclock,hence+*weneedtosyncthepowerdomainperformancestateinaaccordanceto+*theclockrateifclockisresumed.+*/+err=tegra_clock_sync_pd_state(clk_dev);+if(err)+gotounreg_clk;++return0;++unreg_clk:+clk_notifier_unregister(clk,&clk_dev->clk_nb);++returnerr;+}++static__maybe_unusedinttegra_clock_pm_suspend(structdevice*dev)+{+structtegra_clk_device*clk_dev=dev_get_drvdata(dev);++/*+*PowermanagementoftheclockisentangledwiththeTegraPMC+*GENPDbecausePMCdriverenables/disablesclocksfortoggling+*ofthePD'son/offstate.+*+*ThePMCGENPDisresumedinNOIRQphase,beforeRPMoftheclocks+*becomesavailable,hencePMCcan'tuseclocksattheearlyresume+*phaseifRPMisinvolved.Forexamplewhen3dclockisenabled,+*itmayenabletheparentPLLclockthatneedstobeRPM-resumed.+*+*Secondly,thePLLclocksmaybeenabledbythelowlevelsuspend+*code,soweneedtoassumethatPLLisinenabledstateduring+*suspend.+*+*WewillkeepPLLsandsystemclockresumedduringsuspendtime.+*AllPLLsonallSoCsarelowpowerandsystemclockisalways-on,+*sopracticallynotmuchischangedhere.+*/++returnclk_prepare(clk_dev->hw->clk);+}++static__maybe_unusedinttegra_clock_pm_resume(structdevice*dev)+{+structtegra_clk_device*clk_dev=dev_get_drvdata(dev);++clk_unprepare(clk_dev->hw->clk);++return0;+}++staticvoidtegra_clock_shutdown(structplatform_device*pdev)+{+structtegra_clk_device*clk_dev=platform_get_drvdata(pdev);++clk_prepare(clk_dev->hw->clk);+}++staticconststructdev_pm_opstegra_clock_pm={+SET_SYSTEM_SLEEP_PM_OPS(tegra_clock_pm_suspend,+tegra_clock_pm_resume)+};++staticconststructof_device_idtegra_clock_match[]={+{.compatible="nvidia,tegra20-sclk"},+{.compatible="nvidia,tegra30-sclk"},+{.compatible="nvidia,tegra30-pllc"},+{.compatible="nvidia,tegra30-plle"},+{.compatible="nvidia,tegra30-pllm"},+{}+};++staticstructplatform_drivertegra_clock_driver={+.driver={+.name="tegra-clock",+.of_match_table=tegra_clock_match,+.pm=&tegra_clock_pm,+.suppress_bind_attrs=true,+},+.probe=tegra_clock_probe,+.shutdown=tegra_clock_shutdown,+};+builtin_platform_driver(tegra_clock_driver);
@@ -1914,7 +1914,7 @@ static struct clk *_tegra_clk_register_pll(struct tegra_clk_pll *pll,/* Data in .init is copied by clk_register(), so stack variable OK */pll->hw.init=&init;-returnclk_register(NULL,&pll->hw);+returntegra_clk_register(&pll->hw);}structclk*tegra_clk_register_pll(constchar*name,constchar*parent_name,
@@ -226,7 +226,7 @@ struct clk *tegra_clk_register_super_mux(const char *name,/* Data in .init is copied by clk_register(), so stack variable OK */super->hw.init=&init;-clk=clk_register(NULL,&super->hw);+clk=tegra_clk_register(&super->hw);if(IS_ERR(clk))kfree(super);
@@ -9,14 +9,19 @@#include<linux/delay.h>#include<linux/io.h>#include<linux/of.h>+#include<linux/of_device.h>#include<linux/clk/tegra.h>+#include<linux/platform_device.h>+#include<linux/pm_runtime.h>#include<linux/reset-controller.h>+#include<linux/string.h>#include<soc/tegra/fuse.h>#include"clk.h"/* Global data of Tegra CPU CAR ops */+staticstructdevice_node*tegra_car_np;staticstructtegra_cpu_car_opsdummy_car_ops;structtegra_cpu_car_ops*tegra_cpu_car_ops=&dummy_car_ops;
Memory Client should be blocked before hardware reset is asserted in order
to prevent memory corruption and hanging of memory controller.
Document Memory Client resets of Host1x, GR2D and GR3D hardware units.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/display/tegra/nvidia,tegra20-host1x.txt | 4 ++++
1 file changed, 4 insertions(+)
@@ -18,6 +18,7 @@ Required properties: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - host1x Optional properties:
@@ -197,6 +198,7 @@ of the following host1x client modules: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - 2d Optional properties:
@@ -222,6 +224,8 @@ of the following host1x client modules: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc+ - mc2 (Only required on SoCs with two 3D clocks) - 3d - 3d2 (Only required on SoCs with two 3D clocks)
Add host1x_channel_stop() which waits till channel becomes idle and then
stops the channel hardware. This is needed for supporting suspend/resume
by host1x drivers since the hardware state is lost after power-gating,
thus the channel needs to be stopped before client enters into suspend.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/gpu/host1x/channel.c | 8 ++++++++
include/linux/host1x.h | 1 +
2 files changed, 9 insertions(+)
Document new DVFS OPP table and power domain properties of the Host1x bus
and devices sitting on the bus.
Reviewed-by: Rob Herring <robh@kernel.org>
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../display/tegra/nvidia,tegra20-host1x.txt | 49 +++++++++++++++++++
1 file changed, 49 insertions(+)
@@ -20,6 +20,18 @@ Required properties: - reset-names: Must include the following entries: - host1x+Optional properties:+- operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to HEG or core power domain.++For each opp entry in 'operating-points-v2' table of host1x and its modules:+- opp-supported-hw: One bitfield indicating:+ On Tegra20: SoC process ID mask+ On Tegra30+: SoC speedo ID mask++ A bitwise AND is performed against the value and if any bit+ matches, the OPP gets enabled.+ Each host1x client module having to perform DMA through the Memory Controller should have the interconnect endpoints set to the Memory Client and External Memory respectively.
@@ -45,6 +57,8 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to MPE power domain. - vi: video input
@@ -128,6 +142,8 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to VENC power domain. - epp: encoder pre-processor
@@ -147,6 +163,8 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to HEG or core power domain. - isp: image signal processor
@@ -166,6 +184,7 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - power-domains: Phandle to VENC or core power domain. - gr2d: 2D graphics engine
@@ -185,6 +204,8 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to HEG or core power domain. - gr3d: 3D graphics engine
@@ -209,6 +230,8 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandles to 3D or core power domain. - dc: display controller
@@ -241,6 +264,8 @@ of the following host1x client modules: - interconnect-names: Must include name of the interconnect path for each interconnect entry. Consult TRM documentation for information about available memory clients, see MEMORY CONTROLLER section.+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to core power domain. - hdmi: High Definition Multimedia Interface
@@ -267,6 +292,7 @@ of the following host1x client modules: - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection - nvidia,edid: supplies a binary EDID blob - nvidia,panel: phandle of a display panel+ - operating-points-v2: See ../bindings/opp/opp.txt for details. - tvo: TV encoder output
@@ -277,6 +303,10 @@ of the following host1x client modules: - clocks: Must contain one entry, for the module clock. See ../clocks/clock-bindings.txt for details.+ Optional properties:+ - operating-points-v2: See ../bindings/opp/opp.txt for details.+ - power-domains: Phandle to core power domain.+ - dsi: display serial interface Required properties:
@@ -305,6 +335,7 @@ of the following host1x client modules: - nvidia,panel: phandle of a display panel - nvidia,ganged-mode: contains a phandle to a second DSI controller to gang up with in order to support up to 8 data lanes+ - operating-points-v2: See ../bindings/opp/opp.txt for details. - sor: serial output resource
The FUSE controller is enabled at a boot time. Reset it in order to put
hardware and clock into clean and disabled state.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/fuse/fuse-tegra.c | 25 +++++++++++++++++++++++++
drivers/soc/tegra/fuse/fuse.h | 1 +
2 files changed, 26 insertions(+)
@@ -248,6 +249,30 @@ static int tegra_fuse_probe(struct platform_device *pdev)gotorestore;}+fuse->rst=devm_reset_control_get_optional(&pdev->dev,"fuse");+if(IS_ERR(fuse->rst)){+err=PTR_ERR(fuse->rst);+dev_err(&pdev->dev,"failed to get FUSE reset: %pe\n",+fuse->rst);+gotorestore;+}++/*+*FUSEclockisenabledataboottime,hencethisresume/suspend+*disablestheclockbesidestheh/wresetting.+*/+err=pm_runtime_resume_and_get(&pdev->dev);+if(err)+gotorestore;++err=reset_control_reset(fuse->rst);+pm_runtime_put(&pdev->dev);++if(err<0){+dev_err(&pdev->dev,"failed to reset FUSE: %d\n",err);+gotorestore;+}+/* release the early I/O memory mapping */iounmap(base);
Depending on hardware version, Tegra SoC may require a higher voltages
during resume from system suspend, otherwise hardware will crash. Set
SoC voltages to a nominal levels during suspend.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/regulators-tegra20.c | 99 ++++++++++++++++++++
drivers/soc/tegra/regulators-tegra30.c | 122 +++++++++++++++++++++++++
2 files changed, 221 insertions(+)
@@ -105,6 +110,28 @@ static int tegra20_core_rtc_max_spread(struct regulator_dev *core_rdev,return150000;}+staticinttegra20_cpu_nominal_uV(void)+{+switch(tegra_sku_info.soc_speedo_id){+case0:+return1100000;+case1:+return1025000;+default:+return1125000;+}+}++staticinttegra20_core_nominal_uV(void)+{+switch(tegra_sku_info.soc_speedo_id){+default:+return1225000;+case2:+return1300000;+}+}+staticinttegra20_core_rtc_update(structtegra_regulator_coupler*tegra,structregulator_dev*core_rdev,structregulator_dev*rtc_rdev,
@@ -144,6 +171,11 @@ static int tegra20_core_rtc_update(struct tegra_regulator_coupler *tegra,if(err)returnerr;+/* prepare voltage level for suspend */+if(tegra->sys_suspend_mode)+core_min_uV=clamp(tegra20_core_nominal_uV(),+core_min_uV,core_max_uV);+core_uV=regulator_get_voltage_rdev(core_rdev);if(core_uV<0)returncore_uV;
@@ -279,6 +311,11 @@ static int tegra20_cpu_voltage_update(struct tegra_regulator_coupler *tegra,if(tegra->sys_reboot_mode)cpu_min_uV=max(cpu_min_uV,tegra->cpu_min_uV);+/* prepare voltage level for suspend */+if(tegra->sys_suspend_mode)+cpu_min_uV=clamp(tegra20_cpu_nominal_uV(),+cpu_min_uV,cpu_max_uV);+if(cpu_min_uV>cpu_uV){err=tegra20_core_rtc_update(tegra,core_rdev,rtc_rdev,cpu_uV,cpu_min_uV);
@@ -320,6 +357,7 @@ static int tegra20_regulator_balance_voltage(struct regulator_coupler *coupler,}tegra->sys_reboot_mode=READ_ONCE(tegra->sys_reboot_mode_req);+tegra->sys_suspend_mode=READ_ONCE(tegra->sys_suspend_mode_req);if(rdev==cpu_rdev)returntegra20_cpu_voltage_update(tegra,cpu_rdev,
@@ -334,6 +372,63 @@ static int tegra20_regulator_balance_voltage(struct regulator_coupler *coupler,return-EPERM;}+staticinttegra20_regulator_prepare_suspend(structtegra_regulator_coupler*tegra,+boolsys_suspend_mode)+{+interr;++if(!tegra->core_rdev||!tegra->rtc_rdev||!tegra->cpu_rdev)+return0;++/*+*Allpowerdomainsareenabledearlyduringresumefromsuspend+*byGENPDcore.DomainslikeVENCmayrequireahighervoltage+*whenenabledduringresumefromsuspend.Thisalsoprepares+*hardwareforresumingfromLP0.+*/++WRITE_ONCE(tegra->sys_suspend_mode_req,sys_suspend_mode);++err=regulator_sync_voltage_rdev(tegra->cpu_rdev);+if(err)+returnerr;++err=regulator_sync_voltage_rdev(tegra->core_rdev);+if(err)+returnerr;++return0;+}++staticinttegra20_regulator_suspend(structnotifier_block*notifier,+unsignedlongmode,void*arg)+{+structtegra_regulator_coupler*tegra;+intret=0;++tegra=container_of(notifier,structtegra_regulator_coupler,+suspend_notifier);++switch(mode){+casePM_HIBERNATION_PREPARE:+casePM_RESTORE_PREPARE:+casePM_SUSPEND_PREPARE:+ret=tegra20_regulator_prepare_suspend(tegra,true);+break;++casePM_POST_HIBERNATION:+casePM_POST_RESTORE:+casePM_POST_SUSPEND:+ret=tegra20_regulator_prepare_suspend(tegra,false);+break;+}++if(ret)+pr_err("failed to prepare regulators: %d\n",ret);++returnnotifier_from_errno(ret);+}+staticinttegra20_regulator_prepare_reboot(structtegra_regulator_coupler*tegra,boolsys_reboot_mode){
@@ -113,6 +117,52 @@ static int tegra30_core_cpu_limit(int cpu_uV)return-EINVAL;}+staticinttegra30_cpu_nominal_uV(void)+{+switch(tegra_sku_info.cpu_speedo_id){+case10...11:+return850000;++case9:+return912000;++case1...3:+case7...8:+return1050000;++default:+return1125000;++case4...6:+case12...13:+return1237000;+}+}++staticinttegra30_core_nominal_uV(void)+{+switch(tegra_sku_info.soc_speedo_id){+case0:+return1200000;++case1:+if(tegra_sku_info.cpu_speedo_id!=7&&+tegra_sku_info.cpu_speedo_id!=8)+return1200000;++fallthrough;++case2:+if(tegra_sku_info.cpu_speedo_id!=13)+return1300000;++return1350000;++default:+return1250000;+}+}+staticinttegra30_voltage_update(structtegra_regulator_coupler*tegra,structregulator_dev*cpu_rdev,structregulator_dev*core_rdev)
@@ -168,6 +218,11 @@ static int tegra30_voltage_update(struct tegra_regulator_coupler *tegra,if(err)returnerr;+/* prepare voltage level for suspend */+if(tegra->sys_suspend_mode)+core_min_uV=clamp(tegra30_core_nominal_uV(),+core_min_uV,core_max_uV);+core_uV=regulator_get_voltage_rdev(core_rdev);if(core_uV<0)returncore_uV;
@@ -223,6 +278,11 @@ static int tegra30_voltage_update(struct tegra_regulator_coupler *tegra,if(tegra->sys_reboot_mode)cpu_min_uV=max(cpu_min_uV,tegra->cpu_min_uV);+/* prepare voltage level for suspend */+if(tegra->sys_suspend_mode)+cpu_min_uV=clamp(tegra30_cpu_nominal_uV(),+cpu_min_uV,cpu_max_uV);+if(core_min_limited_uV>core_uV){pr_err("core voltage constraint violated: %d %d %d\n",core_uV,core_min_limited_uV,cpu_uV);
@@ -292,10 +352,68 @@ static int tegra30_regulator_balance_voltage(struct regulator_coupler *coupler,}tegra->sys_reboot_mode=READ_ONCE(tegra->sys_reboot_mode_req);+tegra->sys_suspend_mode=READ_ONCE(tegra->sys_suspend_mode_req);returntegra30_voltage_update(tegra,cpu_rdev,core_rdev);}+staticinttegra30_regulator_prepare_suspend(structtegra_regulator_coupler*tegra,+boolsys_suspend_mode)+{+interr;++if(!tegra->core_rdev||!tegra->cpu_rdev)+return0;++/*+*Allpowerdomainsareenabledearlyduringresumefromsuspend+*byGENPDcore.DomainslikeVENCmayrequireahighervoltage+*whenenabledduringresumefromsuspend.Thisalsoprepares+*hardwareforresumingfromLP0.+*/++WRITE_ONCE(tegra->sys_suspend_mode_req,sys_suspend_mode);++err=regulator_sync_voltage_rdev(tegra->cpu_rdev);+if(err)+returnerr;++err=regulator_sync_voltage_rdev(tegra->core_rdev);+if(err)+returnerr;++return0;+}++staticinttegra30_regulator_suspend(structnotifier_block*notifier,+unsignedlongmode,void*arg)+{+structtegra_regulator_coupler*tegra;+intret=0;++tegra=container_of(notifier,structtegra_regulator_coupler,+suspend_notifier);++switch(mode){+casePM_HIBERNATION_PREPARE:+casePM_RESTORE_PREPARE:+casePM_SUSPEND_PREPARE:+ret=tegra30_regulator_prepare_suspend(tegra,true);+break;++casePM_POST_HIBERNATION:+casePM_POST_RESTORE:+casePM_POST_SUSPEND:+ret=tegra30_regulator_prepare_suspend(tegra,false);+break;+}++if(ret)+pr_err("failed to prepare regulators: %d\n",ret);++returnnotifier_from_errno(ret);+}+staticinttegra30_regulator_prepare_reboot(structtegra_regulator_coupler*tegra,boolsys_reboot_mode){
Initialize and sync FUSE OPP to set up SoC core voltage vote needed by
FUSE hardware.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/fuse/fuse-tegra.c | 11 +++++++++++
1 file changed, 11 insertions(+)
Currently driver supports legacy power domain API, this patch adds generic
power domain and OPP support. This allows us to utilize a modern GENPD API
for newer device-trees and support DVFS of the video decoder hardware.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/staging/media/tegra-vde/vde.c | 65 ++++++++++++++++++++++-----
1 file changed, 54 insertions(+), 11 deletions(-)
@@ -920,13 +923,17 @@ static __maybe_unused int tegra_vde_runtime_suspend(struct device *dev)structtegra_vde*vde=dev_get_drvdata(dev);interr;-err=tegra_powergate_power_off(TEGRA_POWERGATE_VDEC);-if(err){-dev_err(dev,"Failed to power down HW: %d\n",err);-returnerr;+if(!dev->pm_domain){+err=tegra_powergate_power_off(TEGRA_POWERGATE_VDEC);+if(err){+dev_err(dev,"Failed to power down HW: %d\n",err);+returnerr;+}}clk_disable_unprepare(vde->clk);+reset_control_release(vde->rst);+reset_control_release(vde->rst_mc);return0;}
@@ -936,14 +943,47 @@ static __maybe_unused int tegra_vde_runtime_resume(struct device *dev)structtegra_vde*vde=dev_get_drvdata(dev);interr;-err=tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VDEC,-vde->clk,vde->rst);+err=reset_control_acquire(vde->rst_mc);+if(err){+dev_err(dev,"Failed to acquire mc reset: %d\n",err);+returnerr;+}++err=reset_control_acquire(vde->rst);+if(err){+dev_err(dev,"Failed to acquire reset: %d\n",err);+gotorelease_mc_reset;+}++if(!dev->pm_domain){+err=tegra_powergate_sequence_power_up(TEGRA_POWERGATE_VDEC,+vde->clk,vde->rst);+if(err){+dev_err(dev,"Failed to power up HW : %d\n",err);+gotorelease_reset;+}+}++err=dev_pm_opp_sync(dev);if(err){-dev_err(dev,"Failed to power up HW : %d\n",err);+dev_err(dev,"failed to sync OPP: %d\n",err);returnerr;}+err=clk_prepare_enable(vde->clk);+if(err){+dev_err(dev,"Failed to enable clock: %d\n",err);+gotorelease_reset;+}+return0;++release_reset:+reset_control_release(vde->rst);+release_mc_reset:+reset_control_release(vde->rst_mc);++returnerr;}staticinttegra_vde_probe(structplatform_device*pdev)
@@ -1001,14 +1041,14 @@ static int tegra_vde_probe(struct platform_device *pdev)returnerr;}-vde->rst=devm_reset_control_get(dev,NULL);+vde->rst=devm_reset_control_get_exclusive_released(dev,NULL);if(IS_ERR(vde->rst)){err=PTR_ERR(vde->rst);dev_err(dev,"Could not get VDE reset %d\n",err);returnerr;}-vde->rst_mc=devm_reset_control_get_optional(dev,"mc");+vde->rst_mc=devm_reset_control_get_optional_exclusive_released(dev,"mc");if(IS_ERR(vde->rst_mc)){err=PTR_ERR(vde->rst_mc);dev_err(dev,"Could not get MC reset %d\n",err);
@@ -1026,6 +1066,10 @@ static int tegra_vde_probe(struct platform_device *pdev)returnerr;}+err=devm_tegra_core_dev_init_opp_table_simple(dev);+if(err)+returnerr;+vde->iram_pool=of_gen_pool_get(dev->of_node,"iram",0);if(!vde->iram_pool){dev_err(dev,"Could not get IRAM pool\n");
Document new OPP table and power domain properties of the video decoder
hardware.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../devicetree/bindings/media/nvidia,tegra-vde.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
@@ -68,6 +68,16 @@ properties:description:Phandle of the SRAM MMIO node.+operating-points-v2:+description:+Should contain freqs and voltages and opp-supported-hw property,+which is a bitfield indicating SoC speedo or process ID mask.++power-domains:+maxItems:1+description:+Phandle to the SoC core power domain.+required:-compatible-reg
All device drivers got runtime PM and OPP support. Flip the core domain
support status for Tegra20 and Tegra30 SoCs.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/pmc.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Memory access must be blocked before hardware reset is asserted and before
power is gated, otherwise a serious hardware fault is inevitable. Add
reset for memory clients to the GR2D, GR3D and Host1x nodes.
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
arch/arm/boot/dts/tegra20.dtsi | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
MPE, VI, EPP and ISP were never used and we don't have drivers for them.
Since these modules are enabled by default in a device-tree, a device is
created for them, blocking voltage scaling because there is no driver to
bind, and thus, state of PMC driver is never synced. Disable them.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
arch/arm/boot/dts/tegra20.dtsi | 4 ++++
arch/arm/boot/dts/tegra30.dtsi | 8 ++++++++
2 files changed, 12 insertions(+)
Memory access must be blocked before hardware reset is asserted and before
power is gated, otherwise a serious hardware fault is inevitable. Add
reset for memory clients to the GR2D, GR3D and Host1x nodes.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
arch/arm/boot/dts/tegra30.dtsi | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
@@ -914,6 +924,7 @@ pmc@7000e400 {nvidia,core-pwr-off-time=<0>;nvidia,core-power-req-active-high;nvidia,sys-clock-req-active-high;+core-supply=<&vdd_core>;/* Set DEV_OFF bit in DCDC control register of TPS65911 PMIC */i2c-thermtrip{
@@ -601,6 +601,7 @@ pmc@7000e400 {nvidia,core-pwr-good-time=<38453845>;nvidia,core-pwr-off-time=<3875>;nvidia,sys-clock-req-active-high;+core-supply=<&vdd_core>;/* Set SLEEP MODE bit in SUPPLYENE register of TPS658643 PMIC */i2c-thermtrip{
The SPI on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now SPI driver must use OPP
API for driving the controller's clock rate because OPP API takes care
of reconfiguring the domain's performance state in accordance to the
rate. Add OPP support to the driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/spi/spi-tegra20-slink.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
@@ -683,7 +686,7 @@ static int tegra_slink_start_transfer_one(struct spi_device *spi,bits_per_word=t->bits_per_word;speed=t->speed_hz;if(speed!=tspi->cur_speed){-clk_set_rate(tspi->clk,speed*4);+dev_pm_opp_set_rate(tspi->dev,speed*4);tspi->cur_speed=speed;}
@@ -1054,6 +1057,10 @@ static int tegra_slink_probe(struct platform_device *pdev)gotoexit_free_master;}+ret=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+if(ret)+returnret;+/* disabled clock may cause interrupt storm upon request */tspi->clk=devm_clk_get(&pdev->dev,NULL);if(IS_ERR(tspi->clk)){
@@ -1205,6 +1212,12 @@ static int tegra_slink_runtime_resume(struct device *dev)structtegra_slink_data*tspi=spi_master_get_devdata(master);intret;+ret=dev_pm_opp_sync(dev);+if(ret){+dev_err(dev,"failed to sync OPP: %d\n",ret);+returnret;+}+ret=clk_prepare_enable(tspi->clk);if(ret<0){dev_err(tspi->dev,"clk_prepare failed: %d\n",ret);
@@ -1,64 +0,0 @@-NVIDIA Tegra Video Decoder Engine--Required properties:-- compatible : Must contain one of the following values:- - "nvidia,tegra20-vde"- - "nvidia,tegra30-vde"- - "nvidia,tegra114-vde"- - "nvidia,tegra124-vde"- - "nvidia,tegra132-vde"-- reg : Must contain an entry for each entry in reg-names.-- reg-names : Must include the following entries:- - sxe- - bsev- - mbe- - ppe- - mce- - tfe- - ppb- - vdma- - frameid-- iram : Must contain phandle to the mmio-sram device node that represents- IRAM region used by VDE.-- interrupts : Must contain an entry for each entry in interrupt-names.-- interrupt-names : Must include the following entries:- - sync-token- - bsev- - sxe-- clocks : Must include the following entries:- - vde-- resets : Must contain an entry for each entry in reset-names.-- reset-names : Should include the following entries:- - vde--Optional properties:-- resets : Must contain an entry for each entry in reset-names.-- reset-names : Must include the following entries:- - mc-- iommus: Must contain phandle to the IOMMU device node.--Example:--video-codec@6001a000 {- compatible = "nvidia,tegra20-vde";- reg = <0x6001a000 0x1000 /* Syntax Engine */- 0x6001b000 0x1000 /* Video Bitstream Engine */- 0x6001c000 0x100 /* Macroblock Engine */- 0x6001c200 0x100 /* Post-processing Engine */- 0x6001c400 0x100 /* Motion Compensation Engine */- 0x6001c600 0x100 /* Transform Engine */- 0x6001c800 0x100 /* Pixel prediction block */- 0x6001ca00 0x100 /* Video DMA */- 0x6001d800 0x300 /* Video frame controls */>;- reg-names = "sxe", "bsev", "mbe", "ppe", "mce",- "tfe", "ppb", "vdma", "frameid";- iram = <&vde_pool>; /* IRAM region */- interrupts = <GIC_SPI 9 IRQ_TYPE_LEVEL_HIGH>, /* Sync token interrupt */- <GIC_SPI 10 IRQ_TYPE_LEVEL_HIGH>, /* BSE-V interrupt */- <GIC_SPI 12 IRQ_TYPE_LEVEL_HIGH>; /* SXE interrupt */- interrupt-names = "sync-token", "bsev", "sxe";- clocks = <&tegra_car TEGRA20_CLK_VDE>;- reset-names = "vde", "mc";- resets = <&tegra_car 61>, <&mc TEGRA20_MC_RESET_VDE>;- iommus = <&mc TEGRA_SWGROUP_VDE>;-};
The NAND on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now NAND must be resumed using
runtime PM API in order to initialize the NAND power state. Add runtime PM
and OPP support to the NAND driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/mtd/nand/raw/tegra_nand.c | 62 +++++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 8 deletions(-)
@@ -1152,6 +1156,7 @@ static int tegra_nand_probe(struct platform_device *pdev)return-ENOMEM;ctrl->dev=&pdev->dev;+platform_set_drvdata(pdev,ctrl);nand_controller_init(&ctrl->controller);ctrl->controller.ops=&tegra_nand_controller_ops;
@@ -1168,14 +1173,19 @@ static int tegra_nand_probe(struct platform_device *pdev)if(IS_ERR(ctrl->clk))returnPTR_ERR(ctrl->clk);-err=clk_prepare_enable(ctrl->clk);+err=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);if(err)returnerr;+pm_runtime_enable(ctrl->dev);+err=pm_runtime_resume_and_get(ctrl->dev);+if(err)+gotoerr_disable_pm;+err=reset_control_reset(rst);if(err){dev_err(ctrl->dev,"Failed to reset HW: %d\n",err);-gotoerr_disable_clk;+gotoerr_put_pm;}writel_relaxed(HWSTATUS_CMD_DEFAULT,ctrl->regs+HWSTATUS_CMD);
@@ -1190,21 +1200,21 @@ static int tegra_nand_probe(struct platform_device *pdev)dev_name(&pdev->dev),ctrl);if(err){dev_err(ctrl->dev,"Failed to get IRQ: %d\n",err);-gotoerr_disable_clk;+gotoerr_put_pm;}writel_relaxed(DMA_MST_CTRL_IS_DONE,ctrl->regs+DMA_MST_CTRL);err=tegra_nand_chips_init(ctrl->dev,ctrl);if(err)-gotoerr_disable_clk;--platform_set_drvdata(pdev,ctrl);+gotoerr_put_pm;return0;-err_disable_clk:-clk_disable_unprepare(ctrl->clk);+err_put_pm:+pm_runtime_put(ctrl->dev);+err_disable_pm:+pm_runtime_disable(ctrl->dev);returnerr;}
@@ -1221,11 +1231,46 @@ static int tegra_nand_remove(struct platform_device *pdev)nand_cleanup(chip);+pm_runtime_put(ctrl->dev);+pm_runtime_disable(ctrl->dev);++return0;+}++staticint__maybe_unusedtegra_nand_runtime_resume(structdevice*dev)+{+structtegra_nand_controller*ctrl=dev_get_drvdata(dev);+interr;++err=dev_pm_opp_sync(dev);+if(err){+dev_err(dev,"Failed to sync OPP: %d\n",err);+returnerr;+}++err=clk_prepare_enable(ctrl->clk);+if(err){+dev_err(dev,"Failed to enable clock: %d\n",err);+returnerr;+}++return0;+}++staticint__maybe_unusedtegra_nand_runtime_suspend(structdevice*dev)+{+structtegra_nand_controller*ctrl=dev_get_drvdata(dev);+clk_disable_unprepare(ctrl->clk);return0;}+staticconststructdev_pm_opstegra_nand_pm={+SET_RUNTIME_PM_OPS(tegra_nand_runtime_suspend,tegra_nand_runtime_resume,+NULL)+};+staticconststructof_device_idtegra_nand_of_match[]={{.compatible="nvidia,tegra20-nand"},{/* sentinel */}
The SDHCI on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now SDHCI must be resumed using
runtime PM API in order to initialize the SDHCI power state. The SDHCI
clock rate must be changed using OPP API that will reconfigure the power
domain performance state in accordance to the rate. Add runtime PM and OPP
support to the SDHCI driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/mmc/host/sdhci-tegra.c | 146 ++++++++++++++++++++++++---------
1 file changed, 105 insertions(+), 41 deletions(-)
@@ -776,7 +793,12 @@ static void tegra_sdhci_set_clock(struct sdhci_host *host, unsigned int clock)*fromclk_get_rate()isused.*/host_clk=tegra_host->ddr_signaling?clock*2:clock;-clk_set_rate(pltfm_host->clk,host_clk);++err=dev_pm_opp_set_rate(dev,host_clk);+if(err)+dev_err(dev,"failed to set clk rate to %luHz: %d\n",+host_clk,err);+tegra_host->curr_clk_rate=host_clk;if(tegra_host->ddr_signaling)host->max_clk=host_clk;
@@ -1653,6 +1675,21 @@ static int sdhci_tegra_probe(struct platform_device *pdev)gotoerr_power_req;}+tegra_host->clocks[TEGRA_CLK_BULK_SDHCI].id=NULL;+tegra_host->clocks[TEGRA_CLK_BULK_TMCLK].id="tmclk";++rc=devm_clk_bulk_get_optional(&pdev->dev,TEGRA_CLK_BULK_NUM,+tegra_host->clocks);++/* controller clock is mandatory */+if(!rc&&!tegra_host->clocks[TEGRA_CLK_BULK_SDHCI].clk)+rc=-ENOENT;++if(rc){+dev_err_probe(&pdev->dev,rc,"failed to get clock\n");+gotoerr_power_req;+}+/**Tegra210hasaseparateSDMMC_LEGACY_TMclockusedforhost*timeoutclockandSWcanchooseTMCLKorSDCLKforhardware
@@ -1669,34 +1706,19 @@ static int sdhci_tegra_probe(struct platform_device *pdev)*/if(soc_data->nvquirks&NVQUIRK_HAS_TMCLK){-clk=devm_clk_get(&pdev->dev,"tmclk");-if(IS_ERR(clk)){-rc=PTR_ERR(clk);-if(rc==-EPROBE_DEFER)-gotoerr_power_req;--dev_warn(&pdev->dev,"failed to get tmclk: %d\n",rc);-clk=NULL;-}+clk=tegra_host->clocks[TEGRA_CLK_BULK_TMCLK].clk;+if(!clk)+dev_warn(&pdev->dev,"failed to get tmclk");clk_set_rate(clk,12000000);-rc=clk_prepare_enable(clk);-if(rc){-dev_err(&pdev->dev,-"failed to enable tmclk: %d\n",rc);-gotoerr_power_req;-}--tegra_host->tmclk=clk;}-clk=devm_clk_get(mmc_dev(host->mmc),NULL);-if(IS_ERR(clk)){-rc=dev_err_probe(&pdev->dev,PTR_ERR(clk),-"failed to get clock\n");-gotoerr_clk_get;+clk=tegra_host->clocks[TEGRA_CLK_BULK_SDHCI].clk;+if(!clk){+dev_err(&pdev->dev,"failed to get sdhci clock\n");+rc=-ENOENT;+gotoerr_power_req;}-clk_prepare_enable(clk);pltfm_host->clk=clk;tegra_host->rst=devm_reset_control_get_exclusive(&pdev->dev,
@@ -1704,9 +1726,18 @@ static int sdhci_tegra_probe(struct platform_device *pdev)if(IS_ERR(tegra_host->rst)){rc=PTR_ERR(tegra_host->rst);dev_err(&pdev->dev,"failed to get reset control: %d\n",rc);-gotoerr_rst_get;+gotoerr_power_req;}+rc=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+if(rc)+gotoerr_power_req;++pm_runtime_enable(&pdev->dev);+rc=pm_runtime_resume_and_get(&pdev->dev);+if(rc)+gotoerr_pm_get;+rc=reset_control_assert(tegra_host->rst);if(rc)gotoerr_rst_get;
@@ -1728,9 +1759,9 @@ static int sdhci_tegra_probe(struct platform_device *pdev)err_add_host:reset_control_assert(tegra_host->rst);err_rst_get:-clk_disable_unprepare(pltfm_host->clk);-err_clk_get:-clk_disable_unprepare(tegra_host->tmclk);+pm_runtime_put(&pdev->dev);+err_pm_get:+pm_runtime_disable(&pdev->dev);err_power_req:err_parse_dt:sdhci_pltfm_free(pdev);
The GMI bus on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now GMI must be resumed using
runtime PM API in order to initialize the GMI power state. Add runtime PM
and OPP support to the GMI driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/bus/tegra-gmi.c | 92 ++++++++++++++++++++++++++++++-----------
1 file changed, 68 insertions(+), 24 deletions(-)
The PWM on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now PWM must be resumed using
runtime PM API in order to initialize the PWM power state. The PWM clock
rate must be changed using OPP API that will reconfigure the power domain
performance state in accordance to the rate. Add runtime PM and OPP
support to the PWM driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/pwm/pwm-tegra.c | 104 ++++++++++++++++++++++++++++++++--------
1 file changed, 85 insertions(+), 19 deletions(-)
@@ -145,12 +149,25 @@ static int tegra_pwm_config(struct pwm_chip *chip, struct pwm_device *pwm,required_clk_rate=(NSEC_PER_SEC/period_ns)<<PWM_DUTY_WIDTH;-err=clk_set_rate(pc->clk,required_clk_rate);-if(err<0)+/*+*GENPDperformancestateshouldbechangedonlyfor+*theresumeddevicebecauseGENPDcoretracksperformance+*stateanddrops/restoresthestateonRPMsuspend/resume.+*/+err=pm_runtime_resume_and_get(pc->dev);+if(err)+returnerr;++err=dev_pm_opp_set_rate(pc->dev,required_clk_rate);+if(err<0){+pm_runtime_put(pc->dev);return-EINVAL;+}/* Store the new rate for further references */pc->clk_rate=clk_get_rate(pc->clk);++pm_runtime_put(pc->dev);}rate=pc->clk_rate>>PWM_DUTY_WIDTH;
@@ -256,11 +273,21 @@ static int tegra_pwm_probe(struct platform_device *pdev)if(IS_ERR(pwm->clk))returnPTR_ERR(pwm->clk);+ret=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+if(ret)+returnret;++pm_runtime_enable(&pdev->dev);++ret=pm_runtime_resume_and_get(&pdev->dev);+if(ret)+gotodisable_pm;+/* Set maximum frequency of the IP */-ret=clk_set_rate(pwm->clk,pwm->soc->max_frequency);+ret=dev_pm_opp_set_rate(pwm->dev,pwm->soc->max_frequency);if(ret<0){dev_err(&pdev->dev,"Failed to set max frequency: %d\n",ret);-returnret;+gotoput_pm;}/*
@@ -278,7 +305,7 @@ static int tegra_pwm_probe(struct platform_device *pdev)if(IS_ERR(pwm->rst)){ret=PTR_ERR(pwm->rst);dev_err(&pdev->dev,"Reset control is not found: %d\n",ret);-returnret;+gotoput_pm;}reset_control_deassert(pwm->rst);
The Tegra USB controller belongs to the core power domain and we're going
to enable GENPD support for the core domain. Now USB controller must be
resumed using runtime PM API in order to initialize the USB power state.
We already support runtime PM for the CI device, but CI's PM is separated
from the RPM managed by tegra-usb driver. Add runtime PM and OPP support
to tegra-usb driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/usb/chipidea/ci_hdrc_tegra.c | 61 ++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 7 deletions(-)
@@ -278,6 +282,8 @@ static int tegra_usb_probe(struct platform_device *pdev)if(!usb)return-ENOMEM;+platform_set_drvdata(pdev,usb);+soc=of_device_get_match_data(&pdev->dev);if(!soc){dev_err(&pdev->dev,"failed to match OF data\n");
@@ -296,11 +302,14 @@ static int tegra_usb_probe(struct platform_device *pdev)returnerr;}-err=clk_prepare_enable(usb->clk);-if(err<0){-dev_err(&pdev->dev,"failed to enable clock: %d\n",err);+err=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+if(err)returnerr;-}++pm_runtime_enable(&pdev->dev);+err=pm_runtime_resume_and_get(&pdev->dev);+if(err)+gotodisable_pm;if(device_property_present(&pdev->dev,"nvidia,needs-double-reset"))usb->needs_double_reset=true;
@@ -320,8 +329,6 @@ static int tegra_usb_probe(struct platform_device *pdev)if(err)gotofail_power_off;-platform_set_drvdata(pdev,usb);-/* setup and register ChipIdea HDRC device */usb->soc=soc;usb->data.name="tegra-usb";
@@ -350,7 +357,10 @@ static int tegra_usb_probe(struct platform_device *pdev)phy_shutdown:usb_phy_shutdown(usb->phy);fail_power_off:-clk_disable_unprepare(usb->clk);+pm_runtime_put(&pdev->dev);+disable_pm:+pm_runtime_disable(&pdev->dev);+returnerr;}
@@ -360,15 +370,52 @@ static int tegra_usb_remove(struct platform_device *pdev)ci_hdrc_remove_device(usb->dev);usb_phy_shutdown(usb->phy);++pm_runtime_put(&pdev->dev);+pm_runtime_disable(&pdev->dev);++return0;+}++staticint__maybe_unusedtegra_usb_runtime_resume(structdevice*dev)+{+structtegra_usb*usb=dev_get_drvdata(dev);+interr;++err=dev_pm_opp_sync(dev);+if(err){+dev_err(dev,"failed to sync OPP: %d\n",err);+returnerr;+}++err=clk_prepare_enable(usb->clk);+if(err<0){+dev_err(dev,"failed to enable clock: %d\n",err);+returnerr;+}++return0;+}++staticint__maybe_unusedtegra_usb_runtime_suspend(structdevice*dev)+{+structtegra_usb*usb=dev_get_drvdata(dev);+clk_disable_unprepare(usb->clk);return0;}+staticconststructdev_pm_opstegra_usb_pm={+SET_RUNTIME_PM_OPS(tegra_usb_runtime_suspend,tegra_usb_runtime_resume,+NULL)+};+staticstructplatform_drivertegra_usb_driver={.driver={.name="tegra-usb",.of_match_table=tegra_usb_of_match,+.pm=&tegra_usb_pm,},.probe=tegra_usb_probe,.remove=tegra_usb_remove,
Hardware must be stopped before system is suspended. Add suspend-resume
callbacks.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/gpu/drm/tegra/vic.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -190,6 +204,27 @@ static const u32 gr2d_addr_regs[] = {GR2D_VA_BASE_ADDR_SB,};+staticintgr2d_get_resets(structdevice*dev,structgr2d*gr2d)+{+interr;++gr2d->resets[RST_GR2D_MC].id="mc";+gr2d->resets[RST_GR2D].id="2d";+gr2d->nresets=RST_GR2D_MAX;++err=devm_reset_control_bulk_get_optional_exclusive_released(+dev,gr2d->nresets,gr2d->resets);+if(err){+dev_err(dev,"failed to get reset: %d\n",err);+returnerr;+}++if(WARN_ON(!gr2d->resets[RST_GR2D].rstc))+return-ENOENT;++return0;+}+staticintgr2d_probe(structplatform_device*pdev){structdevice*dev=&pdev->dev;
@@ -202,6 +237,8 @@ static int gr2d_probe(struct platform_device *pdev)if(!gr2d)return-ENOMEM;+platform_set_drvdata(pdev,gr2d);+gr2d->soc=of_device_get_match_data(dev);syncpts=devm_kzalloc(dev,sizeof(*syncpts),GFP_KERNEL);
@@ -214,11 +251,13 @@ static int gr2d_probe(struct platform_device *pdev)returnPTR_ERR(gr2d->clk);}-err=clk_prepare_enable(gr2d->clk);-if(err){-dev_err(dev,"cannot turn on clock\n");+err=devm_tegra_core_dev_init_opp_table_simple(dev);+if(err)+returnerr;++err=gr2d_get_resets(dev,gr2d);+if(err)returnerr;-}INIT_LIST_HEAD(&gr2d->client.base.list);gr2d->client.base.ops=&gr2d_client_ops;
@@ -231,20 +270,27 @@ static int gr2d_probe(struct platform_device *pdev)gr2d->client.version=gr2d->soc->version;gr2d->client.ops=&gr2d_ops;+pm_runtime_enable(dev);+pm_runtime_use_autosuspend(dev);+pm_runtime_set_autosuspend_delay(dev,200);+err=host1x_client_register(&gr2d->client.base);if(err<0){dev_err(dev,"failed to register host1x client: %d\n",err);-clk_disable_unprepare(gr2d->clk);-returnerr;+gotodisable_rpm;}/* initialize address register map */for(i=0;i<ARRAY_SIZE(gr2d_addr_regs);i++)set_bit(gr2d_addr_regs[i],gr2d->addr_regs);-platform_set_drvdata(pdev,gr2d);-return0;++disable_rpm:+pm_runtime_dont_use_autosuspend(dev);+pm_runtime_disable(dev);++returnerr;}staticintgr2d_remove(structplatform_device*pdev)
@@ -259,15 +305,107 @@ static int gr2d_remove(struct platform_device *pdev)returnerr;}+pm_runtime_dont_use_autosuspend(&pdev->dev);+pm_runtime_disable(&pdev->dev);++return0;+}++staticint__maybe_unusedgr2d_runtime_suspend(structdevice*dev)+{+structgr2d*gr2d=dev_get_drvdata(dev);+interr;++host1x_channel_stop(gr2d->channel);+reset_control_bulk_release(gr2d->nresets,gr2d->resets);++/*+*GR2Dmoduleshouldn'tberesetwhilehardwareisidling,otherwise+*host1x'scmdprocwillstuckontryingtoaccessanyG2register+*afterreset.GR2Dmodulecouldbeeitherhot-resetorresetafter+*power-gatingoftheHEGpartition.Hencewewillputinresetonly+*thememoryclientpartofthemodule,theHEGGENPDwilltakecare+*ofresettingGR2Dmoduleacrosspower-gating.+*+*OnTegra20thereisnoHEGpartition,butit'sokaytohave+*undeterminedh/wstatesinceuserspaceisexpectedtoreprogram+*thestateoneachjobsubmissionanyways.+*/+err=reset_control_acquire(gr2d->resets[RST_GR2D_MC].rstc);+if(err){+dev_err(dev,"failed to acquire MC reset: %d\n",err);+gotoacquire_reset;+}++err=reset_control_assert(gr2d->resets[RST_GR2D_MC].rstc);+reset_control_release(gr2d->resets[RST_GR2D_MC].rstc);+if(err){+dev_err(dev,"failed to assert MC reset: %d\n",err);+gotoacquire_reset;+}+clk_disable_unprepare(gr2d->clk);return0;++acquire_reset:+reset_control_bulk_acquire(gr2d->nresets,gr2d->resets);+reset_control_bulk_deassert(gr2d->nresets,gr2d->resets);++returnerr;}+staticint__maybe_unusedgr2d_runtime_resume(structdevice*dev)+{+structgr2d*gr2d=dev_get_drvdata(dev);+interr;++err=reset_control_bulk_acquire(gr2d->nresets,gr2d->resets);+if(err){+dev_err(dev,"failed to acquire reset: %d\n",err);+returnerr;+}++err=dev_pm_opp_sync(dev);+if(err){+dev_err(dev,"failed to sync OPP: %d\n",err);+gotorelease_reset;+}++err=clk_prepare_enable(gr2d->clk);+if(err){+dev_err(dev,"failed to enable clock: %d\n",err);+gotorelease_reset;+}++/* this is a reset array which deasserts both 2D MC and 2D itself */+err=reset_control_bulk_deassert(gr2d->nresets,gr2d->resets);+if(err){+dev_err(dev,"failed to deassert reset: %d\n",err);+gotodisable_clk;+}++return0;++disable_clk:+clk_disable_unprepare(gr2d->clk);+release_reset:+reset_control_bulk_release(gr2d->nresets,gr2d->resets);++returnerr;+}++staticconststructdev_pm_opstegra_gr2d_pm={+SET_RUNTIME_PM_OPS(gr2d_runtime_suspend,gr2d_runtime_resume,NULL)+SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,+pm_runtime_force_resume)+};+structplatform_drivertegra_gr2d_driver={.driver={.name="tegra-gr2d",.of_match_table=gr2d_match,+.pm=&tegra_gr2d_pm,},.probe=gr2d_probe,.remove=gr2d_remove,
@@ -278,9 +300,132 @@ static const u32 gr3d_addr_regs[] = {GR3D_GLOBAL_SAMP23SURFADDR(15),};+staticvoidgr3d_del_link(void*link)+{+device_link_del(link);+}++staticintgr3d_init_power(structdevice*dev,structgr3d*gr3d)+{+constu32link_flags=DL_FLAG_STATELESS|DL_FLAG_PM_RUNTIME;+constchar*opp_genpd_names[]={"3d0","3d1",NULL};+structdevice**opp_virt_dev;+structdevice_link*link;+structdevice*pd_dev;+unsignedinti;+interr;++err=of_count_phandle_with_args(dev->of_node,"power-domains",+"#power-domain-cells");+if(err<0){+if(err!=-ENOENT)+returnerr;++/*+*Olderdevice-treesdon'tuseGENPD.Inthiscaseweshould+*togglepowerdomainmanually.+*/+gr3d->legacy_pd=true;+return0;+}++/*+*ThePMdomaincoreautomaticallyattachesasinglepowerdomain,+*otherwiseitskipsattachingcompletely.Wehaveasingledomain+*onTegra20andtwodomainsonTegra30+.+*/+if(dev->pm_domain)+return0;++err=devm_pm_opp_attach_genpd(dev,opp_genpd_names,&opp_virt_dev);+if(err)+returnerr;++for(i=0;opp_genpd_names[i];i++){+pd_dev=opp_virt_dev[i];+if(!pd_dev){+dev_err(dev,"failed to get %s power domain\n",+opp_genpd_names[i]);+return-EINVAL;+}++link=device_link_add(dev,pd_dev,link_flags);+if(!link){+dev_err(dev,"failed to link to %s\n",dev_name(pd_dev));+return-EINVAL;+}++err=devm_add_action_or_reset(dev,gr3d_del_link,link);+if(err)+returnerr;+}++return0;+}++staticintgr3d_set_opp(structdev_pm_set_opp_data*data)+{+structgr3d*gr3d=dev_get_drvdata(data->dev);+unsignedinti;+interr;++for(i=0;i<gr3d->nclocks;i++){+err=clk_set_rate(gr3d->clocks[i].clk,data->new_opp.rate);+if(err){+dev_err(data->dev,"failed to set %s rate to %lu: %d\n",+gr3d->clocks[i].id,data->new_opp.rate,err);+returnerr;+}+}++return0;+}++staticintgr3d_get_clocks(structdevice*dev,structgr3d*gr3d)+{+interr;++err=devm_clk_bulk_get_all(dev,&gr3d->clocks);+if(err<0){+dev_err(dev,"failed to get clock: %d\n",err);+returnerr;+}+gr3d->nclocks=err;++if(gr3d->nclocks!=gr3d->soc->num_clocks){+dev_err(dev,"invalid number of clocks: %u\n",gr3d->nclocks);+return-ENOENT;+}++return0;+}++staticintgr3d_get_resets(structdevice*dev,structgr3d*gr3d)+{+interr;++gr3d->nresets=gr3d->soc->num_resets;+gr3d->resets[RST_GR3D2_MC].id="mc2";+gr3d->resets[RST_GR3D_MC].id="mc";+gr3d->resets[RST_GR3D2].id="3d2";+gr3d->resets[RST_GR3D].id="3d";++err=devm_reset_control_bulk_get_optional_exclusive_released(+dev,gr3d->nresets,gr3d->resets);+if(err){+dev_err(dev,"failed to get reset: %d\n",err);+returnerr;+}++if(WARN_ON(!gr3d->resets[RST_GR3D].rstc)||+WARN_ON(!gr3d->resets[RST_GR3D2].rstc&&gr3d->nresets==4))+return-ENOENT;++return0;+}+staticintgr3d_probe(structplatform_device*pdev){-structdevice_node*np=pdev->dev.of_node;structhost1x_syncpt**syncpts;structgr3d*gr3d;unsignedinti;
@@ -290,56 +435,33 @@ static int gr3d_probe(struct platform_device *pdev)if(!gr3d)return-ENOMEM;+platform_set_drvdata(pdev,gr3d);+gr3d->soc=of_device_get_match_data(&pdev->dev);syncpts=devm_kzalloc(&pdev->dev,sizeof(*syncpts),GFP_KERNEL);if(!syncpts)return-ENOMEM;-gr3d->clk=devm_clk_get(&pdev->dev,NULL);-if(IS_ERR(gr3d->clk)){-dev_err(&pdev->dev,"cannot get clock\n");-returnPTR_ERR(gr3d->clk);-}--gr3d->rst=devm_reset_control_get(&pdev->dev,"3d");-if(IS_ERR(gr3d->rst)){-dev_err(&pdev->dev,"cannot get reset\n");-returnPTR_ERR(gr3d->rst);-}+err=gr3d_get_clocks(&pdev->dev,gr3d);+if(err)+returnerr;-if(of_device_is_compatible(np,"nvidia,tegra30-gr3d")){-gr3d->clk_secondary=devm_clk_get(&pdev->dev,"3d2");-if(IS_ERR(gr3d->clk_secondary)){-dev_err(&pdev->dev,"cannot get secondary clock\n");-returnPTR_ERR(gr3d->clk_secondary);-}+err=gr3d_get_resets(&pdev->dev,gr3d);+if(err)+returnerr;-gr3d->rst_secondary=devm_reset_control_get(&pdev->dev,-"3d2");-if(IS_ERR(gr3d->rst_secondary)){-dev_err(&pdev->dev,"cannot get secondary reset\n");-returnPTR_ERR(gr3d->rst_secondary);-}-}+err=gr3d_init_power(&pdev->dev,gr3d);+if(err)+returnerr;-err=tegra_powergate_sequence_power_up(TEGRA_POWERGATE_3D,gr3d->clk,-gr3d->rst);-if(err<0){-dev_err(&pdev->dev,"failed to power up 3D unit\n");+err=devm_pm_opp_register_set_opp_helper(&pdev->dev,gr3d_set_opp);+if(err)returnerr;-}-if(gr3d->clk_secondary){-err=tegra_powergate_sequence_power_up(TEGRA_POWERGATE_3D1,-gr3d->clk_secondary,-gr3d->rst_secondary);-if(err<0){-dev_err(&pdev->dev,-"failed to power up secondary 3D unit\n");-returnerr;-}-}+err=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+if(err)+returnerr;INIT_LIST_HEAD(&gr3d->client.base.list);gr3d->client.base.ops=&gr3d_client_ops;
@@ -352,20 +474,28 @@ static int gr3d_probe(struct platform_device *pdev)gr3d->client.version=gr3d->soc->version;gr3d->client.ops=&gr3d_ops;+pm_runtime_enable(&pdev->dev);+pm_runtime_use_autosuspend(&pdev->dev);+pm_runtime_set_autosuspend_delay(&pdev->dev,200);+err=host1x_client_register(&gr3d->client.base);if(err<0){dev_err(&pdev->dev,"failed to register host1x client: %d\n",err);-returnerr;+gotodisable_rpm;}/* initialize address register map */for(i=0;i<ARRAY_SIZE(gr3d_addr_regs);i++)set_bit(gr3d_addr_regs[i],gr3d->addr_regs);-platform_set_drvdata(pdev,gr3d);-return0;++disable_rpm:+pm_runtime_dont_use_autosuspend(&pdev->dev);+pm_runtime_disable(&pdev->dev);++returnerr;}staticintgr3d_remove(structplatform_device*pdev)
@@ -380,23 +510,178 @@ static int gr3d_remove(struct platform_device *pdev)returnerr;}-if(gr3d->clk_secondary){-reset_control_assert(gr3d->rst_secondary);-tegra_powergate_power_off(TEGRA_POWERGATE_3D1);-clk_disable_unprepare(gr3d->clk_secondary);+pm_runtime_dont_use_autosuspend(&pdev->dev);+pm_runtime_disable(&pdev->dev);++return0;+}++staticintgr3d_legacy_domain_power_up(structdevice*dev,constchar*name,+unsignedintid)+{+structgr3d*gr3d=dev_get_drvdata(dev);+structreset_control*reset;+structclk*clk;+unsignedinti;+interr;++/*+*Tegra20device-treedoesn'tspecify3dclocknameandthereisonly+*oneclockforTegra20.Tegra30+device-treesalwaysspecifiednames+*fortheclocks.+*/+if(gr3d->nclocks==1){+if(id==TEGRA_POWERGATE_3D1)+return0;++clk=gr3d->clocks[0].clk;+}else{+for(i=0;i<gr3d->nclocks;i++){+if(WARN_ON_ONCE(!gr3d->clocks[i].id))+continue;++if(!strcmp(gr3d->clocks[i].id,name)){+clk=gr3d->clocks[i].clk;+break;+}+}++if(WARN_ON_ONCE(i==gr3d->nclocks))+return-EINVAL;+}++/*+*Weusearrayofresets,whichincludesMCresets,andMC+*resetshouldn'tbeassertedwhilehardwareisgatedbecause+*MCflushingwillfailforgatedhardware.Henceforlegacy+*PDwerequesttheindividualresetseparately.+*/+reset=reset_control_get_exclusive_released(dev,name);+if(IS_ERR(reset))+returnPTR_ERR(reset);++err=reset_control_acquire(reset);+if(err){+dev_err(dev,"failed to acquire %s reset: %d\n",name,err);+}else{+err=tegra_powergate_sequence_power_up(id,clk,reset);+reset_control_release(reset);}-reset_control_assert(gr3d->rst);-tegra_powergate_power_off(TEGRA_POWERGATE_3D);-clk_disable_unprepare(gr3d->clk);+reset_control_put(reset);+if(err)+returnerr;++/*+*tegra_powergate_sequence_power_up()leavesclocksenabled+*whileGENPDnot,hencekeepclock-enablebalanced.+*/+clk_disable_unprepare(clk);++return0;+}++staticintgr3d_legacy_power_up(structdevice*dev)+{+structgr3d*gr3d=dev_get_drvdata(dev);+interr;++if(gr3d->legacy_pd){+err=gr3d_legacy_domain_power_up(dev,"3d",+TEGRA_POWERGATE_3D);+if(err)+returnerr;++err=gr3d_legacy_domain_power_up(dev,"3d2",+TEGRA_POWERGATE_3D1);+if(err)+returnerr;+}return0;}+staticint__maybe_unusedgr3d_runtime_suspend(structdevice*dev)+{+structgr3d*gr3d=dev_get_drvdata(dev);+interr;++host1x_channel_stop(gr3d->channel);++err=reset_control_bulk_assert(gr3d->nresets,gr3d->resets);+if(err){+dev_err(dev,"failed to assert reset: %d\n",err);+returnerr;+}++usleep_range(10,20);++/*+*Olderdevice-treesdon'tspecifyMCresetsandpower-gatingcan't+*bedonesafelyinthatcase.Hencewewillkeepthepowerungated+*forolderDTBs.FornewerDTBs,GENPDwillperformthepower-gating.+*/++clk_bulk_disable_unprepare(gr3d->nclocks,gr3d->clocks);+reset_control_bulk_release(gr3d->nresets,gr3d->resets);++return0;+}++staticint__maybe_unusedgr3d_runtime_resume(structdevice*dev)+{+structgr3d*gr3d=dev_get_drvdata(dev);+interr;++err=gr3d_legacy_power_up(dev);+if(err)+returnerr;++err=reset_control_bulk_acquire(gr3d->nresets,gr3d->resets);+if(err){+dev_err(dev,"failed to acquire reset: %d\n",err);+returnerr;+}++err=dev_pm_opp_sync(dev);+if(err){+dev_err(dev,"failed to sync OPP: %d\n",err);+gotorelease_reset;+}++err=clk_bulk_prepare_enable(gr3d->nclocks,gr3d->clocks);+if(err){+dev_err(dev,"failed to enable clock: %d\n",err);+gotorelease_reset;+}++err=reset_control_bulk_deassert(gr3d->nresets,gr3d->resets);+if(err){+dev_err(dev,"failed to deassert reset: %d\n",err);+gotodisable_clk;+}++return0;++disable_clk:+clk_bulk_disable_unprepare(gr3d->nclocks,gr3d->clocks);+release_reset:+reset_control_bulk_release(gr3d->nresets,gr3d->resets);++returnerr;+}++staticconststructdev_pm_opstegra_gr3d_pm={+SET_RUNTIME_PM_OPS(gr3d_runtime_suspend,gr3d_runtime_resume,NULL)+SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,+pm_runtime_force_resume)+};+structplatform_drivertegra_gr3d_driver={.driver={.name="tegra-gr3d",.of_match_table=tegra_gr3d_match,+.pm=&tegra_gr3d_pm,},.probe=gr3d_probe,.remove=gr3d_remove,
The HDMI on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now HDMI driver must use
OPP API for driving the controller's clock rate because OPP API takes
care of reconfiguring the domain's performance state based on HDMI clock
rate. Add OPP support to the HDMI driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/gpu/drm/tegra/hdmi.c | 15 ++++++++++++++-
1 file changed, 14 insertions(+), 1 deletion(-)
Add OPP and SoC core voltage scaling support to the display controller
driver. This is required for enabling system-wide DVFS on pre-Tegra186
SoCs.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/gpu/drm/tegra/dc.c | 74 ++++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/tegra/dc.h | 2 ++
2 files changed, 76 insertions(+)
@@ -1762,6 +1765,47 @@ int tegra_dc_state_setup_clock(struct tegra_dc *dc,return0;}+staticvoidtegra_dc_update_voltage_state(structtegra_dc*dc,+structtegra_dc_state*state)+{+unsignedlongrate,pstate;+structdev_pm_opp*opp;+interr;++if(!dc->has_opp_table)+return;++/* calculate actual pixel clock rate which depends on internal divider */+rate=DIV_ROUND_UP(clk_get_rate(dc->clk)*2,state->div+2);++/* find suitable OPP for the rate */+opp=dev_pm_opp_find_freq_ceil(dc->dev,&rate);++if(opp==ERR_PTR(-ERANGE))+opp=dev_pm_opp_find_freq_floor(dc->dev,&rate);++if(IS_ERR(opp)){+dev_err(dc->dev,"failed to find OPP for %luHz: %pe\n",+rate,opp);+return;+}++pstate=dev_pm_opp_get_required_pstate(opp,0);+dev_pm_opp_put(opp);++/*+*Theminimumcorevoltagedependsonthepixelclockrate(which+*dependsoninternalclockdivideroftheCRTC)andnotonthe+*rateofthedisplaycontrollerclock.Thisiswhywe'renotusing+*dev_pm_opp_set_rate()APIandinsteadcontrollingthepowerdomain+*directly.+*/+err=dev_pm_genpd_set_performance_state(dc->dev,pstate);+if(err)+dev_err(dc->dev,"failed to set power domain state to %lu: %d\n",+pstate,err);+}+staticvoidtegra_dc_commit_state(structtegra_dc*dc,structtegra_dc_state*state){
Add runtime PM and OPP support to the Host1x driver. It's required for
enabling system-wide DVFS and supporting dynamic power management using
a generic power domain. For the starter we will keep host1x always-on
because dynamic power management require a major refactoring of the driver
code since lot's of code paths will need the RPM handling and we're going
to remove some of these paths in the future. Host1x doesn't consume much
power so it is good enough, we at least need to resume Host1x in order
to initialize the power state.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/gpu/host1x/debug.c | 15 +++
drivers/gpu/host1x/dev.c | 157 ++++++++++++++++++++++++-----
drivers/gpu/host1x/dev.h | 3 +-
drivers/gpu/host1x/hw/channel_hw.c | 44 ++++----
drivers/gpu/host1x/intr.c | 3 -
drivers/gpu/host1x/syncpt.c | 5 +-
6 files changed, 171 insertions(+), 56 deletions(-)
@@ -347,6 +355,27 @@ static void host1x_iommu_exit(struct host1x *host)}}+staticinthost1x_get_resets(structhost1x*host)+{+interr;++host->resets[0].id="mc";+host->resets[1].id="host1x";+host->nresets=ARRAY_SIZE(host->resets);++err=devm_reset_control_bulk_get_optional_exclusive_released(+host->dev,host->nresets,host->resets);+if(err){+dev_err(host->dev,"failed to get reset: %d\n",err);+returnerr;+}++if(WARN_ON(!host->resets[1].rstc))+return-EINVAL;++return0;+}+staticinthost1x_probe(structplatform_device*pdev){structhost1x*host;
@@ -394,6 +423,10 @@ static int host1x_probe(struct platform_device *pdev)/* set common host1x device data */platform_set_drvdata(pdev,host);+err=devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+if(err)+returnerr;+host->regs=devm_ioremap_resource(&pdev->dev,regs);if(IS_ERR(host->regs))returnPTR_ERR(host->regs);
@@ -423,12 +456,9 @@ static int host1x_probe(struct platform_device *pdev)returnerr;}-host->rst=devm_reset_control_get(&pdev->dev,"host1x");-if(IS_ERR(host->rst)){-err=PTR_ERR(host->rst);-dev_err(&pdev->dev,"failed to get reset: %d\n",err);+err=host1x_get_resets(host);+if(err)returnerr;-}err=host1x_iommu_init(host);if(err<0){
@@ -443,22 +473,10 @@ static int host1x_probe(struct platform_device *pdev)gotoiommu_exit;}-err=clk_prepare_enable(host->clk);-if(err<0){-dev_err(&pdev->dev,"failed to enable clock\n");-gotofree_channels;-}--err=reset_control_deassert(host->rst);-if(err<0){-dev_err(&pdev->dev,"failed to deassert reset: %d\n",err);-gotounprepare_disable;-}-err=host1x_syncpt_init(host);if(err){dev_err(&pdev->dev,"failed to initialize syncpts\n");-gotoreset_assert;+gotofree_channels;}err=host1x_intr_init(host,syncpt_irq);
@@ -467,10 +485,14 @@ static int host1x_probe(struct platform_device *pdev)gotodeinit_syncpt;}-host1x_debug_init(host);+pm_runtime_enable(&pdev->dev);-if(host->info->has_hypervisor)-host1x_setup_sid_table(host);+/* the driver's code isn't ready yet for the dynamic RPM */+err=pm_runtime_resume_and_get(&pdev->dev);+if(err)+gotodeinit_intr;++host1x_debug_init(host);err=host1x_register(host);if(err<0)
@@ -486,13 +508,13 @@ static int host1x_probe(struct platform_device *pdev)host1x_unregister(host);deinit_debugfs:host1x_debug_deinit(host);++pm_runtime_put(&pdev->dev);+pm_runtime_disable(&pdev->dev);+deinit_intr:host1x_intr_deinit(host);deinit_syncpt:host1x_syncpt_deinit(host);-reset_assert:-reset_control_assert(host->rst);-unprepare_disable:-clk_disable_unprepare(host->clk);free_channels:host1x_channel_list_free(&host->channel_list);iommu_exit:
@@ -507,19 +529,100 @@ static int host1x_remove(struct platform_device *pdev)host1x_unregister(host);host1x_debug_deinit(host);++pm_runtime_put(&pdev->dev);+pm_runtime_disable(&pdev->dev);+host1x_intr_deinit(host);host1x_syncpt_deinit(host);-reset_control_assert(host->rst);-clk_disable_unprepare(host->clk);host1x_iommu_exit(host);return0;}+staticint__maybe_unusedhost1x_runtime_suspend(structdevice*dev)+{+structhost1x*host=dev_get_drvdata(dev);+interr;++host1x_intr_stop(host);+host1x_syncpt_save(host);++err=reset_control_bulk_assert(host->nresets,host->resets);+if(err){+dev_err(dev,"failed to assert reset: %d\n",err);+gotoresume_host1x;+}++usleep_range(1000,2000);++clk_disable_unprepare(host->clk);+reset_control_bulk_release(host->nresets,host->resets);++return0;++resume_host1x:+host1x_setup_sid_table(host);+host1x_syncpt_restore(host);+host1x_intr_start(host);++returnerr;+}++staticint__maybe_unusedhost1x_runtime_resume(structdevice*dev)+{+structhost1x*host=dev_get_drvdata(dev);+interr;++err=reset_control_bulk_acquire(host->nresets,host->resets);+if(err){+dev_err(dev,"failed to acquire reset: %d\n",err);+returnerr;+}++err=dev_pm_opp_sync(dev);+if(err){+dev_err(dev,"failed to sync OPP: %d\n",err);+gotorelease_reset;+}++err=clk_prepare_enable(host->clk);+if(err){+dev_err(dev,"failed to enable clock: %d\n",err);+gotorelease_reset;+}++err=reset_control_bulk_deassert(host->nresets,host->resets);+if(err<0){+dev_err(dev,"failed to deassert reset: %d\n",err);+gotodisable_clk;+}++host1x_setup_sid_table(host);+host1x_syncpt_restore(host);+host1x_intr_start(host);++return0;++disable_clk:+clk_disable_unprepare(host->clk);+release_reset:+reset_control_bulk_release(host->nresets,host->resets);++returnerr;+}++staticconststructdev_pm_opshost1x_pm={+SET_RUNTIME_PM_OPS(host1x_runtime_suspend,host1x_runtime_resume,+NULL)+/* TODO: add system suspend-resume once driver will be ready for that */+};+staticstructplatform_drivertegra_host1x_driver={.driver={.name="tegra-host1x",.of_match_table=host1x_of_match,+.pm=&host1x_pm,},.probe=host1x_probe,.remove=host1x_remove,
@@ -190,6 +211,7 @@ static int channel_submit(struct host1x_job *job)}host1x_channel_set_streamid(ch);+host1x_enable_gather_filter(ch);/* begin a CDMA submit */err=host1x_cdma_begin(&ch->cdma,job);
@@ -366,9 +368,6 @@ int host1x_syncpt_init(struct host1x *host)host->syncpt=syncpt;host->bases=bases;-host1x_syncpt_restore(host);-host1x_hw_syncpt_enable_protection(host);-/* Allocate sync point to use for clearing waits for expired fences */host->nop_sp=host1x_syncpt_alloc(host,0,"reserved-nop");if(!host->nop_sp)
Only couple drivers need to get the -ENODEV error code and explicitly
initialize the performance state. Add new helper that allows to avoid
the extra boilerplate code in majority of drivers.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
include/soc/tegra/common.h | 13 +++++++++++++
1 file changed, 13 insertions(+)
Use new generic dev_pm_opp_sync() helper which initializes voltage vote
based on clock rate.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/soc/tegra/common.c | 30 +-----------------------------
1 file changed, 1 insertion(+), 29 deletions(-)
@@ -39,34 +39,6 @@ bool soc_is_tegra(void)returnmatch!=NULL;}-staticinttegra_core_dev_init_opp_state(structdevice*dev)-{-unsignedlongrate;-structclk*clk;-interr;--clk=devm_clk_get(dev,NULL);-if(IS_ERR(clk)){-dev_err(dev,"failed to get clk: %pe\n",clk);-returnPTR_ERR(clk);-}--rate=clk_get_rate(clk);-if(!rate){-dev_err(dev,"failed to get clk rate\n");-return-EINVAL;-}--/* first dummy rate-setting initializes voltage vote */-err=dev_pm_opp_set_rate(dev,rate);-if(err){-dev_err(dev,"failed to initialize OPP clock: %d\n",err);-returnerr;-}--return0;-}-/***devm_tegra_core_dev_init_opp_table()-initializeOPPtable*@dev:deviceforwhichOPPtableisinitialized
@@ -118,7 +90,7 @@ int devm_tegra_core_dev_init_opp_table(struct device *dev,}if(params->init_state){-err=tegra_core_dev_init_opp_state(dev);+err=dev_pm_opp_sync(dev);if(err)returnerr;}
@@ -1003,7 +1004,7 @@ static int _set_opp(struct device *dev, struct opp_table *opp_table,/* Find the currently set OPP if we don't know already */if(unlikely(!opp_table->current_opp))-_find_current_opp(dev,opp_table);+opp_table->current_opp=_find_current_opp(dev,opp_table);old_opp=opp_table->current_opp;
@@ -2931,3 +2932,38 @@ int dev_pm_opp_sync_regulators(struct device *dev)returnret;}EXPORT_SYMBOL_GPL(dev_pm_opp_sync_regulators);++/**+*dev_pm_opp_sync()-SyncOPPstate+*@dev:deviceforwhichwedothisoperation+*+*InitializeOPPtableaccordinglytocurrentclockrateor+*firstavailableOPPifclocknotavailableforthisdevice.+*+*Return:0onsuccessoranegativeerrorvalue.+*/+intdev_pm_opp_sync(structdevice*dev)+{+structopp_table*opp_table;+structdev_pm_opp*opp;+intret=0;++/* Device may not have OPP table */+opp_table=_find_opp_table(dev);+if(IS_ERR(opp_table))+return0;++if(!_get_opp_count(opp_table))+gotoput_table;++opp=_find_current_opp(dev,opp_table);+ret=_set_opp(dev,opp_table,opp,opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
what exactly are you trying to do here ? Those details would be good
to have in commit log as well, I haven't really followed V7 of your
series.
--
viresh
Hi Dmitry,
Dmitry Osipenko [off-list ref] wrote on Tue, 17 Aug 2021 04:27:41
+0300:
The NAND on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now NAND must be resumed using
runtime PM API in order to initialize the NAND power state. Add runtime PM
and OPP support to the NAND driver.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
drivers/mtd/nand/raw/tegra_nand.c | 62 +++++++++++++++++++++++++++----
1 file changed, 54 insertions(+), 8 deletions(-)
On Tue, 17 Aug 2021 at 03:30, Dmitry Osipenko [off-list ref] wrote:
Add runtime PM and OPP support to the Host1x driver. It's required for
enabling system-wide DVFS and supporting dynamic power management using
a generic power domain. For the starter we will keep host1x always-on
because dynamic power management require a major refactoring of the driver
code since lot's of code paths will need the RPM handling and we're going
to remove some of these paths in the future. Host1x doesn't consume much
power so it is good enough, we at least need to resume Host1x in order
to initialize the power state.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
[...]
quoted hunk
+ static int host1x_probe(struct platform_device *pdev) { struct host1x *host;
@@ -394,6 +423,10 @@ static int host1x_probe(struct platform_device *pdev) /* set common host1x device data */ platform_set_drvdata(pdev, host);+ err = devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+ if (err)+ return err;+ host->regs = devm_ioremap_resource(&pdev->dev, regs); if (IS_ERR(host->regs)) return PTR_ERR(host->regs);
@@ -423,12 +456,9 @@ static int host1x_probe(struct platform_device *pdev) return err; }- host->rst = devm_reset_control_get(&pdev->dev, "host1x");- if (IS_ERR(host->rst)) {- err = PTR_ERR(host->rst);- dev_err(&pdev->dev, "failed to get reset: %d\n", err);+ err = host1x_get_resets(host);+ if (err) return err;- } err = host1x_iommu_init(host); if (err < 0) {
@@ -443,22 +473,10 @@ static int host1x_probe(struct platform_device *pdev) goto iommu_exit; }- err = clk_prepare_enable(host->clk);- if (err < 0) {- dev_err(&pdev->dev, "failed to enable clock\n");- goto free_channels;- }-- err = reset_control_deassert(host->rst);- if (err < 0) {- dev_err(&pdev->dev, "failed to deassert reset: %d\n", err);- goto unprepare_disable;- }-
Removing the clk_prepare_enable() and reset_control_deassert() from
host1x_probe(), might not be a good idea. See more about why, below.
quoted hunk
err = host1x_syncpt_init(host); if (err) { dev_err(&pdev->dev, "failed to initialize syncpts\n");- goto reset_assert;+ goto free_channels; } err = host1x_intr_init(host, syncpt_irq);
@@ -467,10 +485,14 @@ static int host1x_probe(struct platform_device *pdev) goto deinit_syncpt; }- host1x_debug_init(host);+ pm_runtime_enable(&pdev->dev);- if (host->info->has_hypervisor)- host1x_setup_sid_table(host);+ /* the driver's code isn't ready yet for the dynamic RPM */+ err = pm_runtime_resume_and_get(&pdev->dev);
If the driver is being built with the CONFIG_PM Kconfig option being
unset, pm_runtime_resume_and_get() will return 0 to indicate success -
and without calling the ->runtime_resume() callback.
In other words, the clock will remain gated and the reset will not be
deasserted, likely causing the driver to be malfunctioning.
If the driver isn't ever being built with CONFIG_PM unset, feel free
to ignore my above comments.
Otherwise, if it needs to work both with and without CONFIG_PM being
set, you may use the following pattern in host1x_probe() to deploy
runtime PM support:
"Enable the needed resources to probe the device"
pm_runtime_get_noresume()
pm_runtime_set_active()
pm_runtime_enable()
"Before successfully completing probe"
pm_runtime_put()
quoted hunk
+ if (err)+ goto deinit_intr;++ host1x_debug_init(host); err = host1x_register(host); if (err < 0)
From: Mark Brown <broonie@kernel.org> Date: 2021-08-17 12:23:12
On Tue, Aug 17, 2021 at 04:27:42AM +0300, Dmitry Osipenko wrote:
The SPI on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now SPI driver must use OPP
API for driving the controller's clock rate because OPP API takes care
of reconfiguring the domain's performance state in accordance to the
rate. Add OPP support to the driver.
Acked-by: Mark Brown <broonie@kernel.org>
Is there a concrete dependency here or can I merge this separately?
On Tue, Aug 17, 2021 at 02:04:38PM +0200, Ulf Hansson wrote:
On Tue, 17 Aug 2021 at 03:30, Dmitry Osipenko [off-list ref] wrote:
quoted
Add runtime PM and OPP support to the Host1x driver. It's required for
enabling system-wide DVFS and supporting dynamic power management using
a generic power domain. For the starter we will keep host1x always-on
because dynamic power management require a major refactoring of the driver
code since lot's of code paths will need the RPM handling and we're going
to remove some of these paths in the future. Host1x doesn't consume much
power so it is good enough, we at least need to resume Host1x in order
to initialize the power state.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
[...]
quoted
+ static int host1x_probe(struct platform_device *pdev) { struct host1x *host;
@@ -394,6 +423,10 @@ static int host1x_probe(struct platform_device *pdev) /* set common host1x device data */ platform_set_drvdata(pdev, host);+ err = devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+ if (err)+ return err;+ host->regs = devm_ioremap_resource(&pdev->dev, regs); if (IS_ERR(host->regs)) return PTR_ERR(host->regs);
@@ -423,12 +456,9 @@ static int host1x_probe(struct platform_device *pdev) return err; }- host->rst = devm_reset_control_get(&pdev->dev, "host1x");- if (IS_ERR(host->rst)) {- err = PTR_ERR(host->rst);- dev_err(&pdev->dev, "failed to get reset: %d\n", err);+ err = host1x_get_resets(host);+ if (err) return err;- } err = host1x_iommu_init(host); if (err < 0) {
@@ -443,22 +473,10 @@ static int host1x_probe(struct platform_device *pdev) goto iommu_exit; }- err = clk_prepare_enable(host->clk);- if (err < 0) {- dev_err(&pdev->dev, "failed to enable clock\n");- goto free_channels;- }-- err = reset_control_deassert(host->rst);- if (err < 0) {- dev_err(&pdev->dev, "failed to deassert reset: %d\n", err);- goto unprepare_disable;- }-
Removing the clk_prepare_enable() and reset_control_deassert() from
host1x_probe(), might not be a good idea. See more about why, below.
quoted
err = host1x_syncpt_init(host); if (err) { dev_err(&pdev->dev, "failed to initialize syncpts\n");- goto reset_assert;+ goto free_channels; } err = host1x_intr_init(host, syncpt_irq);
@@ -467,10 +485,14 @@ static int host1x_probe(struct platform_device *pdev) goto deinit_syncpt; }- host1x_debug_init(host);+ pm_runtime_enable(&pdev->dev);- if (host->info->has_hypervisor)- host1x_setup_sid_table(host);+ /* the driver's code isn't ready yet for the dynamic RPM */+ err = pm_runtime_resume_and_get(&pdev->dev);
If the driver is being built with the CONFIG_PM Kconfig option being
unset, pm_runtime_resume_and_get() will return 0 to indicate success -
and without calling the ->runtime_resume() callback.
In other words, the clock will remain gated and the reset will not be
deasserted, likely causing the driver to be malfunctioning.
If the driver isn't ever being built with CONFIG_PM unset, feel free
to ignore my above comments.
Otherwise, if it needs to work both with and without CONFIG_PM being
set, you may use the following pattern in host1x_probe() to deploy
runtime PM support:
"Enable the needed resources to probe the device"
pm_runtime_get_noresume()
pm_runtime_set_active()
pm_runtime_enable()
"Before successfully completing probe"
pm_runtime_put()
We made a conscious decision a few years ago to have ARCH_TEGRA select
PM on both 32-bit and 64-bit ARM, specifically to avoid the need to do
this dance (though there are still a few drivers left that do this, I
think).
So I think this should be unnecessary. Unless perhaps if the sysfs PM
controls have any influence on this. As far as I know, as long as the
PM kconfig option is enabled, the sysfs control only influence the
runtime behaviour (i.e. setting the sysfs PM control to "on" is going
to force runtime PM to be resumed) but there's no way to disable
runtime PM altogether via sysfs that would make the above necessary.
Thierry
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
what exactly are you trying to do here ? Those details would be good
to have in commit log as well, I haven't really followed V7 of your
series.
I'm initializing voltage/power state of OPP table in accordance to the
clock rate, bumping voltage before clock is enabled by device driver.
I'll improve the commit message.
An alternative to the explicit syncing could be something like a new
dev_pm_opp_resume/suspend helpers that will take care of
enabling/disabling the OPP table clock/etc and syncing the OPP state
with h/w.
On Tue, Aug 17, 2021 at 04:27:42AM +0300, Dmitry Osipenko wrote:
quoted
The SPI on Tegra belongs to the core power domain and we're going to
enable GENPD support for the core domain. Now SPI driver must use OPP
API for driving the controller's clock rate because OPP API takes care
of reconfiguring the domain's performance state in accordance to the
rate. Add OPP support to the driver.
Acked-by: Mark Brown <broonie@kernel.org>
Is there a concrete dependency here or can I merge this separately?
This patch depends on the new OPP helpers added earlier in this series.
In particular it depends on these patches:
opp: Add dev_pm_opp_sync() helper
soc/tegra: Add devm_tegra_core_dev_init_opp_table_simple()
Thank you for the ack!
From: Rob Herring <robh@kernel.org> Date: 2021-08-18 01:15:16
On Tue, Aug 17, 2021 at 04:27:26AM +0300, Dmitry Osipenko wrote:
quoted hunk
Document tegra-clocks sub-node which describes Tegra SoC clocks that
require a higher voltage of the core power domain in order to operate
properly on a higher clock rates. Each node contains a phandle to OPP
table and power domain.
The root PLLs and system clocks don't have any specific device dedicated
to them, clock controller is in charge of managing power for them.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/clock/nvidia,tegra20-car.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
@@ -42,6 +42,48 @@ properties:"#reset-cells":const:1+tegra-clocks:+description:child nodes are the output clocks from the CAR+type:object++patternProperties:+"^[a-z]+[0-9]+$":+type:object+properties:+compatible:+allOf:+-items:+-enum:+-nvidia,tegra20-sclk+-nvidia,tegra30-sclk+-nvidia,tegra30-pllc+-nvidia,tegra30-plle+-nvidia,tegra30-pllm+-const:nvidia,tegra-clock
You are saying the first string must be both one of the enums and
'nvidia,tegra-clock'. You don't get an error because your pattern
doesn't match 'sclk'.
quoted hunk
++ operating-points-v2:+ $ref: /schemas/types.yaml#/definitions/phandle+ description:+ Phandle to OPP table that contains frequencies, voltages and+ opp-supported-hw property, which is a bitfield indicating+ SoC process or speedo ID mask.
From: Rob Herring <robh@kernel.org> Date: 2021-08-18 01:16:28
On Tue, Aug 17, 2021 at 04:27:29AM +0300, Dmitry Osipenko wrote:
quoted hunk
Memory Client should be blocked before hardware reset is asserted in order
to prevent memory corruption and hanging of memory controller.
Document Memory Client resets of Host1x, GR2D and GR3D hardware units.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/display/tegra/nvidia,tegra20-host1x.txt | 4 ++++
1 file changed, 4 insertions(+)
@@ -18,6 +18,7 @@ Required properties: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - host1x
New entries should be at the end. Order matters.
quoted hunk
Optional properties:
@@ -197,6 +198,7 @@ of the following host1x client modules: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - 2d Optional properties:
@@ -222,6 +224,8 @@ of the following host1x client modules: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc+ - mc2 (Only required on SoCs with two 3D clocks) - 3d - 3d2 (Only required on SoCs with two 3D clocks)
From: Rob Herring <robh@kernel.org> Date: 2021-08-18 01:17:42
On Tue, 17 Aug 2021 04:27:44 +0300, Dmitry Osipenko wrote:
Document new OPP table and power domain properties of the video decoder
hardware.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../devicetree/bindings/media/nvidia,tegra-vde.yaml | 12 ++++++++++++
1 file changed, 12 insertions(+)
On Tue, Aug 17, 2021 at 04:27:29AM +0300, Dmitry Osipenko wrote:
quoted
Memory Client should be blocked before hardware reset is asserted in order
to prevent memory corruption and hanging of memory controller.
Document Memory Client resets of Host1x, GR2D and GR3D hardware units.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/display/tegra/nvidia,tegra20-host1x.txt | 4 ++++
1 file changed, 4 insertions(+)
@@ -18,6 +18,7 @@ Required properties: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - host1x
New entries should be at the end. Order matters.
Indeed, order matters. In this case it matters by the hardware because
memory reset must be asserted before the controller's reset. We rely on
it in the code of the GENPD driver. Hence it's the intended order in
this patch.
+ tegra-clocks:+ description: child nodes are the output clocks from the CAR+ type: object++ patternProperties:+ "^[a-z]+[0-9]+$":+ type: object+ properties:+ compatible:+ allOf:+ - items:+ - enum:+ - nvidia,tegra20-sclk+ - nvidia,tegra30-sclk+ - nvidia,tegra30-pllc+ - nvidia,tegra30-plle+ - nvidia,tegra30-pllm+ - const: nvidia,tegra-clock
You are saying the first string must be both one of the enums and
'nvidia,tegra-clock'. You don't get an error because your pattern
doesn't match 'sclk'.
Could you please rephrase or clarify? If pattern doesn't match 'sclk',
then it must match any other enum. I'm not sure what you're meaning.
The 'nvidia,tegra-clock' actually could be removed since it's
superfluous now. I'll consider the removal in v9.
On Tue, Aug 17, 2021 at 04:27:29AM +0300, Dmitry Osipenko wrote:
quoted
Memory Client should be blocked before hardware reset is asserted in order
to prevent memory corruption and hanging of memory controller.
Document Memory Client resets of Host1x, GR2D and GR3D hardware units.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/display/tegra/nvidia,tegra20-host1x.txt | 4 ++++
1 file changed, 4 insertions(+)
@@ -18,6 +18,7 @@ Required properties: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - host1x
New entries should be at the end. Order matters.
Indeed, order matters. In this case it matters by the hardware because
memory reset must be asserted before the controller's reset. We rely on
it in the code of the GENPD driver. Hence it's the intended order in
this patch.
Although, my bad. It should be to reorder items here, it's not a GENPD
binding.
On Tue, Aug 17, 2021 at 04:27:29AM +0300, Dmitry Osipenko wrote:
quoted
Memory Client should be blocked before hardware reset is asserted in order
to prevent memory corruption and hanging of memory controller.
Document Memory Client resets of Host1x, GR2D and GR3D hardware units.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/display/tegra/nvidia,tegra20-host1x.txt | 4 ++++
1 file changed, 4 insertions(+)
@@ -18,6 +18,7 @@ Required properties: - resets: Must contain an entry for each entry in reset-names. See ../reset/reset.txt for details. - reset-names: Must include the following entries:+ - mc - host1x
New entries should be at the end. Order matters.
Indeed, order matters. In this case it matters by the hardware because
memory reset must be asserted before the controller's reset. We rely on
it in the code of the GENPD driver. Hence it's the intended order in
this patch.
Although, my bad. It should be to reorder items here, it's not a GENPD
binding.
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
Right, but how will this work from Resume ? Won't that be a no-op ?
--
viresh
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
Right, but how will this work from Resume ? Won't that be a no-op ?
The first resume initializes the OPP state on sync, all further syncs on
resume are no-ops.
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
Right, but how will this work from Resume ? Won't that be a no-op ?
The first resume initializes the OPP state on sync, all further syncs on
resume are no-ops.
Notice that we use GENPD here. GENPD core takes care of storing PD's
performance state (voltage in case of Tegra) and dropping it to 0 after
rpm-suspend, GENPD core also restores the state before rpm-resume.
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
Right, but how will this work from Resume ? Won't that be a no-op ?
The first resume initializes the OPP state on sync, all further syncs on
resume are no-ops.
Notice that we use GENPD here. GENPD core takes care of storing PD's
performance state (voltage in case of Tegra) and dropping it to 0 after
rpm-suspend, GENPD core also restores the state before rpm-resume.
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
Right, but how will this work from Resume ? Won't that be a no-op ?
The first resume initializes the OPP state on sync, all further syncs on
resume are no-ops.
But the OPPs should already be initialized as someone must have called
opp-set-rate earlier ? Why do this from resume and not probe ?
--
viresh
The first resume initializes the OPP state on sync, all further syncs on
resume are no-ops.
Notice that we use GENPD here. GENPD core takes care of storing PD's
performance state (voltage in case of Tegra) and dropping it to 0 after
rpm-suspend, GENPD core also restores the state before rpm-resume.
By 'here' I mean in this series.
It is still not clear to me why you need to this on resume, and not
probe.
--
viresh
+int dev_pm_opp_sync(struct device *dev)+{+ struct opp_table *opp_table;+ struct dev_pm_opp *opp;+ int ret = 0;++ /* Device may not have OPP table */+ opp_table = _find_opp_table(dev);+ if (IS_ERR(opp_table))+ return 0;++ if (!_get_opp_count(opp_table))+ goto put_table;++ opp = _find_current_opp(dev, opp_table);+ ret = _set_opp(dev, opp_table, opp, opp->rate);
And I am not sure how this will end up working, since new OPP will be
equal to old one. Since I see you call this from resume() at many
places.
Initially OPP table is "uninitialized" and opp_table->enabled=false,
hence the first sync always works even if OPP is equal to old one. Once
OPP has been synced, all further syncs are NO-OPs, hence it doesn't
matter how many times syncing is called.
https://elixir.bootlin.com/linux/v5.14-rc6/source/drivers/opp/core.c#L1012
Right, but how will this work from Resume ? Won't that be a no-op ?
The first resume initializes the OPP state on sync, all further syncs on
resume are no-ops.
But the OPPs should already be initialized as someone must have called
opp-set-rate earlier ? Why do this from resume and not probe ?
This will set voltage level without having an actively used hardware.
Take a 3d driver for example, if you set the rate on probe and
rpm-resume will never be called, then the voltage will be set high,
while hardware is kept suspended if userspace will never wake it up by
executing a 3d job.
This will set voltage level without having an actively used hardware.
Take a 3d driver for example, if you set the rate on probe and
rpm-resume will never be called, then the voltage will be set high,
while hardware is kept suspended if userspace will never wake it up by
executing a 3d job.
What exactly are we looking to achieve with this stuff ? Cache the
current performance state with genpd (based on the state bootloader's
has set) ?
Or anything else as well ?
--
viresh
This will set voltage level without having an actively used hardware.
Take a 3d driver for example, if you set the rate on probe and
rpm-resume will never be called, then the voltage will be set high,
while hardware is kept suspended if userspace will never wake it up by
executing a 3d job.
What exactly are we looking to achieve with this stuff ? Cache the
current performance state with genpd (based on the state bootloader's
has set) ?
Yes, GENPD will cache the perf state across suspend/resume and initially
cached value is out of sync with h/w.
Or anything else as well ?
Nothing else. But let me clarify it all again.
Initially the performance state of all GENPDs is 0 for all devices.
The clock rate is preinitialized for all devices to a some default rate
by clk driver, or by bootloader or by assigned-clocks in DT.
When device is rpm-resumed, the resume callback of a device driver
enables the clock.
Before clock is enabled, the voltage needs to be configured in
accordance to the clk rate.
So now we have a GENPD with pstate=0 on a first rpm-resume, which
doesn't match the h/w configuration. Calling dev_pm_opp_sync() sets the
pstate in accordance to the h/w config.
In a previous v7 I proposed to preset the rpm_pstate of GENPD (perf
level that is restored before device is rpm-resumed) from PD's
attach_dev callback, but Ulf didn't like that because it requires to use
and modify GENPD 'private' variables from a PD driver. We decided that
will be better to make device drivers to explicitly sync the perf state,
which I implemented in this v8.
Yes, GENPD will cache the perf state across suspend/resume and initially
cached value is out of sync with h/w.
Nothing else. But let me clarify it all again.
Thanks for your explanation.
Initially the performance state of all GENPDs is 0 for all devices.
The clock rate is preinitialized for all devices to a some default rate
by clk driver, or by bootloader or by assigned-clocks in DT.
When device is rpm-resumed, the resume callback of a device driver
enables the clock.
Before clock is enabled, the voltage needs to be configured in
accordance to the clk rate.
So now we have a GENPD with pstate=0 on a first rpm-resume, which
doesn't match the h/w configuration. Calling dev_pm_opp_sync() sets the
pstate in accordance to the h/w config.
What about calling dev_pm_opp_set_rate(dev, clk_get_rate(dev)) here
instead ? That will work, right ? The advantage is it works without
any special routine to do so.
I also wonder looking at your gr3d.c changes, you set a set-opp
helper, but the driver doesn't call set_opp_rate at all. Who calls it
?
And if it is all about just syncing the genpd core, then can the genpd
core do something like what clk framework does? i.e. allow a new
optional genpd callback, get_performance_state() (just like
set_performance_state()), which can be called initially by the core to
get the performance to something other than zero. opp-set-rate is
there to set the performance state and enable the stuff as well.
That's why it looks incorrect in your case, where the function was
only required to be called once, and you are ending up calling it on
each resume. Limiting that with another local variable is bad as well.
In a previous v7 I proposed to preset the rpm_pstate of GENPD (perf
level that is restored before device is rpm-resumed) from PD's
attach_dev callback, but Ulf didn't like that because it requires to use
and modify GENPD 'private' variables from a PD driver. We decided that
will be better to make device drivers to explicitly sync the perf state,
which I implemented in this v8.
Yes, GENPD will cache the perf state across suspend/resume and initially
cached value is out of sync with h/w.
Nothing else. But let me clarify it all again.
Thanks for your explanation.
quoted
Initially the performance state of all GENPDs is 0 for all devices.
The clock rate is preinitialized for all devices to a some default rate
by clk driver, or by bootloader or by assigned-clocks in DT.
When device is rpm-resumed, the resume callback of a device driver
enables the clock.
Before clock is enabled, the voltage needs to be configured in
accordance to the clk rate.
So now we have a GENPD with pstate=0 on a first rpm-resume, which
doesn't match the h/w configuration. Calling dev_pm_opp_sync() sets the
pstate in accordance to the h/w config.
What about calling dev_pm_opp_set_rate(dev, clk_get_rate(dev)) here
instead ? That will work, right ? The advantage is it works without
any special routine to do so.
I also wonder looking at your gr3d.c changes, you set a set-opp
helper, but the driver doesn't call set_opp_rate at all. Who calls it
?
And if it is all about just syncing the genpd core, then can the genpd
core do something like what clk framework does? i.e. allow a new
optional genpd callback, get_performance_state() (just like
set_performance_state()), which can be called initially by the core to
get the performance to something other than zero. opp-set-rate is
there to set the performance state and enable the stuff as well.
That's why it looks incorrect in your case, where the function was
only required to be called once, and you are ending up calling it on
each resume. Limiting that with another local variable is bad as well.
Yes, GENPD will cache the perf state across suspend/resume and initially
cached value is out of sync with h/w.
Nothing else. But let me clarify it all again.
Thanks for your explanation.
quoted
Initially the performance state of all GENPDs is 0 for all devices.
The clock rate is preinitialized for all devices to a some default rate
by clk driver, or by bootloader or by assigned-clocks in DT.
When device is rpm-resumed, the resume callback of a device driver
enables the clock.
Before clock is enabled, the voltage needs to be configured in
accordance to the clk rate.
So now we have a GENPD with pstate=0 on a first rpm-resume, which
doesn't match the h/w configuration. Calling dev_pm_opp_sync() sets the
pstate in accordance to the h/w config.
What about calling dev_pm_opp_set_rate(dev, clk_get_rate(dev)) here
instead ? That will work, right ? The advantage is it works without
any special routine to do so.
It will work, but a dedicated helper is nicer.
I also wonder looking at your gr3d.c changes, you set a set-opp
helper, but the driver doesn't call set_opp_rate at all. Who calls it
?
dev_pm_opp_sync() calls it from _set_opp().
And if it is all about just syncing the genpd core, then can the genpd
core do something like what clk framework does? i.e. allow a new
optional genpd callback, get_performance_state() (just like
set_performance_state()), which can be called initially by the core to
get the performance to something other than zero. opp-set-rate is
there to set the performance state and enable the stuff as well.
That's why it looks incorrect in your case, where the function was
only required to be called once, and you are ending up calling it on
each resume. Limiting that with another local variable is bad as well.
We discussed variant with get_performance_state() previously and Ulf
didn't like it either since it still requires to touch 'internals' of GENPD.
What about calling dev_pm_opp_set_rate(dev, clk_get_rate(dev)) here
instead ? That will work, right ? The advantage is it works without
any special routine to do so.
It will work, but a dedicated helper is nicer.
quoted
I also wonder looking at your gr3d.c changes, you set a set-opp
helper, but the driver doesn't call set_opp_rate at all. Who calls it
?
dev_pm_opp_sync() calls it from _set_opp().
Okay, please use dev_pm_opp_set_rate() instead then. New helper just
adds to the confusion and isn't doing anything special apart from
doing clk_get_rate() for you.
quoted
And if it is all about just syncing the genpd core, then can the genpd
core do something like what clk framework does? i.e. allow a new
optional genpd callback, get_performance_state() (just like
set_performance_state()), which can be called initially by the core to
get the performance to something other than zero. opp-set-rate is
there to set the performance state and enable the stuff as well.
That's why it looks incorrect in your case, where the function was
only required to be called once, and you are ending up calling it on
each resume. Limiting that with another local variable is bad as well.
We discussed variant with get_performance_state() previously and Ulf
didn't like it either since it still requires to touch 'internals' of GENPD.
Hmm, I wonder if that would be a problem since only genpd core is
going to call that routine anyway.
--
viresh
On Wed, 18 Aug 2021 at 08:27, Viresh Kumar [off-list ref] wrote:
On 18-08-21, 09:22, Dmitry Osipenko wrote:
quoted
18.08.2021 08:58, Viresh Kumar пишет:
quoted
What about calling dev_pm_opp_set_rate(dev, clk_get_rate(dev)) here
instead ? That will work, right ? The advantage is it works without
any special routine to do so.
It will work, but a dedicated helper is nicer.
quoted
I also wonder looking at your gr3d.c changes, you set a set-opp
helper, but the driver doesn't call set_opp_rate at all. Who calls it
?
dev_pm_opp_sync() calls it from _set_opp().
Okay, please use dev_pm_opp_set_rate() instead then. New helper just
adds to the confusion and isn't doing anything special apart from
doing clk_get_rate() for you.
quoted
quoted
And if it is all about just syncing the genpd core, then can the genpd
core do something like what clk framework does? i.e. allow a new
optional genpd callback, get_performance_state() (just like
set_performance_state()), which can be called initially by the core to
get the performance to something other than zero. opp-set-rate is
there to set the performance state and enable the stuff as well.
That's why it looks incorrect in your case, where the function was
only required to be called once, and you are ending up calling it on
each resume. Limiting that with another local variable is bad as well.
We discussed variant with get_performance_state() previously and Ulf
didn't like it either since it still requires to touch 'internals' of GENPD.
Hmm, I wonder if that would be a problem since only genpd core is
going to call that routine anyway.
Me and Dmitry discussed adding a new genpd callback for this. I agreed
that it seems like a reasonable thing to add, if he insists.
The intent was to invoke the new callback from __genpd_dev_pm_attach()
when the device has been attached to its genpd. This allows the
callback, to invoke clk_get_rate() and then dev_pm_opp_set_rate(), to
update the vote according to the current state of the HW.
I am not sure if/why that approach seemed insufficient?
Another option to solve the problem, I think, is simply to patch
drivers to let them call dev_pm_opp_set_rate() during ->probe(), this
should synchronize the HW state too.
Dmitry, can you please elaborate on this?
Kind regards
Uffe
On Tue, 17 Aug 2021 at 16:03, Thierry Reding [off-list ref] wrote:
On Tue, Aug 17, 2021 at 02:04:38PM +0200, Ulf Hansson wrote:
quoted
On Tue, 17 Aug 2021 at 03:30, Dmitry Osipenko [off-list ref] wrote:
quoted
Add runtime PM and OPP support to the Host1x driver. It's required for
enabling system-wide DVFS and supporting dynamic power management using
a generic power domain. For the starter we will keep host1x always-on
because dynamic power management require a major refactoring of the driver
code since lot's of code paths will need the RPM handling and we're going
to remove some of these paths in the future. Host1x doesn't consume much
power so it is good enough, we at least need to resume Host1x in order
to initialize the power state.
Tested-by: Peter Geis <redacted> # Ouya T30
Tested-by: Paul Fertser <fercerpav@gmail.com> # PAZ00 T20
Tested-by: Nicolas Chauvet <redacted> # PAZ00 T20 and TK1 T124
Tested-by: Matt Merhar <redacted> # Ouya T30
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
[...]
quoted
+ static int host1x_probe(struct platform_device *pdev) { struct host1x *host;
@@ -394,6 +423,10 @@ static int host1x_probe(struct platform_device *pdev) /* set common host1x device data */ platform_set_drvdata(pdev, host);+ err = devm_tegra_core_dev_init_opp_table_simple(&pdev->dev);+ if (err)+ return err;+ host->regs = devm_ioremap_resource(&pdev->dev, regs); if (IS_ERR(host->regs)) return PTR_ERR(host->regs);
@@ -423,12 +456,9 @@ static int host1x_probe(struct platform_device *pdev) return err; }- host->rst = devm_reset_control_get(&pdev->dev, "host1x");- if (IS_ERR(host->rst)) {- err = PTR_ERR(host->rst);- dev_err(&pdev->dev, "failed to get reset: %d\n", err);+ err = host1x_get_resets(host);+ if (err) return err;- } err = host1x_iommu_init(host); if (err < 0) {
@@ -443,22 +473,10 @@ static int host1x_probe(struct platform_device *pdev) goto iommu_exit; }- err = clk_prepare_enable(host->clk);- if (err < 0) {- dev_err(&pdev->dev, "failed to enable clock\n");- goto free_channels;- }-- err = reset_control_deassert(host->rst);- if (err < 0) {- dev_err(&pdev->dev, "failed to deassert reset: %d\n", err);- goto unprepare_disable;- }-
Removing the clk_prepare_enable() and reset_control_deassert() from
host1x_probe(), might not be a good idea. See more about why, below.
quoted
err = host1x_syncpt_init(host); if (err) { dev_err(&pdev->dev, "failed to initialize syncpts\n");- goto reset_assert;+ goto free_channels; } err = host1x_intr_init(host, syncpt_irq);
@@ -467,10 +485,14 @@ static int host1x_probe(struct platform_device *pdev) goto deinit_syncpt; }- host1x_debug_init(host);+ pm_runtime_enable(&pdev->dev);- if (host->info->has_hypervisor)- host1x_setup_sid_table(host);+ /* the driver's code isn't ready yet for the dynamic RPM */+ err = pm_runtime_resume_and_get(&pdev->dev);
If the driver is being built with the CONFIG_PM Kconfig option being
unset, pm_runtime_resume_and_get() will return 0 to indicate success -
and without calling the ->runtime_resume() callback.
In other words, the clock will remain gated and the reset will not be
deasserted, likely causing the driver to be malfunctioning.
If the driver isn't ever being built with CONFIG_PM unset, feel free
to ignore my above comments.
Otherwise, if it needs to work both with and without CONFIG_PM being
set, you may use the following pattern in host1x_probe() to deploy
runtime PM support:
"Enable the needed resources to probe the device"
pm_runtime_get_noresume()
pm_runtime_set_active()
pm_runtime_enable()
"Before successfully completing probe"
pm_runtime_put()
We made a conscious decision a few years ago to have ARCH_TEGRA select
PM on both 32-bit and 64-bit ARM, specifically to avoid the need to do
this dance (though there are still a few drivers left that do this, I
think).
So I think this should be unnecessary. Unless perhaps if the sysfs PM
controls have any influence on this. As far as I know, as long as the
PM kconfig option is enabled, the sysfs control only influence the
runtime behaviour (i.e. setting the sysfs PM control to "on" is going
to force runtime PM to be resumed) but there's no way to disable
runtime PM altogether via sysfs that would make the above necessary.
Thanks for clarifying! As I said, feel free to ignore my comments then.
For this and the other patches in the series, I assume you only need
to care about whether the driver is a cross SoC driver and used on
other platforms than Tegra then.
Me and Dmitry discussed adding a new genpd callback for this. I agreed
that it seems like a reasonable thing to add, if he insists.
The intent was to invoke the new callback from __genpd_dev_pm_attach()
when the device has been attached to its genpd. This allows the
callback, to invoke clk_get_rate() and then dev_pm_opp_set_rate(), to
update the vote according to the current state of the HW.
I wouldn't call dev_pm_opp_set_rate() from there, since it means
configure and enable (both) for different resources, clk, regulator,
genpd, etc..
What we need here is just configure. So something like this then:
- genpd->get_performance_state()
-> dev_pm_opp_get_current_opp() //New API
-> dev_pm_genpd_set_performance_state(dev, current_opp->pstate);
This can be done just once from probe() then.
I am not sure if/why that approach seemed insufficient?
Another option to solve the problem, I think, is simply to patch
drivers to let them call dev_pm_opp_set_rate() during ->probe(), this
should synchronize the HW state too.
Dmitry already mentioned that this will make the device start
consuming power, and he doesn't want that, else we need an explicit
disble call as well.
--
viresh
On Wed, 18 Aug 2021 at 11:14, Viresh Kumar [off-list ref] wrote:
On 18-08-21, 10:29, Ulf Hansson wrote:
quoted
Me and Dmitry discussed adding a new genpd callback for this. I agreed
that it seems like a reasonable thing to add, if he insists.
The intent was to invoke the new callback from __genpd_dev_pm_attach()
when the device has been attached to its genpd. This allows the
callback, to invoke clk_get_rate() and then dev_pm_opp_set_rate(), to
update the vote according to the current state of the HW.
I wouldn't call dev_pm_opp_set_rate() from there, since it means
configure and enable (both) for different resources, clk, regulator,
genpd, etc..
Right, good point!
dev_pm_opp_set_rate() is best called from consumer drivers, as they
need to be in control.
What we need here is just configure. So something like this then:
- genpd->get_performance_state()
-> dev_pm_opp_get_current_opp() //New API
-> dev_pm_genpd_set_performance_state(dev, current_opp->pstate);
This can be done just once from probe() then.
How would dev_pm_opp_get_current_opp() work? Do you have a suggestion?
quoted
I am not sure if/why that approach seemed insufficient?
Another option to solve the problem, I think, is simply to patch
drivers to let them call dev_pm_opp_set_rate() during ->probe(), this
should synchronize the HW state too.
Dmitry already mentioned that this will make the device start
consuming power, and he doesn't want that, else we need an explicit
disble call as well.
I am sure I understand the problem. When a device is getting probed,
it needs to consume power, how else can the corresponding driver
successfully probe it?
On Wed, 18 Aug 2021 at 11:41, Ulf Hansson [off-list ref] wrote:
On Wed, 18 Aug 2021 at 11:14, Viresh Kumar [off-list ref] wrote:
quoted
On 18-08-21, 10:29, Ulf Hansson wrote:
quoted
Me and Dmitry discussed adding a new genpd callback for this. I agreed
that it seems like a reasonable thing to add, if he insists.
The intent was to invoke the new callback from __genpd_dev_pm_attach()
when the device has been attached to its genpd. This allows the
callback, to invoke clk_get_rate() and then dev_pm_opp_set_rate(), to
update the vote according to the current state of the HW.
I wouldn't call dev_pm_opp_set_rate() from there, since it means
configure and enable (both) for different resources, clk, regulator,
genpd, etc..
Right, good point!
dev_pm_opp_set_rate() is best called from consumer drivers, as they
need to be in control.
quoted
What we need here is just configure. So something like this then:
- genpd->get_performance_state()
-> dev_pm_opp_get_current_opp() //New API
-> dev_pm_genpd_set_performance_state(dev, current_opp->pstate);
This can be done just once from probe() then.
How would dev_pm_opp_get_current_opp() work? Do you have a suggestion?
quoted
quoted
I am not sure if/why that approach seemed insufficient?
Another option to solve the problem, I think, is simply to patch
drivers to let them call dev_pm_opp_set_rate() during ->probe(), this
should synchronize the HW state too.
Dmitry already mentioned that this will make the device start
consuming power, and he doesn't want that, else we need an explicit
disble call as well.
I am sure I understand the problem. When a device is getting probed,
/s/I am sure/I am not sure
it needs to consume power, how else can the corresponding driver
successfully probe it?
On Wed, 18 Aug 2021 at 11:14, Viresh Kumar [off-list ref] wrote:
quoted
What we need here is just configure. So something like this then:
- genpd->get_performance_state()
-> dev_pm_opp_get_current_opp() //New API
-> dev_pm_genpd_set_performance_state(dev, current_opp->pstate);
This can be done just once from probe() then.
How would dev_pm_opp_get_current_opp() work? Do you have a suggestion?
The opp core already has a way of finding current OPP, that's what
Dmitry is trying to use here. It finds it using clk_get_rate(), if
that is zero, it picks the lowest freq possible.
I am sure I understand the problem. When a device is getting probed,
it needs to consume power, how else can the corresponding driver
successfully probe it?
Dmitry can answer that better, but a device doesn't necessarily need
to consume energy in probe. It can consume bus clock, like APB we
have, but the more energy consuming stuff can be left disabled until
the time a user comes up. Probe will just end up registering the
driver and initializing it.
--
viresh
On Wed, 18 Aug 2021 at 11:50, Viresh Kumar [off-list ref] wrote:
On 18-08-21, 11:41, Ulf Hansson wrote:
quoted
On Wed, 18 Aug 2021 at 11:14, Viresh Kumar [off-list ref] wrote:
quoted
What we need here is just configure. So something like this then:
- genpd->get_performance_state()
-> dev_pm_opp_get_current_opp() //New API
-> dev_pm_genpd_set_performance_state(dev, current_opp->pstate);
This can be done just once from probe() then.
How would dev_pm_opp_get_current_opp() work? Do you have a suggestion?
The opp core already has a way of finding current OPP, that's what
Dmitry is trying to use here. It finds it using clk_get_rate(), if
that is zero, it picks the lowest freq possible.
quoted
I am sure I understand the problem. When a device is getting probed,
it needs to consume power, how else can the corresponding driver
successfully probe it?
Dmitry can answer that better, but a device doesn't necessarily need
to consume energy in probe. It can consume bus clock, like APB we
have, but the more energy consuming stuff can be left disabled until
the time a user comes up. Probe will just end up registering the
driver and initializing it.
That's perfectly fine, as then it's likely that it won't vote for an
OPP, but can postpone that as well.
Perhaps the problem is rather that the HW may already carry a non-zero
vote made from a bootloader. If the consumer driver tries to clear
that vote (calling dev_pm_opp_set_rate(dev, 0), for example), it would
still not lead to any updates of the performance state in genpd,
because genpd internally has initialized the performance-state to
zero.
Dmitry?
On Wed, Aug 18, 2021 at 04:44:30AM +0300, Dmitry Osipenko wrote:
18.08.2021 04:15, Rob Herring пишет:
quoted
quoted
+ tegra-clocks:+ description: child nodes are the output clocks from the CAR+ type: object++ patternProperties:+ "^[a-z]+[0-9]+$":+ type: object+ properties:+ compatible:+ allOf:+ - items:+ - enum:+ - nvidia,tegra20-sclk+ - nvidia,tegra30-sclk+ - nvidia,tegra30-pllc+ - nvidia,tegra30-plle+ - nvidia,tegra30-pllm+ - const: nvidia,tegra-clock
You are saying the first string must be both one of the enums and
'nvidia,tegra-clock'. You don't get an error because your pattern
doesn't match 'sclk'.
Could you please rephrase or clarify? If pattern doesn't match 'sclk',
then it must match any other enum. I'm not sure what you're meaning.
"sclk" doesn't match "^[a-z]+[0-9]+$" because it's missing at least one
digit at the end. Perhaps that last + was supposed to be *?
The 'nvidia,tegra-clock' actually could be removed since it's
superfluous now. I'll consider the removal in v9.
It also looks like your schema was meant to be something like:
compatible:
- items:
- enum:
- nvidia,tegra20-sclk
- nvidia,tegra30-sclk
- nvidia,tegra30-pllc
- nvidia,tegra30-plle
- nvidia,tegra30-pllm
- const: nvidia,tegra-clock
Note how the const: element is indented one more level. Now this means:
one of the enumeration values, followed by the constant value. That
matches what the example has.
That said, I agree that nvidia,tegra-clock seems a bit useless. There's
really no such thing as a generic clock, they're all different in some
way.
Thierry
On Tue, Aug 17, 2021 at 04:27:26AM +0300, Dmitry Osipenko wrote:
quoted hunk
Document tegra-clocks sub-node which describes Tegra SoC clocks that
require a higher voltage of the core power domain in order to operate
properly on a higher clock rates. Each node contains a phandle to OPP
table and power domain.
The root PLLs and system clocks don't have any specific device dedicated
to them, clock controller is in charge of managing power for them.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/clock/nvidia,tegra20-car.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
@@ -42,6 +42,48 @@ properties:"#reset-cells":const:1+tegra-clocks:+description:child nodes are the output clocks from the CAR+type:object++patternProperties:+"^[a-z]+[0-9]+$":+type:object+properties:+compatible:+allOf:+-items:+-enum:+-nvidia,tegra20-sclk+-nvidia,tegra30-sclk+-nvidia,tegra30-pllc+-nvidia,tegra30-plle+-nvidia,tegra30-pllm+-const:nvidia,tegra-clock++operating-points-v2:+$ref:/schemas/types.yaml#/definitions/phandle+description:+Phandle to OPP table that contains frequencies, voltages and+opp-supported-hw property, which is a bitfield indicating+SoC process or speedo ID mask.++clocks:+items:+-description:node's clock++power-domains:+maxItems:1+description:phandle to the core SoC power domain++required:+-compatible+-operating-points-v2+-clocks+-power-domains++additionalProperties:false+required:-compatible-reg
I wonder if it'd be better to match on the name of the node rather than
add an artificial compatible string. We usually use the compatible
string to match a device, but here you're really trying to add
information about a resource provided by the CAR controller.
We do similar things for example in PMIC bindings where the individual
regulators are represented in the device tree via nodes named after the
regulator.
You could then also leave out the clocks property, which is weird as it
is because it's basically a self-reference. But you don't really need
the reference here in the first place because the CAR is already the
parent of SCLK.
Also, I don't think the tegra- prefix is necessary here. The parent node
is already identified as Tegra via the compatible string.
In the case of CAR, I'd imagine something like:
clocks {
sclk {
operating-points-v2 = <&opp_table>;
power-domains = <&domain>;
};
};
Now you've only got the bare minimum in here that you actually add. All
the other data that you used to have is simply derived from the parent.
Thierry
This looks wrong. Why do we need struct platform_device objects for each
of these clocks? That's going to be a massive amount of platform devices
and they will completely mess up sysfs.
Thierry
On Wed, Aug 18, 2021 at 04:44:30AM +0300, Dmitry Osipenko wrote:
quoted
18.08.2021 04:15, Rob Herring пишет:
quoted
quoted
+ tegra-clocks:+ description: child nodes are the output clocks from the CAR+ type: object++ patternProperties:+ "^[a-z]+[0-9]+$":+ type: object+ properties:+ compatible:+ allOf:+ - items:+ - enum:+ - nvidia,tegra20-sclk+ - nvidia,tegra30-sclk+ - nvidia,tegra30-pllc+ - nvidia,tegra30-plle+ - nvidia,tegra30-pllm+ - const: nvidia,tegra-clock
You are saying the first string must be both one of the enums and
'nvidia,tegra-clock'. You don't get an error because your pattern
doesn't match 'sclk'.
Could you please rephrase or clarify? If pattern doesn't match 'sclk',
then it must match any other enum. I'm not sure what you're meaning.
"sclk" doesn't match "^[a-z]+[0-9]+$" because it's missing at least one
digit at the end. Perhaps that last + was supposed to be *?
Ah, the regex pattern. Yes, I forgot to update it.
quoted
The 'nvidia,tegra-clock' actually could be removed since it's
superfluous now. I'll consider the removal in v9.
It also looks like your schema was meant to be something like:
compatible:
- items:
- enum:
- nvidia,tegra20-sclk
- nvidia,tegra30-sclk
- nvidia,tegra30-pllc
- nvidia,tegra30-plle
- nvidia,tegra30-pllm
- const: nvidia,tegra-clock
Note how the const: element is indented one more level. Now this means:
one of the enumeration values, followed by the constant value. That
matches what the example has.
That said, I agree that nvidia,tegra-clock seems a bit useless. There's
really no such thing as a generic clock, they're all different in some
way.
It's a leftover from older versions of this patchset, I'll remove it.
On Tue, Aug 17, 2021 at 04:27:26AM +0300, Dmitry Osipenko wrote:
quoted
Document tegra-clocks sub-node which describes Tegra SoC clocks that
require a higher voltage of the core power domain in order to operate
properly on a higher clock rates. Each node contains a phandle to OPP
table and power domain.
The root PLLs and system clocks don't have any specific device dedicated
to them, clock controller is in charge of managing power for them.
Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
---
.../bindings/clock/nvidia,tegra20-car.yaml | 51 +++++++++++++++++++
1 file changed, 51 insertions(+)
@@ -42,6 +42,48 @@ properties:"#reset-cells":const:1+tegra-clocks:+description:child nodes are the output clocks from the CAR+type:object++patternProperties:+"^[a-z]+[0-9]+$":+type:object+properties:+compatible:+allOf:+-items:+-enum:+-nvidia,tegra20-sclk+-nvidia,tegra30-sclk+-nvidia,tegra30-pllc+-nvidia,tegra30-plle+-nvidia,tegra30-pllm+-const:nvidia,tegra-clock++operating-points-v2:+$ref:/schemas/types.yaml#/definitions/phandle+description:+Phandle to OPP table that contains frequencies, voltages and+opp-supported-hw property, which is a bitfield indicating+SoC process or speedo ID mask.++clocks:+items:+-description:node's clock++power-domains:+maxItems:1+description:phandle to the core SoC power domain++required:+-compatible+-operating-points-v2+-clocks+-power-domains++additionalProperties:false+required:-compatible-reg
I wonder if it'd be better to match on the name of the node rather than
add an artificial compatible string. We usually use the compatible
string to match a device, but here you're really trying to add
information about a resource provided by the CAR controller.
We do similar things for example in PMIC bindings where the individual
regulators are represented in the device tree via nodes named after the
regulator.
You could then also leave out the clocks property, which is weird as it
is because it's basically a self-reference. But you don't really need
the reference here in the first place because the CAR is already the
parent of SCLK.
We don't have a platform device for CaR. I don't see how it's going to
work. We need to create a platform device for each RPM-capable clock
because that's how RPM works. The compatible string is required for
instantiating OF-devices from a node, otherwise we will have to
re-invent the OF core.
Also, I don't think the tegra- prefix is necessary here. The parent node
is already identified as Tegra via the compatible string.
In the case of CAR, I'd imagine something like:
clocks {
sclk {
operating-points-v2 = <&opp_table>;
power-domains = <&domain>;
};
};
Now you've only got the bare minimum in here that you actually add. All
the other data that you used to have is simply derived from the parent.
'clocks' is already a generic keyword in DT. It's probably not okay to
redefine it.