drm/tegra: dsi: Add suspend/resume support
PATCH 1/1 drm/tegra: dsi: Add suspend/resume support
This patch adds the suspend/resume support for Tegra drm driver by calling the corresponding DPMS functions.
---
Hi,
This patch hooks DSI driver's suspend/resume to implement the whole display system's suspend/resume. I know this is a super ugly way, but as we all know, Tegra DRM driver doesn't have a dedicate drm device so honestly I didn't find a better way to do that.
Thanks,
Mark
drivers/gpu/drm/tegra/dsi.c | 96 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 6 deletions(-)
drivers/gpu/drm/tegra/dsi.c +90 -6
@@ -61,6 +61,9 @@ struct tegra_dsi {struct tegra_dsi *slave; }; +static int tegra_dsi_pad_calibrate (struct tegra_dsi *); +static int tegra_dsi_ganged_setup (struct tegra_dsi *dsi); +static inline struct tegra_dsi *host1x_client_to_dsi (struct host1x_client *client) {
@@ -805,6 +808,20 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output, lanes =tegra_dsi_get_lanes (dsi); + err =tegra_dsi_pad_calibrate (dsi); +if (err <0 ) { +dev_err (dsi->dev,"MIPI calibration failed: %d\n" , err); +return err; + } +if (dsi->slave) { + err =tegra_dsi_pad_calibrate (dsi->slave); +if (err <0 ) { +dev_err (dsi->slave->dev, +"MIPI calibration failed: %d\n" , err); +return err; + } + } + err =tegra_dsi_get_muldiv (dsi->format, &mul, &div);if (err <0 )return err;
@@ -833,6 +850,13 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,dev_err (dsi->dev,"failed to set parent clock: %d\n" , err);return err; } +if (dsi->slave) { + err =tegra_dsi_ganged_setup (dsi); +if (err <0 ) { +dev_err (dsi->dev,"DSI ganged setup failed: %d\n" , err); +return err; + } + } err =clk_set_rate (dsi->clk_parent, plld);if (err <0 ) {
@@ -1470,12 +1494,6 @@ static int tegra_dsi_probe(struct platform_device *pdev) goto disable_vdd; } - err =tegra_dsi_pad_calibrate (dsi); -if (err <0 ) { -dev_err (dsi->dev,"MIPI calibration failed: %d\n" , err); - goto mipi_free; - } - dsi->host.ops = &tegra_dsi_host_ops; dsi->host.dev = &pdev->dev;
@@ -1544,6 +1562,67 @@ static int tegra_dsi_remove(struct platform_device *pdev)return 0 ; } +#ifdef CONFIG_PM +static int tegra_dsi_suspend (struct platform_device *pdev,pm_message_t state) +{ +struct tegra_dsi *dsi =platform_get_drvdata (pdev); +struct tegra_drm *tegra =dev_get_drvdata (dsi->client.parent); +struct drm_connector *connector; + +if (dsi->master) { +regulator_disable (dsi->vdd); +return 0 ; + } + +drm_modeset_lock_all (tegra->drm); +list_for_each_entry (connector, &tegra->drm->mode_config.connector_list, + head) { +int old_dpms = connector->dpms; + +if (connector->funcs->dpms) + connector->funcs->dpms (connector,DRM_MODE_DPMS_OFF ); + +/* Set the old mode back to the connector for resume */ + connector->dpms = old_dpms; + } +drm_modeset_unlock_all (tegra->drm); + +regulator_disable (dsi->vdd); + +return 0 ; +} + +static int tegra_dsi_resume (struct platform_device *pdev) +{ +struct tegra_dsi *dsi =platform_get_drvdata (pdev); +struct tegra_drm *tegra =dev_get_drvdata (dsi->client.parent); +struct drm_connector *connector; +int err =0 ; + + err =regulator_enable (dsi->vdd); +if (err <0 ) { +dev_err (&pdev->dev,"Enable DSI vdd failed: %d\n" , err); +return err; + } + +if (dsi->master) +return 0 ; + +drm_modeset_lock_all (tegra->drm); +list_for_each_entry (connector, &tegra->drm->mode_config.connector_list, + head) { +if (connector->funcs->dpms) + connector->funcs->dpms (connector, connector->dpms); + } +drm_modeset_unlock_all (tegra->drm); + +drm_helper_resume_force_mode (tegra->drm); + +return 0 ; +} +#endif + +static const struct of_device_id tegra_dsi_of_match[] = { { .compatible ="nvidia,tegra114-dsi" , }, { },
@@ -1557,4 +1636,9 @@ struct platform_driver tegra_dsi_driver = { }, .probe = tegra_dsi_probe, .remove = tegra_dsi_remove, +#ifdef CONFIG_PM + .suspend = tegra_dsi_suspend, + .resume = tegra_dsi_resume, +#endif + };
Signature · 1 line
1.8.1.5
Quoted 7 lines · Mark Zhang
This patch adds the suspend/resume support for Tegra drm driver by calling the corresponding DPMS functions.
Signed-off-by Mark Zhang <[email protected]>---
Hi,
Quoted 8 lines
This patch hooks DSI driver's suspend/resume to implement the whole display system's suspend/resume. I know this is a super ugly way, but as we all know, Tegra DRM driver doesn't have a dedicate drm device so honestly I didn't find a better way to do that.
Thanks,
Mark
Hi Mark,
Thanks for posting this. I've reproduced my Gerrit comments from the CrOS tree below for the benefit of others.
Quoted 24 lines
drivers/gpu/drm/tegra/dsi.c | 96 ++++++++++++++++++++++++++++++++++++++++++--- 1 file changed, 90 insertions(+), 6 deletions(-)drivers/gpu/drm/tegra/dsi.c +6
-0@@ -61,6 +61,9 @@ struct tegra_dsi {struct tegra_dsi *slave; }; +static int tegra_dsi_pad_calibrate (struct tegra_dsi *); +static int tegra_dsi_ganged_setup (struct tegra_dsi *dsi); +static inline struct tegra_dsi *host1x_client_to_dsi (struct host1x_client *client) {@@ -805,6 +808,20 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output, lanes =tegra_dsi_get_lanes (dsi); + err =tegra_dsi_pad_calibrate (dsi); +if (err <0 ) { +dev_err (dsi->dev,"MIPI calibration failed: %d\n" , err);
drivers/gpu/drm/tegra/dsi.c static int tegra_output_dsi_setup_clock(struct tegra_output *output, · line 823
Quoted 10 lines
drivers/gpu/drm/tegra/dsi.c +10
-0+return err; + } +if (dsi->slave) { + err =tegra_dsi_pad_calibrate (dsi->slave); +if (err <0 ) { +dev_err (dsi->slave->dev, +"MIPI calibration failed: %d\n" , err); +return err; + } + }
Move this duplicate call into tegra_dsi_pad_calibrate() to be consistent with the other functions in this file (eg:tegra_dsi_set_timeout).
Quoted 5 lines
drivers/gpu/drm/tegra/dsi.c +1
-0+ err =tegra_dsi_get_muldiv (dsi->format, &mul, &div);if (err <0 )return err;
drivers/gpu/drm/tegra/dsi.c static int tegra_output_dsi_setup_clock(struct tegra_output *output, · line 859
Quoted 10 lines
drivers/gpu/drm/tegra/dsi.c +7
-0@@ -833,6 +850,13 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,dev_err (dsi->dev,"failed to set parent clock: %d\n" , err);return err; } +if (dsi->slave) { + err =tegra_dsi_ganged_setup (dsi); +if (err <0 ) { +dev_err (dsi->dev,"DSI ganged setup failed: %d\n" , err); +return err; + } + }
I think this belongs in ->enable() instead of here.
Quoted 69 lines
drivers/gpu/drm/tegra/dsi.c +49
-6err =clk_set_rate (dsi->clk_parent, plld);if (err <0 ) {@@ -1470,12 +1494,6 @@ static int tegra_dsi_probe(struct platform_device *pdev) goto disable_vdd; } - err =tegra_dsi_pad_calibrate (dsi); -if (err <0 ) { -dev_err (dsi->dev,"MIPI calibration failed: %d\n" , err); - goto mipi_free; - } - dsi->host.ops = &tegra_dsi_host_ops; dsi->host.dev = &pdev->dev;@@ -1544,6 +1562,67 @@ static int tegra_dsi_remove(struct platform_device *pdev)return 0 ; } +#ifdef CONFIG_PM +static int tegra_dsi_suspend (struct platform_device *pdev,pm_message_t state) +{ +struct tegra_dsi *dsi =platform_get_drvdata (pdev); +struct tegra_drm *tegra =dev_get_drvdata (dsi->client.parent); +struct drm_connector *connector; + +if (dsi->master) { +regulator_disable (dsi->vdd); +return 0 ; + } + +drm_modeset_lock_all (tegra->drm); +list_for_each_entry (connector, &tegra->drm->mode_config.connector_list, + head) { +int old_dpms = connector->dpms; + +if (connector->funcs->dpms) + connector->funcs->dpms (connector,DRM_MODE_DPMS_OFF ); + +/* Set the old mode back to the connector for resume */ + connector->dpms = old_dpms; + } +drm_modeset_unlock_all (tegra->drm); + +regulator_disable (dsi->vdd); + +return 0 ; +} + +static int tegra_dsi_resume (struct platform_device *pdev) +{ +struct tegra_dsi *dsi =platform_get_drvdata (pdev); +struct tegra_drm *tegra =dev_get_drvdata (dsi->client.parent); +struct drm_connector *connector; +int err =0 ; + + err =regulator_enable (dsi->vdd); +if (err <0 ) { +dev_err (&pdev->dev,"Enable DSI vdd failed: %d\n" , err); +return err; + } + +if (dsi->master) +return 0 ; + +drm_modeset_lock_all (tegra->drm); +list_for_each_entry (connector, &tegra->drm->mode_config.connector_list, + head) {
drivers/gpu/drm/tegra/dsi.c static int tegra_dsi_remove(struct platform_device *pdev) · line 1623
Quoted 10 lines
drivers/gpu/drm/tegra/dsi.c +10
-0+if (connector->funcs->dpms) + connector->funcs->dpms(connector, connector->dpms); + } +drm_modeset_unlock_all (tegra->drm); + +drm_helper_resume_force_mode (tegra->drm); + +return 0 ; +} +#endif
So this is the tricky chunk, it definitely does not belong here. I think it makes most sense for this to live in drm.c, however host1x_driver doesn't have suspend/resume hooks.
I suspect the correct thing to do here is to add them to host1x_driver, but I'm not sure how much effort is involved in doing that.
Sean
Quoted 18 lines
drivers/gpu/drm/tegra/dsi.c +7
-1+ +static const struct of_device_id tegra_dsi_of_match[] = { { .compatible ="nvidia,tegra114-dsi" , }, { },@@ -1557,4 +1636,9 @@ struct platform_driver tegra_dsi_driver = { }, .probe = tegra_dsi_probe, .remove = tegra_dsi_remove, +#ifdef CONFIG_PM + .suspend = tegra_dsi_suspend, + .resume = tegra_dsi_resume, +#endif + }; --1.8.1.5
Quoted 5 lines
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Quoted 3 lines · Sean Paul
Quoted 2 lines · Mark Zhang
This patch adds the suspend/resume support for Tegra drm driver by calling the corresponding DPMS functions.
[...]
Quoted 9 lines
drivers/gpu/drm/tegra/dsi.c static int tegra_output_dsi_setup_clock(struct tegra_output *output, · line 823
Quoted 8 lines
drivers/gpu/drm/tegra/dsi.c +8
-0+if (dsi->slave) { + err =tegra_dsi_pad_calibrate (dsi->slave); +if (err <0 ) { +dev_err (dsi->slave->dev, +"MIPI calibration failed: %d\n" , err); +return err; + } + }
Quoted 4 lines
Move this duplicate call into
tegra_dsi_pad_calibrate()to be consistent with the other functions in this file (eg:tegra_dsi_set_timeout).
Yeah, will do.
Quoted 9 lines
Quoted 9 lines
drivers/gpu/drm/tegra/dsi.c +2
-0+ err =tegra_dsi_get_muldiv (dsi->format, &mul, &div);if (err <0 )return err;@@ -833,6 +850,13 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,dev_err (dsi->dev,"failed to set parent clock: %d\n" , err);return err; } +if (dsi->slave) {
Quoted 9 lines
drivers/gpu/drm/tegra/dsi.c static int tegra_output_dsi_setup_clock(struct tegra_output *output, · line 859
Quoted 6 lines
drivers/gpu/drm/tegra/dsi.c +6
-0+ err =tegra_dsi_ganged_setup (dsi); +if (err <0 ) { +dev_err (dsi->dev,"DSI ganged setup failed: %d\n" , err); +return err; + } + }I think this belongs in ->enable() instead of here.
Actually "tegra_dsi_ganged_setup" is not needed any more. This function ensures that DSIA & DSIB use the same PLL in ganged mode. But if you read the Tegra124/Tegra132 TRM, you'll find there is no way to select the clock source for DSIB. I.E, DSIB always uses the same clock source with DSIA. Besides, PLLD is the only clock source for DSIA. I.E, "clk_get_parent"/"clk_set_parent" doesn't make sense for dsia & dsib clock now.
I've posted a patch(in tegra clock driver) to fix this:
http://article.gmane.org/gmane.linux.ports.tegra/20313
And the corresponding changes in dsi driver will arrive soon.
Quoted 3 lines
Quoted 3 lines
drivers/gpu/drm/tegra/dsi.c +0
-0err =clk_set_rate (dsi->clk_parent, plld);if (err <0 ) {
[...]
Quoted 10 lines
Quoted 10 lines
drivers/gpu/drm/tegra/dsi.c +10
-0+ +drm_modeset_lock_all (tegra->drm); +list_for_each_entry (connector, &tegra->drm->mode_config.connector_list, + head) { +if (connector->funcs->dpms) + connector->funcs->dpms (connector, connector->dpms); + } +drm_modeset_unlock_all (tegra->drm); + +drm_helper_resume_force_mode (tegra->drm);
Quoted 9 lines
drivers/gpu/drm/tegra/dsi.c static int tegra_dsi_remove(struct platform_device *pdev) · line 1623
Quoted 4 lines
drivers/gpu/drm/tegra/dsi.c +4
-0+ + return 0; +} +#endif So this is the tricky chunk, it definitely does not belong here. I think it makes most sense for this to live in
drm.c, howeverhost1x_driverdoesn't have suspend/resume hooks.
Agree. drm.c is the best place for putting this.
Quoted 4 lines
I suspect the correct thing to do here is to add them to
host1x_driver, but I'm not sure how much effort is involved in doing that.
I thought about this before. If we do it in host1x driver, IIUC, it means that when host1x starts suspending, it will suspend all host1x client devices, right? Honestly I feel this is not a good thing despite I can't tell what's the problem in this right now...
Mark
Quoted 25 lines
Sean
Quoted 23 lines
drivers/gpu/drm/tegra/dsi.c +7
-1+ +static const struct of_device_id tegra_dsi_of_match[] = { { .compatible ="nvidia,tegra114-dsi" , }, { },@@ -1557,4 +1636,9 @@ struct platform_driver tegra_dsi_driver = { }, .probe = tegra_dsi_probe, .remove = tegra_dsi_remove, +#ifdef CONFIG_PM + .suspend = tegra_dsi_suspend, + .resume = tegra_dsi_resume, +#endif + }; --1.8.1.5
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Quoted 2 lines · Mark Zhang
Quoted 1 line · Sean Paul
On Sun, Dec 7, 2014 at 10:40 PM, Mark Zhang <[email protected]> wrote:
[...]
Quoted 31 lines
Quoted 3 lines
Quoted 3 lines
err = clk_set_rate(dsi->clk_parent, plld); if (err < 0) {[...]
Quoted 10 lines
Quoted 10 lines
drivers/gpu/drm/tegra/dsi.c +10
-0+ +drm_modeset_lock_all (tegra->drm); +list_for_each_entry (connector, &tegra->drm->mode_config.connector_list, + head) { +if (connector->funcs->dpms) + connector->funcs->dpms (connector, connector->dpms); + } +drm_modeset_unlock_all (tegra->drm); + +drm_helper_resume_force_mode (tegra->drm);Quoted 9 lines
drivers/gpu/drm/tegra/dsi.c static int tegra_dsi_remove(struct platform_device *pdev) · line 1623
Quoted 4 lines
drivers/gpu/drm/tegra/dsi.c +4
-0+ + return 0; +} +#endif So this is the tricky chunk, it definitely does not belong here. I think it makes most sense for this to live in
drm.c, howeverhost1x_driverdoesn't have suspend/resume hooks.Agree.
drm.cis the best place for putting this.Quoted 4 lines
I suspect the correct thing to do here is to add them to
host1x_driver, but I'm not sure how much effort is involved in doing that.
Quoted 4 lines
I thought about this before. If we do it in host1x driver, IIUC, it means that when host1x starts suspending, it will suspend all host1x client devices, right? Honestly I feel this is not a good thing despite I can't tell what's the problem in this right now...
I've just sent out patches for review that add the missing infrastructure to properly do suspend/resume. The idea behind this is that the DRM host1x device's ->pm_ops are responsible for dealing with the suspend/resume at a subsystem level (call connectors' ->dpms() callbacks, etc.) whereas more fine-grained suspend/resume can further be done in the ->pm_ops of the individual subdevices. The infrastructure makes sure that these get called in the right order.
Thierry