drm/tegra: Turn off and reset hardware across suspend-resume
PATCH v2 1/1 drm/tegra: Turn off and reset hardware across suspend-resume
The drivers core bumps runtime PM refcount during of entering into suspend to workaround some problem where parent device may become turned off before its children. In order to disable and reset CRTCs/HDMI/etc hardware, the runtime PM needs to be "forced" into suspend mode.
---
Changelog:
v2: The SYSTEM_SLEEP_PM_OPS are now set for all of the relevant drivers and
not only for the DC because turned out that they all should enforce the suspending.
drivers/gpu/drm/tegra/dc.c | 2 ++ drivers/gpu/drm/tegra/dpaux.c | 2 ++ drivers/gpu/drm/tegra/dsi.c | 2 ++ drivers/gpu/drm/tegra/hdmi.c | 2 ++ drivers/gpu/drm/tegra/hub.c | 2 ++ drivers/gpu/drm/tegra/sor.c | 2 ++ drivers/gpu/drm/tegra/vic.c | 2 ++ 7 files changed, 14 insertions(+)
drivers/gpu/drm/tegra/dc.c +2 -0
@@ -2572,6 +2572,8 @@ static int tegra_dc_resume(struct device *dev)static const struct dev_pm_ops tegra_dc_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dc_suspend, tegra_dc_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_dc_driver = {
drivers/gpu/drm/tegra/dpaux.c +2 -0
@@ -638,6 +638,8 @@ static int tegra_dpaux_resume(struct device *dev)static const struct dev_pm_ops tegra_dpaux_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dpaux_suspend, tegra_dpaux_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };static const struct of_device_id tegra_dpaux_of_match[] = {
drivers/gpu/drm/tegra/dsi.c +2 -0
@@ -1665,6 +1665,8 @@ static int tegra_dsi_resume(struct device *dev)static const struct dev_pm_ops tegra_dsi_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dsi_suspend, tegra_dsi_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };static const struct of_device_id tegra_dsi_of_match[] = {
drivers/gpu/drm/tegra/hdmi.c +2 -0
@@ -1739,6 +1739,8 @@ static int tegra_hdmi_resume(struct device *dev)static const struct dev_pm_ops tegra_hdmi_pm_ops = { SET_RUNTIME_PM_OPS(tegra_hdmi_suspend, tegra_hdmi_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_hdmi_driver = {
drivers/gpu/drm/tegra/hub.c +2 -0
@@ -931,6 +931,8 @@ static int __maybe_unused tegra_display_hub_resume(struct device *dev)static const struct dev_pm_ops tegra_display_hub_pm_ops = { SET_RUNTIME_PM_OPS(tegra_display_hub_suspend, tegra_display_hub_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };static const struct tegra_display_hub_soc tegra186_display_hub = {
drivers/gpu/drm/tegra/sor.c +2 -0
@@ -3572,6 +3572,8 @@ static int tegra_sor_resume(struct device *dev)static const struct dev_pm_ops tegra_sor_pm_ops = { SET_RUNTIME_PM_OPS(tegra_sor_suspend, tegra_sor_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_sor_driver = {
drivers/gpu/drm/tegra/vic.c +2 -0
@@ -476,6 +476,8 @@ static int vic_remove(struct platform_device *pdev)static const struct dev_pm_ops vic_pm_ops = { SET_RUNTIME_PM_OPS(vic_runtime_suspend, vic_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_vic_driver = {
Signature · 1 line
2.22.0
11.08.2019 21:39, Dmitry Osipenko пишет:
Quoted 7 lines
The drivers core bumps runtime PM refcount during of entering into suspend to workaround some problem where parent device may become turned off before its children. In order to disable and reset CRTCs/HDMI/etc hardware, the runtime PM needs to be "forced" into suspend mode.
Signed-off-by Dmitry Osipenko <[email protected]>---
Hello Thierry,
Is there any particular reason why you're skipping this patch? The driver's suspend-resume doesn't work properly without it.
Quoted 14 lines · Dmitry Osipenko
The drivers core bumps runtime PM refcount during of entering into suspend to workaround some problem where parent device may become turned off before its children. In order to disable and reset CRTCs/HDMI/etc hardware, the runtime PM needs to be "forced" into suspend mode.
Signed-off-by Dmitry Osipenko <[email protected]>---
Changelog:
v2: The
SYSTEM_SLEEP_PM_OPSare now set for all of the relevant drivers and
not only for the DC because turned out that they all should enforce the suspending.
Quoted 8 lines
drivers/gpu/drm/tegra/dc.c | 2 ++ drivers/gpu/drm/tegra/dpaux.c | 2 ++ drivers/gpu/drm/tegra/dsi.c | 2 ++ drivers/gpu/drm/tegra/hdmi.c | 2 ++ drivers/gpu/drm/tegra/hub.c | 2 ++ drivers/gpu/drm/tegra/sor.c | 2 ++ drivers/gpu/drm/tegra/vic.c | 2 ++ 7 files changed, 14 insertions(+)
I'm not exactly sure I understand why this is necessary. Runtime PM is controlled by the drivers themselves so that when an output (say SOR) is disabled, it drops the runtime PM reference. The idea is that since the disabled output is no longer needed it can just go into a low power mode which on Tegra usually means clocks off and reset asserted (and in some cases also power domain off).
DRM/KMS has system-level suspend support, which we use to disable all outputs when entering suspend. I see that, unfortunately, this doesn't seem to actually cause the devices to runtime suspend. I'm pretty sure that this used to work at some point, so I don't know what changed. I'd have to look into this a little more. The core doing something like this behind the driver's back seems wrong and having to force the device into suspend mode seems like it's just piling up on the workarounds.
Thierry
Quoted 95 lines
drivers/gpu/drm/tegra/dc.c +2
-0@@ -2572,6 +2572,8 @@ static int tegra_dc_resume(struct device *dev)static const struct dev_pm_ops tegra_dc_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dc_suspend, tegra_dc_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_dc_driver = {drivers/gpu/drm/tegra/dpaux.c +2
-0@@ -638,6 +638,8 @@ static int tegra_dpaux_resume(struct device *dev)static const struct dev_pm_ops tegra_dpaux_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dpaux_suspend, tegra_dpaux_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };static const struct of_device_id tegra_dpaux_of_match[] = {drivers/gpu/drm/tegra/dsi.c +2
-0@@ -1665,6 +1665,8 @@ static int tegra_dsi_resume(struct device *dev)static const struct dev_pm_ops tegra_dsi_pm_ops = { SET_RUNTIME_PM_OPS(tegra_dsi_suspend, tegra_dsi_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };static const struct of_device_id tegra_dsi_of_match[] = {drivers/gpu/drm/tegra/hdmi.c +2
-0@@ -1739,6 +1739,8 @@ static int tegra_hdmi_resume(struct device *dev)static const struct dev_pm_ops tegra_hdmi_pm_ops = { SET_RUNTIME_PM_OPS(tegra_hdmi_suspend, tegra_hdmi_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_hdmi_driver = {drivers/gpu/drm/tegra/hub.c +2
-0@@ -931,6 +931,8 @@ static int __maybe_unused tegra_display_hub_resume(struct device *dev)static const struct dev_pm_ops tegra_display_hub_pm_ops = { SET_RUNTIME_PM_OPS(tegra_display_hub_suspend, tegra_display_hub_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };static const struct tegra_display_hub_soc tegra186_display_hub = {drivers/gpu/drm/tegra/sor.c +2
-0@@ -3572,6 +3572,8 @@ static int tegra_sor_resume(struct device *dev)static const struct dev_pm_ops tegra_sor_pm_ops = { SET_RUNTIME_PM_OPS(tegra_sor_suspend, tegra_sor_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_sor_driver = {drivers/gpu/drm/tegra/vic.c +2
-0@@ -476,6 +476,8 @@ static int vic_remove(struct platform_device *pdev)static const struct dev_pm_ops vic_pm_ops = { SET_RUNTIME_PM_OPS(vic_runtime_suspend, vic_runtime_resume, NULL) +SET_SYSTEM_SLEEP_PM_OPS (pm_runtime_force_suspend, + pm_runtime_force_resume) };struct platform_driver tegra_vic_driver = {Signature · 1 line
2.22.0
14.11.2019 23:31, Thierry Reding пишет:
Quoted 31 lines · Dmitry Osipenko
Quoted 14 lines
The drivers core bumps runtime PM refcount during of entering into suspend to workaround some problem where parent device may become turned off before its children. In order to disable and reset CRTCs/HDMI/etc hardware, the runtime PM needs to be "forced" into suspend mode.
Signed-off-by Dmitry Osipenko <[email protected]>---
Changelog:
v2: The
SYSTEM_SLEEP_PM_OPSare now set for all of the relevant drivers and
not only for the DC because turned out that they all should enforce the suspending.Quoted 8 lines
drivers/gpu/drm/tegra/dc.c | 2 ++ drivers/gpu/drm/tegra/dpaux.c | 2 ++ drivers/gpu/drm/tegra/dsi.c | 2 ++ drivers/gpu/drm/tegra/hdmi.c | 2 ++ drivers/gpu/drm/tegra/hub.c | 2 ++ drivers/gpu/drm/tegra/sor.c | 2 ++ drivers/gpu/drm/tegra/vic.c | 2 ++ 7 files changed, 14 insertions(+)I'm not exactly sure I understand why this is necessary. Runtime PM is controlled by the drivers themselves so that when an output (say SOR) is disabled, it drops the runtime PM reference. The idea is that since the disabled output is no longer needed it can just go into a low power mode which on Tegra usually means clocks off and reset asserted (and in some cases also power domain off).
Quoted 7 lines
DRM/KMS has system-level suspend support, which we use to disable all outputs when entering suspend. I see that, unfortunately, this doesn't seem to actually cause the devices to runtime suspend. I'm pretty sure that this used to work at some point, so I don't know what changed. I'd have to look into this a little more. The core doing something like this behind the driver's back seems wrong and having to force the device into suspend mode seems like it's just piling up on the workarounds.
Please let me know if you'll find a better solution.