Patchwork drm/exynos: dp: remove suspend/resume functions

login
register
mail settings
Submitter Inki Dae
Date Sept. 30, 2015, 11:21 a.m.
Message ID <1443612079-28094-1-git-send-email-inki.dae@samsung.com>
Download mbox | patch
Permalink /patch/7297051/
State New
Headers show

Comments

Inki Dae - Sept. 30, 2015, 11:21 a.m.
This patch removes unnecessary pm suspend/resume functions.

All kms sub drivers will be controlled by top of Exynos drm driver
and connector dpms so these sub drivers shouldn't have their own
pm interfaces.

Signed-off-by: Inki Dae <inki.dae@samsung.com>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c | 23 -----------------------
 1 file changed, 23 deletions(-)
Emil Velikov - Sept. 30, 2015, 12:19 p.m.
Hi Inki,

On 30 September 2015 at 12:21, Inki Dae <inki.dae@samsung.com> wrote:
> This patch removes unnecessary pm suspend/resume functions.
>
> All kms sub drivers will be controlled by top of Exynos drm driver
> and connector dpms so these sub drivers shouldn't have their own
> pm interfaces.
>
Not sure if you've noticed but this patch seems to do the opposite of
what Gustavo was aiming with an earlier series [1].

Cheers,
Emil

[1] http://lists.freedesktop.org/archives/dri-devel/2015-September/089800.html
Inki Dae - Sept. 30, 2015, 1:46 p.m.
Hi Emil,

On 2015? 09? 30? 21:19, Emil Velikov wrote:
> Hi Inki,
> 
> On 30 September 2015 at 12:21, Inki Dae <inki.dae@samsung.com> wrote:
>> This patch removes unnecessary pm suspend/resume functions.
>>
>> All kms sub drivers will be controlled by top of Exynos drm driver
>> and connector dpms so these sub drivers shouldn't have their own
>> pm interfaces.
>>
> Not sure if you've noticed but this patch seems to do the opposite of
> what Gustavo was aiming with an earlier series [1].

I removed just the interfaces related to sleep pm not runtime pm. From
long ago, Linux DRM drivers - especially ARM DRM drivers - have been
stepping forward to a integrated DRM driver so it'd be reasonable for
sleep pm operations of all KMS drivers are controlled by top of DRM
driver. It means that Exynos drm driver has already sleep pm interfaces.

However, I think a power domain of each kms device couldn't be
controlled by the top in case of ARM SoC because kms devices can use a
different power domain each other according to Vendor SoC so runtime pm
should be controlled by each kms driver, which will be triggered by DRM top.

And Gustavo's patch set you mentioned - now being reviewed - will add
only runtime pm interfaces to each kms driver.

Thanks,
Inki Dae

> 
> Cheers,
> Emil
> 
> [1] http://lists.freedesktop.org/archives/dri-devel/2015-September/089800.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Gustavo F. Padovan - Sept. 30, 2015, 2:25 p.m.
Hi Inki,

2015-09-30 Inki Dae <inki.dae@samsung.com>:

> This patch removes unnecessary pm suspend/resume functions.
> 
> All kms sub drivers will be controlled by top of Exynos drm driver
> and connector dpms so these sub drivers shouldn't have their own
> pm interfaces.
> 
> Signed-off-by: Inki Dae <inki.dae@samsung.com>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c | 23 -----------------------
>  1 file changed, 23 deletions(-)

This sounds reasonable to me.

Reviewed-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

	Gustavo
Emil Velikov - Sept. 30, 2015, 2:36 p.m.
On 30 September 2015 at 14:46, Inki Dae <inki.dae@samsung.com> wrote:
> Hi Emil,
>
> On 2015? 09? 30? 21:19, Emil Velikov wrote:
>> Hi Inki,
>>
>> On 30 September 2015 at 12:21, Inki Dae <inki.dae@samsung.com> wrote:
>>> This patch removes unnecessary pm suspend/resume functions.
>>>
>>> All kms sub drivers will be controlled by top of Exynos drm driver
>>> and connector dpms so these sub drivers shouldn't have their own
>>> pm interfaces.
>>>
>> Not sure if you've noticed but this patch seems to do the opposite of
>> what Gustavo was aiming with an earlier series [1].
>
> I removed just the interfaces related to sleep pm not runtime pm. From
> long ago, Linux DRM drivers - especially ARM DRM drivers - have been
> stepping forward to a integrated DRM driver so it'd be reasonable for
> sleep pm operations of all KMS drivers are controlled by top of DRM
> driver. It means that Exynos drm driver has already sleep pm interfaces.
>
> However, I think a power domain of each kms device couldn't be
> controlled by the top in case of ARM SoC because kms devices can use a
> different power domain each other according to Vendor SoC so runtime pm
> should be controlled by each kms driver, which will be triggered by DRM top.
>
> And Gustavo's patch set you mentioned - now being reviewed - will add
> only runtime pm interfaces to each kms driver.
>
Ack. Seems like I misread your patch. Thank you for the comprehensive
answer and apologies for the noise.

Thanks
Emil

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index d66ade0..124fb9a 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1383,28 +1383,6 @@  static int exynos_dp_remove(struct platform_device *pdev)
 	return 0;
 }
 
-#ifdef CONFIG_PM_SLEEP
-static int exynos_dp_suspend(struct device *dev)
-{
-	struct exynos_dp_device *dp = dev_get_drvdata(dev);
-
-	exynos_dp_disable(&dp->encoder);
-	return 0;
-}
-
-static int exynos_dp_resume(struct device *dev)
-{
-	struct exynos_dp_device *dp = dev_get_drvdata(dev);
-
-	exynos_dp_enable(&dp->encoder);
-	return 0;
-}
-#endif
-
-static const struct dev_pm_ops exynos_dp_pm_ops = {
-	SET_SYSTEM_SLEEP_PM_OPS(exynos_dp_suspend, exynos_dp_resume)
-};
-
 static const struct of_device_id exynos_dp_match[] = {
 	{ .compatible = "samsung,exynos5-dp" },
 	{},
@@ -1417,7 +1395,6 @@  struct platform_driver dp_driver = {
 	.driver		= {
 		.name	= "exynos-dp",
 		.owner	= THIS_MODULE,
-		.pm	= &exynos_dp_pm_ops,
 		.of_match_table = exynos_dp_match,
 	},
 };