This patch-set series adds runtime pm support for host1x, gr2d & dc. It retains the current behaviour if CONFIG_PM_RUNTIME is not enabled.
For host1x & gr2d, the clocks are now enabled in .probe and disabled on its exit. This is needed for correct init of hardware.
Additionally for gr2d, the clocks are also enabled when a new work is submitted and disabled when the work is done. Due to parent->child relations between host1x->gr2d, this scheme also ends up in enabling & disabling host1x clock
For dc, the clocks are enabled in .probe and disabled in .remove but via runtime pm instead of direct clock APIs.
Mayuresh Kulkarni (4): gpu: host1x: shuffle job APIs gpu: host1x: add runtime pm support for gr2d gpu: host1x: add runtime pm support for dc gpu: host1x: add runtime pm support for host1x
drivers/gpu/host1x/cdma.c | 2 ++ drivers/gpu/host1x/channel.c | 8 ------ drivers/gpu/host1x/channel.h | 1 - drivers/gpu/host1x/dev.c | 57 ++++++++++++++++++++++++++++++++++++++++ drivers/gpu/host1x/drm/dc.c | 60 +++++++++++++++++++++++++++++++++++++++---- drivers/gpu/host1x/drm/gr2d.c | 56 +++++++++++++++++++++++++++++++++++++++- drivers/gpu/host1x/job.c | 21 +++++++++++++++ drivers/gpu/host1x/job.h | 3 +++ 8 files changed, 193 insertions(+), 15 deletions(-)
This patch moves the API host1x_job_submit to job.c file. It also adds a new API host1x_job_complete.
This is in preparation to add runtime PM support to host1x & its modules. The idea is to call pm_runtime_get from host1x_job_submit and pm_runtime_put from host1x_job_complete.
This way the runtime PM calls are localized to single file & easy to maintain as well as debug
Signed-off-by: Mayuresh Kulkarni mkulkarni@nvidia.com --- drivers/gpu/host1x/cdma.c | 2 ++ drivers/gpu/host1x/channel.c | 8 -------- drivers/gpu/host1x/channel.h | 1 - drivers/gpu/host1x/job.c | 12 ++++++++++++ drivers/gpu/host1x/job.h | 3 +++ 5 files changed, 17 insertions(+), 9 deletions(-)
diff --git a/drivers/gpu/host1x/cdma.c b/drivers/gpu/host1x/cdma.c index de72172..910087b 100644 --- a/drivers/gpu/host1x/cdma.c +++ b/drivers/gpu/host1x/cdma.c @@ -252,6 +252,8 @@ static void update_cdma_locked(struct host1x_cdma *cdma) signal = true; }
+ host1x_job_complete(job); + list_del(&job->list); host1x_job_put(job); } diff --git a/drivers/gpu/host1x/channel.c b/drivers/gpu/host1x/channel.c index 83ea51b..c381441 100644 --- a/drivers/gpu/host1x/channel.c +++ b/drivers/gpu/host1x/channel.c @@ -21,7 +21,6 @@
#include "channel.h" #include "dev.h" -#include "job.h"
/* Constructor for the host1x device list */ int host1x_channel_list_init(struct host1x *host) @@ -37,13 +36,6 @@ int host1x_channel_list_init(struct host1x *host) return 0; }
-int host1x_job_submit(struct host1x_job *job) -{ - struct host1x *host = dev_get_drvdata(job->channel->dev->parent); - - return host1x_hw_channel_submit(host, job); -} - struct host1x_channel *host1x_channel_get(struct host1x_channel *channel) { int err = 0; diff --git a/drivers/gpu/host1x/channel.h b/drivers/gpu/host1x/channel.h index 48723b8..8401f25 100644 --- a/drivers/gpu/host1x/channel.h +++ b/drivers/gpu/host1x/channel.h @@ -44,7 +44,6 @@ struct host1x_channel *host1x_channel_request(struct device *dev); void host1x_channel_free(struct host1x_channel *channel); struct host1x_channel *host1x_channel_get(struct host1x_channel *channel); void host1x_channel_put(struct host1x_channel *channel); -int host1x_job_submit(struct host1x_job *job);
#define host1x_for_each_channel(host, channel) \ list_for_each_entry(channel, &host->chlist.list, list) diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index cc80766..05bafa4 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -586,3 +586,15 @@ void host1x_job_dump(struct device *dev, struct host1x_job *job) dev_dbg(dev, " NUM_SLOTS %d\n", job->num_slots); dev_dbg(dev, " NUM_HANDLES %d\n", job->num_unpins); } + +int host1x_job_submit(struct host1x_job *job) +{ + struct host1x *host = dev_get_drvdata(job->channel->dev->parent); + + return host1x_hw_channel_submit(host, job); +} + +int host1x_job_complete(struct host1x_job *job) +{ + return 0; +} diff --git a/drivers/gpu/host1x/job.h b/drivers/gpu/host1x/job.h index fba45f2..e0249c3 100644 --- a/drivers/gpu/host1x/job.h +++ b/drivers/gpu/host1x/job.h @@ -159,4 +159,7 @@ void host1x_job_unpin(struct host1x_job *job); */ void host1x_job_dump(struct device *dev, struct host1x_job *job);
+int host1x_job_submit(struct host1x_job *job); +int host1x_job_complete(struct host1x_job *job); + #endif
Signed-off-by: Mayuresh Kulkarni mkulkarni@nvidia.com --- drivers/gpu/host1x/drm/gr2d.c | 56 ++++++++++++++++++++++++++++++++++++++++++- drivers/gpu/host1x/job.c | 9 +++++++ 2 files changed, 64 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c index 27ffcf1..eb506cd 100644 --- a/drivers/gpu/host1x/drm/gr2d.c +++ b/drivers/gpu/host1x/drm/gr2d.c @@ -22,6 +22,7 @@ #include <linux/of.h> #include <linux/of_device.h> #include <linux/clk.h> +#include <linux/pm_runtime.h>
#include "channel.h" #include "drm.h" @@ -275,11 +276,18 @@ static int gr2d_probe(struct platform_device *pdev) return PTR_ERR(gr2d->clk); }
+ platform_set_drvdata(pdev, gr2d); + +#ifdef CONFIG_PM_RUNTIME + pm_runtime_enable(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); +#else err = clk_prepare_enable(gr2d->clk); if (err) { dev_err(dev, "cannot turn on clock\n"); return err; } +#endif
gr2d->channel = host1x_channel_request(dev); if (!gr2d->channel) @@ -305,7 +313,9 @@ static int gr2d_probe(struct platform_device *pdev)
gr2d_init_addr_reg_map(dev, gr2d);
- platform_set_drvdata(pdev, gr2d); +#ifdef CONFIG_PM_RUNTIME + pm_runtime_put(&pdev->dev); +#endif
return 0; } @@ -328,10 +338,51 @@ static int __exit gr2d_remove(struct platform_device *pdev)
host1x_channel_free(gr2d->channel); clk_disable_unprepare(gr2d->clk); +#ifdef CONFIG_PM_RUNTIME + pm_runtime_disable(&pdev->dev); +#endif + + return 0; +} + +#ifdef CONFIG_PM_RUNTIME +static int gr2d_runtime_suspend(struct device *dev) +{ + struct gr2d *gr2d; + + gr2d = dev_get_drvdata(dev); + if (!gr2d) + return -EINVAL; + + clk_disable_unprepare(gr2d->clk);
return 0; }
+static int gr2d_runtime_resume(struct device *dev) +{ + int err = 0; + struct gr2d *gr2d; + + gr2d = dev_get_drvdata(dev); + if (!gr2d) + return -EINVAL; + + err = clk_prepare_enable(gr2d->clk); + if (err < 0) + dev_err(dev, "failed to enable clock\n"); + + return err; +} +#endif /* CONFIG_PM_RUNTIME */ + +#ifdef CONFIG_PM +static const struct dev_pm_ops gr2d_pm_ops = { + SET_RUNTIME_PM_OPS(gr2d_runtime_suspend, + gr2d_runtime_resume, NULL) +}; +#endif + struct platform_driver tegra_gr2d_driver = { .probe = gr2d_probe, .remove = __exit_p(gr2d_remove), @@ -339,5 +390,8 @@ struct platform_driver tegra_gr2d_driver = { .owner = THIS_MODULE, .name = "gr2d", .of_match_table = gr2d_match, +#ifdef CONFIG_PM + .pm = &gr2d_pm_ops, +#endif } }; diff --git a/drivers/gpu/host1x/job.c b/drivers/gpu/host1x/job.c index 05bafa4..a1b05f0 100644 --- a/drivers/gpu/host1x/job.c +++ b/drivers/gpu/host1x/job.c @@ -23,6 +23,7 @@ #include <linux/scatterlist.h> #include <linux/slab.h> #include <linux/vmalloc.h> +#include <linux/pm_runtime.h> #include <trace/events/host1x.h>
#include "channel.h" @@ -591,10 +592,18 @@ int host1x_job_submit(struct host1x_job *job) { struct host1x *host = dev_get_drvdata(job->channel->dev->parent);
+#ifdef CONFIG_PM_RUNTIME + pm_runtime_get_sync(job->channel->dev); +#endif + return host1x_hw_channel_submit(host, job); }
int host1x_job_complete(struct host1x_job *job) { +#ifdef CONFIG_PM_RUNTIME + return pm_runtime_put(job->channel->dev); +#else return 0; +#endif }
On 06/13/2013 03:53 AM, Mayuresh Kulkarni wrote:
Patch description?
diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c
+#ifdef CONFIG_PM_RUNTIME
- pm_runtime_enable(&pdev->dev);
- pm_runtime_get_sync(&pdev->dev);
+#else err = clk_prepare_enable(gr2d->clk); if (err) { dev_err(dev, "cannot turn on clock\n"); return err; } +#endif
The #else block here is a cut/paste of the body of gr2d_runtime_resume(). It'd be better to call that function instead. The following is what I ended up with in the Tegra ASoC driver in order to support runtime PM on or off:
pm_runtime_enable(&pdev->dev); if (!pm_runtime_enabled(&pdev->dev)) { ret = tegra20_i2s_runtime_resume(&pdev->dev); if (ret) goto err_pm_disable; }
@@ -328,10 +338,51 @@ static int __exit gr2d_remove(struct platform_device *pdev)
host1x_channel_free(gr2d->channel); clk_disable_unprepare(gr2d->clk);
Don't you need to remove that clk disable, or make it conditional upon !PM_RUNTIME?
+#ifdef CONFIG_PM_RUNTIME
- pm_runtime_disable(&pdev->dev);
+#endif
Similarly, perhaps something like the following here:
pm_runtime_disable(&pdev->dev); if (!pm_runtime_status_suspended(&pdev->dev)) tegra20_i2s_runtime_suspend(&pdev->dev);
@ -591,10 +592,18 @@ int host1x_job_submit(struct host1x_job *job) { struct host1x *host = dev_get_drvdata(job->channel->dev->parent);
+#ifdef CONFIG_PM_RUNTIME
- pm_runtime_get_sync(job->channel->dev);
+#endif
- return host1x_hw_channel_submit(host, job);
}
int host1x_job_complete(struct host1x_job *job) { +#ifdef CONFIG_PM_RUNTIME
- return pm_runtime_put(job->channel->dev);
+#else return 0; +#endif }
I don't think you need any of those ifdefs; simply call the pm_runtime_*() functions all the time, and they'll be successful no-ops if !PM_RUNTIME.
On Thursday 13 June 2013 08:44 PM, Stephen Warren wrote:
On 06/13/2013 03:53 AM, Mayuresh Kulkarni wrote:
Patch description?
I thought the patch subject is sufficient to tell what it is it doing. Description here would be repetition in my opinion.
Also, the cover letter for the patch-set series is verbose enough.
diff --git a/drivers/gpu/host1x/drm/gr2d.c b/drivers/gpu/host1x/drm/gr2d.c
+#ifdef CONFIG_PM_RUNTIME
- pm_runtime_enable(&pdev->dev);
- pm_runtime_get_sync(&pdev->dev);
+#else err = clk_prepare_enable(gr2d->clk); if (err) { dev_err(dev, "cannot turn on clock\n"); return err; } +#endif
The #else block here is a cut/paste of the body of gr2d_runtime_resume(). It'd be better to call that function instead. The following is what I ended up with in the Tegra ASoC driver in order to support runtime PM on or off:
pm_runtime_enable(&pdev->dev); if (!pm_runtime_enabled(&pdev->dev)) { ret = tegra20_i2s_runtime_resume(&pdev->dev); if (ret) goto err_pm_disable; }
Thanks for the tip. Runtime detection is better than compile time here.
@@ -328,10 +338,51 @@ static int __exit gr2d_remove(struct platform_device *pdev)
host1x_channel_free(gr2d->channel); clk_disable_unprepare(gr2d->clk);
Don't you need to remove that clk disable, or make it conditional upon !PM_RUNTIME?
Yes you are correct.
+#ifdef CONFIG_PM_RUNTIME
- pm_runtime_disable(&pdev->dev);
+#endif
Similarly, perhaps something like the following here:
pm_runtime_disable(&pdev->dev); if (!pm_runtime_status_suspended(&pdev->dev)) tegra20_i2s_runtime_suspend(&pdev->dev);@ -591,10 +592,18 @@ int host1x_job_submit(struct host1x_job *job) { struct host1x *host = dev_get_drvdata(job->channel->dev->parent);
+#ifdef CONFIG_PM_RUNTIME
- pm_runtime_get_sync(job->channel->dev);
+#endif
return host1x_hw_channel_submit(host, job); }
int host1x_job_complete(struct host1x_job *job) {
+#ifdef CONFIG_PM_RUNTIME
- return pm_runtime_put(job->channel->dev);
+#else return 0; +#endif }
I don't think you need any of those ifdefs; simply call the pm_runtime_*() functions all the time, and they'll be successful no-ops if !PM_RUNTIME.
OK. But I thought it will be better to be verbose (which is not needed).
As of now, the dc clock is enabled in its .probe via runtime pm and disabled in .remove
Signed-off-by: Mayuresh Kulkarni mkulkarni@nvidia.com --- drivers/gpu/host1x/drm/dc.c | 60 +++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 55 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/host1x/drm/dc.c b/drivers/gpu/host1x/drm/dc.c index 5360e5a..8e669a9 100644 --- a/drivers/gpu/host1x/drm/dc.c +++ b/drivers/gpu/host1x/drm/dc.c @@ -13,6 +13,7 @@ #include <linux/of.h> #include <linux/platform_device.h> #include <linux/clk/tegra.h> +#include <linux/pm_runtime.h>
#include "host1x_client.h" #include "dc.h" @@ -1128,9 +1129,7 @@ static int tegra_dc_probe(struct platform_device *pdev) return PTR_ERR(dc->clk); }
- err = clk_prepare_enable(dc->clk); - if (err < 0) - return err; + platform_set_drvdata(pdev, dc);
regs = platform_get_resource(pdev, IORESOURCE_MEM, 0); dc->regs = devm_ioremap_resource(&pdev->dev, regs); @@ -1147,6 +1146,15 @@ static int tegra_dc_probe(struct platform_device *pdev) dc->client.ops = &dc_client_ops; dc->client.dev = &pdev->dev;
+#ifdef CONFIG_PM_RUNTIME + pm_runtime_enable(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); +#else + err = clk_prepare_enable(dc->clk); + if (err < 0) + return err; +#endif + err = tegra_dc_rgb_probe(dc); if (err < 0 && err != -ENODEV) { dev_err(&pdev->dev, "failed to probe RGB output: %d\n", err); @@ -1160,8 +1168,6 @@ static int tegra_dc_probe(struct platform_device *pdev) return err; }
- platform_set_drvdata(pdev, dc); - return 0; }
@@ -1178,11 +1184,52 @@ static int tegra_dc_remove(struct platform_device *pdev) return err; }
+#ifdef CONFIG_PM_RUNTIME + pm_runtime_put(&pdev->dev); + pm_runtime_disable(&pdev->dev); +#endif + + return 0; +} + +#ifdef CONFIG_PM_RUNTIME +static int tegra_dc_runtime_suspend(struct device *dev) +{ + struct tegra_dc *dc; + + dc = dev_get_drvdata(dev); + if (!dc) + return -EINVAL; + clk_disable_unprepare(dc->clk);
return 0; }
+static int tegra_dc_runtime_resume(struct device *dev) +{ + int err = 0; + struct tegra_dc *dc; + + dc = dev_get_drvdata(dev); + if (!dc) + return -EINVAL; + + err = clk_prepare_enable(dc->clk); + if (err < 0) + dev_err(dev, "failed to enable clock\n"); + + return err; +} +#endif /* CONFIG_PM_RUNTIME */ + +#ifdef CONFIG_PM +static const struct dev_pm_ops tegra_dc_pm_ops = { + SET_RUNTIME_PM_OPS(tegra_dc_runtime_suspend, + tegra_dc_runtime_resume, NULL) +}; +#endif + static struct of_device_id tegra_dc_of_match[] = { { .compatible = "nvidia,tegra30-dc", }, { .compatible = "nvidia,tegra20-dc", }, @@ -1194,6 +1241,9 @@ struct platform_driver tegra_dc_driver = { .name = "tegra-dc", .owner = THIS_MODULE, .of_match_table = tegra_dc_of_match, +#ifdef CONFIG_PM + .pm = &tegra_dc_pm_ops, +#endif }, .probe = tegra_dc_probe, .remove = tegra_dc_remove,
On Thu, Jun 13, 2013 at 03:23:37PM +0530, Mayuresh Kulkarni wrote: [...]
diff --git a/drivers/gpu/host1x/drm/dc.c b/drivers/gpu/host1x/drm/dc.c
[...]
@@ -1128,9 +1129,7 @@ static int tegra_dc_probe(struct platform_device *pdev) return PTR_ERR(dc->clk); }
- err = clk_prepare_enable(dc->clk);
- if (err < 0)
return err;
- platform_set_drvdata(pdev, dc);
Why do you move the call to platform_set_drvdata() up here?
Thierry
On 06/13/2013 12:49 PM, Thierry Reding wrote:
On Thu, Jun 13, 2013 at 03:23:37PM +0530, Mayuresh Kulkarni wrote: [...]
diff --git a/drivers/gpu/host1x/drm/dc.c b/drivers/gpu/host1x/drm/dc.c
[...]
@@ -1128,9 +1129,7 @@ static int tegra_dc_probe(struct platform_device *pdev) return PTR_ERR(dc->clk); }
- err = clk_prepare_enable(dc->clk); - if (err < 0) - return
err; + platform_set_drvdata(pdev, dc);
Why do you move the call to platform_set_drvdata() up here?
Presumably the suspend/resume functions need to get the value out of the device they're passed, so it needs to be set up early?
On Friday 14 June 2013 12:39 AM, Stephen Warren wrote:
On 06/13/2013 12:49 PM, Thierry Reding wrote:
On Thu, Jun 13, 2013 at 03:23:37PM +0530, Mayuresh Kulkarni wrote: [...]
diff --git a/drivers/gpu/host1x/drm/dc.c b/drivers/gpu/host1x/drm/dc.c
[...]
@@ -1128,9 +1129,7 @@ static int tegra_dc_probe(struct platform_device *pdev) return PTR_ERR(dc->clk); }
- err = clk_prepare_enable(dc->clk); - if (err < 0) - return
err; + platform_set_drvdata(pdev, dc);
Why do you move the call to platform_set_drvdata() up here?
Presumably the suspend/resume functions need to get the value out of the device they're passed, so it needs to be set up early?
Yes that is correct, Stephen. The run-time pm call-backs need 2 things: correct driver data pointer and correct clock pointer within it. Hence I had to move this line upper in sequence.
Same is applicable to gr2d and host1x patches as well.
Signed-off-by: Mayuresh Kulkarni mkulkarni@nvidia.com --- drivers/gpu/host1x/dev.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)
diff --git a/drivers/gpu/host1x/dev.c b/drivers/gpu/host1x/dev.c index 28e28a2..b43eb29 100644 --- a/drivers/gpu/host1x/dev.c +++ b/drivers/gpu/host1x/dev.c @@ -23,6 +23,7 @@ #include <linux/of_device.h> #include <linux/clk.h> #include <linux/io.h> +#include <linux/pm_runtime.h>
#define CREATE_TRACE_POINTS #include <trace/events/host1x.h> @@ -143,11 +144,16 @@ static int host1x_probe(struct platform_device *pdev) return err; }
+#ifdef CONFIG_PM_RUNTIME + pm_runtime_enable(&pdev->dev); + pm_runtime_get_sync(&pdev->dev); +#else err = clk_prepare_enable(host->clk); if (err < 0) { dev_err(&pdev->dev, "failed to enable clock\n"); return err; } +#endif
err = host1x_syncpt_init(host); if (err) { @@ -165,10 +171,17 @@ static int host1x_probe(struct platform_device *pdev)
host1x_drm_alloc(pdev);
+#ifdef CONFIG_PM_RUNTIME + pm_runtime_put(&pdev->dev); +#endif + return 0;
fail_deinit_syncpt: host1x_syncpt_deinit(host); +#ifdef CONFIG_PM_RUNTIME + pm_runtime_put(&pdev->dev); +#endif return err; }
@@ -179,10 +192,51 @@ static int __exit host1x_remove(struct platform_device *pdev) host1x_intr_deinit(host); host1x_syncpt_deinit(host); clk_disable_unprepare(host->clk); +#ifdef CONFIG_PM_RUNTIME + pm_runtime_disable(&pdev->dev); +#endif + + return 0; +} + +#ifdef CONFIG_PM_RUNTIME +static int host1x_runtime_suspend(struct device *dev) +{ + struct host1x *host; + + host = dev_get_drvdata(dev); + if (!host) + return -EINVAL; + + clk_disable_unprepare(host->clk);
return 0; }
+static int host1x_runtime_resume(struct device *dev) +{ + int err = 0; + struct host1x *host; + + host = dev_get_drvdata(dev); + if (!host) + return -EINVAL; + + err = clk_prepare_enable(host->clk); + if (err < 0) + dev_err(dev, "failed to enable clock\n"); + + return err; +} +#endif /* CONFIG_PM_RUNTIME */ + +#ifdef CONFIG_PM +static const struct dev_pm_ops host1x_pm_ops = { + SET_RUNTIME_PM_OPS(host1x_runtime_suspend, + host1x_runtime_resume, NULL) +}; +#endif + static struct platform_driver tegra_host1x_driver = { .probe = host1x_probe, .remove = __exit_p(host1x_remove), @@ -190,6 +244,9 @@ static struct platform_driver tegra_host1x_driver = { .owner = THIS_MODULE, .name = "tegra-host1x", .of_match_table = host1x_of_match, +#ifdef CONFIG_PM + .pm = &host1x_pm_ops, +#endif }, };
dri-devel@lists.freedesktop.org
-
Mayuresh Kulkarni -
Stephen Warren -
Thierry Reding