[PATCH v2 0/4] gpu: host1x: add runtime pm support

[PATCH v2 0/4] gpu: host1x: add runtime pm support

[Mayuresh Kulkarni]

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(-)

-- 
1.8.1.5

[PATCH v2 1/4] gpu: host1x: shuffle job APIs

[Mayuresh Kulkarni]

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-DDmLM1+adcrQT0dZR+AlfA@public.gmane.org>
---
 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
-- 
1.8.1.5

[PATCH v2 2/4] gpu: host1x: add runtime pm support for gr2d

[Mayuresh Kulkarni]

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
 }
-- 
1.8.1.5

[PATCH v2 3/4] gpu: host1x: add runtime pm support for dc

[Mayuresh Kulkarni]

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,
-- 
1.8.1.5

[PATCH v2 4/4] gpu: host1x: add runtime pm support for host1x

[Mayuresh Kulkarni]

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
 	},
 };
 
-- 
1.8.1.5

Re: [PATCH v2 2/4] gpu: host1x: add runtime pm support for gr2d

[Stephen Warren]

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.

Re: [PATCH v2 3/4] gpu: host1x: add runtime pm support for dc

[Thierry Reding]

[-- Attachment #1: Type: text/plain, Size: 467 bytes --]

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

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH v2 3/4] gpu: host1x: add runtime pm support for dc

[Stephen Warren]

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?

Re: [PATCH v2 2/4] gpu: host1x: add runtime pm support for gr2d

[Mayuresh Kulkarni]

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).

Re: [PATCH v2 3/4] gpu: host1x: add runtime pm support for dc

[Mayuresh Kulkarni]

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.