diff mbox

[v9,16/18] drm/exynos: split exynos_crtc->dpms in enable() and disable()

Message ID 1432849376-12828-17-git-send-email-gustavo@padovan.org (mailing list archive)
State New, archived
Headers show

Commit Message

Gustavo Padovan May 28, 2015, 9:42 p.m. UTC 
From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

To follow more closely the new atomic API we split the dpms()
helper into the enable() and disable() helper to get exactly the
same semantics.

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/exynos/exynos7_drm_decon.c | 31 +++++------------
 drivers/gpu/drm/exynos/exynos_drm_crtc.c   |  8 ++---
 drivers/gpu/drm/exynos/exynos_drm_drv.h    |  6 ++--
 drivers/gpu/drm/exynos/exynos_drm_fimd.c   | 31 +++++------------
 drivers/gpu/drm/exynos/exynos_drm_vidi.c   | 55 ++++++++++++------------------
 drivers/gpu/drm/exynos/exynos_mixer.c      | 32 ++++++-----------
 6 files changed, 56 insertions(+), 107 deletions(-)

Comments

Joonyoung Shim May 29, 2015, 6:23 a.m. UTC | #1 
On 05/29/2015 06:42 AM, Gustavo Padovan wrote:
> From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> 
> To follow more closely the new atomic API we split the dpms()
> helper into the enable() and disable() helper to get exactly the
> same semantics.
> 
> Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
> ---
>  drivers/gpu/drm/exynos/exynos7_drm_decon.c | 31 +++++------------
>  drivers/gpu/drm/exynos/exynos_drm_crtc.c   |  8 ++---
>  drivers/gpu/drm/exynos/exynos_drm_drv.h    |  6 ++--
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c   | 31 +++++------------
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c   | 55 ++++++++++++------------------
>  drivers/gpu/drm/exynos/exynos_mixer.c      | 32 ++++++-----------
>  6 files changed, 56 insertions(+), 107 deletions(-)
> 
> diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> index 6714e5b..0008658 100644
> --- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> +++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
> @@ -603,8 +603,9 @@ static void decon_init(struct decon_context *ctx)
>  		writel(VIDCON1_VCLK_HOLD, ctx->regs + VIDCON1(0));
>  }
>  
> -static int decon_poweron(struct decon_context *ctx)
> +static int decon_enable(struct exynos_drm_crtc *crtc)
>  {
> +	struct decon_context *ctx = crtc->ctx;
>  	int ret;
>  
>  	if (!ctx->suspended)
> @@ -668,8 +669,10 @@ pclk_err:
>  	return ret;
>  }
>  
> -static int decon_poweroff(struct decon_context *ctx)
> +static int decon_disable(struct exynos_drm_crtc *crtc)
>  {
> +	struct decon_context *ctx = crtc->ctx;
> +
>  	if (ctx->suspended)
>  		return 0;
>  
> @@ -691,27 +694,9 @@ static int decon_poweroff(struct decon_context *ctx)
>  	return 0;
>  }
>  
> -static void decon_dpms(struct exynos_drm_crtc *crtc, int mode)
> -{
> -	DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode);
> -
> -	switch (mode) {
> -	case DRM_MODE_DPMS_ON:
> -		decon_poweron(crtc->ctx);
> -		break;
> -	case DRM_MODE_DPMS_STANDBY:
> -	case DRM_MODE_DPMS_SUSPEND:
> -	case DRM_MODE_DPMS_OFF:
> -		decon_poweroff(crtc->ctx);
> -		break;
> -	default:
> -		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> -		break;
> -	}
> -}
> -
>  static const struct exynos_drm_crtc_ops decon_crtc_ops = {
> -	.dpms = decon_dpms,
> +	.enable = decon_enable,
> +	.disable = decon_disable,
>  	.mode_fixup = decon_mode_fixup,
>  	.commit = decon_commit,
>  	.enable_vblank = decon_enable_vblank,
> @@ -796,7 +781,7 @@ static void decon_unbind(struct device *dev, struct device *master,
>  {
>  	struct decon_context *ctx = dev_get_drvdata(dev);
>  
> -	decon_dpms(ctx->crtc, DRM_MODE_DPMS_OFF);
> +	decon_disable(ctx->crtc);
>  
>  	if (ctx->display)
>  		exynos_dpi_remove(ctx->display);
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> index b7c6d51..644b4b7 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
> @@ -29,8 +29,8 @@ static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
>  	if (exynos_crtc->enabled)
>  		return;
>  
> -	if (exynos_crtc->ops->dpms)
> -		exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_ON);
> +	if (exynos_crtc->ops->enable)
> +		exynos_crtc->ops->enable(exynos_crtc);
>  
>  	exynos_crtc->enabled = true;
>  
> @@ -51,8 +51,8 @@ static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
>  
>  	drm_crtc_vblank_off(crtc);
>  
> -	if (exynos_crtc->ops->dpms)
> -		exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_OFF);
> +	if (exynos_crtc->ops->disable)
> +		exynos_crtc->ops->disable(exynos_crtc);
>  
>  	exynos_crtc->enabled = false;
>  }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> index 86d6894..a3053a2 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
> @@ -157,7 +157,8 @@ struct exynos_drm_display {
>  /*
>   * Exynos drm crtc ops
>   *
> - * @dpms: control device power.
> + * @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.
> @@ -175,7 +176,8 @@ struct exynos_drm_display {
>   */
>  struct exynos_drm_crtc;
>  struct exynos_drm_crtc_ops {
> -	void (*dpms)(struct exynos_drm_crtc *crtc, int mode);
> +	int (*enable)(struct exynos_drm_crtc *crtc);
> +	int (*disable)(struct exynos_drm_crtc *crtc);

Need return value for .enable and .disable? It's better to use void
because there is no any care about the result. It's possible to not
return .enable functions of fimd and decon driver.

Anyway, more patches including 17/18 can be difficult to merge atomic
patchset. IMHO i think cleanup patches like this would be posted after
atomic patches merged.
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos7_drm_decon.c b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
index 6714e5b..0008658 100644
--- a/drivers/gpu/drm/exynos/exynos7_drm_decon.c
+++ b/drivers/gpu/drm/exynos/exynos7_drm_decon.c
@@ -603,8 +603,9 @@  static void decon_init(struct decon_context *ctx)
 		writel(VIDCON1_VCLK_HOLD, ctx->regs + VIDCON1(0));
 }
 
-static int decon_poweron(struct decon_context *ctx)
+static int decon_enable(struct exynos_drm_crtc *crtc)
 {
+	struct decon_context *ctx = crtc->ctx;
 	int ret;
 
 	if (!ctx->suspended)
@@ -668,8 +669,10 @@  pclk_err:
 	return ret;
 }
 
-static int decon_poweroff(struct decon_context *ctx)
+static int decon_disable(struct exynos_drm_crtc *crtc)
 {
+	struct decon_context *ctx = crtc->ctx;
+
 	if (ctx->suspended)
 		return 0;
 
@@ -691,27 +694,9 @@  static int decon_poweroff(struct decon_context *ctx)
 	return 0;
 }
 
-static void decon_dpms(struct exynos_drm_crtc *crtc, int mode)
-{
-	DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode);
-
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		decon_poweron(crtc->ctx);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-	case DRM_MODE_DPMS_SUSPEND:
-	case DRM_MODE_DPMS_OFF:
-		decon_poweroff(crtc->ctx);
-		break;
-	default:
-		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
-		break;
-	}
-}
-
 static const struct exynos_drm_crtc_ops decon_crtc_ops = {
-	.dpms = decon_dpms,
+	.enable = decon_enable,
+	.disable = decon_disable,
 	.mode_fixup = decon_mode_fixup,
 	.commit = decon_commit,
 	.enable_vblank = decon_enable_vblank,
@@ -796,7 +781,7 @@  static void decon_unbind(struct device *dev, struct device *master,
 {
 	struct decon_context *ctx = dev_get_drvdata(dev);
 
-	decon_dpms(ctx->crtc, DRM_MODE_DPMS_OFF);
+	decon_disable(ctx->crtc);
 
 	if (ctx->display)
 		exynos_dpi_remove(ctx->display);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index b7c6d51..644b4b7 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -29,8 +29,8 @@  static void exynos_drm_crtc_enable(struct drm_crtc *crtc)
 	if (exynos_crtc->enabled)
 		return;
 
-	if (exynos_crtc->ops->dpms)
-		exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_ON);
+	if (exynos_crtc->ops->enable)
+		exynos_crtc->ops->enable(exynos_crtc);
 
 	exynos_crtc->enabled = true;
 
@@ -51,8 +51,8 @@  static void exynos_drm_crtc_disable(struct drm_crtc *crtc)
 
 	drm_crtc_vblank_off(crtc);
 
-	if (exynos_crtc->ops->dpms)
-		exynos_crtc->ops->dpms(exynos_crtc, DRM_MODE_DPMS_OFF);
+	if (exynos_crtc->ops->disable)
+		exynos_crtc->ops->disable(exynos_crtc);
 
 	exynos_crtc->enabled = false;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index 86d6894..a3053a2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -157,7 +157,8 @@  struct exynos_drm_display {
 /*
  * Exynos drm crtc ops
  *
- * @dpms: control device power.
+ * @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.
@@ -175,7 +176,8 @@  struct exynos_drm_display {
  */
 struct exynos_drm_crtc;
 struct exynos_drm_crtc_ops {
-	void (*dpms)(struct exynos_drm_crtc *crtc, int mode);
+	int (*enable)(struct exynos_drm_crtc *crtc);
+	int (*disable)(struct exynos_drm_crtc *crtc);
 	bool (*mode_fixup)(struct exynos_drm_crtc *crtc,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index a0edab8..433c298 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -805,8 +805,9 @@  static void fimd_apply(struct fimd_context *ctx)
 	fimd_commit(ctx->crtc);
 }
 
-static int fimd_poweron(struct fimd_context *ctx)
+static int fimd_enable(struct exynos_drm_crtc *crtc)
 {
+	struct fimd_context *ctx = crtc->ctx;
 	int ret;
 
 	if (!ctx->suspended)
@@ -852,8 +853,10 @@  bus_clk_err:
 	return ret;
 }
 
-static int fimd_poweroff(struct fimd_context *ctx)
+static int fimd_disable(struct exynos_drm_crtc *crtc)
 {
+	struct fimd_context *ctx = crtc->ctx;
+
 	if (ctx->suspended)
 		return 0;
 
@@ -873,25 +876,6 @@  static int fimd_poweroff(struct fimd_context *ctx)
 	return 0;
 }
 
-static void fimd_dpms(struct exynos_drm_crtc *crtc, int mode)
-{
-	DRM_DEBUG_KMS("%s, %d\n", __FILE__, mode);
-
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		fimd_poweron(crtc->ctx);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-	case DRM_MODE_DPMS_SUSPEND:
-	case DRM_MODE_DPMS_OFF:
-		fimd_poweroff(crtc->ctx);
-		break;
-	default:
-		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
-		break;
-	}
-}
-
 static void fimd_trigger(struct device *dev)
 {
 	struct fimd_context *ctx = dev_get_drvdata(dev);
@@ -964,7 +948,8 @@  static void fimd_dp_clock_enable(struct exynos_drm_crtc *crtc, bool enable)
 }
 
 static const struct exynos_drm_crtc_ops fimd_crtc_ops = {
-	.dpms = fimd_dpms,
+	.enable = fimd_enable,
+	.disable = fimd_disable,
 	.mode_fixup = fimd_mode_fixup,
 	.commit = fimd_commit,
 	.enable_vblank = fimd_enable_vblank,
@@ -1051,7 +1036,7 @@  static void fimd_unbind(struct device *dev, struct device *master,
 {
 	struct fimd_context *ctx = dev_get_drvdata(dev);
 
-	fimd_dpms(ctx->crtc, DRM_MODE_DPMS_OFF);
+	fimd_disable(ctx->crtc);
 
 	fimd_iommu_detach_devices(ctx);
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 63c1536..e0b1923 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -153,58 +153,44 @@  static void vidi_win_disable(struct exynos_drm_crtc *crtc, unsigned int win)
 	/* TODO. */
 }
 
-static int vidi_power_on(struct vidi_context *ctx, bool enable)
+static int vidi_enable(struct exynos_drm_crtc *crtc)
 {
+	struct vidi_context *ctx = crtc->ctx;
 	struct exynos_drm_plane *plane;
 	int i;
 
-	DRM_DEBUG_KMS("%s\n", __FILE__);
-
-	if (enable != false && enable != true)
-		return -EINVAL;
+	mutex_lock(&ctx->lock);
 
-	if (enable) {
-		ctx->suspended = false;
+	ctx->suspended = false;
 
-		/* if vblank was enabled status, enable it again. */
-		if (test_and_clear_bit(0, &ctx->irq_flags))
-			vidi_enable_vblank(ctx->crtc);
+	/* if vblank was enabled status, enable it again. */
+	if (test_and_clear_bit(0, &ctx->irq_flags))
+		vidi_enable_vblank(ctx->crtc);
 
-		for (i = 0; i < WINDOWS_NR; i++) {
-			plane = &ctx->planes[i];
-			if (plane->enabled)
-				vidi_win_commit(ctx->crtc, i);
-		}
-	} else {
-		ctx->suspended = true;
+	for (i = 0; i < WINDOWS_NR; i++) {
+		plane = &ctx->planes[i];
+		if (plane->enabled)
+			vidi_win_commit(ctx->crtc, i);
 	}
 
+	mutex_unlock(&ctx->lock);
+
 	return 0;
 }
 
-static void vidi_dpms(struct exynos_drm_crtc *crtc, int mode)
+static int vidi_disable(struct exynos_drm_crtc *crtc)
 {
 	struct vidi_context *ctx = crtc->ctx;
-
-	DRM_DEBUG_KMS("%d\n", mode);
+	struct exynos_drm_plane *plane;
+	int i;
 
 	mutex_lock(&ctx->lock);
 
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		vidi_power_on(ctx, true);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-	case DRM_MODE_DPMS_SUSPEND:
-	case DRM_MODE_DPMS_OFF:
-		vidi_power_on(ctx, false);
-		break;
-	default:
-		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
-		break;
-	}
+	ctx->suspended = true;
 
 	mutex_unlock(&ctx->lock);
+
+	return 0;
 }
 
 static int vidi_ctx_initialize(struct vidi_context *ctx,
@@ -219,7 +205,8 @@  static int vidi_ctx_initialize(struct vidi_context *ctx,
 }
 
 static const struct exynos_drm_crtc_ops vidi_crtc_ops = {
-	.dpms = vidi_dpms,
+	.enable = vidi_enable,
+	.disable = vidi_disable,
 	.enable_vblank = vidi_enable_vblank,
 	.disable_vblank = vidi_disable_vblank,
 	.win_commit = vidi_win_commit,
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 8874c1f..44d58ba 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -1027,14 +1027,15 @@  static void mixer_window_resume(struct mixer_context *ctx)
 	}
 }
 
-static void mixer_poweron(struct mixer_context *ctx)
+static int mixer_enable(struct exynos_drm_crtc *crtc)
 {
+	struct mixer_context *ctx = crtc->ctx;
 	struct mixer_resources *res = &ctx->mixer_res;
 
 	mutex_lock(&ctx->mixer_mutex);
 	if (ctx->powered) {
 		mutex_unlock(&ctx->mixer_mutex);
-		return;
+		return 0;
 	}
 
 	mutex_unlock(&ctx->mixer_mutex);
@@ -1059,16 +1060,19 @@  static void mixer_poweron(struct mixer_context *ctx)
 	mixer_win_reset(ctx);
 
 	mixer_window_resume(ctx);
+
+	return 0;
 }
 
-static void mixer_poweroff(struct mixer_context *ctx)
+static int mixer_disable(struct exynos_drm_crtc *crtc)
 {
+	struct mixer_context *ctx = crtc->ctx;
 	struct mixer_resources *res = &ctx->mixer_res;
 
 	mutex_lock(&ctx->mixer_mutex);
 	if (!ctx->powered) {
 		mutex_unlock(&ctx->mixer_mutex);
-		return;
+		return 0;
 	}
 	mutex_unlock(&ctx->mixer_mutex);
 
@@ -1091,23 +1095,8 @@  static void mixer_poweroff(struct mixer_context *ctx)
 	}
 
 	pm_runtime_put_sync(ctx->dev);
-}
 
-static void mixer_dpms(struct exynos_drm_crtc *crtc, int mode)
-{
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		mixer_poweron(crtc->ctx);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-	case DRM_MODE_DPMS_SUSPEND:
-	case DRM_MODE_DPMS_OFF:
-		mixer_poweroff(crtc->ctx);
-		break;
-	default:
-		DRM_DEBUG_KMS("unknown dpms mode: %d\n", mode);
-		break;
-	}
+	return 0;
 }
 
 /* Only valid for Mixer version 16.0.33.0 */
@@ -1131,7 +1120,8 @@  int mixer_check_mode(struct drm_display_mode *mode)
 }
 
 static const struct exynos_drm_crtc_ops mixer_crtc_ops = {
-	.dpms			= mixer_dpms,
+	.enable			= mixer_enable,
+	.disable		= mixer_disable,
 	.enable_vblank		= mixer_enable_vblank,
 	.disable_vblank		= mixer_disable_vblank,
 	.wait_for_vblank	= mixer_wait_for_vblank,