[PATCH 00/10] drm/exynos: add pm_runtime support

[PATCH 00/10] drm/exynos: add pm_runtime support

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Hi,

This series adds proper runtime PM suport to CRTCs and Encoders, so
now instead of relying on 'suspended' or 'enabled' flags to track when
the CRTC or Encoder is enabled we let the pm_runtime subsystem do it for us
and remove all the flags. This is a important step to the atomic suspend/resume
support that will land in drm anytime soon.

Please review!

	Gustavo

Gustavo Padovan (10):
  drm/exynos: do not start enabling DP at bind() phase
  drm/exynos: add pm_runtime to DP
  drm/exynos: add pm_runtime to HDMI
  drm/exynos: add pm_runtime to Mixer
  drm/exynos: remove exynos_crtc commit() callback
  drm/exynos: Remove exynos_crtc commit() callback
  drm/exynos: add pm_runtime to FIMD
  drm/exynos: Enable DP clock directly from FIMD
  drm/exynos: add pm_runtime to DECON 5433
  drm/exynos: add pm_runtime to DECON 7

 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  90 ++++++++----------
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 126 +++++++++++--------------
 drivers/gpu/drm/exynos/exynos_dp_core.c       |  57 ++++++------
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      |  10 --
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |   7 --
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 129 ++++++++++++--------------
 drivers/gpu/drm/exynos/exynos_hdmi.c          |  56 +++++++----
 drivers/gpu/drm/exynos/exynos_mixer.c         | 125 ++++++++++++-------------
 8 files changed, 275 insertions(+), 325 deletions(-)

-- 
2.1.0

[PATCH 01/10] drm/exynos: do not start enabling DP at bind() phase

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

The DP device will be properly enabled at the enable() call just
after the bind call finishes.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index d66ade0..c73aff1 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1289,10 +1289,6 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 
 	INIT_WORK(&dp->hotplug_work, exynos_dp_hotplug);
 
-	phy_power_on(dp->phy);
-
-	exynos_dp_init_dp(dp);
-
 	ret = devm_request_irq(&pdev->dev, dp->irq, exynos_dp_irq_handler,
 			irq_flags, "exynos-dp", dp);
 	if (ret) {
-- 
2.1.0

[PATCH 02/10] drm/exynos: add pm_runtime to DP

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Let pm_runtime handle the enabling/disabling of the device with
proper refcnt instead of rely on specific flags to track the enabled
state.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c | 40 +++++++++++++++++++++------------
 1 file changed, 26 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index c73aff1..6794982 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -1070,8 +1070,7 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
 	struct exynos_dp_device *dp = encoder_to_dp(encoder);
 	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
 
-	if (dp->dpms_mode == DRM_MODE_DPMS_ON)
-		return;
+	pm_runtime_get_sync(dp->dev);
 
 	if (dp->panel) {
 		if (drm_panel_prepare(dp->panel)) {
@@ -1083,13 +1082,10 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
 	if (crtc->ops->clock_enable)
 		crtc->ops->clock_enable(dp_to_crtc(dp), true);
 
-	clk_prepare_enable(dp->clock);
 	phy_power_on(dp->phy);
 	exynos_dp_init_dp(dp);
 	enable_irq(dp->irq);
 	exynos_dp_commit(&dp->encoder);
-
-	dp->dpms_mode = DRM_MODE_DPMS_ON;
 }
 
 static void exynos_dp_disable(struct drm_encoder *encoder)
@@ -1097,9 +1093,6 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
 	struct exynos_dp_device *dp = encoder_to_dp(encoder);
 	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
 
-	if (dp->dpms_mode != DRM_MODE_DPMS_ON)
-		return;
-
 	if (dp->panel) {
 		if (drm_panel_disable(dp->panel)) {
 			DRM_ERROR("failed to disable the panel\n");
@@ -1110,7 +1103,6 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
 	disable_irq(dp->irq);
 	flush_work(&dp->hotplug_work);
 	phy_power_off(dp->phy);
-	clk_disable_unprepare(dp->clock);
 
 	if (crtc->ops->clock_enable)
 		crtc->ops->clock_enable(dp_to_crtc(dp), false);
@@ -1120,7 +1112,7 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
 			DRM_ERROR("failed to turnoff the panel\n");
 	}
 
-	dp->dpms_mode = DRM_MODE_DPMS_OFF;
+	pm_runtime_put_sync(dp->dev);
 }
 
 static struct drm_encoder_helper_funcs exynos_dp_encoder_helper_funcs = {
@@ -1216,7 +1208,6 @@ static int exynos_dp_bind(struct device *dev, struct device *master, void *data)
 	int pipe, ret = 0;
 
 	dp->dev = &pdev->dev;
-	dp->dpms_mode = DRM_MODE_DPMS_OFF;
 
 	dp->video_info = exynos_dp_dt_parse_pdata(&pdev->dev);
 	if (IS_ERR(dp->video_info))
@@ -1341,6 +1332,7 @@ static int exynos_dp_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct device_node *panel_node, *bridge_node, *endpoint;
 	struct exynos_dp_device *dp;
+	int ret;
 
 	dp = devm_kzalloc(&pdev->dev, sizeof(struct exynos_dp_device),
 				GFP_KERNEL);
@@ -1369,11 +1361,23 @@ static int exynos_dp_probe(struct platform_device *pdev)
 			return -EPROBE_DEFER;
 	}
 
-	return component_add(&pdev->dev, &exynos_dp_ops);
+	pm_runtime_enable(dev);
+
+	ret = component_add(&pdev->dev, &exynos_dp_ops);
+	if (ret)
+		goto err_disable_pm_runtime;
+
+	return ret;
+
+err_disable_pm_runtime:
+	pm_runtime_disable(dev);
+
+	return ret;
 }
 
 static int exynos_dp_remove(struct platform_device *pdev)
 {
+	pm_runtime_disable(&pdev->dev);
 	component_del(&pdev->dev, &exynos_dp_ops);
 
 	return 0;
@@ -1384,21 +1388,29 @@ static int exynos_dp_suspend(struct device *dev)
 {
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
 
-	exynos_dp_disable(&dp->encoder);
+	clk_disable_unprepare(dp->clock);
+
 	return 0;
 }
 
 static int exynos_dp_resume(struct device *dev)
 {
 	struct exynos_dp_device *dp = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(dp->clock);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the clock clk [%d]\n", ret);
+		return ret;
+	}
 
-	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)
+	SET_RUNTIME_PM_OPS(exynos_dp_suspend, exynos_dp_resume, NULL)
 };
 
 static const struct of_device_id exynos_dp_match[] = {
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/10] drm/exynos: add pm_runtime to HDMI

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Let pm_runtime handle the enabling/disabling of the device with proper
refcnt instead of rely on specific flags to track the enabled state.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 56 +++++++++++++++++++++++++-----------
 1 file changed, 39 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 932f7fa..5fcbdda 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -92,7 +92,6 @@ struct hdmi_context {
 	struct drm_device		*drm_dev;
 	struct drm_connector		connector;
 	bool				hpd;
-	bool				powered;
 	bool				dvi_mode;
 
 	void __iomem			*regs;
@@ -1726,11 +1725,6 @@ static void hdmi_enable(struct drm_encoder *encoder)
 	struct hdmi_context *hdata = encoder_to_hdmi(encoder);
 	struct hdmi_resources *res = &hdata->res;
 
-	if (hdata->powered)
-		return;
-
-	hdata->powered = true;
-
 	pm_runtime_get_sync(hdata->dev);
 
 	if (regulator_bulk_enable(res->regul_count, res->regul_bulk))
@@ -1740,9 +1734,6 @@ static void hdmi_enable(struct drm_encoder *encoder)
 	regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
 			PMU_HDMI_PHY_ENABLE_BIT, 1);
 
-	clk_prepare_enable(res->hdmi);
-	clk_prepare_enable(res->sclk_hdmi);
-
 	hdmiphy_poweron(hdata);
 	hdmi_conf_apply(hdata);
 }
@@ -1754,9 +1745,6 @@ static void hdmi_disable(struct drm_encoder *encoder)
 	struct drm_crtc *crtc = encoder->crtc;
 	const struct drm_crtc_helper_funcs *funcs = NULL;
 
-	if (!hdata->powered)
-		return;
-
 	/*
 	 * The SFRs of VP and Mixer are updated by Vertical Sync of
 	 * Timing generator which is a part of HDMI so the sequence
@@ -1778,9 +1766,6 @@ static void hdmi_disable(struct drm_encoder *encoder)
 
 	cancel_delayed_work(&hdata->hotplug_work);
 
-	clk_disable_unprepare(res->sclk_hdmi);
-	clk_disable_unprepare(res->hdmi);
-
 	/* reset pmu hdmiphy control bit to disable hdmiphy */
 	regmap_update_bits(hdata->pmureg, PMU_HDMI_PHY_CONTROL,
 			PMU_HDMI_PHY_ENABLE_BIT, 0);
@@ -1788,8 +1773,6 @@ static void hdmi_disable(struct drm_encoder *encoder)
 	regulator_bulk_disable(res->regul_count, res->regul_bulk);
 
 	pm_runtime_put_sync(hdata->dev);
-
-	hdata->powered = false;
 }
 
 static struct drm_encoder_helper_funcs exynos_hdmi_encoder_helper_funcs = {
@@ -2146,12 +2129,51 @@ static int hdmi_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int exynos_hdmi_suspend(struct device *dev)
+{
+	struct hdmi_context *hdata = dev_get_drvdata(dev);
+	struct hdmi_resources *res = &hdata->res;
+
+	clk_disable_unprepare(res->sclk_hdmi);
+	clk_disable_unprepare(res->hdmi);
+
+	return 0;
+}
+
+static int exynos_hdmi_resume(struct device *dev)
+{
+	struct hdmi_context *hdata = dev_get_drvdata(dev);
+	struct hdmi_resources *res = &hdata->res;
+	int ret;
+
+	ret = clk_prepare_enable(res->hdmi);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the hdmi clk [%d]\n", ret);
+		return ret;
+	}
+	ret = clk_prepare_enable(res->sclk_hdmi);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the sclk_mixer clk [%d]\n",
+			  ret);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops exynos_hdmi_pm_ops = {
+	SET_RUNTIME_PM_OPS(exynos_hdmi_suspend, exynos_hdmi_resume, NULL)
+};
+
 struct platform_driver hdmi_driver = {
 	.probe		= hdmi_probe,
 	.remove		= hdmi_remove,
 	.driver		= {
 		.name	= "exynos-hdmi",
 		.owner	= THIS_MODULE,
+		.pm	= &exynos_hdmi_pm_ops,
 		.of_match_table = hdmi_match_types,
 	},
 };
-- 
2.1.0

[PATCH 04/10] drm/exynos: add pm_runtime to Mixer

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Let pm_runtime handle the enabling/disabling of the device with proper
refcnt instead of rely on specific flags to track the enabled state.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_mixer.c | 125 +++++++++++++++++-----------------
 1 file changed, 61 insertions(+), 64 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 2484277..dc48084 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -71,7 +71,6 @@ enum mixer_version_id {
 };
 
 enum mixer_flag_bits {
-	MXR_BIT_POWERED,
 	MXR_BIT_VSYNC,
 };
 
@@ -927,8 +926,6 @@ static int mixer_enable_vblank(struct exynos_drm_crtc *crtc)
 	struct mixer_resources *res = &mixer_ctx->mixer_res;
 
 	__set_bit(MXR_BIT_VSYNC, &mixer_ctx->flags);
-	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
-		return 0;
 
 	/* enable vsync interrupt */
 	mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
@@ -944,9 +941,6 @@ static void mixer_disable_vblank(struct exynos_drm_crtc *crtc)
 
 	__clear_bit(MXR_BIT_VSYNC, &mixer_ctx->flags);
 
-	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
-		return;
-
 	/* disable vsync interrupt */
 	mixer_reg_writemask(res, MXR_INT_STATUS, ~0, MXR_INT_CLEAR_VSYNC);
 	mixer_reg_writemask(res, MXR_INT_EN, 0, MXR_INT_EN_VSYNC);
@@ -959,9 +953,6 @@ static void mixer_update_plane(struct exynos_drm_crtc *crtc,
 
 	DRM_DEBUG_KMS("win: %d\n", plane->zpos);
 
-	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
-		return;
-
 	if (plane->zpos > 1 && mixer_ctx->vp_enabled)
 		vp_video_buffer(mixer_ctx, plane);
 	else
@@ -977,9 +968,6 @@ static void mixer_disable_plane(struct exynos_drm_crtc *crtc,
 
 	DRM_DEBUG_KMS("win: %d\n", plane->zpos);
 
-	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
-		return;
-
 	spin_lock_irqsave(&res->reg_slock, flags);
 	mixer_vsync_set_update(mixer_ctx, false);
 
@@ -994,9 +982,6 @@ static void mixer_wait_for_vblank(struct exynos_drm_crtc *crtc)
 	struct mixer_context *mixer_ctx = crtc->ctx;
 	int err;
 
-	if (!test_bit(MXR_BIT_POWERED, &mixer_ctx->flags))
-		return;
-
 	err = drm_vblank_get(mixer_ctx->drm_dev, mixer_ctx->pipe);
 	if (err < 0) {
 		DRM_DEBUG_KMS("failed to acquire vblank counter\n");
@@ -1021,43 +1006,9 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)
 {
 	struct mixer_context *ctx = crtc->ctx;
 	struct mixer_resources *res = &ctx->mixer_res;
-	int ret;
-
-	if (test_bit(MXR_BIT_POWERED, &ctx->flags))
-		return;
 
 	pm_runtime_get_sync(ctx->dev);
 
-	ret = clk_prepare_enable(res->mixer);
-	if (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the mixer clk [%d]\n", ret);
-		return;
-	}
-	ret = clk_prepare_enable(res->hdmi);
-	if (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the hdmi clk [%d]\n", ret);
-		return;
-	}
-	if (ctx->vp_enabled) {
-		ret = clk_prepare_enable(res->vp);
-		if (ret < 0) {
-			DRM_ERROR("Failed to prepare_enable the vp clk [%d]\n",
-				  ret);
-			return;
-		}
-		if (ctx->has_sclk) {
-			ret = clk_prepare_enable(res->sclk_mixer);
-			if (ret < 0) {
-				DRM_ERROR("Failed to prepare_enable the " \
-					   "sclk_mixer clk [%d]\n",
-					  ret);
-				return;
-			}
-		}
-	}
-
-	set_bit(MXR_BIT_POWERED, &ctx->flags);
-
 	mixer_reg_writemask(res, MXR_STATUS, ~0, MXR_STATUS_SOFT_RESET);
 
 	if (test_bit(MXR_BIT_VSYNC, &ctx->flags)) {
@@ -1070,29 +1021,15 @@ static void mixer_enable(struct exynos_drm_crtc *crtc)
 static void mixer_disable(struct exynos_drm_crtc *crtc)
 {
 	struct mixer_context *ctx = crtc->ctx;
-	struct mixer_resources *res = &ctx->mixer_res;
 	int i;
 
-	if (!test_bit(MXR_BIT_POWERED, &ctx->flags))
-		return;
-
 	mixer_stop(ctx);
 	mixer_regs_dump(ctx);
 
 	for (i = 0; i < MIXER_WIN_NR; i++)
 		mixer_disable_plane(crtc, &ctx->planes[i]);
 
-	clear_bit(MXR_BIT_POWERED, &ctx->flags);
-
-	clk_disable_unprepare(res->hdmi);
-	clk_disable_unprepare(res->mixer);
-	if (ctx->vp_enabled) {
-		clk_disable_unprepare(res->vp);
-		if (ctx->has_sclk)
-			clk_disable_unprepare(res->sclk_mixer);
-	}
-
-	pm_runtime_put_sync(ctx->dev);
+	pm_runtime_put(ctx->dev);
 }
 
 /* Only valid for Mixer version 16.0.33.0 */
@@ -1288,10 +1225,70 @@ static int mixer_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int exynos_mixer_suspend(struct device *dev)
+{
+	struct mixer_context *ctx = dev_get_drvdata(dev);
+	struct mixer_resources *res = &ctx->mixer_res;
+
+	clk_disable_unprepare(res->hdmi);
+	clk_disable_unprepare(res->mixer);
+	if (ctx->vp_enabled) {
+		clk_disable_unprepare(res->vp);
+		if (ctx->has_sclk)
+			clk_disable_unprepare(res->sclk_mixer);
+	}
+
+	return 0;
+}
+
+static int exynos_mixer_resume(struct device *dev)
+{
+	struct mixer_context *ctx = dev_get_drvdata(dev);
+	struct mixer_resources *res = &ctx->mixer_res;
+	int ret;
+
+	ret = clk_prepare_enable(res->mixer);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the mixer clk [%d]\n", ret);
+		return ret;
+	}
+	ret = clk_prepare_enable(res->hdmi);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the hdmi clk [%d]\n", ret);
+		return ret;
+	}
+	if (ctx->vp_enabled) {
+		ret = clk_prepare_enable(res->vp);
+		if (ret < 0) {
+			DRM_ERROR("Failed to prepare_enable the vp clk [%d]\n",
+				  ret);
+			return ret;
+		}
+		if (ctx->has_sclk) {
+			ret = clk_prepare_enable(res->sclk_mixer);
+			if (ret < 0) {
+				DRM_ERROR("Failed to prepare_enable the " \
+					   "sclk_mixer clk [%d]\n",
+					  ret);
+				return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops exynos_mixer_pm_ops = {
+	SET_RUNTIME_PM_OPS(exynos_mixer_suspend, exynos_mixer_resume, NULL)
+};
+
 struct platform_driver mixer_driver = {
 	.driver = {
 		.name = "exynos-mixer",
 		.owner = THIS_MODULE,
+		.pm = &exynos_mixer_pm_ops,
 		.of_match_table = mixer_match_types,
 	},
 	.probe = mixer_probe,
-- 
2.1.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/10] drm/exynos: remove exynos_crtc commit() callback

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

It turns out that .commit() was never executed, because
at the time .mode_set_nofb() called it ctx->suspended was still false
and .commit() would return. It removes the callback from FIMD DECON 7 and
DECON 5433.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 2697ebc..0bbe537 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -888,7 +888,6 @@ static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.enable = fimd_enable,
 	.disable = fimd_disable,
 	.mode_fixup = fimd_mode_fixup,
-	.commit = fimd_commit,
 	.enable_vblank = fimd_enable_vblank,
 	.disable_vblank = fimd_disable_vblank,
 	.wait_for_vblank = fimd_wait_for_vblank,
-- 
2.1.0

[PATCH 06/10] drm/exynos: Remove exynos_crtc commit() callback

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

This callback is no longer used by any of the exynos_crtc drivers, remove
it.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_drm_crtc.c | 10 ----------
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  2 --
 2 files changed, 12 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index 0872aa2f..deff5a7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -55,15 +55,6 @@ exynos_drm_crtc_mode_fixup(struct drm_crtc *crtc,
 	return true;
 }
 
-static void
-exynos_drm_crtc_mode_set_nofb(struct drm_crtc *crtc)
-{
-	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
-
-	if (exynos_crtc->ops->commit)
-		exynos_crtc->ops->commit(exynos_crtc);
-}
-
 static void exynos_crtc_atomic_begin(struct drm_crtc *crtc,
 				     struct drm_crtc_state *old_crtc_state)
 {
@@ -100,7 +91,6 @@ static struct drm_crtc_helper_funcs exynos_crtc_helper_funcs = {
 	.enable		= exynos_drm_crtc_enable,
 	.disable	= exynos_drm_crtc_disable,
 	.mode_fixup	= exynos_drm_crtc_mode_fixup,
-	.mode_set_nofb	= exynos_drm_crtc_mode_set_nofb,
 	.atomic_begin	= exynos_crtc_atomic_begin,
 	.atomic_flush	= exynos_crtc_atomic_flush,
 };
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index cc56c3d..5f1a4d6 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -86,7 +86,6 @@ struct exynos_drm_plane {
  * @enable: enable the device
  * @disable: disable the device
  * @mode_fixup: fix mode data before applying it
- * @commit: set current hw specific display mode to hw.
  * @enable_vblank: specific driver callback for enabling vblank interrupt.
  * @disable_vblank: specific driver callback for disabling vblank interrupt.
  * @wait_for_vblank: wait for vblank interrupt to make sure that
@@ -109,7 +108,6 @@ struct exynos_drm_crtc_ops {
 	bool (*mode_fixup)(struct exynos_drm_crtc *crtc,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode);
-	void (*commit)(struct exynos_drm_crtc *crtc);
 	int (*enable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*disable_vblank)(struct exynos_drm_crtc *crtc);
 	void (*wait_for_vblank)(struct exynos_drm_crtc *crtc);
-- 
2.1.0

[PATCH 07/10] drm/exynos: add pm_runtime to FIMD

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Let pm_runtime handle the enabling/disabling of the device with proper
refcnt instead of rely on specific flags to track the enabled state.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  1 -
 drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  1 -
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 91 +++++++++++----------------
 3 files changed, 37 insertions(+), 56 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 79b2b22..838a9c1 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -478,7 +478,6 @@ static struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.commit			= decon_commit,
 	.enable_vblank		= decon_enable_vblank,
 	.disable_vblank		= decon_disable_vblank,
-	.commit			= decon_commit,
 	.atomic_begin		= decon_atomic_begin,
 	.update_plane		= decon_update_plane,
 	.disable_plane		= decon_disable_plane,
diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index f3826dc..e4646e2 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -637,7 +637,6 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.enable = decon_enable,
 	.disable = decon_disable,
 	.mode_fixup = decon_mode_fixup,
-	.commit = decon_commit,
 	.enable_vblank = decon_enable_vblank,
 	.disable_vblank = decon_disable_vblank,
 	.wait_for_vblank = decon_wait_for_vblank,
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 0bbe537..0f17ae0 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -160,7 +160,6 @@ struct fimd_context {
 	u32				vidout_con;
 	u32				i80ifcon;
 	bool				i80_if;
-	bool				suspended;
 	int				pipe;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
@@ -209,9 +208,6 @@ static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
 	struct fimd_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (ctx->suspended)
-		return -EPERM;
-
 	if (!test_and_set_bit(0, &ctx->irq_flags)) {
 		val = readl(ctx->regs + VIDINTCON0);
 
@@ -241,9 +237,6 @@ static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
 	struct fimd_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (ctx->suspended)
-		return;
-
 	if (test_and_clear_bit(0, &ctx->irq_flags)) {
 		val = readl(ctx->regs + VIDINTCON0);
 
@@ -264,9 +257,6 @@ static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct fimd_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	atomic_set(&ctx->wait_vsync_event, 1);
 
 	/*
@@ -339,14 +329,12 @@ static void fimd_clear_channels(struct exynos_drm_crtc *crtc)
 		int pipe = ctx->pipe;
 
 		/* ensure that vblank interrupt won't be reported to core */
-		ctx->suspended = false;
 		ctx->pipe = -1;
 
 		fimd_enable_vblank(ctx->crtc);
 		fimd_wait_for_vblank(ctx->crtc);
 		fimd_disable_vblank(ctx->crtc);
 
-		ctx->suspended = true;
 		ctx->pipe = pipe;
 	}
 
@@ -394,9 +382,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
 	void *timing_base = ctx->regs + driver_data->timing_base;
 	u32 val, clkdiv;
 
-	if (ctx->suspended)
-		return;
-
 	/* nothing to do if we haven't set the mode yet */
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return;
@@ -630,9 +615,6 @@ static void fimd_atomic_begin(struct exynos_drm_crtc *crtc,
 {
 	struct fimd_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	fimd_shadow_protect_win(ctx, plane->zpos, true);
 }
 
@@ -641,9 +623,6 @@ static void fimd_atomic_flush(struct exynos_drm_crtc *crtc,
 {
 	struct fimd_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	fimd_shadow_protect_win(ctx, plane->zpos, false);
 }
 
@@ -659,9 +638,6 @@ static void fimd_update_plane(struct exynos_drm_crtc *crtc,
 	unsigned int bpp = state->fb->bits_per_pixel >> 3;
 	unsigned int pitch = state->fb->pitches[0];
 
-	if (ctx->suspended)
-		return;
-
 	offset = plane->src_x * bpp;
 	offset += plane->src_y * pitch;
 
@@ -743,9 +719,6 @@ static void fimd_disable_plane(struct exynos_drm_crtc *crtc,
 	struct fimd_context *ctx = crtc->ctx;
 	unsigned int win = plane->zpos;
 
-	if (ctx->suspended)
-		return;
-
 	fimd_enable_video_output(ctx, win, false);
 
 	if (ctx->driver_data->has_shadowcon)
@@ -755,27 +728,9 @@ static void fimd_disable_plane(struct exynos_drm_crtc *crtc,
 static void fimd_enable(struct exynos_drm_crtc *crtc)
 {
 	struct fimd_context *ctx = crtc->ctx;
-	int ret;
-
-	if (!ctx->suspended)
-		return;
-
-	ctx->suspended = false;
 
 	pm_runtime_get_sync(ctx->dev);
 
-	ret = clk_prepare_enable(ctx->bus_clk);
-	if (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
-		return;
-	}
-
-	ret = clk_prepare_enable(ctx->lcd_clk);
-	if  (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret);
-		return;
-	}
-
 	/* if vblank was enabled status, enable it again. */
 	if (test_and_clear_bit(0, &ctx->irq_flags))
 		fimd_enable_vblank(ctx->crtc);
@@ -788,9 +743,6 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
 	struct fimd_context *ctx = crtc->ctx;
 	int i;
 
-	if (ctx->suspended)
-		return;
-
 	/*
 	 * We need to make sure that all windows are disabled before we
 	 * suspend that connector. Otherwise we might try to scan from
@@ -805,12 +757,7 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
 
 	writel(0, ctx->regs + VIDCON0);
 
-	clk_disable_unprepare(ctx->lcd_clk);
-	clk_disable_unprepare(ctx->bus_clk);
-
 	pm_runtime_put_sync(ctx->dev);
-
-	ctx->suspended = true;
 }
 
 static void fimd_trigger(struct device *dev)
@@ -1019,7 +966,6 @@ static int fimd_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ctx->dev = dev;
-	ctx->suspended = true;
 	ctx->driver_data = drm_fimd_get_driver_data(pdev);
 
 	if (of_property_read_bool(dev->of_node, "samsung,invert-vden"))
@@ -1129,12 +1075,49 @@ static int fimd_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int exynos_fimd_suspend(struct device *dev)
+{
+	struct fimd_context *ctx = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(ctx->lcd_clk);
+	clk_disable_unprepare(ctx->bus_clk);
+
+	return 0;
+}
+
+static int exynos_fimd_resume(struct device *dev)
+{
+	struct fimd_context *ctx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(ctx->bus_clk);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ctx->lcd_clk);
+	if  (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops exynos_fimd_pm_ops = {
+	SET_RUNTIME_PM_OPS(exynos_fimd_suspend, exynos_fimd_resume, NULL)
+};
+
 struct platform_driver fimd_driver = {
 	.probe		= fimd_probe,
 	.remove		= fimd_remove,
 	.driver		= {
 		.name	= "exynos4-fb",
 		.owner	= THIS_MODULE,
+		.pm	= &exynos_fimd_pm_ops,
 		.of_match_table = fimd_driver_dt_match,
 	},
 };
-- 
2.1.0

Re: [PATCH 07/10] drm/exynos: add pm_runtime to FIMD

[Inki Dae]

On 2015년 09월 05일 05:15, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Let pm_runtime handle the enabling/disabling of the device with proper
> refcnt instead of rely on specific flags to track the enabled state.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  1 -
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  1 -
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 91 +++++++++++----------------
>  3 files changed, 37 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> index 79b2b22..838a9c1 100644
> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
> @@ -478,7 +478,6 @@ static struct exynos_drm_crtc_ops decon_crtc_ops = {
>  	.commit			= decon_commit,
>  	.enable_vblank		= decon_enable_vblank,
>  	.disable_vblank		= decon_disable_vblank,
> -	.commit			= decon_commit,

Above wouldn't be related to this patch.

>  	.atomic_begin		= decon_atomic_begin,
>  	.update_plane		= decon_update_plane,
>  	.disable_plane		= decon_disable_plane,
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index f3826dc..e4646e2 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -637,7 +637,6 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
>  	.enable = decon_enable,
>  	.disable = decon_disable,
>  	.mode_fixup = decon_mode_fixup,
> -	.commit = decon_commit,

Ditto.

Thanks,
Inki Dae

>  	.enable_vblank = decon_enable_vblank,
>  	.disable_vblank = decon_disable_vblank,
>  	.wait_for_vblank = decon_wait_for_vblank,
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 0bbe537..0f17ae0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -160,7 +160,6 @@ struct fimd_context {
>  	u32				vidout_con;
>  	u32				i80ifcon;
>  	bool				i80_if;
> -	bool				suspended;
>  	int				pipe;
>  	wait_queue_head_t		wait_vsync_queue;
>  	atomic_t			wait_vsync_event;
> @@ -209,9 +208,6 @@ static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
>  	struct fimd_context *ctx = crtc->ctx;
>  	u32 val;
>  
> -	if (ctx->suspended)
> -		return -EPERM;
> -
>  	if (!test_and_set_bit(0, &ctx->irq_flags)) {
>  		val = readl(ctx->regs + VIDINTCON0);
>  
> @@ -241,9 +237,6 @@ static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
>  	struct fimd_context *ctx = crtc->ctx;
>  	u32 val;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	if (test_and_clear_bit(0, &ctx->irq_flags)) {
>  		val = readl(ctx->regs + VIDINTCON0);
>  
> @@ -264,9 +257,6 @@ static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
>  {
>  	struct fimd_context *ctx = crtc->ctx;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	atomic_set(&ctx->wait_vsync_event, 1);
>  
>  	/*
> @@ -339,14 +329,12 @@ static void fimd_clear_channels(struct exynos_drm_crtc *crtc)
>  		int pipe = ctx->pipe;
>  
>  		/* ensure that vblank interrupt won't be reported to core */
> -		ctx->suspended = false;
>  		ctx->pipe = -1;
>  
>  		fimd_enable_vblank(ctx->crtc);
>  		fimd_wait_for_vblank(ctx->crtc);
>  		fimd_disable_vblank(ctx->crtc);
>  
> -		ctx->suspended = true;
>  		ctx->pipe = pipe;
>  	}
>  
> @@ -394,9 +382,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
>  	void *timing_base = ctx->regs + driver_data->timing_base;
>  	u32 val, clkdiv;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	/* nothing to do if we haven't set the mode yet */
>  	if (mode->htotal == 0 || mode->vtotal == 0)
>  		return;
> @@ -630,9 +615,6 @@ static void fimd_atomic_begin(struct exynos_drm_crtc *crtc,
>  {
>  	struct fimd_context *ctx = crtc->ctx;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	fimd_shadow_protect_win(ctx, plane->zpos, true);
>  }
>  
> @@ -641,9 +623,6 @@ static void fimd_atomic_flush(struct exynos_drm_crtc *crtc,
>  {
>  	struct fimd_context *ctx = crtc->ctx;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	fimd_shadow_protect_win(ctx, plane->zpos, false);
>  }
>  
> @@ -659,9 +638,6 @@ static void fimd_update_plane(struct exynos_drm_crtc *crtc,
>  	unsigned int bpp = state->fb->bits_per_pixel >> 3;
>  	unsigned int pitch = state->fb->pitches[0];
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	offset = plane->src_x * bpp;
>  	offset += plane->src_y * pitch;
>  
> @@ -743,9 +719,6 @@ static void fimd_disable_plane(struct exynos_drm_crtc *crtc,
>  	struct fimd_context *ctx = crtc->ctx;
>  	unsigned int win = plane->zpos;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	fimd_enable_video_output(ctx, win, false);
>  
>  	if (ctx->driver_data->has_shadowcon)
> @@ -755,27 +728,9 @@ static void fimd_disable_plane(struct exynos_drm_crtc *crtc,
>  static void fimd_enable(struct exynos_drm_crtc *crtc)
>  {
>  	struct fimd_context *ctx = crtc->ctx;
> -	int ret;
> -
> -	if (!ctx->suspended)
> -		return;
> -
> -	ctx->suspended = false;
>  
>  	pm_runtime_get_sync(ctx->dev);
>  
> -	ret = clk_prepare_enable(ctx->bus_clk);
> -	if (ret < 0) {
> -		DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
> -		return;
> -	}
> -
> -	ret = clk_prepare_enable(ctx->lcd_clk);
> -	if  (ret < 0) {
> -		DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret);
> -		return;
> -	}
> -
>  	/* if vblank was enabled status, enable it again. */
>  	if (test_and_clear_bit(0, &ctx->irq_flags))
>  		fimd_enable_vblank(ctx->crtc);
> @@ -788,9 +743,6 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
>  	struct fimd_context *ctx = crtc->ctx;
>  	int i;
>  
> -	if (ctx->suspended)
> -		return;
> -
>  	/*
>  	 * We need to make sure that all windows are disabled before we
>  	 * suspend that connector. Otherwise we might try to scan from
> @@ -805,12 +757,7 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
>  
>  	writel(0, ctx->regs + VIDCON0);
>  
> -	clk_disable_unprepare(ctx->lcd_clk);
> -	clk_disable_unprepare(ctx->bus_clk);
> -
>  	pm_runtime_put_sync(ctx->dev);
> -
> -	ctx->suspended = true;
>  }
>  
>  static void fimd_trigger(struct device *dev)
> @@ -1019,7 +966,6 @@ static int fimd_probe(struct platform_device *pdev)
>  		return -ENOMEM;
>  
>  	ctx->dev = dev;
> -	ctx->suspended = true;
>  	ctx->driver_data = drm_fimd_get_driver_data(pdev);
>  
>  	if (of_property_read_bool(dev->of_node, "samsung,invert-vden"))
> @@ -1129,12 +1075,49 @@ static int fimd_remove(struct platform_device *pdev)
>  	return 0;
>  }
>  
> +#ifdef CONFIG_PM_SLEEP
> +static int exynos_fimd_suspend(struct device *dev)
> +{
> +	struct fimd_context *ctx = dev_get_drvdata(dev);
> +
> +	clk_disable_unprepare(ctx->lcd_clk);
> +	clk_disable_unprepare(ctx->bus_clk);
> +
> +	return 0;
> +}
> +
> +static int exynos_fimd_resume(struct device *dev)
> +{
> +	struct fimd_context *ctx = dev_get_drvdata(dev);
> +	int ret;
> +
> +	ret = clk_prepare_enable(ctx->bus_clk);
> +	if (ret < 0) {
> +		DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
> +		return ret;
> +	}
> +
> +	ret = clk_prepare_enable(ctx->lcd_clk);
> +	if  (ret < 0) {
> +		DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret);
> +		return ret;
> +	}
> +
> +	return 0;
> +}
> +#endif
> +
> +static const struct dev_pm_ops exynos_fimd_pm_ops = {
> +	SET_RUNTIME_PM_OPS(exynos_fimd_suspend, exynos_fimd_resume, NULL)
> +};
> +
>  struct platform_driver fimd_driver = {
>  	.probe		= fimd_probe,
>  	.remove		= fimd_remove,
>  	.driver		= {
>  		.name	= "exynos4-fb",
>  		.owner	= THIS_MODULE,
> +		.pm	= &exynos_fimd_pm_ops,
>  		.of_match_table = fimd_driver_dt_match,
>  	},
>  };
> 

Re: [PATCH 07/10] drm/exynos: add pm_runtime to FIMD

[Inki Dae]

Also ping~~

2015년 09월 19일 12:53에 Inki Dae 이(가) 쓴 글:
> On 2015년 09월 05일 05:15, Gustavo Padovan wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>
>> Let pm_runtime handle the enabling/disabling of the device with proper
>> refcnt instead of rely on specific flags to track the enabled state.
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> ---
>>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  1 -
>>   drivers/gpu/drm/exynos/exynos7_drm_decon.c    |  1 -
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 91 +++++++++++----------------
>>   3 files changed, 37 insertions(+), 56 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> index 79b2b22..838a9c1 100644
>> --- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
>> @@ -478,7 +478,6 @@ static struct exynos_drm_crtc_ops decon_crtc_ops = {
>>   	.commit			= decon_commit,
>>   	.enable_vblank		= decon_enable_vblank,
>>   	.disable_vblank		= decon_disable_vblank,
>> -	.commit			= decon_commit,
>
> Above wouldn't be related to this patch.
>
>>   	.atomic_begin		= decon_atomic_begin,
>>   	.update_plane		= decon_update_plane,
>>   	.disable_plane		= decon_disable_plane,
>> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> index f3826dc..e4646e2 100644
>> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
>> @@ -637,7 +637,6 @@ static const struct exynos_drm_crtc_ops decon_crtc_ops = {
>>   	.enable = decon_enable,
>>   	.disable = decon_disable,
>>   	.mode_fixup = decon_mode_fixup,
>> -	.commit = decon_commit,
>
> Ditto.
>
> Thanks,
> Inki Dae
>
>>   	.enable_vblank = decon_enable_vblank,
>>   	.disable_vblank = decon_disable_vblank,
>>   	.wait_for_vblank = decon_wait_for_vblank,
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 0bbe537..0f17ae0 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -160,7 +160,6 @@ struct fimd_context {
>>   	u32				vidout_con;
>>   	u32				i80ifcon;
>>   	bool				i80_if;
>> -	bool				suspended;
>>   	int				pipe;
>>   	wait_queue_head_t		wait_vsync_queue;
>>   	atomic_t			wait_vsync_event;
>> @@ -209,9 +208,6 @@ static int fimd_enable_vblank(struct exynos_drm_crtc *crtc)
>>   	struct fimd_context *ctx = crtc->ctx;
>>   	u32 val;
>>
>> -	if (ctx->suspended)
>> -		return -EPERM;
>> -
>>   	if (!test_and_set_bit(0, &ctx->irq_flags)) {
>>   		val = readl(ctx->regs + VIDINTCON0);
>>
>> @@ -241,9 +237,6 @@ static void fimd_disable_vblank(struct exynos_drm_crtc *crtc)
>>   	struct fimd_context *ctx = crtc->ctx;
>>   	u32 val;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	if (test_and_clear_bit(0, &ctx->irq_flags)) {
>>   		val = readl(ctx->regs + VIDINTCON0);
>>
>> @@ -264,9 +257,6 @@ static void fimd_wait_for_vblank(struct exynos_drm_crtc *crtc)
>>   {
>>   	struct fimd_context *ctx = crtc->ctx;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	atomic_set(&ctx->wait_vsync_event, 1);
>>
>>   	/*
>> @@ -339,14 +329,12 @@ static void fimd_clear_channels(struct exynos_drm_crtc *crtc)
>>   		int pipe = ctx->pipe;
>>
>>   		/* ensure that vblank interrupt won't be reported to core */
>> -		ctx->suspended = false;
>>   		ctx->pipe = -1;
>>
>>   		fimd_enable_vblank(ctx->crtc);
>>   		fimd_wait_for_vblank(ctx->crtc);
>>   		fimd_disable_vblank(ctx->crtc);
>>
>> -		ctx->suspended = true;
>>   		ctx->pipe = pipe;
>>   	}
>>
>> @@ -394,9 +382,6 @@ static void fimd_commit(struct exynos_drm_crtc *crtc)
>>   	void *timing_base = ctx->regs + driver_data->timing_base;
>>   	u32 val, clkdiv;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	/* nothing to do if we haven't set the mode yet */
>>   	if (mode->htotal == 0 || mode->vtotal == 0)
>>   		return;
>> @@ -630,9 +615,6 @@ static void fimd_atomic_begin(struct exynos_drm_crtc *crtc,
>>   {
>>   	struct fimd_context *ctx = crtc->ctx;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	fimd_shadow_protect_win(ctx, plane->zpos, true);
>>   }
>>
>> @@ -641,9 +623,6 @@ static void fimd_atomic_flush(struct exynos_drm_crtc *crtc,
>>   {
>>   	struct fimd_context *ctx = crtc->ctx;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	fimd_shadow_protect_win(ctx, plane->zpos, false);
>>   }
>>
>> @@ -659,9 +638,6 @@ static void fimd_update_plane(struct exynos_drm_crtc *crtc,
>>   	unsigned int bpp = state->fb->bits_per_pixel >> 3;
>>   	unsigned int pitch = state->fb->pitches[0];
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	offset = plane->src_x * bpp;
>>   	offset += plane->src_y * pitch;
>>
>> @@ -743,9 +719,6 @@ static void fimd_disable_plane(struct exynos_drm_crtc *crtc,
>>   	struct fimd_context *ctx = crtc->ctx;
>>   	unsigned int win = plane->zpos;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	fimd_enable_video_output(ctx, win, false);
>>
>>   	if (ctx->driver_data->has_shadowcon)
>> @@ -755,27 +728,9 @@ static void fimd_disable_plane(struct exynos_drm_crtc *crtc,
>>   static void fimd_enable(struct exynos_drm_crtc *crtc)
>>   {
>>   	struct fimd_context *ctx = crtc->ctx;
>> -	int ret;
>> -
>> -	if (!ctx->suspended)
>> -		return;
>> -
>> -	ctx->suspended = false;
>>
>>   	pm_runtime_get_sync(ctx->dev);
>>
>> -	ret = clk_prepare_enable(ctx->bus_clk);
>> -	if (ret < 0) {
>> -		DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
>> -		return;
>> -	}
>> -
>> -	ret = clk_prepare_enable(ctx->lcd_clk);
>> -	if  (ret < 0) {
>> -		DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret);
>> -		return;
>> -	}
>> -
>>   	/* if vblank was enabled status, enable it again. */
>>   	if (test_and_clear_bit(0, &ctx->irq_flags))
>>   		fimd_enable_vblank(ctx->crtc);
>> @@ -788,9 +743,6 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
>>   	struct fimd_context *ctx = crtc->ctx;
>>   	int i;
>>
>> -	if (ctx->suspended)
>> -		return;
>> -
>>   	/*
>>   	 * We need to make sure that all windows are disabled before we
>>   	 * suspend that connector. Otherwise we might try to scan from
>> @@ -805,12 +757,7 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
>>
>>   	writel(0, ctx->regs + VIDCON0);
>>
>> -	clk_disable_unprepare(ctx->lcd_clk);
>> -	clk_disable_unprepare(ctx->bus_clk);
>> -
>>   	pm_runtime_put_sync(ctx->dev);
>> -
>> -	ctx->suspended = true;
>>   }
>>
>>   static void fimd_trigger(struct device *dev)
>> @@ -1019,7 +966,6 @@ static int fimd_probe(struct platform_device *pdev)
>>   		return -ENOMEM;
>>
>>   	ctx->dev = dev;
>> -	ctx->suspended = true;
>>   	ctx->driver_data = drm_fimd_get_driver_data(pdev);
>>
>>   	if (of_property_read_bool(dev->of_node, "samsung,invert-vden"))
>> @@ -1129,12 +1075,49 @@ static int fimd_remove(struct platform_device *pdev)
>>   	return 0;
>>   }
>>
>> +#ifdef CONFIG_PM_SLEEP
>> +static int exynos_fimd_suspend(struct device *dev)
>> +{
>> +	struct fimd_context *ctx = dev_get_drvdata(dev);
>> +
>> +	clk_disable_unprepare(ctx->lcd_clk);
>> +	clk_disable_unprepare(ctx->bus_clk);
>> +
>> +	return 0;
>> +}
>> +
>> +static int exynos_fimd_resume(struct device *dev)
>> +{
>> +	struct fimd_context *ctx = dev_get_drvdata(dev);
>> +	int ret;
>> +
>> +	ret = clk_prepare_enable(ctx->bus_clk);
>> +	if (ret < 0) {
>> +		DRM_ERROR("Failed to prepare_enable the bus clk [%d]\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	ret = clk_prepare_enable(ctx->lcd_clk);
>> +	if  (ret < 0) {
>> +		DRM_ERROR("Failed to prepare_enable the lcd clk [%d]\n", ret);
>> +		return ret;
>> +	}
>> +
>> +	return 0;
>> +}
>> +#endif
>> +
>> +static const struct dev_pm_ops exynos_fimd_pm_ops = {
>> +	SET_RUNTIME_PM_OPS(exynos_fimd_suspend, exynos_fimd_resume, NULL)
>> +};
>> +
>>   struct platform_driver fimd_driver = {
>>   	.probe		= fimd_probe,
>>   	.remove		= fimd_remove,
>>   	.driver		= {
>>   		.name	= "exynos4-fb",
>>   		.owner	= THIS_MODULE,
>> +		.pm	= &exynos_fimd_pm_ops,
>>   		.of_match_table = fimd_driver_dt_match,
>>   	},
>>   };
>>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/10] drm/exynos: Enable DP clock directly from FIMD

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Instead of having a .clock_enable callback enable the dp clock directly
from FIMD.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos_dp_core.c  | 13 -----------
 drivers/gpu/drm/exynos/exynos_drm_drv.h  |  5 ----
 drivers/gpu/drm/exynos/exynos_drm_fimd.c | 39 +++++++++++++++++---------------
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
index 6794982..aa11d18 100644
--- a/drivers/gpu/drm/exynos/exynos_dp_core.c
+++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
@@ -37,11 +37,6 @@
 #define ctx_from_connector(c)	container_of(c, struct exynos_dp_device, \
 					connector)
 
-static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
-{
-	return to_exynos_crtc(dp->encoder.crtc);
-}
-
 static inline struct exynos_dp_device *encoder_to_dp(
 						struct drm_encoder *e)
 {
@@ -1068,7 +1063,6 @@ static void exynos_dp_mode_set(struct drm_encoder *encoder,
 static void exynos_dp_enable(struct drm_encoder *encoder)
 {
 	struct exynos_dp_device *dp = encoder_to_dp(encoder);
-	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
 
 	pm_runtime_get_sync(dp->dev);
 
@@ -1079,9 +1073,6 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
 		}
 	}
 
-	if (crtc->ops->clock_enable)
-		crtc->ops->clock_enable(dp_to_crtc(dp), true);
-
 	phy_power_on(dp->phy);
 	exynos_dp_init_dp(dp);
 	enable_irq(dp->irq);
@@ -1091,7 +1082,6 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
 static void exynos_dp_disable(struct drm_encoder *encoder)
 {
 	struct exynos_dp_device *dp = encoder_to_dp(encoder);
-	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
 
 	if (dp->panel) {
 		if (drm_panel_disable(dp->panel)) {
@@ -1104,9 +1094,6 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
 	flush_work(&dp->hotplug_work);
 	phy_power_off(dp->phy);
 
-	if (crtc->ops->clock_enable)
-		crtc->ops->clock_enable(dp_to_crtc(dp), false);
-
 	if (dp->panel) {
 		if (drm_panel_unprepare(dp->panel))
 			DRM_ERROR("failed to turnoff the panel\n");
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 5f1a4d6..ee60619 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -96,10 +96,6 @@ struct exynos_drm_plane {
  * @disable_plane: disable hardware specific overlay.
  * @te_handler: trigger to transfer video image at the tearing effect
  *	synchronization signal if there is a page flip request.
- * @clock_enable: optional function enabling/disabling display domain clock,
- *	called from exynos-dp driver before powering up (with
- *	'enable' argument as true) and after powering down (with
- *	'enable' as false).
  */
 struct exynos_drm_crtc;
 struct exynos_drm_crtc_ops {
@@ -120,7 +116,6 @@ struct exynos_drm_crtc_ops {
 	void (*atomic_flush)(struct exynos_drm_crtc *crtc,
 			      struct exynos_drm_plane *plane);
 	void (*te_handler)(struct exynos_drm_crtc *crtc);
-	void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable);
 };
 
 /*
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 0f17ae0..3cf2b80 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -573,6 +573,23 @@ static void fimd_win_set_colkey(struct fimd_context *ctx, unsigned int win)
 	writel(keycon1, ctx->regs + WKEYCON1_BASE(win));
 }
 
+static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
+{
+	struct fimd_context *ctx = crtc->ctx;
+	u32 val;
+
+	/*
+	 * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
+	 * clock. On these SoCs the bootloader may enable it but any
+	 * power domain off/on will reset it to disable state.
+	 */
+	if (ctx->driver_data != &exynos5_fimd_driver_data)
+		return;
+
+	val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
+	writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
+}
+
 /**
  * shadow_protect_win() - disable updating values from shadow registers at vsync
  *
@@ -735,6 +752,8 @@ static void fimd_enable(struct exynos_drm_crtc *crtc)
 	if (test_and_clear_bit(0, &ctx->irq_flags))
 		fimd_enable_vblank(ctx->crtc);
 
+	fimd_dp_clock_enable(crtc, true);
+
 	fimd_commit(ctx->crtc);
 }
 
@@ -743,6 +762,8 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
 	struct fimd_context *ctx = crtc->ctx;
 	int i;
 
+	fimd_dp_clock_enable(crtc, false);
+
 	/*
 	 * We need to make sure that all windows are disabled before we
 	 * suspend that connector. Otherwise we might try to scan from
@@ -814,23 +835,6 @@ static void fimd_te_handler(struct exynos_drm_crtc *crtc)
 		drm_crtc_handle_vblank(&ctx->crtc->base);
 }
 
-static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
-{
-	struct fimd_context *ctx = crtc->ctx;
-	u32 val;
-
-	/*
-	 * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
-	 * clock. On these SoCs the bootloader may enable it but any
-	 * power domain off/on will reset it to disable state.
-	 */
-	if (ctx->driver_data != &exynos5_fimd_driver_data)
-		return;
-
-	val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
-	writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
-}
-
 static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.enable = fimd_enable,
 	.disable = fimd_disable,
@@ -843,7 +847,6 @@ static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
 	.disable_plane = fimd_disable_plane,
 	.atomic_flush = fimd_atomic_flush,
 	.te_handler = fimd_te_handler,
-	.clock_enable = fimd_dp_clock_enable,
 };
 
 static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
-- 
2.1.0

Re: [PATCH 08/10] drm/exynos: Enable DP clock directly from FIMD

[Inki Dae]

Hi Gustavo,

On 2015년 09월 05일 05:15, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Instead of having a .clock_enable callback enable the dp clock directly
> from FIMD.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos_dp_core.c  | 13 -----------
>  drivers/gpu/drm/exynos/exynos_drm_drv.h  |  5 ----
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c | 39 +++++++++++++++++---------------
>  3 files changed, 21 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
> index 6794982..aa11d18 100644
> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
> @@ -37,11 +37,6 @@
>  #define ctx_from_connector(c)	container_of(c, struct exynos_dp_device, \
>  					connector)
>  
> -static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
> -{
> -	return to_exynos_crtc(dp->encoder.crtc);
> -}
> -
>  static inline struct exynos_dp_device *encoder_to_dp(
>  						struct drm_encoder *e)
>  {
> @@ -1068,7 +1063,6 @@ static void exynos_dp_mode_set(struct drm_encoder *encoder,
>  static void exynos_dp_enable(struct drm_encoder *encoder)
>  {
>  	struct exynos_dp_device *dp = encoder_to_dp(encoder);
> -	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
>  
>  	pm_runtime_get_sync(dp->dev);
>  
> @@ -1079,9 +1073,6 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
>  		}
>  	}
>  
> -	if (crtc->ops->clock_enable)
> -		crtc->ops->clock_enable(dp_to_crtc(dp), true);
> -
>  	phy_power_on(dp->phy);
>  	exynos_dp_init_dp(dp);
>  	enable_irq(dp->irq);
> @@ -1091,7 +1082,6 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
>  static void exynos_dp_disable(struct drm_encoder *encoder)
>  {
>  	struct exynos_dp_device *dp = encoder_to_dp(encoder);
> -	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
>  
>  	if (dp->panel) {
>  		if (drm_panel_disable(dp->panel)) {
> @@ -1104,9 +1094,6 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
>  	flush_work(&dp->hotplug_work);
>  	phy_power_off(dp->phy);
>  
> -	if (crtc->ops->clock_enable)
> -		crtc->ops->clock_enable(dp_to_crtc(dp), false);
> -
>  	if (dp->panel) {
>  		if (drm_panel_unprepare(dp->panel))
>  			DRM_ERROR("failed to turnoff the panel\n");
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 5f1a4d6..ee60619 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -96,10 +96,6 @@ struct exynos_drm_plane {
>   * @disable_plane: disable hardware specific overlay.
>   * @te_handler: trigger to transfer video image at the tearing effect
>   *	synchronization signal if there is a page flip request.
> - * @clock_enable: optional function enabling/disabling display domain clock,
> - *	called from exynos-dp driver before powering up (with
> - *	'enable' argument as true) and after powering down (with
> - *	'enable' as false).
>   */
>  struct exynos_drm_crtc;
>  struct exynos_drm_crtc_ops {
> @@ -120,7 +116,6 @@ struct exynos_drm_crtc_ops {
>  	void (*atomic_flush)(struct exynos_drm_crtc *crtc,
>  			      struct exynos_drm_plane *plane);
>  	void (*te_handler)(struct exynos_drm_crtc *crtc);
> -	void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable);
>  };
>  
>  /*
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 0f17ae0..3cf2b80 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -573,6 +573,23 @@ static void fimd_win_set_colkey(struct fimd_context *ctx, unsigned int win)
>  	writel(keycon1, ctx->regs + WKEYCON1_BASE(win));
>  }
>  
> +static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
> +{
> +	struct fimd_context *ctx = crtc->ctx;
> +	u32 val;
> +
> +	/*
> +	 * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
> +	 * clock. On these SoCs the bootloader may enable it but any
> +	 * power domain off/on will reset it to disable state.
> +	 */
> +	if (ctx->driver_data != &exynos5_fimd_driver_data)
> +		return;
> +
> +	val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
> +	writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
> +}
> +
>  /**
>   * shadow_protect_win() - disable updating values from shadow registers at vsync
>   *
> @@ -735,6 +752,8 @@ static void fimd_enable(struct exynos_drm_crtc *crtc)
>  	if (test_and_clear_bit(0, &ctx->irq_flags))
>  		fimd_enable_vblank(ctx->crtc);
>  
> +	fimd_dp_clock_enable(crtc, true);

You are forcing FIMD driver to enable DP clock every time FIMD is
enabled. Please know that in Exynos Display pipeline, Encoder device
could be used according to how Display path is configured.

For example,
FIMD ----- Panel
FIMD ----- MIPI-DSI ----- Panel
FIMD ----- DP ----- Panel
...

In previous codes, DP clock will be enabled by DP driver even through
enable callback of the DP clock is registered by FIMD driver to
fimd_crtc_ops, which means that DP clock can be enabled only in case
that DP driver is available.

Thanks,
Inki Dae

> +
>  	fimd_commit(ctx->crtc);
>  }
>  
> @@ -743,6 +762,8 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
>  	struct fimd_context *ctx = crtc->ctx;
>  	int i;
>  
> +	fimd_dp_clock_enable(crtc, false);
> +
>  	/*
>  	 * We need to make sure that all windows are disabled before we
>  	 * suspend that connector. Otherwise we might try to scan from
> @@ -814,23 +835,6 @@ static void fimd_te_handler(struct exynos_drm_crtc *crtc)
>  		drm_crtc_handle_vblank(&ctx->crtc->base);
>  }
>  
> -static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
> -{
> -	struct fimd_context *ctx = crtc->ctx;
> -	u32 val;
> -
> -	/*
> -	 * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
> -	 * clock. On these SoCs the bootloader may enable it but any
> -	 * power domain off/on will reset it to disable state.
> -	 */
> -	if (ctx->driver_data != &exynos5_fimd_driver_data)
> -		return;
> -
> -	val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
> -	writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
> -}
> -
>  static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
>  	.enable = fimd_enable,
>  	.disable = fimd_disable,
> @@ -843,7 +847,6 @@ static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
>  	.disable_plane = fimd_disable_plane,
>  	.atomic_flush = fimd_atomic_flush,
>  	.te_handler = fimd_te_handler,
> -	.clock_enable = fimd_dp_clock_enable,
>  };
>  
>  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
> 

Re: [PATCH 08/10] drm/exynos: Enable DP clock directly from FIMD

[Inki Dae]

Gustavo, please ping~~~

2015년 09월 19일 12:51에 Inki Dae 이(가) 쓴 글:
> Hi Gustavo,
>
> On 2015년 09월 05일 05:15, Gustavo Padovan wrote:
>> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>>
>> Instead of having a .clock_enable callback enable the dp clock directly
>> from FIMD.
>>
>> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>> ---
>>   drivers/gpu/drm/exynos/exynos_dp_core.c  | 13 -----------
>>   drivers/gpu/drm/exynos/exynos_drm_drv.h  |  5 ----
>>   drivers/gpu/drm/exynos/exynos_drm_fimd.c | 39 +++++++++++++++++---------------
>>   3 files changed, 21 insertions(+), 36 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_dp_core.c b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> index 6794982..aa11d18 100644
>> --- a/drivers/gpu/drm/exynos/exynos_dp_core.c
>> +++ b/drivers/gpu/drm/exynos/exynos_dp_core.c
>> @@ -37,11 +37,6 @@
>>   #define ctx_from_connector(c)	container_of(c, struct exynos_dp_device, \
>>   					connector)
>>
>> -static inline struct exynos_drm_crtc *dp_to_crtc(struct exynos_dp_device *dp)
>> -{
>> -	return to_exynos_crtc(dp->encoder.crtc);
>> -}
>> -
>>   static inline struct exynos_dp_device *encoder_to_dp(
>>   						struct drm_encoder *e)
>>   {
>> @@ -1068,7 +1063,6 @@ static void exynos_dp_mode_set(struct drm_encoder *encoder,
>>   static void exynos_dp_enable(struct drm_encoder *encoder)
>>   {
>>   	struct exynos_dp_device *dp = encoder_to_dp(encoder);
>> -	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
>>
>>   	pm_runtime_get_sync(dp->dev);
>>
>> @@ -1079,9 +1073,6 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
>>   		}
>>   	}
>>
>> -	if (crtc->ops->clock_enable)
>> -		crtc->ops->clock_enable(dp_to_crtc(dp), true);
>> -
>>   	phy_power_on(dp->phy);
>>   	exynos_dp_init_dp(dp);
>>   	enable_irq(dp->irq);
>> @@ -1091,7 +1082,6 @@ static void exynos_dp_enable(struct drm_encoder *encoder)
>>   static void exynos_dp_disable(struct drm_encoder *encoder)
>>   {
>>   	struct exynos_dp_device *dp = encoder_to_dp(encoder);
>> -	struct exynos_drm_crtc *crtc = dp_to_crtc(dp);
>>
>>   	if (dp->panel) {
>>   		if (drm_panel_disable(dp->panel)) {
>> @@ -1104,9 +1094,6 @@ static void exynos_dp_disable(struct drm_encoder *encoder)
>>   	flush_work(&dp->hotplug_work);
>>   	phy_power_off(dp->phy);
>>
>> -	if (crtc->ops->clock_enable)
>> -		crtc->ops->clock_enable(dp_to_crtc(dp), false);
>> -
>>   	if (dp->panel) {
>>   		if (drm_panel_unprepare(dp->panel))
>>   			DRM_ERROR("failed to turnoff the panel\n");
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> index 5f1a4d6..ee60619 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
>> @@ -96,10 +96,6 @@ struct exynos_drm_plane {
>>    * @disable_plane: disable hardware specific overlay.
>>    * @te_handler: trigger to transfer video image at the tearing effect
>>    *	synchronization signal if there is a page flip request.
>> - * @clock_enable: optional function enabling/disabling display domain clock,
>> - *	called from exynos-dp driver before powering up (with
>> - *	'enable' argument as true) and after powering down (with
>> - *	'enable' as false).
>>    */
>>   struct exynos_drm_crtc;
>>   struct exynos_drm_crtc_ops {
>> @@ -120,7 +116,6 @@ struct exynos_drm_crtc_ops {
>>   	void (*atomic_flush)(struct exynos_drm_crtc *crtc,
>>   			      struct exynos_drm_plane *plane);
>>   	void (*te_handler)(struct exynos_drm_crtc *crtc);
>> -	void (*clock_enable)(struct exynos_drm_crtc *crtc, bool enable);
>>   };
>>
>>   /*
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 0f17ae0..3cf2b80 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -573,6 +573,23 @@ static void fimd_win_set_colkey(struct fimd_context *ctx, unsigned int win)
>>   	writel(keycon1, ctx->regs + WKEYCON1_BASE(win));
>>   }
>>
>> +static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
>> +{
>> +	struct fimd_context *ctx = crtc->ctx;
>> +	u32 val;
>> +
>> +	/*
>> +	 * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
>> +	 * clock. On these SoCs the bootloader may enable it but any
>> +	 * power domain off/on will reset it to disable state.
>> +	 */
>> +	if (ctx->driver_data != &exynos5_fimd_driver_data)
>> +		return;
>> +
>> +	val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
>> +	writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
>> +}
>> +
>>   /**
>>    * shadow_protect_win() - disable updating values from shadow registers at vsync
>>    *
>> @@ -735,6 +752,8 @@ static void fimd_enable(struct exynos_drm_crtc *crtc)
>>   	if (test_and_clear_bit(0, &ctx->irq_flags))
>>   		fimd_enable_vblank(ctx->crtc);
>>
>> +	fimd_dp_clock_enable(crtc, true);
>
> You are forcing FIMD driver to enable DP clock every time FIMD is
> enabled. Please know that in Exynos Display pipeline, Encoder device
> could be used according to how Display path is configured.
>
> For example,
> FIMD ----- Panel
> FIMD ----- MIPI-DSI ----- Panel
> FIMD ----- DP ----- Panel
> ...
>
> In previous codes, DP clock will be enabled by DP driver even through
> enable callback of the DP clock is registered by FIMD driver to
> fimd_crtc_ops, which means that DP clock can be enabled only in case
> that DP driver is available.
>
> Thanks,
> Inki Dae
>
>> +
>>   	fimd_commit(ctx->crtc);
>>   }
>>
>> @@ -743,6 +762,8 @@ static void fimd_disable(struct exynos_drm_crtc *crtc)
>>   	struct fimd_context *ctx = crtc->ctx;
>>   	int i;
>>
>> +	fimd_dp_clock_enable(crtc, false);
>> +
>>   	/*
>>   	 * We need to make sure that all windows are disabled before we
>>   	 * suspend that connector. Otherwise we might try to scan from
>> @@ -814,23 +835,6 @@ static void fimd_te_handler(struct exynos_drm_crtc *crtc)
>>   		drm_crtc_handle_vblank(&ctx->crtc->base);
>>   }
>>
>> -static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
>> -{
>> -	struct fimd_context *ctx = crtc->ctx;
>> -	u32 val;
>> -
>> -	/*
>> -	 * Only Exynos 5250, 5260, 5410 and 542x requires enabling DP/MIE
>> -	 * clock. On these SoCs the bootloader may enable it but any
>> -	 * power domain off/on will reset it to disable state.
>> -	 */
>> -	if (ctx->driver_data != &exynos5_fimd_driver_data)
>> -		return;
>> -
>> -	val = enable ? DP_MIE_CLK_DP_ENABLE : DP_MIE_CLK_DISABLE;
>> -	writel(DP_MIE_CLK_DP_ENABLE, ctx->regs + DP_MIE_CLKCON);
>> -}
>> -
>>   static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
>>   	.enable = fimd_enable,
>>   	.disable = fimd_disable,
>> @@ -843,7 +847,6 @@ static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
>>   	.disable_plane = fimd_disable_plane,
>>   	.atomic_flush = fimd_atomic_flush,
>>   	.te_handler = fimd_te_handler,
>> -	.clock_enable = fimd_dp_clock_enable,
>>   };
>>
>>   static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>
>

[PATCH 09/10] drm/exynos: add pm_runtime to DECON 5433

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Let pm_runtime handle the enabling/disabling of the device with
proper refcnt instead of rely on specific flags to track the enabled
state.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos5433_drm_decon.c | 89 ++++++++++++---------------
 1 file changed, 39 insertions(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
index 838a9c1..f0b7804 100644
--- a/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos5433_drm_decon.c
@@ -35,7 +35,6 @@ struct decon_context {
 	struct clk			*clks[6];
 	unsigned long			irq_flags;
 	int				pipe;
-	bool				suspended;
 
 #define BIT_CLKS_ENABLED		0
 #define BIT_IRQS_ENABLED		1
@@ -65,9 +64,6 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (ctx->suspended)
-		return -EPERM;
-
 	if (test_and_set_bit(0, &ctx->irq_flags)) {
 		val = VIDINTCON0_INTEN;
 		if (ctx->i80_if)
@@ -85,9 +81,6 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	if (test_and_clear_bit(0, &ctx->irq_flags))
 		writel(0, ctx->addr + DECON_VIDINTCON0);
 }
@@ -105,9 +98,6 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	struct drm_display_mode *mode = &crtc->base.mode;
 	u32 val;
 
-	if (ctx->suspended)
-		return;
-
 	/* enable clock gate */
 	val = CMU_CLKGAGE_MODE_SFR_F | CMU_CLKGAGE_MODE_MEM_F;
 	writel(val, ctx->addr + DECON_CMU);
@@ -230,9 +220,6 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	decon_shadow_protect_win(ctx, plane->zpos, true);
 }
 
@@ -246,9 +233,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	unsigned int pitch = state->fb->pitches[0];
 	u32 val;
 
-	if (ctx->suspended)
-		return;
-
 	val = COORDINATE_X(plane->crtc_x) | COORDINATE_Y(plane->crtc_y);
 	writel(val, ctx->addr + DECON_VIDOSDxA(win));
 
@@ -293,9 +277,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 	unsigned int win = plane->zpos;
 	u32 val;
 
-	if (ctx->suspended)
-		return;
-
 	decon_shadow_protect_win(ctx, win, true);
 
 	/* window disable */
@@ -316,9 +297,6 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	decon_shadow_protect_win(ctx, plane->zpos, false);
 
 	if (ctx->i80_if)
@@ -351,22 +329,9 @@ static void decon_swreset(struct decon_context *ctx)
 static void decon_enable(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
-	int ret;
-	int i;
-
-	if (!ctx->suspended)
-		return;
-
-	ctx->suspended = false;
 
 	pm_runtime_get_sync(ctx->dev);
 
-	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
-		ret = clk_prepare_enable(ctx->clks[i]);
-		if (ret < 0)
-			goto err;
-	}
-
 	set_bit(BIT_CLKS_ENABLED, &ctx->enabled);
 
 	/* if vblank was enabled status, enable it again. */
@@ -376,11 +341,6 @@ static void decon_enable(struct exynos_drm_crtc *crtc)
 	decon_commit(ctx->crtc);
 
 	return;
-err:
-	while (--i >= 0)
-		clk_disable_unprepare(ctx->clks[i]);
-
-	ctx->suspended = true;
 }
 
 static void decon_disable(struct exynos_drm_crtc *crtc)
@@ -388,9 +348,6 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	int i;
 
-	if (ctx->suspended)
-		return;
-
 	/*
 	 * We need to make sure that all windows are disabled before we
 	 * suspend that connector. Otherwise we might try to scan from
@@ -401,14 +358,9 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 
 	decon_swreset(ctx);
 
-	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++)
-		clk_disable_unprepare(ctx->clks[i]);
-
 	clear_bit(BIT_CLKS_ENABLED, &ctx->enabled);
 
 	pm_runtime_put_sync(ctx->dev);
-
-	ctx->suspended = true;
 }
 
 void decon_te_irq_handler(struct exynos_drm_crtc *crtc)
@@ -475,7 +427,6 @@ err:
 static struct exynos_drm_crtc_ops decon_crtc_ops = {
 	.enable			= decon_enable,
 	.disable		= decon_disable,
-	.commit			= decon_commit,
 	.enable_vblank		= decon_enable_vblank,
 	.disable_vblank		= decon_disable_vblank,
 	.atomic_begin		= decon_atomic_begin,
@@ -602,7 +553,6 @@ static int exynos5433_decon_probe(struct platform_device *pdev)
 	if (!ctx)
 		return -ENOMEM;
 
-	ctx->suspended = true;
 	ctx->dev = dev;
 	if (of_get_child_by_name(dev->of_node, "i80-if-timings"))
 		ctx->i80_if = true;
@@ -669,6 +619,44 @@ static int exynos5433_decon_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int exynos5433_decon_suspend(struct device *dev)
+{
+	struct decon_context *ctx = dev_get_drvdata(dev);
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++)
+		clk_disable_unprepare(ctx->clks[i]);
+
+	return 0;
+}
+
+static int exynos5433_decon_resume(struct device *dev)
+{
+	struct decon_context *ctx = dev_get_drvdata(dev);
+	int i, ret;
+
+	for (i = 0; i < ARRAY_SIZE(decon_clks_name); i++) {
+		ret = clk_prepare_enable(ctx->clks[i]);
+		if (ret < 0)
+			goto err;
+	}
+
+	return 0;
+
+err:
+	while (--i >= 0)
+		clk_disable_unprepare(ctx->clks[i]);
+
+	return ret;
+}
+#endif
+
+static const struct dev_pm_ops exynos5433_decon_pm_ops = {
+	SET_RUNTIME_PM_OPS(exynos5433_decon_suspend, exynos5433_decon_resume,
+			   NULL)
+};
+
 static const struct of_device_id exynos5433_decon_driver_dt_match[] = {
 	{ .compatible = "samsung,exynos5433-decon" },
 	{},
@@ -680,6 +668,7 @@ struct platform_driver exynos5433_decon_driver = {
 	.remove		= exynos5433_decon_remove,
 	.driver		= {
 		.name	= "exynos5433-decon",
+		.pm	= &exynos5433_decon_pm_ops,
 		.of_match_table = exynos5433_decon_driver_dt_match,
 	},
 };
-- 
2.1.0

[PATCH 10/10] drm/exynos: add pm_runtime to DECON 7

[Gustavo Padovan]

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Let pm_runtime handle the enabling/disabling of the device with
proper refcnt instead of rely on specific flags to track the enabled
state.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c | 125 ++++++++++++-----------------
 1 file changed, 53 insertions(+), 72 deletions(-)

diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index e4646e2..517ba7a 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -54,7 +54,6 @@ struct decon_context {
 	void __iomem			*regs;
 	unsigned long			irq_flags;
 	bool				i80_if;
-	bool				suspended;
 	int				pipe;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
@@ -85,9 +84,6 @@ static void decon_wait_for_vblank(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	atomic_set(&ctx->wait_vsync_event, 1);
 
 	/*
@@ -119,13 +115,8 @@ static void decon_clear_channels(struct exynos_drm_crtc *crtc)
 	}
 
 	/* Wait for vsync, as disable channel takes effect at next vsync */
-	if (ch_enabled) {
-		unsigned int state = ctx->suspended;
-
-		ctx->suspended = 0;
+	if (ch_enabled)
 		decon_wait_for_vblank(ctx->crtc);
-		ctx->suspended = state;
-	}
 }
 
 static int decon_ctx_initialize(struct decon_context *ctx,
@@ -180,9 +171,6 @@ static void decon_commit(struct exynos_drm_crtc *crtc)
 	struct drm_display_mode *mode = &crtc->base.state->adjusted_mode;
 	u32 val, clkdiv;
 
-	if (ctx->suspended)
-		return;
-
 	/* nothing to do if we haven't set the mode yet */
 	if (mode->htotal == 0 || mode->vtotal == 0)
 		return;
@@ -244,9 +232,6 @@ static int decon_enable_vblank(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (ctx->suspended)
-		return -EPERM;
-
 	if (!test_and_set_bit(0, &ctx->irq_flags)) {
 		val = readl(ctx->regs + VIDINTCON0);
 
@@ -269,9 +254,6 @@ static void decon_disable_vblank(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	u32 val;
 
-	if (ctx->suspended)
-		return;
-
 	if (test_and_clear_bit(0, &ctx->irq_flags)) {
 		val = readl(ctx->regs + VIDINTCON0);
 
@@ -399,9 +381,6 @@ static void decon_atomic_begin(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	decon_shadow_protect_win(ctx, plane->zpos, true);
 }
 
@@ -419,9 +398,6 @@ static void decon_update_plane(struct exynos_drm_crtc *crtc,
 	unsigned int bpp = state->fb->bits_per_pixel >> 3;
 	unsigned int pitch = state->fb->pitches[0];
 
-	if (ctx->suspended)
-		return;
-
 	/*
 	 * SHADOWCON/PRTCON register is used for enabling timing.
 	 *
@@ -518,9 +494,6 @@ static void decon_disable_plane(struct exynos_drm_crtc *crtc,
 	unsigned int win = plane->zpos;
 	u32 val;
 
-	if (ctx->suspended)
-		return;
-
 	/* protect windows */
 	decon_shadow_protect_win(ctx, win, true);
 
@@ -539,9 +512,6 @@ static void decon_atomic_flush(struct exynos_drm_crtc *crtc,
 {
 	struct decon_context *ctx = crtc->ctx;
 
-	if (ctx->suspended)
-		return;
-
 	decon_shadow_protect_win(ctx, plane->zpos, false);
 }
 
@@ -565,39 +535,9 @@ static void decon_init(struct decon_context *ctx)
 static void decon_enable(struct exynos_drm_crtc *crtc)
 {
 	struct decon_context *ctx = crtc->ctx;
-	int ret;
-
-	if (!ctx->suspended)
-		return;
-
-	ctx->suspended = false;
 
 	pm_runtime_get_sync(ctx->dev);
 
-	ret = clk_prepare_enable(ctx->pclk);
-	if (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the pclk [%d]\n", ret);
-		return;
-	}
-
-	ret = clk_prepare_enable(ctx->aclk);
-	if (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the aclk [%d]\n", ret);
-		return;
-	}
-
-	ret = clk_prepare_enable(ctx->eclk);
-	if  (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the eclk [%d]\n", ret);
-		return;
-	}
-
-	ret = clk_prepare_enable(ctx->vclk);
-	if  (ret < 0) {
-		DRM_ERROR("Failed to prepare_enable the vclk [%d]\n", ret);
-		return;
-	}
-
 	decon_init(ctx);
 
 	/* if vblank was enabled status, enable it again. */
@@ -612,9 +552,6 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	struct decon_context *ctx = crtc->ctx;
 	int i;
 
-	if (ctx->suspended)
-		return;
-
 	/*
 	 * We need to make sure that all windows are disabled before we
 	 * suspend that connector. Otherwise we might try to scan from
@@ -623,14 +560,7 @@ static void decon_disable(struct exynos_drm_crtc *crtc)
 	for (i = 0; i < WINDOWS_NR; i++)
 		decon_disable_plane(crtc, &ctx->planes[i]);
 
-	clk_disable_unprepare(ctx->vclk);
-	clk_disable_unprepare(ctx->eclk);
-	clk_disable_unprepare(ctx->aclk);
-	clk_disable_unprepare(ctx->pclk);
-
 	pm_runtime_put_sync(ctx->dev);
-
-	ctx->suspended = true;
 }
 
 static const struct exynos_drm_crtc_ops decon_crtc_ops = {
@@ -756,7 +686,6 @@ static int decon_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	ctx->dev = dev;
-	ctx->suspended = true;
 
 	i80_if_timings = of_get_child_by_name(dev->of_node, "i80-if-timings");
 	if (i80_if_timings)
@@ -851,11 +780,63 @@ static int decon_remove(struct platform_device *pdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM_SLEEP
+static int exynos7_decon_suspend(struct device *dev)
+{
+	struct decon_context *ctx = dev_get_drvdata(dev);
+
+	clk_disable_unprepare(ctx->vclk);
+	clk_disable_unprepare(ctx->eclk);
+	clk_disable_unprepare(ctx->aclk);
+	clk_disable_unprepare(ctx->pclk);
+
+	return 0;
+}
+
+static int exynos7_decon_resume(struct device *dev)
+{
+	struct decon_context *ctx = dev_get_drvdata(dev);
+	int ret;
+
+	ret = clk_prepare_enable(ctx->pclk);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the pclk [%d]\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ctx->aclk);
+	if (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the aclk [%d]\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ctx->eclk);
+	if  (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the eclk [%d]\n", ret);
+		return ret;
+	}
+
+	ret = clk_prepare_enable(ctx->vclk);
+	if  (ret < 0) {
+		DRM_ERROR("Failed to prepare_enable the vclk [%d]\n", ret);
+		return ret;
+	}
+
+	return 0;
+}
+#endif
+
+static const struct dev_pm_ops exynos7_decon_pm_ops = {
+	SET_RUNTIME_PM_OPS(exynos7_decon_suspend, exynos7_decon_resume,
+			   NULL)
+};
+
 struct platform_driver decon_driver = {
 	.probe		= decon_probe,
 	.remove		= decon_remove,
 	.driver		= {
 		.name	= "exynos-decon",
+		.pm	= &exynos7_decon_pm_ops,
 		.of_match_table = decon_driver_dt_match,
 	},
 };
-- 
2.1.0

Re: [PATCH 00/10] drm/exynos: add pm_runtime support

[Gustavo Padovan]

Hi Inki,

Any comment on this series?

	Gustavo

2015-09-04 Gustavo Padovan <gustavo@padovan.org>:

> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Hi,
> 
> This series adds proper runtime PM suport to CRTCs and Encoders, so
> now instead of relying on 'suspended' or 'enabled' flags to track when
> the CRTC or Encoder is enabled we let the pm_runtime subsystem do it for us
> and remove all the flags. This is a important step to the atomic suspend/resume
> support that will land in drm anytime soon.
> 
> Please review!
> 
> 	Gustavo
> 
> Gustavo Padovan (10):
>   drm/exynos: do not start enabling DP at bind() phase
>   drm/exynos: add pm_runtime to DP
>   drm/exynos: add pm_runtime to HDMI
>   drm/exynos: add pm_runtime to Mixer
>   drm/exynos: remove exynos_crtc commit() callback
>   drm/exynos: Remove exynos_crtc commit() callback
>   drm/exynos: add pm_runtime to FIMD
>   drm/exynos: Enable DP clock directly from FIMD
>   drm/exynos: add pm_runtime to DECON 5433
>   drm/exynos: add pm_runtime to DECON 7
> 
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  90 ++++++++----------
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 126 +++++++++++--------------
>  drivers/gpu/drm/exynos/exynos_dp_core.c       |  57 ++++++------
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c      |  10 --
>  drivers/gpu/drm/exynos/exynos_drm_drv.h       |   7 --
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 129 ++++++++++++--------------
>  drivers/gpu/drm/exynos/exynos_hdmi.c          |  56 +++++++----
>  drivers/gpu/drm/exynos/exynos_mixer.c         | 125 ++++++++++++-------------
>  8 files changed, 275 insertions(+), 325 deletions(-)
> 
> -- 
> 2.1.0
> 

Re: [PATCH 00/10] drm/exynos: add pm_runtime support

[Andrzej Hajda]

On 09/04/2015 10:15 PM, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Hi,
> 
> This series adds proper runtime PM suport to CRTCs and Encoders, so
> now instead of relying on 'suspended' or 'enabled' flags to track when
> the CRTC or Encoder is enabled we let the pm_runtime subsystem do it for us
> and remove all the flags. This is a important step to the atomic suspend/resume
> support that will land in drm anytime soon.
> 
> Please review!
> 
> 	Gustavo

The title is quite misleading, exynos_drm has already pm_runtime support :)
Could you explain more why do you want to change it, what are requirements of
atomic pm? What is wrong with the current one?

If I remember correctly one of arguments for replacing pm callbacks in
components by one centralized callback in exynos_drm_drv was to avoid issues
with non-deterministic order of calling components suspend callback.
Are there any recent changes in PM, DRM which invalidates the reason above?

On the other side your patchset adds implicit dependency on PM_SLEEP config
option, it should be avoided or at least it should be explicit.

Regards
Andrzej

Re: [PATCH 00/10] drm/exynos: add pm_runtime support

[Tobias Jakobi]

Hello Gustavo,

just wanted to ask if the series is still meant to go upstream. I tried
applying the series to torvalds/master and I get a conflict because of
Inki's commit 148ba09c465cc54d8e68f041bf9a30332b315c39 ('drm/exynos: dp:
remove suspend/resume functions').

With best wishes,
Tobias


Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> Hi,
> 
> This series adds proper runtime PM suport to CRTCs and Encoders, so
> now instead of relying on 'suspended' or 'enabled' flags to track when
> the CRTC or Encoder is enabled we let the pm_runtime subsystem do it for us
> and remove all the flags. This is a important step to the atomic suspend/resume
> support that will land in drm anytime soon.
> 
> Please review!
> 
> 	Gustavo
> 
> Gustavo Padovan (10):
>   drm/exynos: do not start enabling DP at bind() phase
>   drm/exynos: add pm_runtime to DP
>   drm/exynos: add pm_runtime to HDMI
>   drm/exynos: add pm_runtime to Mixer
>   drm/exynos: remove exynos_crtc commit() callback
>   drm/exynos: Remove exynos_crtc commit() callback
>   drm/exynos: add pm_runtime to FIMD
>   drm/exynos: Enable DP clock directly from FIMD
>   drm/exynos: add pm_runtime to DECON 5433
>   drm/exynos: add pm_runtime to DECON 7
> 
>  drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  90 ++++++++----------
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 126 +++++++++++--------------
>  drivers/gpu/drm/exynos/exynos_dp_core.c       |  57 ++++++------
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c      |  10 --
>  drivers/gpu/drm/exynos/exynos_drm_drv.h       |   7 --
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 129 ++++++++++++--------------
>  drivers/gpu/drm/exynos/exynos_hdmi.c          |  56 +++++++----
>  drivers/gpu/drm/exynos/exynos_mixer.c         | 125 ++++++++++++-------------
>  8 files changed, 275 insertions(+), 325 deletions(-)
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/10] drm/exynos: add pm_runtime support

[Inki Dae]

Hi Gustavo,

Please ping~ and re-base on top of exynos-drm-next.

Thanks,
Inki Dae

2015년 09월 05일 05:15에 Gustavo Padovan 이(가) 쓴 글:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
> Hi,
>
> This series adds proper runtime PM suport to CRTCs and Encoders, so
> now instead of relying on 'suspended' or 'enabled' flags to track when
> the CRTC or Encoder is enabled we let the pm_runtime subsystem do it for us
> and remove all the flags. This is a important step to the atomic suspend/resume
> support that will land in drm anytime soon.
>
> Please review!
>
> 	Gustavo
>
> Gustavo Padovan (10):
>    drm/exynos: do not start enabling DP at bind() phase
>    drm/exynos: add pm_runtime to DP
>    drm/exynos: add pm_runtime to HDMI
>    drm/exynos: add pm_runtime to Mixer
>    drm/exynos: remove exynos_crtc commit() callback
>    drm/exynos: Remove exynos_crtc commit() callback
>    drm/exynos: add pm_runtime to FIMD
>    drm/exynos: Enable DP clock directly from FIMD
>    drm/exynos: add pm_runtime to DECON 5433
>    drm/exynos: add pm_runtime to DECON 7
>
>   drivers/gpu/drm/exynos/exynos5433_drm_decon.c |  90 ++++++++----------
>   drivers/gpu/drm/exynos/exynos7_drm_decon.c    | 126 +++++++++++--------------
>   drivers/gpu/drm/exynos/exynos_dp_core.c       |  57 ++++++------
>   drivers/gpu/drm/exynos/exynos_drm_crtc.c      |  10 --
>   drivers/gpu/drm/exynos/exynos_drm_drv.h       |   7 --
>   drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 129 ++++++++++++--------------
>   drivers/gpu/drm/exynos/exynos_hdmi.c          |  56 +++++++----
>   drivers/gpu/drm/exynos/exynos_mixer.c         | 125 ++++++++++++-------------
>   8 files changed, 275 insertions(+), 325 deletions(-)
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel