drm/msm: fix fallout of atomic dpms changes

As a result of atomic DPMS support, the various prepare/commit hooks get
called in a way that msm dislikes.  We were expecting prepare/commit to
bracket a modeset, which is no longer the case.  This was needed to hold
various extra clk's (such as interface clks) on while we are touching
registers, and in the case of mdp4 holding vblank enabled.

The most straightforward way to deal with this, since we already have
our own atomic_commit(), is to just handle prepare/commit internally to
the driver (with some additional vfuncs for mdp4 vs mdp5), and switch
everything over to instead use the new enable/disable hooks.  It doesn't
really change too much, despite the code motion.  What used to be in the
encoder/crtc dpms() fxns is split out into enable/disable.

We should be able to drop our own enable-state tracking, as the atomic
helpers should do this for us.  But keeping that for the short term for
extra debugging as atomic stablizes.

Signed-off-by: Rob Clark <robdclark@gmail.com>

12 files changed, 303 insertions, 263 deletions

diff --git a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
index b4e70e0e3cfa..6bd38c73c6f7 100644
--- a/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
+++ b/drivers/gpu/drm/msm/hdmi/hdmi_connector.c
@@ -386,7 +386,7 @@ hdmi_connector_best_encoder(struct drm_connector *connector)
 }
 
 static const struct drm_connector_funcs hdmi_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = hdmi_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = hdmi_connector_destroy,
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c
index 67b42a43a1b1..73afa21822b4 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_crtc.c
@@ -140,26 +140,6 @@ static void mdp4_crtc_destroy(struct drm_crtc *crtc)
 	kfree(mdp4_crtc);
 }
 
-static void mdp4_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
-	struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
-	struct mdp4_kms *mdp4_kms = get_kms(crtc);
-	bool enabled = (mode == DRM_MODE_DPMS_ON);
-
-	DBG("%s: mode=%d", mdp4_crtc->name, mode);
-
-	if (enabled != mdp4_crtc->enabled) {
-		if (enabled) {
-			mdp4_enable(mdp4_kms);
-			mdp_irq_register(&mdp4_kms->base, &mdp4_crtc->err);
-		} else {
-			mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
-			mdp4_disable(mdp4_kms);
-		}
-		mdp4_crtc->enabled = enabled;
-	}
-}
-
 static bool mdp4_crtc_mode_fixup(struct drm_crtc *crtc,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
@@ -304,23 +284,38 @@ static void mdp4_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	}
 }
 
-static void mdp4_crtc_prepare(struct drm_crtc *crtc)
+static void mdp4_crtc_disable(struct drm_crtc *crtc)
 {
 	struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
+	struct mdp4_kms *mdp4_kms = get_kms(crtc);
+
 	DBG("%s", mdp4_crtc->name);
-	/* make sure we hold a ref to mdp clks while setting up mode: */
-	drm_crtc_vblank_get(crtc);
-	mdp4_enable(get_kms(crtc));
-	mdp4_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+
+	if (WARN_ON(!mdp4_crtc->enabled))
+		return;
+
+	mdp_irq_unregister(&mdp4_kms->base, &mdp4_crtc->err);
+	mdp4_disable(mdp4_kms);
+
+	mdp4_crtc->enabled = false;
 }
 
-static void mdp4_crtc_commit(struct drm_crtc *crtc)
+static void mdp4_crtc_enable(struct drm_crtc *crtc)
 {
-	mdp4_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
+	struct mdp4_crtc *mdp4_crtc = to_mdp4_crtc(crtc);
+	struct mdp4_kms *mdp4_kms = get_kms(crtc);
+
+	DBG("%s", mdp4_crtc->name);
+
+	if (WARN_ON(mdp4_crtc->enabled))
+		return;
+
+	mdp4_enable(mdp4_kms);
+	mdp_irq_register(&mdp4_kms->base, &mdp4_crtc->err);
+
 	crtc_flush(crtc);
-	/* drop the ref to mdp clk's that we got in prepare: */
-	mdp4_disable(get_kms(crtc));
-	drm_crtc_vblank_put(crtc);
+
+	mdp4_crtc->enabled = true;
 }
 
 static int mdp4_crtc_atomic_check(struct drm_crtc *crtc,
@@ -504,11 +499,10 @@ static const struct drm_crtc_funcs mdp4_crtc_funcs = {
 };
 
 static const struct drm_crtc_helper_funcs mdp4_crtc_helper_funcs = {
-	.dpms = mdp4_crtc_dpms,
 	.mode_fixup = mdp4_crtc_mode_fixup,
 	.mode_set_nofb = mdp4_crtc_mode_set_nofb,
-	.prepare = mdp4_crtc_prepare,
-	.commit = mdp4_crtc_commit,
+	.disable = mdp4_crtc_disable,
+	.enable = mdp4_crtc_enable,
 	.atomic_check = mdp4_crtc_atomic_check,
 	.atomic_begin = mdp4_crtc_atomic_begin,
 	.atomic_flush = mdp4_crtc_atomic_flush,
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c
index c3878420180b..7896323b2631 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_dtv_encoder.c
@@ -94,61 +94,6 @@ static const struct drm_encoder_funcs mdp4_dtv_encoder_funcs = {
 	.destroy = mdp4_dtv_encoder_destroy,
 };
 
-static void mdp4_dtv_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct drm_device *dev = encoder->dev;
-	struct mdp4_dtv_encoder *mdp4_dtv_encoder = to_mdp4_dtv_encoder(encoder);
-	struct mdp4_kms *mdp4_kms = get_kms(encoder);
-	bool enabled = (mode == DRM_MODE_DPMS_ON);
-
-	DBG("mode=%d", mode);
-
-	if (enabled == mdp4_dtv_encoder->enabled)
-		return;
-
-	if (enabled) {
-		unsigned long pc = mdp4_dtv_encoder->pixclock;
-		int ret;
-
-		bs_set(mdp4_dtv_encoder, 1);
-
-		DBG("setting src_clk=%lu", pc);
-
-		ret = clk_set_rate(mdp4_dtv_encoder->src_clk, pc);
-		if (ret)
-			dev_err(dev->dev, "failed to set src_clk to %lu: %d\n", pc, ret);
-		clk_prepare_enable(mdp4_dtv_encoder->src_clk);
-		ret = clk_prepare_enable(mdp4_dtv_encoder->hdmi_clk);
-		if (ret)
-			dev_err(dev->dev, "failed to enable hdmi_clk: %d\n", ret);
-		ret = clk_prepare_enable(mdp4_dtv_encoder->mdp_clk);
-		if (ret)
-			dev_err(dev->dev, "failed to enabled mdp_clk: %d\n", ret);
-
-		mdp4_write(mdp4_kms, REG_MDP4_DTV_ENABLE, 1);
-	} else {
-		mdp4_write(mdp4_kms, REG_MDP4_DTV_ENABLE, 0);
-
-		/*
-		 * Wait for a vsync so we know the ENABLE=0 latched before
-		 * the (connector) source of the vsync's gets disabled,
-		 * otherwise we end up in a funny state if we re-enable
-		 * before the disable latches, which results that some of
-		 * the settings changes for the new modeset (like new
-		 * scanout buffer) don't latch properly..
-		 */
-		mdp_irq_wait(&mdp4_kms->base, MDP4_IRQ_EXTERNAL_VSYNC);
-
-		clk_disable_unprepare(mdp4_dtv_encoder->src_clk);
-		clk_disable_unprepare(mdp4_dtv_encoder->hdmi_clk);
-		clk_disable_unprepare(mdp4_dtv_encoder->mdp_clk);
-
-		bs_set(mdp4_dtv_encoder, 0);
-	}
-
-	mdp4_dtv_encoder->enabled = enabled;
-}
-
 static bool mdp4_dtv_encoder_mode_fixup(struct drm_encoder *encoder,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
@@ -221,28 +166,78 @@ static void mdp4_dtv_encoder_mode_set(struct drm_encoder *encoder,
 	mdp4_write(mdp4_kms, REG_MDP4_DTV_ACTIVE_VEND, 0);
 }
 
-static void mdp4_dtv_encoder_prepare(struct drm_encoder *encoder)
+static void mdp4_dtv_encoder_disable(struct drm_encoder *encoder)
 {
-	mdp4_dtv_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+	struct mdp4_dtv_encoder *mdp4_dtv_encoder = to_mdp4_dtv_encoder(encoder);
+	struct mdp4_kms *mdp4_kms = get_kms(encoder);
+
+	if (WARN_ON(!mdp4_dtv_encoder->enabled))
+		return;
+
+	mdp4_write(mdp4_kms, REG_MDP4_DTV_ENABLE, 0);
+
+	/*
+	 * Wait for a vsync so we know the ENABLE=0 latched before
+	 * the (connector) source of the vsync's gets disabled,
+	 * otherwise we end up in a funny state if we re-enable
+	 * before the disable latches, which results that some of
+	 * the settings changes for the new modeset (like new
+	 * scanout buffer) don't latch properly..
+	 */
+	mdp_irq_wait(&mdp4_kms->base, MDP4_IRQ_EXTERNAL_VSYNC);
+
+	clk_disable_unprepare(mdp4_dtv_encoder->src_clk);
+	clk_disable_unprepare(mdp4_dtv_encoder->hdmi_clk);
+	clk_disable_unprepare(mdp4_dtv_encoder->mdp_clk);
+
+	bs_set(mdp4_dtv_encoder, 0);
+
+	mdp4_dtv_encoder->enabled = false;
 }
 
-static void mdp4_dtv_encoder_commit(struct drm_encoder *encoder)
+static void mdp4_dtv_encoder_enable(struct drm_encoder *encoder)
 {
+	struct drm_device *dev = encoder->dev;
+	struct mdp4_dtv_encoder *mdp4_dtv_encoder = to_mdp4_dtv_encoder(encoder);
+	struct mdp4_kms *mdp4_kms = get_kms(encoder);
+	unsigned long pc = mdp4_dtv_encoder->pixclock;
+	int ret;
+
+	if (WARN_ON(mdp4_dtv_encoder->enabled))
+		return;
+
 	mdp4_crtc_set_config(encoder->crtc,
 			MDP4_DMA_CONFIG_R_BPC(BPC8) |
 			MDP4_DMA_CONFIG_G_BPC(BPC8) |
 			MDP4_DMA_CONFIG_B_BPC(BPC8) |
 			MDP4_DMA_CONFIG_PACK(0x21));
 	mdp4_crtc_set_intf(encoder->crtc, INTF_LCDC_DTV, 1);
-	mdp4_dtv_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+
+	bs_set(mdp4_dtv_encoder, 1);
+
+	DBG("setting src_clk=%lu", pc);
+
+	ret = clk_set_rate(mdp4_dtv_encoder->src_clk, pc);
+	if (ret)
+		dev_err(dev->dev, "failed to set src_clk to %lu: %d\n", pc, ret);
+	clk_prepare_enable(mdp4_dtv_encoder->src_clk);
+	ret = clk_prepare_enable(mdp4_dtv_encoder->hdmi_clk);
+	if (ret)
+		dev_err(dev->dev, "failed to enable hdmi_clk: %d\n", ret);
+	ret = clk_prepare_enable(mdp4_dtv_encoder->mdp_clk);
+	if (ret)
+		dev_err(dev->dev, "failed to enabled mdp_clk: %d\n", ret);
+
+	mdp4_write(mdp4_kms, REG_MDP4_DTV_ENABLE, 1);
+
+	mdp4_dtv_encoder->enabled = true;
 }
 
 static const struct drm_encoder_helper_funcs mdp4_dtv_encoder_helper_funcs = {
-	.dpms = mdp4_dtv_encoder_dpms,
 	.mode_fixup = mdp4_dtv_encoder_mode_fixup,
 	.mode_set = mdp4_dtv_encoder_mode_set,
-	.prepare = mdp4_dtv_encoder_prepare,
-	.commit = mdp4_dtv_encoder_commit,
+	.enable = mdp4_dtv_encoder_enable,
+	.disable = mdp4_dtv_encoder_disable,
 };
 
 long mdp4_dtv_round_pixclk(struct drm_encoder *encoder, unsigned long rate)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
index a62109e4ae0d..d847b9436194 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_kms.c
@@ -125,6 +125,38 @@ out:
 	return ret;
 }
 
+static void mdp4_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+{
+	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
+	int i, ncrtcs = state->dev->mode_config.num_crtc;
+
+	mdp4_enable(mdp4_kms);
+
+	/* see 119ecb7fd */
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc *crtc = state->crtcs[i];
+		if (!crtc)
+			continue;
+		drm_crtc_vblank_get(crtc);
+	}
+}
+
+static void mdp4_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+{
+	struct mdp4_kms *mdp4_kms = to_mdp4_kms(to_mdp_kms(kms));
+	int i, ncrtcs = state->dev->mode_config.num_crtc;
+
+	/* see 119ecb7fd */
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc *crtc = state->crtcs[i];
+		if (!crtc)
+			continue;
+		drm_crtc_vblank_put(crtc);
+	}
+
+	mdp4_disable(mdp4_kms);
+}
+
 static long mdp4_round_pixclk(struct msm_kms *kms, unsigned long rate,
 		struct drm_encoder *encoder)
 {
@@ -161,6 +193,8 @@ static const struct mdp_kms_funcs kms_funcs = {
 		.irq             = mdp4_irq,
 		.enable_vblank   = mdp4_enable_vblank,
 		.disable_vblank  = mdp4_disable_vblank,
+		.prepare_commit  = mdp4_prepare_commit,
+		.complete_commit = mdp4_complete_commit,
 		.get_format      = mdp_get_format,
 		.round_pixclk    = mdp4_round_pixclk,
 		.preclose        = mdp4_preclose,
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
index 41f6436754fc..60ec8222c9f6 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lcdc_encoder.c
@@ -259,77 +259,6 @@ static void setup_phy(struct drm_encoder *encoder)
 	mdp4_write(mdp4_kms, REG_MDP4_LVDS_PHY_CFG0, lvds_phy_cfg0);
 }
 
-static void mdp4_lcdc_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct drm_device *dev = encoder->dev;
-	struct mdp4_lcdc_encoder *mdp4_lcdc_encoder =
-			to_mdp4_lcdc_encoder(encoder);
-	struct mdp4_kms *mdp4_kms = get_kms(encoder);
-	struct drm_panel *panel = mdp4_lcdc_encoder->panel;
-	bool enabled = (mode == DRM_MODE_DPMS_ON);
-	int i, ret;
-
-	DBG("mode=%d", mode);
-
-	if (enabled == mdp4_lcdc_encoder->enabled)
-		return;
-
-	if (enabled) {
-		unsigned long pc = mdp4_lcdc_encoder->pixclock;
-		int ret;
-
-		bs_set(mdp4_lcdc_encoder, 1);
-
-		for (i = 0; i < ARRAY_SIZE(mdp4_lcdc_encoder->regs); i++) {
-			ret = regulator_enable(mdp4_lcdc_encoder->regs[i]);
-			if (ret)
-				dev_err(dev->dev, "failed to enable regulator: %d\n", ret);
-		}
-
-		DBG("setting lcdc_clk=%lu", pc);
-		ret = clk_set_rate(mdp4_lcdc_encoder->lcdc_clk, pc);
-		if (ret)
-			dev_err(dev->dev, "failed to configure lcdc_clk: %d\n", ret);
-		ret = clk_prepare_enable(mdp4_lcdc_encoder->lcdc_clk);
-		if (ret)
-			dev_err(dev->dev, "failed to enable lcdc_clk: %d\n", ret);
-
-		if (panel)
-			drm_panel_enable(panel);
-
-		setup_phy(encoder);
-
-		mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 1);
-	} else {
-		mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 0);
-
-		if (panel)
-			drm_panel_disable(panel);
-
-		/*
-		 * Wait for a vsync so we know the ENABLE=0 latched before
-		 * the (connector) source of the vsync's gets disabled,
-		 * otherwise we end up in a funny state if we re-enable
-		 * before the disable latches, which results that some of
-		 * the settings changes for the new modeset (like new
-		 * scanout buffer) don't latch properly..
-		 */
-		mdp_irq_wait(&mdp4_kms->base, MDP4_IRQ_PRIMARY_VSYNC);
-
-		clk_disable_unprepare(mdp4_lcdc_encoder->lcdc_clk);
-
-		for (i = 0; i < ARRAY_SIZE(mdp4_lcdc_encoder->regs); i++) {
-			ret = regulator_disable(mdp4_lcdc_encoder->regs[i]);
-			if (ret)
-				dev_err(dev->dev, "failed to disable regulator: %d\n", ret);
-		}
-
-		bs_set(mdp4_lcdc_encoder, 0);
-	}
-
-	mdp4_lcdc_encoder->enabled = enabled;
-}
-
 static bool mdp4_lcdc_encoder_mode_fixup(struct drm_encoder *encoder,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
@@ -403,13 +332,59 @@ static void mdp4_lcdc_encoder_mode_set(struct drm_encoder *encoder,
 	mdp4_write(mdp4_kms, REG_MDP4_LCDC_ACTIVE_VEND, 0);
 }
 
-static void mdp4_lcdc_encoder_prepare(struct drm_encoder *encoder)
+static void mdp4_lcdc_encoder_disable(struct drm_encoder *encoder)
 {
-	mdp4_lcdc_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+	struct drm_device *dev = encoder->dev;
+	struct mdp4_lcdc_encoder *mdp4_lcdc_encoder =
+			to_mdp4_lcdc_encoder(encoder);
+	struct mdp4_kms *mdp4_kms = get_kms(encoder);
+	struct drm_panel *panel = mdp4_lcdc_encoder->panel;
+	int i, ret;
+
+	if (WARN_ON(!mdp4_lcdc_encoder->enabled))
+		return;
+
+	mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 0);
+
+	if (panel)
+		drm_panel_disable(panel);
+
+	/*
+	 * Wait for a vsync so we know the ENABLE=0 latched before
+	 * the (connector) source of the vsync's gets disabled,
+	 * otherwise we end up in a funny state if we re-enable
+	 * before the disable latches, which results that some of
+	 * the settings changes for the new modeset (like new
+	 * scanout buffer) don't latch properly..
+	 */
+	mdp_irq_wait(&mdp4_kms->base, MDP4_IRQ_PRIMARY_VSYNC);
+
+	clk_disable_unprepare(mdp4_lcdc_encoder->lcdc_clk);
+
+	for (i = 0; i < ARRAY_SIZE(mdp4_lcdc_encoder->regs); i++) {
+		ret = regulator_disable(mdp4_lcdc_encoder->regs[i]);
+		if (ret)
+			dev_err(dev->dev, "failed to disable regulator: %d\n", ret);
+	}
+
+	bs_set(mdp4_lcdc_encoder, 0);
+
+	mdp4_lcdc_encoder->enabled = false;
 }
 
-static void mdp4_lcdc_encoder_commit(struct drm_encoder *encoder)
+static void mdp4_lcdc_encoder_enable(struct drm_encoder *encoder)
 {
+	struct drm_device *dev = encoder->dev;
+	struct mdp4_lcdc_encoder *mdp4_lcdc_encoder =
+			to_mdp4_lcdc_encoder(encoder);
+	unsigned long pc = mdp4_lcdc_encoder->pixclock;
+	struct mdp4_kms *mdp4_kms = get_kms(encoder);
+	struct drm_panel *panel = mdp4_lcdc_encoder->panel;
+	int i, ret;
+
+	if (WARN_ON(mdp4_lcdc_encoder->enabled))
+		return;
+
 	/* TODO: hard-coded for 18bpp: */
 	mdp4_crtc_set_config(encoder->crtc,
 			MDP4_DMA_CONFIG_R_BPC(BPC6) |
@@ -420,15 +395,38 @@ static void mdp4_lcdc_encoder_commit(struct drm_encoder *encoder)
 			MDP4_DMA_CONFIG_DEFLKR_EN |
 			MDP4_DMA_CONFIG_DITHER_EN);
 	mdp4_crtc_set_intf(encoder->crtc, INTF_LCDC_DTV, 0);
-	mdp4_lcdc_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+
+	bs_set(mdp4_lcdc_encoder, 1);
+
+	for (i = 0; i < ARRAY_SIZE(mdp4_lcdc_encoder->regs); i++) {
+		ret = regulator_enable(mdp4_lcdc_encoder->regs[i]);
+		if (ret)
+			dev_err(dev->dev, "failed to enable regulator: %d\n", ret);
+	}
+
+	DBG("setting lcdc_clk=%lu", pc);
+	ret = clk_set_rate(mdp4_lcdc_encoder->lcdc_clk, pc);
+	if (ret)
+		dev_err(dev->dev, "failed to configure lcdc_clk: %d\n", ret);
+	ret = clk_prepare_enable(mdp4_lcdc_encoder->lcdc_clk);
+	if (ret)
+		dev_err(dev->dev, "failed to enable lcdc_clk: %d\n", ret);
+
+	if (panel)
+		drm_panel_enable(panel);
+
+	setup_phy(encoder);
+
+	mdp4_write(mdp4_kms, REG_MDP4_LCDC_ENABLE, 1);
+
+	mdp4_lcdc_encoder->enabled = true;
 }
 
 static const struct drm_encoder_helper_funcs mdp4_lcdc_encoder_helper_funcs = {
-	.dpms = mdp4_lcdc_encoder_dpms,
 	.mode_fixup = mdp4_lcdc_encoder_mode_fixup,
 	.mode_set = mdp4_lcdc_encoder_mode_set,
-	.prepare = mdp4_lcdc_encoder_prepare,
-	.commit = mdp4_lcdc_encoder_commit,
+	.disable = mdp4_lcdc_encoder_disable,
+	.enable = mdp4_lcdc_encoder_enable,
 };
 
 long mdp4_lcdc_round_pixclk(struct drm_encoder *encoder, unsigned long rate)
diff --git a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
index 4ddc28e1275b..921185133d38 100644
--- a/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
+++ b/drivers/gpu/drm/msm/mdp/mdp4/mdp4_lvds_connector.c
@@ -94,7 +94,7 @@ mdp4_lvds_connector_best_encoder(struct drm_connector *connector)
 }
 
 static const struct drm_connector_funcs mdp4_lvds_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.dpms = drm_atomic_helper_connector_dpms,
 	.detect = mdp4_lvds_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = mdp4_lvds_connector_destroy,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
index 47f101d2f9fa..7416cae60479 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_crtc.c
@@ -138,28 +138,6 @@ static void mdp5_crtc_destroy(struct drm_crtc *crtc)
 	kfree(mdp5_crtc);
 }
 
-static void mdp5_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
-	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
-	struct mdp5_kms *mdp5_kms = get_kms(crtc);
-	bool enabled = (mode == DRM_MODE_DPMS_ON);
-
-	DBG("%s: mode=%d", mdp5_crtc->name, mode);
-
-	if (enabled != mdp5_crtc->enabled) {
-		if (enabled) {
-			mdp5_enable(mdp5_kms);
-			mdp_irq_register(&mdp5_kms->base, &mdp5_crtc->err);
-		} else {
-			/* set STAGE_UNUSED for all layers */
-			mdp5_ctl_blend(mdp5_crtc->ctl, mdp5_crtc->lm, 0x00000000);
-			mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
-			mdp5_disable(mdp5_kms);
-		}
-		mdp5_crtc->enabled = enabled;
-	}
-}
-
 static bool mdp5_crtc_mode_fixup(struct drm_crtc *crtc,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
@@ -256,23 +234,41 @@ static void mdp5_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	spin_unlock_irqrestore(&mdp5_crtc->lm_lock, flags);
 }
 
-static void mdp5_crtc_prepare(struct drm_crtc *crtc)
+static void mdp5_crtc_disable(struct drm_crtc *crtc)
 {
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
+	struct mdp5_kms *mdp5_kms = get_kms(crtc);
+
 	DBG("%s", mdp5_crtc->name);
-	/* make sure we hold a ref to mdp clks while setting up mode: */
-	mdp5_enable(get_kms(crtc));
-	mdp5_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
+
+	if (WARN_ON(!mdp5_crtc->enabled))
+		return;
+
+	/* set STAGE_UNUSED for all layers */
+	mdp5_ctl_blend(mdp5_crtc->ctl, mdp5_crtc->lm, 0x00000000);
+
+	mdp_irq_unregister(&mdp5_kms->base, &mdp5_crtc->err);
+	mdp5_disable(mdp5_kms);
+
+	mdp5_crtc->enabled = false;
 }
 
-static void mdp5_crtc_commit(struct drm_crtc *crtc)
+static void mdp5_crtc_enable(struct drm_crtc *crtc)
 {
 	struct mdp5_crtc *mdp5_crtc = to_mdp5_crtc(crtc);
+	struct mdp5_kms *mdp5_kms = get_kms(crtc);
+
 	DBG("%s", mdp5_crtc->name);
-	mdp5_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
+
+	if (WARN_ON(mdp5_crtc->enabled))
+		return;
+
+	mdp5_enable(mdp5_kms);
+	mdp_irq_register(&mdp5_kms->base, &mdp5_crtc->err);
+
 	crtc_flush_all(crtc);
-	/* drop the ref to mdp clk's that we got in prepare: */
-	mdp5_disable(get_kms(crtc));
+
+	mdp5_crtc->enabled = true;
 }
 
 struct plane_state {
@@ -391,11 +387,10 @@ static const struct drm_crtc_funcs mdp5_crtc_funcs = {
 };
 
 static const struct drm_crtc_helper_funcs mdp5_crtc_helper_funcs = {
-	.dpms = mdp5_crtc_dpms,
 	.mode_fixup = mdp5_crtc_mode_fixup,
 	.mode_set_nofb = mdp5_crtc_mode_set_nofb,
-	.prepare = mdp5_crtc_prepare,
-	.commit = mdp5_crtc_commit,
+	.prepare = mdp5_crtc_disable,
+	.commit = mdp5_crtc_enable,
 	.atomic_check = mdp5_crtc_atomic_check,
 	.atomic_begin = mdp5_crtc_atomic_begin,
 	.atomic_flush = mdp5_crtc_atomic_flush,
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
index 0254bfdeb92f..b5494315345d 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_encoder.c
@@ -110,45 +110,6 @@ static const struct drm_encoder_funcs mdp5_encoder_funcs = {
 	.destroy = mdp5_encoder_destroy,
 };
 
-static void mdp5_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
-	struct mdp5_kms *mdp5_kms = get_kms(encoder);
-	int intf = mdp5_encoder->intf;
-	bool enabled = (mode == DRM_MODE_DPMS_ON);
-	unsigned long flags;
-
-	DBG("mode=%d", mode);
-
-	if (enabled == mdp5_encoder->enabled)
-		return;
-
-	if (enabled) {
-		bs_set(mdp5_encoder, 1);
-		spin_lock_irqsave(&mdp5_encoder->intf_lock, flags);
-		mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(intf), 1);
-		spin_unlock_irqrestore(&mdp5_encoder->intf_lock, flags);
-	} else {
-		spin_lock_irqsave(&mdp5_encoder->intf_lock, flags);
-		mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(intf), 0);
-		spin_unlock_irqrestore(&mdp5_encoder->intf_lock, flags);
-
-		/*
-		 * Wait for a vsync so we know the ENABLE=0 latched before
-		 * the (connector) source of the vsync's gets disabled,
-		 * otherwise we end up in a funny state if we re-enable
-		 * before the disable latches, which results that some of
-		 * the settings changes for the new modeset (like new
-		 * scanout buffer) don't latch properly..
-		 */
-		mdp_irq_wait(&mdp5_kms->base, intf2vblank(intf));
-
-		bs_set(mdp5_encoder, 0);
-	}
-
-	mdp5_encoder->enabled = enabled;
-}
-
 static bool mdp5_encoder_mode_fixup(struct drm_encoder *encoder,
 		const struct drm_display_mode *mode,
 		struct drm_display_mode *adjusted_mode)
@@ -225,25 +186,61 @@ static void mdp5_encoder_mode_set(struct drm_encoder *encoder,
 	spin_unlock_irqrestore(&mdp5_encoder->intf_lock, flags);
 }
 
-static void mdp5_encoder_prepare(struct drm_encoder *encoder)
+static void mdp5_encoder_disable(struct drm_encoder *encoder)
 {
-	mdp5_encoder_dpms(encoder, DRM_MODE_DPMS_OFF);
+	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
+	struct mdp5_kms *mdp5_kms = get_kms(encoder);
+	int intf = mdp5_encoder->intf;
+	unsigned long flags;
+
+	if (WARN_ON(!mdp5_encoder->enabled))
+		return;
+
+	spin_lock_irqsave(&mdp5_encoder->intf_lock, flags);
+	mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(intf), 0);
+	spin_unlock_irqrestore(&mdp5_encoder->intf_lock, flags);
+
+	/*
+	 * Wait for a vsync so we know the ENABLE=0 latched before
+	 * the (connector) source of the vsync's gets disabled,
+	 * otherwise we end up in a funny state if we re-enable
+	 * before the disable latches, which results that some of
+	 * the settings changes for the new modeset (like new
+	 * scanout buffer) don't latch properly..
+	 */
+	mdp_irq_wait(&mdp5_kms->base, intf2vblank(intf));
+
+	bs_set(mdp5_encoder, 0);
+
+	mdp5_encoder->enabled = false;
 }
 
-static void mdp5_encoder_commit(struct drm_encoder *encoder)
+static void mdp5_encoder_enable(struct drm_encoder *encoder)
 {
 	struct mdp5_encoder *mdp5_encoder = to_mdp5_encoder(encoder);
+	struct mdp5_kms *mdp5_kms = get_kms(encoder);
+	int intf = mdp5_encoder->intf;
+	unsigned long flags;
+
+	if (WARN_ON(mdp5_encoder->enabled))
+		return;
+
 	mdp5_crtc_set_intf(encoder->crtc, mdp5_encoder->intf,
 			mdp5_encoder->intf_id);
-	mdp5_encoder_dpms(encoder, DRM_MODE_DPMS_ON);
+
+	bs_set(mdp5_encoder, 1);
+	spin_lock_irqsave(&mdp5_encoder->intf_lock, flags);
+	mdp5_write(mdp5_kms, REG_MDP5_INTF_TIMING_ENGINE_EN(intf), 1);
+	spin_unlock_irqrestore(&mdp5_encoder->intf_lock, flags);
+
+	mdp5_encoder->enabled = false;
 }
 
 static const struct drm_encoder_helper_funcs mdp5_encoder_helper_funcs = {
-	.dpms = mdp5_encoder_dpms,
 	.mode_fixup = mdp5_encoder_mode_fixup,
 	.mode_set = mdp5_encoder_mode_set,
-	.prepare = mdp5_encoder_prepare,
-	.commit = mdp5_encoder_commit,
+	.prepare = mdp5_encoder_disable,
+	.commit = mdp5_encoder_enable,
 };
 
 /* initialize encoder */
diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 9f01a4f21af2..e13e102372f5 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -68,6 +68,18 @@ static int mdp5_hw_init(struct msm_kms *kms)
 	return 0;
 }
 
+static void mdp5_prepare_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+{
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+	mdp5_enable(mdp5_kms);
+}
+
+static void mdp5_complete_commit(struct msm_kms *kms, struct drm_atomic_state *state)
+{
+	struct mdp5_kms *mdp5_kms = to_mdp5_kms(to_mdp_kms(kms));
+	mdp5_disable(mdp5_kms);
+}
+
 static long mdp5_round_pixclk(struct msm_kms *kms, unsigned long rate,
 		struct drm_encoder *encoder)
 {
@@ -115,6 +127,8 @@ static const struct mdp_kms_funcs kms_funcs = {
 		.irq             = mdp5_irq,
 		.enable_vblank   = mdp5_enable_vblank,
 		.disable_vblank  = mdp5_disable_vblank,
+		.prepare_commit  = mdp5_prepare_commit,
+		.complete_commit = mdp5_complete_commit,
 		.get_format      = mdp_get_format,
 		.round_pixclk    = mdp5_round_pixclk,
 		.preclose        = mdp5_preclose,
diff --git a/drivers/gpu/drm/msm/msm_atomic.c b/drivers/gpu/drm/msm/msm_atomic.c
index 2c396540e279..2beef4e670d0 100644
--- a/drivers/gpu/drm/msm/msm_atomic.c
+++ b/drivers/gpu/drm/msm/msm_atomic.c
@@ -20,6 +20,7 @@
 #include "msm_gem.h"
 
 struct msm_commit {
+	struct drm_device *dev;
 	struct drm_atomic_state *state;
 	uint32_t fence;
 	struct msm_fence_cb fence_cb;
@@ -58,14 +59,16 @@ static void end_atomic(struct msm_drm_private *priv, uint32_t crtc_mask)
 	spin_unlock(&priv->pending_crtcs_event.lock);
 }
 
-static struct msm_commit *new_commit(struct drm_atomic_state *state)
+static struct msm_commit *commit_init(struct drm_atomic_state *state)
 {
 	struct msm_commit *c = kzalloc(sizeof(*c), GFP_KERNEL);
 
 	if (!c)
 		return NULL;
 
+	c->dev = state->dev;
 	c->state = state;
+
 	/* TODO we might need a way to indicate to run the cb on a
 	 * different wq so wait_for_vblanks() doesn't block retiring
 	 * bo's..
@@ -75,6 +78,12 @@ static struct msm_commit *new_commit(struct drm_atomic_state *state)
 	return c;
 }
 
+static void commit_destroy(struct msm_commit *c)
+{
+	end_atomic(c->dev->dev_private, c->crtc_mask);
+	kfree(c);
+}
+
 /* The (potentially) asynchronous part of the commit.  At this point
  * nothing can fail short of armageddon.
  */
@@ -82,6 +91,10 @@ static void complete_commit(struct msm_commit *c)
 {
 	struct drm_atomic_state *state = c->state;
 	struct drm_device *dev = state->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+	struct msm_kms *kms = priv->kms;
+
+	kms->funcs->prepare_commit(kms, state);
 
 	drm_atomic_helper_commit_pre_planes(dev, state);
 
@@ -106,11 +119,11 @@ static void complete_commit(struct msm_commit *c)
 
 	drm_atomic_helper_cleanup_planes(dev, state);
 
-	drm_atomic_state_free(state);
+	kms->funcs->complete_commit(kms, state);
 
-	end_atomic(dev->dev_private, c->crtc_mask);
+	drm_atomic_state_free(state);
 
-	kfree(c);
+	commit_destroy(c);
 }
 
 static void fence_cb(struct msm_fence_cb *cb)
@@ -172,7 +185,7 @@ int msm_atomic_commit(struct drm_device *dev,
 	if (ret)
 		return ret;
 
-	c = new_commit(state);
+	c = commit_init(state);
 	if (!c)
 		return -ENOMEM;
 
@@ -240,7 +253,7 @@ int msm_atomic_commit(struct drm_device *dev,
 	ret = msm_wait_fence_interruptable(dev, c->fence, NULL);
 	if (ret) {
 		WARN_ON(ret);  // TODO unswap state back?  or??
-		kfree(c);
+		commit_destroy(c);
 		return ret;
 	}
 
diff --git a/drivers/gpu/drm/msm/msm_fbdev.c b/drivers/gpu/drm/msm/msm_fbdev.c
index 115b509a4a00..df60f65728ff 100644
--- a/drivers/gpu/drm/msm/msm_fbdev.c
+++ b/drivers/gpu/drm/msm/msm_fbdev.c
@@ -245,9 +245,6 @@ struct drm_fb_helper *msm_fbdev_init(struct drm_device *dev)
 	if (ret)
 		goto fini;
 
-	/* disable all the possible outputs/crtcs before entering KMS mode */
-	drm_helper_disable_unused_functions(dev);
-
 	ret = drm_fb_helper_initial_config(helper, 32);
 	if (ret)
 		goto fini;
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 06437745bc2c..867672eb1fee 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -38,6 +38,9 @@ struct msm_kms_funcs {
 	irqreturn_t (*irq)(struct msm_kms *kms);
 	int (*enable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
 	void (*disable_vblank)(struct msm_kms *kms, struct drm_crtc *crtc);
+	/* modeset, bracketing atomic_commit(): */
+	void (*prepare_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
+	void (*complete_commit)(struct msm_kms *kms, struct drm_atomic_state *state);
 	/* misc: */
 	const struct msm_format *(*get_format)(struct msm_kms *kms, uint32_t format);
 	long (*round_pixclk)(struct msm_kms *kms, unsigned long rate,