Tegra Video Decoder driver power management corrections
Subsystems (3)
v2 cover Tegra Video Decoder driver power management corrections
Hello,
This small series addresses a Runtime PM issue that was discovered during of Tegra VI driver reviewing by balancing RPM usage count on RPM resume failure. Secondly it fixes reboot on some Tegra devices due to bootloader expecting VDE power partition to be ON at the boot time, which wasn't happening in case of a warm re-booting (i.e. by PMC resetting).
Changelog:
v2: - Extended the commit's message of the "Balance runtime PM use-count on
resume failure" patch.
- Re-send for 5.9 inclusion.
Dmitry Osipenko (4):
media: staging: tegra-vde: Balance runtime PM use-count on resume
failure
media: staging: tegra-vde: Runtime PM is always available on Tegra
media: staging: tegra-vde: Turn ON power domain on shutdown
media: staging: tegra-vde: Power-cycle hardware on probe
drivers/staging/media/tegra-vde/vde.c | 45 +++++++++++++++++---------- 1 file changed, 29 insertions(+), 16 deletions(-)
Signature · 1 line
2.26.0
PATCH v2 1/4 media: staging: tegra-vde: Balance runtime PM use-count on resume failure
The RPM's use-count is getting incremented regardless of the pm_runtime_get_sync() success or fail. It's up to a driver how to handle the failed RPM. In the case of VDE driver, the RPM's use-count should be restored if runtime-resume fails. Use pm_runtime_put_noidle(), which is the most straight-forward variant to balance the RPM, confirmed by Rafael J. Wysocki.
--- drivers/staging/media/tegra-vde/vde.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
drivers/staging/media/tegra-vde/vde.c +3 -1
@@ -776,8 +776,10 @@ static int tegra_vde_ioctl_decode_h264(struct tegra_vde *vde, goto release_dpb_frames; ret =pm_runtime_get_sync (dev); -if (ret <0 ) +if (ret <0 ) { +pm_runtime_put_noidle (dev); goto unlock; + }/* * We rely on the VDE registers reset value, otherwise VDE
Signature · 1 line
2.26.0
PATCH v2 2/4 media: staging: tegra-vde: Runtime PM is always available on Tegra
In reply to Dmitry OsipenkoRuntime PM is always available on Tegra nowadays since commit 40b2bb1b132a ("ARM: tegra: enforce PM requirement"), hence the case of unavailable RPM doesn't need to be handled.
--- drivers/staging/media/tegra-vde/vde.c | 16 ---------------- 1 file changed, 16 deletions(-)
drivers/staging/media/tegra-vde/vde.c +0 -16
@@ -1068,17 +1068,8 @@ static int tegra_vde_probe(struct platform_device *pdev)pm_runtime_use_autosuspend (dev);pm_runtime_set_autosuspend_delay (dev,300 ); -if (!pm_runtime_enabled (dev)) { - err =tegra_vde_runtime_resume (dev); -if (err) - goto err_misc_unreg; - } -return 0 ; -err_misc_unreg: -misc_deregister (&vde->miscdev); - err_deinit_iommu:tegra_vde_iommu_deinit (vde);
@@ -1093,13 +1084,6 @@ static int tegra_vde_remove(struct platform_device *pdev) {struct tegra_vde *vde =platform_get_drvdata (pdev);struct device *dev = &pdev->dev; -int err; - -if (!pm_runtime_enabled (dev)) { - err =tegra_vde_runtime_suspend (dev); -if (err) -return err; - }pm_runtime_dont_use_autosuspend (dev);pm_runtime_disable (dev);
Signature · 1 line
2.26.0
Re: [PATCH v2 2/4] media: staging: tegra-vde: Runtime PM is always available on Tegra
Quoted 8 lines · Dmitry Osipenko
Runtime PM is always available on Tegra nowadays since commit
40b2bb1b132a("ARM: tegra: enforce PM requirement"), hence the case of unavailable RPM doesn't need to be handled.Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/staging/media/tegra-vde/vde.c | 16 ---------------- 1 file changed, 16 deletions(-)
PATCH v2 3/4 media: staging: tegra-vde: Turn ON power domain on shutdown
In reply to Dmitry OsipenkoOn some devices bootloader isn't ready to a clamped VDE power, and thus, machine hangs on a warm reboot (CPU reset). The VDE power partition is turned ON by default on a cold boot, hence VDE driver should keep power partition enabled on system's reboot too. This fixes hang on a warm reboot on a Tegra20 Acer A500 device, which is handy if Embedded Controller driver is unavailable, i.e. cold reboot can't be performed.
--- drivers/staging/media/tegra-vde/vde.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
drivers/staging/media/tegra-vde/vde.c +19 -0
@@ -1085,9 +1085,17 @@ static int tegra_vde_remove(struct platform_device *pdev)struct tegra_vde *vde =platform_get_drvdata (pdev);struct device *dev = &pdev->dev; +pm_runtime_get_sync (dev);pm_runtime_dont_use_autosuspend (dev);pm_runtime_disable (dev); +/* +* Balance RPM state, the VDE power domain is left ON and hardware +* is clock-gated. It's safe to reboot machine now. +*/ +pm_runtime_put_noidle (dev); +clk_disable_unprepare (vde->clk); +misc_deregister (&vde->miscdev);tegra_vde_dmabuf_cache_unmap_all (vde);
@@ -1099,6 +1107,16 @@ static int tegra_vde_remove(struct platform_device *pdev)return 0 ; } +static void tegra_vde_shutdown (struct platform_device *pdev) +{ +/* +* On some devices bootloader isn't ready to a power-gated VDE on +* a warm-reboot, machine will hang in that case. +*/ +if (pm_runtime_status_suspended (&pdev->dev)) +tegra_vde_runtime_resume (&pdev->dev); +} +static __maybe_unused int tegra_vde_pm_suspend (struct device *dev) {struct tegra_vde *vde =dev_get_drvdata (dev);
@@ -1144,6 +1162,7 @@ MODULE_DEVICE_TABLE(of, tegra_vde_of_match);static struct platform_driver tegra_vde_driver = { .probe = tegra_vde_probe, .remove = tegra_vde_remove, + .shutdown = tegra_vde_shutdown, .driver = { .name ="tegra-vde" , .of_match_table = tegra_vde_of_match,
Signature · 1 line
2.26.0
Re: [PATCH v2 3/4] media: staging: tegra-vde: Turn ON power domain on shutdown
Quoted 11 lines · Dmitry Osipenko
On some devices bootloader isn't ready to a clamped VDE power, and thus, machine hangs on a warm reboot (CPU reset). The VDE power partition is turned ON by default on a cold boot, hence VDE driver should keep power partition enabled on system's reboot too. This fixes hang on a warm reboot on a Tegra20 Acer A500 device, which is handy if Embedded Controller driver is unavailable, i.e. cold reboot can't be performed.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/staging/media/tegra-vde/vde.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
Unfortunate that we have to do this, but looks fine:
PATCH v2 4/4 media: staging: tegra-vde: Power-cycle hardware on probe
In reply to Dmitry OsipenkoVDE partition is left turned ON after bootloader on most devices, hence let's ensure that it's turned OFF in order to lower power leakage while hardware is idling by turning it ON and OFF during of the driver's probe.
--- drivers/staging/media/tegra-vde/vde.c | 8 ++++++++ 1 file changed, 8 insertions(+)
drivers/staging/media/tegra-vde/vde.c +8 -0
@@ -1068,6 +1068,14 @@ static int tegra_vde_probe(struct platform_device *pdev)pm_runtime_use_autosuspend (dev);pm_runtime_set_autosuspend_delay (dev,300 ); +/* +* VDE partition may be left ON after bootloader, hence let's +* power-cycle it in order to put hardware into a predictable lower +* power state. +*/ +pm_runtime_get_sync (dev); +pm_runtime_put (dev); +return 0 ; err_deinit_iommu:
Signature · 1 line
2.26.0
Re: [PATCH v2 4/4] media: staging: tegra-vde: Power-cycle hardware on probe
Quoted 18 lines · Dmitry Osipenko
VDE partition is left turned ON after bootloader on most devices, hence let's ensure that it's turned OFF in order to lower power leakage while hardware is idling by turning it ON and OFF during of the driver's probe.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/staging/media/tegra-vde/vde.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/staging/media/tegra-vde/vde.c +1
-0@@ -1068,6 +1068,14 @@ static int tegra_vde_probe(struct platform_device *pdev)pm_runtime_use_autosuspend (dev);pm_runtime_set_autosuspend_delay (dev,300 ); +/*
drivers/staging/media/tegra-vde/vde.c static int tegra_vde_probe(struct platform_device *pdev) · line 1081
Quoted 10 lines
drivers/staging/media/tegra-vde/vde.c +7
-0+* VDE partition may be left ON after bootloader, hence let's +* power-cycle it in order to put hardware into a predictable lower +* power state. +*/ +pm_runtime_get_sync (dev); +pm_runtime_put (dev); +return 0 ; err_deinit_iommu:
Shouldn't this happen automatically? My understanding is that power domains are turned on automatically before ->probe() and then unless a runtime PM reference is taken during ->probe() it will get turned off again after ->probe()?
Is that not happening? Is auto-suspend perhaps getting in the way somehow?
Thierry
Re: [PATCH v2 4/4] media: staging: tegra-vde: Power-cycle hardware on probe
26.06.2020 10:48, Thierry Reding пишет:
Quoted 28 lines · Dmitry Osipenko
Quoted 27 lines
VDE partition is left turned ON after bootloader on most devices, hence let's ensure that it's turned OFF in order to lower power leakage while hardware is idling by turning it ON and OFF during of the driver's probe.
Signed-off-by Dmitry Osipenko <[email protected]>--- drivers/staging/media/tegra-vde/vde.c | 8 ++++++++ 1 file changed, 8 insertions(+)drivers/staging/media/tegra-vde/vde.c +8
-0@@ -1068,6 +1068,14 @@ static int tegra_vde_probe(struct platform_device *pdev)pm_runtime_use_autosuspend (dev);pm_runtime_set_autosuspend_delay (dev,300 ); +/* +* VDE partition may be left ON after bootloader, hence let's +* power-cycle it in order to put hardware into a predictable lower +* power state. +*/ +pm_runtime_get_sync (dev); +pm_runtime_put (dev); +return 0 ;
Quoted 6 lines
drivers/staging/media/tegra-vde/vde.c static int tegra_vde_probe(struct platform_device *pdev) · line 1081
Quoted 1 line
drivers/staging/media/tegra-vde/vde.c +0
-0err_deinit_iommu:Shouldn't this happen automatically? My understanding is that power domains are turned on automatically before ->probe() and then unless a runtime PM reference is taken during ->probe() it will get turned off again after ->probe()?
Older Tegra SoCs haven't been converted to use the generic power-domain API and today's VDE driver supports only T20 and T30 SoCs.
Quoted 2 lines
Is that not happening? Is auto-suspend perhaps getting in the way somehow?
We're manually toggling the PD using legacy Tegra-PD API in the driver's RPM callbacks, that's why the RPM needs to be toggled manually as well.
Perhaps this hunk could be removed if older Tergas would get a genpd support. I guess it shouldn't be difficult to implement the genpd support, but then there will be a compatibility trouble with older DTs, so perhaps it's not really worth the effort.
Quoted 7 lines · Dmitry Osipenko
Hello,
This small series addresses a Runtime PM issue that was discovered during of Tegra VI driver reviewing by balancing RPM usage count on RPM resume failure. Secondly it fixes reboot on some Tegra devices due to bootloader expecting VDE power partition to be ON at the boot time, which wasn't happening in case of a warm re-booting (i.e. by PMC resetting).
Can you rebase this on top of the media_tree master branch? I think a variant of patch 1 has already been applied. I found a mail today where you mentioned that you preferred your version (it looks like I missed that) so you'll need to rework patch 1.
Sorry about this,
Hans
Quoted 18 lines
Changelog:
v2: - Extended the commit's message of the "Balance runtime PM use-count on
resume failure" patch.- Re-send for 5.9 inclusion.
Dmitry Osipenko (4):
media: staging: tegra-vde: Balance runtime PM use-count on resume
failure
media: staging: tegra-vde: Runtime PM is always available on Tegra
media: staging: tegra-vde: Turn ON power domain on shutdown
media: staging: tegra-vde: Power-cycle hardware on probedrivers/staging/media/tegra-vde/vde.c | 45 +++++++++++++++++---------- 1 file changed, 29 insertions(+), 16 deletions(-)
24.06.2020 18:16, Hans Verkuil пишет:
Quoted 9 lines · Dmitry Osipenko
Quoted 7 lines
Hello,
This small series addresses a Runtime PM issue that was discovered during of Tegra VI driver reviewing by balancing RPM usage count on RPM resume failure. Secondly it fixes reboot on some Tegra devices due to bootloader expecting VDE power partition to be ON at the boot time, which wasn't happening in case of a warm re-booting (i.e. by PMC resetting).
Quoted 4 lines
Can you rebase this on top of the
media_treemaster branch? I think a variant of patch 1 has already been applied. I found a mail today where you mentioned that you preferred your version (it looks like I missed that) so you'll need to rework patch 1.
Hello Hans,
I'll take a look at what patches has been applied, my bad for sending the v2 too late. Thank you for the heads up!
24.06.2020 18:23, Dmitry Osipenko пишет:
Quoted 15 lines
24.06.2020 18:16, Hans Verkuil пишет:
Quoted 9 lines · Dmitry Osipenko
Quoted 7 lines
Hello,
This small series addresses a Runtime PM issue that was discovered during of Tegra VI driver reviewing by balancing RPM usage count on RPM resume failure. Secondly it fixes reboot on some Tegra devices due to bootloader expecting VDE power partition to be ON at the boot time, which wasn't happening in case of a warm re-booting (i.e. by PMC resetting).
Quoted 4 lines
Can you rebase this on top of the
media_treemaster branch? I think a variant of patch 1 has already been applied. I found a mail today where you mentioned that you preferred your version (it looks like I missed that) so you'll need to rework patch 1.
Quoted 5 lines
Hello Hans,
I'll take a look at what patches has been applied, my bad for sending the v2 too late. Thank you for the heads up!
I tested the already-applied variant of the patch 1 and it has the same behaviour as my variant, so it's okay.
Would you want me to send a v3 without the conflicting patch 1 or you could apply the patches 2-4 from this series?
Quoted 22 lines · Dmitry Osipenko
24.06.2020 18:23, Dmitry Osipenko пишет:
Quoted 15 lines
24.06.2020 18:16, Hans Verkuil пишет:
Quoted 9 lines · Dmitry Osipenko
Quoted 7 lines
Hello,
This small series addresses a Runtime PM issue that was discovered during of Tegra VI driver reviewing by balancing RPM usage count on RPM resume failure. Secondly it fixes reboot on some Tegra devices due to bootloader expecting VDE power partition to be ON at the boot time, which wasn't happening in case of a warm re-booting (i.e. by PMC resetting).
Quoted 4 lines
Can you rebase this on top of the
media_treemaster branch? I think a variant of patch 1 has already been applied. I found a mail today where you mentioned that you preferred your version (it looks like I missed that) so you'll need to rework patch 1.Quoted 5 lines
Hello Hans,
I'll take a look at what patches has been applied, my bad for sending the v2 too late. Thank you for the heads up!
Quoted 6 lines
I tested the already-applied variant of the patch 1 and it has the same behaviour as my variant, so it's okay.
Would you want me to send a v3 without the conflicting patch 1 or you could apply the patches 2-4 from this series?
I'll mark 1/4 as superseded and will apply patches 2-4. No need for you to do anything.
Regards,
Hans
26.06.2020 09:58, Hans Verkuil пишет:
Quoted 26 lines · Dmitry Osipenko
Quoted 25 lines
24.06.2020 18:23, Dmitry Osipenko пишет:
Quoted 15 lines
24.06.2020 18:16, Hans Verkuil пишет:
Quoted 9 lines · Dmitry Osipenko
Quoted 7 lines
Hello,
This small series addresses a Runtime PM issue that was discovered during of Tegra VI driver reviewing by balancing RPM usage count on RPM resume failure. Secondly it fixes reboot on some Tegra devices due to bootloader expecting VDE power partition to be ON at the boot time, which wasn't happening in case of a warm re-booting (i.e. by PMC resetting).
Quoted 4 lines
Can you rebase this on top of the
media_treemaster branch? I think a variant of patch 1 has already been applied. I found a mail today where you mentioned that you preferred your version (it looks like I missed that) so you'll need to rework patch 1.Quoted 5 lines
Hello Hans,
I'll take a look at what patches has been applied, my bad for sending the v2 too late. Thank you for the heads up!
I tested the already-applied variant of the patch 1 and it has the same behaviour as my variant, so it's okay.
Quoted 6 lines
Quoted 3 lines
Would you want me to send a v3 without the conflicting patch 1 or you could apply the patches 2-4 from this series?
I'll mark 1/4 as superseded and will apply patches 2-4. No need for you to do anything.
Thank you!