diff mbox

[v3,02/32] drm/exynos: Merge overlay_ops into manager_ops

Message ID 1383063198-10526-3-git-send-email-seanpaul@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Paul Oct. 29, 2013, 4:12 p.m. UTC
This patch merges overlay_ops into manager_ops. In all cases,
overlay_ops is implemented in the same place as manager ops, it doesn't
serve a functional purpose, and doesn't make things more clear.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---

Changes in v2: None
Changes in v3: None

 drivers/gpu/drm/exynos/exynos_drm_drv.h     |  29 +--
 drivers/gpu/drm/exynos/exynos_drm_encoder.c |  26 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c    | 363 ++++++++++++++--------------
 drivers/gpu/drm/exynos/exynos_drm_hdmi.c    |  36 ++-
 drivers/gpu/drm/exynos/exynos_drm_vidi.c    |  29 +--
 drivers/gpu/drm/exynos/exynos_mixer.c       |   2 -
 6 files changed, 229 insertions(+), 256 deletions(-)

Comments

Tomasz Figa Oct. 31, 2013, 11:39 p.m. UTC | #1
Hi Sean,

On Tuesday 29 of October 2013 12:12:48 Sean Paul wrote:
> This patch merges overlay_ops into manager_ops. In all cases,
> overlay_ops is implemented in the same place as manager ops, it doesn't
> serve a functional purpose, and doesn't make things more clear.
> 
> Signed-off-by: Sean Paul <seanpaul@chromium.org>
> ---
> 
> Changes in v2: None
> Changes in v3: None
> 
>  drivers/gpu/drm/exynos/exynos_drm_drv.h     |  29 +--
>  drivers/gpu/drm/exynos/exynos_drm_encoder.c |  26 +-
>  drivers/gpu/drm/exynos/exynos_drm_fimd.c    | 363
> ++++++++++++++-------------- drivers/gpu/drm/exynos/exynos_drm_hdmi.c  
>  |  36 ++-
>  drivers/gpu/drm/exynos/exynos_drm_vidi.c    |  29 +--
>  drivers/gpu/drm/exynos/exynos_mixer.c       |   2 -
>  6 files changed, 229 insertions(+), 256 deletions(-)
[snip]
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 868a14d..f271f22
> 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -181,185 +181,6 @@ static struct exynos_drm_display_ops
> fimd_display_ops = { .power_on = fimd_display_power_on,
>  };
> 
> -static void fimd_dpms(struct device *subdrv_dev, int mode)
> -{
> -	struct fimd_context *ctx = get_fimd_context(subdrv_dev);
> -
> -	DRM_DEBUG_KMS("%d\n", mode);
> -
> -	mutex_lock(&ctx->lock);
> -
> -	switch (mode) {
> -	case DRM_MODE_DPMS_ON:
> -		/*
> -		 * enable fimd hardware only if suspended status.
> -		 *
> -		 * P.S. fimd_dpms function would be called at booting time 
so
> -		 * clk_enable could be called double time.
> -		 */
> -		if (ctx->suspended)
> -			pm_runtime_get_sync(subdrv_dev);
> -		break;
> -	case DRM_MODE_DPMS_STANDBY:
> -	case DRM_MODE_DPMS_SUSPEND:
> -	case DRM_MODE_DPMS_OFF:
> -		if (!ctx->suspended)
> -			pm_runtime_put_sync(subdrv_dev);
> -		break;
> -	default:
> -		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
> -		break;
> -	}
> -
> -	mutex_unlock(&ctx->lock);
> -}
[snip]
> -static void fimd_wait_for_vblank(struct device *dev)
> -{
> -	struct fimd_context *ctx = get_fimd_context(dev);
> -
> -	if (ctx->suspended)
> -		return;
> -
> -	atomic_set(&ctx->wait_vsync_event, 1);
> -
> -	/*
> -	 * wait for FIMD to signal VSYNC interrupt or return after
> -	 * timeout which is set to 50ms (refresh rate of 20).
> -	 */
> -	if (!wait_event_timeout(ctx->wait_vsync_queue,
> -				!atomic_read(&ctx->wait_vsync_event),
> -				DRM_HZ/20))
> -		DRM_DEBUG_KMS("vblank wait timed out.\n");
> -}

Do you need all the churn of moving all the functions above? I believe it 
would be enough to simply move the structure. This would greatly decrease 
the diffstat and chances of possible merge conflicts.

Otherwise the patch looks good to me and improves things indeed.

Best regards,
Tomasz
Sean Paul Nov. 1, 2013, 7:50 p.m. UTC | #2
On Thu, Oct 31, 2013 at 7:39 PM, Tomasz Figa <tomasz.figa@gmail.com> wrote:
> Hi Sean,
>
> On Tuesday 29 of October 2013 12:12:48 Sean Paul wrote:
>> This patch merges overlay_ops into manager_ops. In all cases,
>> overlay_ops is implemented in the same place as manager ops, it doesn't
>> serve a functional purpose, and doesn't make things more clear.
>>
>> Signed-off-by: Sean Paul <seanpaul@chromium.org>
>> ---
>>
>> Changes in v2: None
>> Changes in v3: None
>>
>>  drivers/gpu/drm/exynos/exynos_drm_drv.h     |  29 +--
>>  drivers/gpu/drm/exynos/exynos_drm_encoder.c |  26 +-
>>  drivers/gpu/drm/exynos/exynos_drm_fimd.c    | 363
>> ++++++++++++++-------------- drivers/gpu/drm/exynos/exynos_drm_hdmi.c
>>  |  36 ++-
>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c    |  29 +--
>>  drivers/gpu/drm/exynos/exynos_mixer.c       |   2 -
>>  6 files changed, 229 insertions(+), 256 deletions(-)
> [snip]
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> b/drivers/gpu/drm/exynos/exynos_drm_fimd.c index 868a14d..f271f22
>> 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -181,185 +181,6 @@ static struct exynos_drm_display_ops
>> fimd_display_ops = { .power_on = fimd_display_power_on,
>>  };
>>
>> -static void fimd_dpms(struct device *subdrv_dev, int mode)
>> -{
>> -     struct fimd_context *ctx = get_fimd_context(subdrv_dev);
>> -
>> -     DRM_DEBUG_KMS("%d\n", mode);
>> -
>> -     mutex_lock(&ctx->lock);
>> -
>> -     switch (mode) {
>> -     case DRM_MODE_DPMS_ON:
>> -             /*
>> -              * enable fimd hardware only if suspended status.
>> -              *
>> -              * P.S. fimd_dpms function would be called at booting time
> so
>> -              * clk_enable could be called double time.
>> -              */
>> -             if (ctx->suspended)
>> -                     pm_runtime_get_sync(subdrv_dev);
>> -             break;
>> -     case DRM_MODE_DPMS_STANDBY:
>> -     case DRM_MODE_DPMS_SUSPEND:
>> -     case DRM_MODE_DPMS_OFF:
>> -             if (!ctx->suspended)
>> -                     pm_runtime_put_sync(subdrv_dev);
>> -             break;
>> -     default:
>> -             DRM_DEBUG_KMS("unspecified mode %d\n", mode);
>> -             break;
>> -     }
>> -
>> -     mutex_unlock(&ctx->lock);
>> -}
> [snip]
>> -static void fimd_wait_for_vblank(struct device *dev)
>> -{
>> -     struct fimd_context *ctx = get_fimd_context(dev);
>> -
>> -     if (ctx->suspended)
>> -             return;
>> -
>> -     atomic_set(&ctx->wait_vsync_event, 1);
>> -
>> -     /*
>> -      * wait for FIMD to signal VSYNC interrupt or return after
>> -      * timeout which is set to 50ms (refresh rate of 20).
>> -      */
>> -     if (!wait_event_timeout(ctx->wait_vsync_queue,
>> -                             !atomic_read(&ctx->wait_vsync_event),
>> -                             DRM_HZ/20))
>> -             DRM_DEBUG_KMS("vblank wait timed out.\n");
>> -}
>
> Do you need all the churn of moving all the functions above? I believe it
> would be enough to simply move the structure. This would greatly decrease
> the diffstat and chances of possible merge conflicts.
>

Hi Tomasz,
I've done as you suggest, I'll post a new version once we settle on
the other issues you brought up.

Sean

> Otherwise the patch looks good to me and improves things indeed.
>
> Best regards,
> Tomasz
>
Tomasz Figa Nov. 1, 2013, 7:55 p.m. UTC | #3
Hi Sean,

On Friday 01 of November 2013 15:50:05 Sean Paul wrote:
> On Thu, Oct 31, 2013 at 7:39 PM, Tomasz Figa <tomasz.figa@gmail.com> 
wrote:
> > Hi Sean,
> > 
> > On Tuesday 29 of October 2013 12:12:48 Sean Paul wrote:
[snip]
> >> -static void fimd_wait_for_vblank(struct device *dev)
> >> -{
> >> -     struct fimd_context *ctx = get_fimd_context(dev);
> >> -
> >> -     if (ctx->suspended)
> >> -             return;
> >> -
> >> -     atomic_set(&ctx->wait_vsync_event, 1);
> >> -
> >> -     /*
> >> -      * wait for FIMD to signal VSYNC interrupt or return after
> >> -      * timeout which is set to 50ms (refresh rate of 20).
> >> -      */
> >> -     if (!wait_event_timeout(ctx->wait_vsync_queue,
> >> -                             !atomic_read(&ctx->wait_vsync_event),
> >> -                             DRM_HZ/20))
> >> -             DRM_DEBUG_KMS("vblank wait timed out.\n");
> >> -}
> > 
> > Do you need all the churn of moving all the functions above? I believe
> > it would be enough to simply move the structure. This would greatly
> > decrease the diffstat and chances of possible merge conflicts.
> 
> Hi Tomasz,
> I've done as you suggest,

Thanks.

> I'll post a new version once we settle on
> the other issues you brought up.

Okay.

I'm yet to review remaining patches from this series (I hope to review 
next 5-7 patches later today), as it is quite a lot of changes, so stay 
tuned for further comments. ;)

Best regards,
Tomasz
Inki Dae Nov. 4, 2013, 7:44 a.m. UTC | #4
2013/11/2 Tomasz Figa <tomasz.figa@gmail.com>:
> Hi Sean,
>
> On Friday 01 of November 2013 15:50:05 Sean Paul wrote:
>> On Thu, Oct 31, 2013 at 7:39 PM, Tomasz Figa <tomasz.figa@gmail.com>
> wrote:
>> > Hi Sean,
>> >
>> > On Tuesday 29 of October 2013 12:12:48 Sean Paul wrote:
> [snip]
>> >> -static void fimd_wait_for_vblank(struct device *dev)
>> >> -{
>> >> -     struct fimd_context *ctx = get_fimd_context(dev);
>> >> -
>> >> -     if (ctx->suspended)
>> >> -             return;
>> >> -
>> >> -     atomic_set(&ctx->wait_vsync_event, 1);
>> >> -
>> >> -     /*
>> >> -      * wait for FIMD to signal VSYNC interrupt or return after
>> >> -      * timeout which is set to 50ms (refresh rate of 20).
>> >> -      */
>> >> -     if (!wait_event_timeout(ctx->wait_vsync_queue,
>> >> -                             !atomic_read(&ctx->wait_vsync_event),
>> >> -                             DRM_HZ/20))
>> >> -             DRM_DEBUG_KMS("vblank wait timed out.\n");
>> >> -}
>> >
>> > Do you need all the churn of moving all the functions above? I believe
>> > it would be enough to simply move the structure. This would greatly
>> > decrease the diffstat and chances of possible merge conflicts.
>>
>> Hi Tomasz,
>> I've done as you suggest,
>
> Thanks.
>
>> I'll post a new version once we settle on
>> the other issues you brought up.
>
> Okay.
>
> I'm yet to review remaining patches from this series (I hope to review
> next 5-7 patches later today), as it is quite a lot of changes, so stay
> tuned for further comments. ;)
>

Hi Tomasz,

Are you reviewing yet? I have looked into this patch series but I
couldn't find any big issues excepting bridge and eDP related patches
so I wanted  to merge them as is, and then fix them up later.

Do you and Sean prefer this patch series to be more reviewed? If so or
If there are quite big issues to should be fixed then I will not merge
all patch series this time so that we can have enough time for more
detailed reviews including bridge, eDP related patches, dt support to
hdmi phy config, and so on.

Thanks,
Inki Dae

> Best regards,
> Tomasz
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox

Patch

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index eaa1966..6f31839 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -54,22 +54,6 @@  enum exynos_drm_output_type {
 };
 
 /*
- * Exynos drm overlay ops structure.
- *
- * @mode_set: copy drm overlay info to hw specific overlay info.
- * @commit: apply hardware specific overlay data to registers.
- * @enable: enable hardware specific overlay.
- * @disable: disable hardware specific overlay.
- */
-struct exynos_drm_overlay_ops {
-	void (*mode_set)(struct device *subdrv_dev,
-			 struct exynos_drm_overlay *overlay);
-	void (*commit)(struct device *subdrv_dev, int zpos);
-	void (*enable)(struct device *subdrv_dev, int zpos);
-	void (*disable)(struct device *subdrv_dev, int zpos);
-};
-
-/*
  * Exynos drm common overlay structure.
  *
  * @fb_x: offset x on a framebuffer to be displayed.
@@ -169,6 +153,10 @@  struct exynos_drm_display_ops {
  * @disable_vblank: specific driver callback for disabling vblank interrupt.
  * @wait_for_vblank: wait for vblank interrupt to make sure that
  *	hardware overlay is updated.
+ * @win_mode_set: copy drm overlay info to hw specific overlay info.
+ * @win_commit: apply hardware specific overlay data to registers.
+ * @win_enable: enable hardware specific overlay.
+ * @win_disable: disable hardware specific overlay.
  */
 struct exynos_drm_manager_ops {
 	void (*dpms)(struct device *subdrv_dev, int mode);
@@ -184,6 +172,11 @@  struct exynos_drm_manager_ops {
 	int (*enable_vblank)(struct device *subdrv_dev);
 	void (*disable_vblank)(struct device *subdrv_dev);
 	void (*wait_for_vblank)(struct device *subdrv_dev);
+	void (*win_mode_set)(struct device *subdrv_dev,
+				struct exynos_drm_overlay *overlay);
+	void (*win_commit)(struct device *subdrv_dev, int zpos);
+	void (*win_enable)(struct device *subdrv_dev, int zpos);
+	void (*win_disable)(struct device *subdrv_dev, int zpos);
 };
 
 /*
@@ -195,9 +188,6 @@  struct exynos_drm_manager_ops {
  * @ops: pointer to callbacks for exynos drm specific framebuffer.
  *	these callbacks should be set by specific drivers such fimd
  *	or hdmi driver and are used to control hardware global registers.
- * @overlay_ops: pointer to callbacks for exynos drm specific framebuffer.
- *	these callbacks should be set by specific drivers such fimd
- *	or hdmi driver and are used to control hardware overlay reigsters.
  * @display: pointer to callbacks for exynos drm specific framebuffer.
  *	these callbacks should be set by specific drivers such fimd
  *	or hdmi driver and are used to control display devices such as
@@ -207,7 +197,6 @@  struct exynos_drm_manager {
 	struct device *dev;
 	int pipe;
 	struct exynos_drm_manager_ops *ops;
-	struct exynos_drm_overlay_ops *overlay_ops;
 	struct exynos_drm_display_ops *display_ops;
 };
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
index 06f1b2a..c255341 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
@@ -133,7 +133,7 @@  static void disable_plane_to_crtc(struct drm_device *dev,
 			 *
 			 * plane->funcs->disable_plane call checks
 			 * if encoder->crtc is same as plane->crtc and if same
-			 * then overlay_ops->disable callback will be called
+			 * then manager_ops->win_disable callback will be called
 			 * to diasble current hw overlay so plane->crtc should
 			 * have new_crtc because new_crtc was set to
 			 * encoder->crtc in advance.
@@ -442,51 +442,51 @@  void exynos_drm_encoder_plane_mode_set(struct drm_encoder *encoder, void *data)
 {
 	struct exynos_drm_manager *manager =
 		to_exynos_encoder(encoder)->manager;
-	struct exynos_drm_overlay_ops *overlay_ops = manager->overlay_ops;
+	struct exynos_drm_manager_ops *manager_ops = manager->ops;
 	struct exynos_drm_overlay *overlay = data;
 
-	if (overlay_ops && overlay_ops->mode_set)
-		overlay_ops->mode_set(manager->dev, overlay);
+	if (manager_ops && manager_ops->win_mode_set)
+		manager_ops->win_mode_set(manager->dev, overlay);
 }
 
 void exynos_drm_encoder_plane_commit(struct drm_encoder *encoder, void *data)
 {
 	struct exynos_drm_manager *manager =
 		to_exynos_encoder(encoder)->manager;
-	struct exynos_drm_overlay_ops *overlay_ops = manager->overlay_ops;
+	struct exynos_drm_manager_ops *manager_ops = manager->ops;
 	int zpos = DEFAULT_ZPOS;
 
 	if (data)
 		zpos = *(int *)data;
 
-	if (overlay_ops && overlay_ops->commit)
-		overlay_ops->commit(manager->dev, zpos);
+	if (manager_ops && manager_ops->win_commit)
+		manager_ops->win_commit(manager->dev, zpos);
 }
 
 void exynos_drm_encoder_plane_enable(struct drm_encoder *encoder, void *data)
 {
 	struct exynos_drm_manager *manager =
 		to_exynos_encoder(encoder)->manager;
-	struct exynos_drm_overlay_ops *overlay_ops = manager->overlay_ops;
+	struct exynos_drm_manager_ops *manager_ops = manager->ops;
 	int zpos = DEFAULT_ZPOS;
 
 	if (data)
 		zpos = *(int *)data;
 
-	if (overlay_ops && overlay_ops->enable)
-		overlay_ops->enable(manager->dev, zpos);
+	if (manager_ops && manager_ops->win_enable)
+		manager_ops->win_enable(manager->dev, zpos);
 }
 
 void exynos_drm_encoder_plane_disable(struct drm_encoder *encoder, void *data)
 {
 	struct exynos_drm_manager *manager =
 		to_exynos_encoder(encoder)->manager;
-	struct exynos_drm_overlay_ops *overlay_ops = manager->overlay_ops;
+	struct exynos_drm_manager_ops *manager_ops = manager->ops;
 	int zpos = DEFAULT_ZPOS;
 
 	if (data)
 		zpos = *(int *)data;
 
-	if (overlay_ops && overlay_ops->disable)
-		overlay_ops->disable(manager->dev, zpos);
+	if (manager_ops && manager_ops->win_disable)
+		manager_ops->win_disable(manager->dev, zpos);
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 868a14d..f271f22 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -181,185 +181,6 @@  static struct exynos_drm_display_ops fimd_display_ops = {
 	.power_on = fimd_display_power_on,
 };
 
-static void fimd_dpms(struct device *subdrv_dev, int mode)
-{
-	struct fimd_context *ctx = get_fimd_context(subdrv_dev);
-
-	DRM_DEBUG_KMS("%d\n", mode);
-
-	mutex_lock(&ctx->lock);
-
-	switch (mode) {
-	case DRM_MODE_DPMS_ON:
-		/*
-		 * enable fimd hardware only if suspended status.
-		 *
-		 * P.S. fimd_dpms function would be called at booting time so
-		 * clk_enable could be called double time.
-		 */
-		if (ctx->suspended)
-			pm_runtime_get_sync(subdrv_dev);
-		break;
-	case DRM_MODE_DPMS_STANDBY:
-	case DRM_MODE_DPMS_SUSPEND:
-	case DRM_MODE_DPMS_OFF:
-		if (!ctx->suspended)
-			pm_runtime_put_sync(subdrv_dev);
-		break;
-	default:
-		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
-		break;
-	}
-
-	mutex_unlock(&ctx->lock);
-}
-
-static void fimd_apply(struct device *subdrv_dev)
-{
-	struct fimd_context *ctx = get_fimd_context(subdrv_dev);
-	struct exynos_drm_manager *mgr = ctx->subdrv.manager;
-	struct exynos_drm_manager_ops *mgr_ops = mgr->ops;
-	struct exynos_drm_overlay_ops *ovl_ops = mgr->overlay_ops;
-	struct fimd_win_data *win_data;
-	int i;
-
-	for (i = 0; i < WINDOWS_NR; i++) {
-		win_data = &ctx->win_data[i];
-		if (win_data->enabled && (ovl_ops && ovl_ops->commit))
-			ovl_ops->commit(subdrv_dev, i);
-	}
-
-	if (mgr_ops && mgr_ops->commit)
-		mgr_ops->commit(subdrv_dev);
-}
-
-static void fimd_commit(struct device *dev)
-{
-	struct fimd_context *ctx = get_fimd_context(dev);
-	struct exynos_drm_panel_info *panel = &ctx->panel;
-	struct videomode *vm = &panel->vm;
-	struct fimd_driver_data *driver_data;
-	u32 val;
-
-	driver_data = ctx->driver_data;
-	if (ctx->suspended)
-		return;
-
-	/* setup polarity values from machine code. */
-	writel(ctx->vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
-
-	/* setup vertical timing values. */
-	val = VIDTCON0_VBPD(vm->vback_porch - 1) |
-	       VIDTCON0_VFPD(vm->vfront_porch - 1) |
-	       VIDTCON0_VSPW(vm->vsync_len - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
-
-	/* setup horizontal timing values.  */
-	val = VIDTCON1_HBPD(vm->hback_porch - 1) |
-	       VIDTCON1_HFPD(vm->hfront_porch - 1) |
-	       VIDTCON1_HSPW(vm->hsync_len - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
-
-	/* setup horizontal and vertical display size. */
-	val = VIDTCON2_LINEVAL(vm->vactive - 1) |
-	       VIDTCON2_HOZVAL(vm->hactive - 1) |
-	       VIDTCON2_LINEVAL_E(vm->vactive - 1) |
-	       VIDTCON2_HOZVAL_E(vm->hactive - 1);
-	writel(val, ctx->regs + driver_data->timing_base + VIDTCON2);
-
-	/* setup clock source, clock divider, enable dma. */
-	val = ctx->vidcon0;
-	val &= ~(VIDCON0_CLKVAL_F_MASK | VIDCON0_CLKDIR);
-
-	if (ctx->driver_data->has_clksel) {
-		val &= ~VIDCON0_CLKSEL_MASK;
-		val |= VIDCON0_CLKSEL_LCD;
-	}
-
-	if (ctx->clkdiv > 1)
-		val |= VIDCON0_CLKVAL_F(ctx->clkdiv - 1) | VIDCON0_CLKDIR;
-	else
-		val &= ~VIDCON0_CLKDIR;	/* 1:1 clock */
-
-	/*
-	 * fields of register with prefix '_F' would be updated
-	 * at vsync(same as dma start)
-	 */
-	val |= VIDCON0_ENVID | VIDCON0_ENVID_F;
-	writel(val, ctx->regs + VIDCON0);
-}
-
-static int fimd_enable_vblank(struct device *dev)
-{
-	struct fimd_context *ctx = get_fimd_context(dev);
-	u32 val;
-
-	if (ctx->suspended)
-		return -EPERM;
-
-	if (!test_and_set_bit(0, &ctx->irq_flags)) {
-		val = readl(ctx->regs + VIDINTCON0);
-
-		val |= VIDINTCON0_INT_ENABLE;
-		val |= VIDINTCON0_INT_FRAME;
-
-		val &= ~VIDINTCON0_FRAMESEL0_MASK;
-		val |= VIDINTCON0_FRAMESEL0_VSYNC;
-		val &= ~VIDINTCON0_FRAMESEL1_MASK;
-		val |= VIDINTCON0_FRAMESEL1_NONE;
-
-		writel(val, ctx->regs + VIDINTCON0);
-	}
-
-	return 0;
-}
-
-static void fimd_disable_vblank(struct device *dev)
-{
-	struct fimd_context *ctx = get_fimd_context(dev);
-	u32 val;
-
-	if (ctx->suspended)
-		return;
-
-	if (test_and_clear_bit(0, &ctx->irq_flags)) {
-		val = readl(ctx->regs + VIDINTCON0);
-
-		val &= ~VIDINTCON0_INT_FRAME;
-		val &= ~VIDINTCON0_INT_ENABLE;
-
-		writel(val, ctx->regs + VIDINTCON0);
-	}
-}
-
-static void fimd_wait_for_vblank(struct device *dev)
-{
-	struct fimd_context *ctx = get_fimd_context(dev);
-
-	if (ctx->suspended)
-		return;
-
-	atomic_set(&ctx->wait_vsync_event, 1);
-
-	/*
-	 * wait for FIMD to signal VSYNC interrupt or return after
-	 * timeout which is set to 50ms (refresh rate of 20).
-	 */
-	if (!wait_event_timeout(ctx->wait_vsync_queue,
-				!atomic_read(&ctx->wait_vsync_event),
-				DRM_HZ/20))
-		DRM_DEBUG_KMS("vblank wait timed out.\n");
-}
-
-static struct exynos_drm_manager_ops fimd_manager_ops = {
-	.dpms = fimd_dpms,
-	.apply = fimd_apply,
-	.commit = fimd_commit,
-	.enable_vblank = fimd_enable_vblank,
-	.disable_vblank = fimd_disable_vblank,
-	.wait_for_vblank = fimd_wait_for_vblank,
-};
-
 static void fimd_win_mode_set(struct device *dev,
 			      struct exynos_drm_overlay *overlay)
 {
@@ -669,16 +490,190 @@  static void fimd_win_disable(struct device *dev, int zpos)
 	win_data->enabled = false;
 }
 
-static struct exynos_drm_overlay_ops fimd_overlay_ops = {
-	.mode_set = fimd_win_mode_set,
-	.commit = fimd_win_commit,
-	.disable = fimd_win_disable,
+static void fimd_dpms(struct device *subdrv_dev, int mode)
+{
+	struct fimd_context *ctx = get_fimd_context(subdrv_dev);
+
+	DRM_DEBUG_KMS("%d\n", mode);
+
+	mutex_lock(&ctx->lock);
+
+	switch (mode) {
+	case DRM_MODE_DPMS_ON:
+		/*
+		 * enable fimd hardware only if suspended status.
+		 *
+		 * P.S. fimd_dpms function would be called at booting time so
+		 * clk_enable could be called double time.
+		 */
+		if (ctx->suspended)
+			pm_runtime_get_sync(subdrv_dev);
+		break;
+	case DRM_MODE_DPMS_STANDBY:
+	case DRM_MODE_DPMS_SUSPEND:
+	case DRM_MODE_DPMS_OFF:
+		if (!ctx->suspended)
+			pm_runtime_put_sync(subdrv_dev);
+		break;
+	default:
+		DRM_DEBUG_KMS("unspecified mode %d\n", mode);
+		break;
+	}
+
+	mutex_unlock(&ctx->lock);
+}
+
+static void fimd_apply(struct device *subdrv_dev)
+{
+	struct fimd_context *ctx = get_fimd_context(subdrv_dev);
+	struct exynos_drm_manager *mgr = ctx->subdrv.manager;
+	struct exynos_drm_manager_ops *mgr_ops = mgr->ops;
+	struct fimd_win_data *win_data;
+	int i;
+
+	for (i = 0; i < WINDOWS_NR; i++) {
+		win_data = &ctx->win_data[i];
+		if (win_data->enabled && (mgr_ops && mgr_ops->win_commit))
+			mgr_ops->win_commit(subdrv_dev, i);
+	}
+
+	if (mgr_ops && mgr_ops->commit)
+		mgr_ops->commit(subdrv_dev);
+}
+
+static void fimd_commit(struct device *dev)
+{
+	struct fimd_context *ctx = get_fimd_context(dev);
+	struct exynos_drm_panel_info *panel = &ctx->panel;
+	struct videomode *vm = &panel->vm;
+	struct fimd_driver_data *driver_data;
+	u32 val;
+
+	driver_data = ctx->driver_data;
+	if (ctx->suspended)
+		return;
+
+	/* setup polarity values from machine code. */
+	writel(ctx->vidcon1, ctx->regs + driver_data->timing_base + VIDCON1);
+
+	/* setup vertical timing values. */
+	val = VIDTCON0_VBPD(vm->vback_porch - 1) |
+	       VIDTCON0_VFPD(vm->vfront_porch - 1) |
+	       VIDTCON0_VSPW(vm->vsync_len - 1);
+	writel(val, ctx->regs + driver_data->timing_base + VIDTCON0);
+
+	/* setup horizontal timing values.  */
+	val = VIDTCON1_HBPD(vm->hback_porch - 1) |
+	       VIDTCON1_HFPD(vm->hfront_porch - 1) |
+	       VIDTCON1_HSPW(vm->hsync_len - 1);
+	writel(val, ctx->regs + driver_data->timing_base + VIDTCON1);
+
+	/* setup horizontal and vertical display size. */
+	val = VIDTCON2_LINEVAL(vm->vactive - 1) |
+	       VIDTCON2_HOZVAL(vm->hactive - 1) |
+	       VIDTCON2_LINEVAL_E(vm->vactive - 1) |
+	       VIDTCON2_HOZVAL_E(vm->hactive - 1);
+	writel(val, ctx->regs + driver_data->timing_base + VIDTCON2);
+
+	/* setup clock source, clock divider, enable dma. */
+	val = ctx->vidcon0;
+	val &= ~(VIDCON0_CLKVAL_F_MASK | VIDCON0_CLKDIR);
+
+	if (ctx->driver_data->has_clksel) {
+		val &= ~VIDCON0_CLKSEL_MASK;
+		val |= VIDCON0_CLKSEL_LCD;
+	}
+
+	if (ctx->clkdiv > 1)
+		val |= VIDCON0_CLKVAL_F(ctx->clkdiv - 1) | VIDCON0_CLKDIR;
+	else
+		val &= ~VIDCON0_CLKDIR;	/* 1:1 clock */
+
+	/*
+	 * fields of register with prefix '_F' would be updated
+	 * at vsync(same as dma start)
+	 */
+	val |= VIDCON0_ENVID | VIDCON0_ENVID_F;
+	writel(val, ctx->regs + VIDCON0);
+}
+
+static int fimd_enable_vblank(struct device *dev)
+{
+	struct fimd_context *ctx = get_fimd_context(dev);
+	u32 val;
+
+	if (ctx->suspended)
+		return -EPERM;
+
+	if (!test_and_set_bit(0, &ctx->irq_flags)) {
+		val = readl(ctx->regs + VIDINTCON0);
+
+		val |= VIDINTCON0_INT_ENABLE;
+		val |= VIDINTCON0_INT_FRAME;
+
+		val &= ~VIDINTCON0_FRAMESEL0_MASK;
+		val |= VIDINTCON0_FRAMESEL0_VSYNC;
+		val &= ~VIDINTCON0_FRAMESEL1_MASK;
+		val |= VIDINTCON0_FRAMESEL1_NONE;
+
+		writel(val, ctx->regs + VIDINTCON0);
+	}
+
+	return 0;
+}
+
+static void fimd_disable_vblank(struct device *dev)
+{
+	struct fimd_context *ctx = get_fimd_context(dev);
+	u32 val;
+
+	if (ctx->suspended)
+		return;
+
+	if (test_and_clear_bit(0, &ctx->irq_flags)) {
+		val = readl(ctx->regs + VIDINTCON0);
+
+		val &= ~VIDINTCON0_INT_FRAME;
+		val &= ~VIDINTCON0_INT_ENABLE;
+
+		writel(val, ctx->regs + VIDINTCON0);
+	}
+}
+
+static void fimd_wait_for_vblank(struct device *dev)
+{
+	struct fimd_context *ctx = get_fimd_context(dev);
+
+	if (ctx->suspended)
+		return;
+
+	atomic_set(&ctx->wait_vsync_event, 1);
+
+	/*
+	 * wait for FIMD to signal VSYNC interrupt or return after
+	 * timeout which is set to 50ms (refresh rate of 20).
+	 */
+	if (!wait_event_timeout(ctx->wait_vsync_queue,
+				!atomic_read(&ctx->wait_vsync_event),
+				DRM_HZ/20))
+		DRM_DEBUG_KMS("vblank wait timed out.\n");
+}
+
+static struct exynos_drm_manager_ops fimd_manager_ops = {
+	.dpms = fimd_dpms,
+	.apply = fimd_apply,
+	.commit = fimd_commit,
+	.enable_vblank = fimd_enable_vblank,
+	.disable_vblank = fimd_disable_vblank,
+	.wait_for_vblank = fimd_wait_for_vblank,
+	.win_mode_set = fimd_win_mode_set,
+	.win_commit = fimd_win_commit,
+	.win_disable = fimd_win_disable,
 };
 
 static struct exynos_drm_manager fimd_manager = {
 	.pipe		= -1,
 	.ops		= &fimd_manager_ops,
-	.overlay_ops	= &fimd_overlay_ops,
 	.display_ops	= &fimd_display_ops,
 };
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
index 8548b97..a1ef3c9 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
@@ -284,19 +284,7 @@  static void drm_hdmi_apply(struct device *subdrv_dev)
 		hdmi_ops->commit(ctx->hdmi_ctx->ctx);
 }
 
-static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
-	.dpms = drm_hdmi_dpms,
-	.apply = drm_hdmi_apply,
-	.enable_vblank = drm_hdmi_enable_vblank,
-	.disable_vblank = drm_hdmi_disable_vblank,
-	.wait_for_vblank = drm_hdmi_wait_for_vblank,
-	.mode_fixup = drm_hdmi_mode_fixup,
-	.mode_set = drm_hdmi_mode_set,
-	.get_max_resol = drm_hdmi_get_max_resol,
-	.commit = drm_hdmi_commit,
-};
-
-static void drm_mixer_mode_set(struct device *subdrv_dev,
+static void drm_mixer_win_mode_set(struct device *subdrv_dev,
 		struct exynos_drm_overlay *overlay)
 {
 	struct drm_hdmi_context *ctx = to_context(subdrv_dev);
@@ -305,7 +293,7 @@  static void drm_mixer_mode_set(struct device *subdrv_dev,
 		mixer_ops->win_mode_set(ctx->mixer_ctx->ctx, overlay);
 }
 
-static void drm_mixer_commit(struct device *subdrv_dev, int zpos)
+static void drm_mixer_win_commit(struct device *subdrv_dev, int zpos)
 {
 	struct drm_hdmi_context *ctx = to_context(subdrv_dev);
 	int win = (zpos == DEFAULT_ZPOS) ? MIXER_DEFAULT_WIN : zpos;
@@ -321,7 +309,7 @@  static void drm_mixer_commit(struct device *subdrv_dev, int zpos)
 	ctx->enabled[win] = true;
 }
 
-static void drm_mixer_disable(struct device *subdrv_dev, int zpos)
+static void drm_mixer_win_disable(struct device *subdrv_dev, int zpos)
 {
 	struct drm_hdmi_context *ctx = to_context(subdrv_dev);
 	int win = (zpos == DEFAULT_ZPOS) ? MIXER_DEFAULT_WIN : zpos;
@@ -337,16 +325,24 @@  static void drm_mixer_disable(struct device *subdrv_dev, int zpos)
 	ctx->enabled[win] = false;
 }
 
-static struct exynos_drm_overlay_ops drm_hdmi_overlay_ops = {
-	.mode_set = drm_mixer_mode_set,
-	.commit = drm_mixer_commit,
-	.disable = drm_mixer_disable,
+static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
+	.dpms = drm_hdmi_dpms,
+	.apply = drm_hdmi_apply,
+	.enable_vblank = drm_hdmi_enable_vblank,
+	.disable_vblank = drm_hdmi_disable_vblank,
+	.wait_for_vblank = drm_hdmi_wait_for_vblank,
+	.mode_fixup = drm_hdmi_mode_fixup,
+	.mode_set = drm_hdmi_mode_set,
+	.get_max_resol = drm_hdmi_get_max_resol,
+	.commit = drm_hdmi_commit,
+	.win_mode_set = drm_mixer_win_mode_set,
+	.win_commit = drm_mixer_win_commit,
+	.win_disable = drm_mixer_win_disable,
 };
 
 static struct exynos_drm_manager hdmi_manager = {
 	.pipe		= -1,
 	.ops		= &drm_hdmi_manager_ops,
-	.overlay_ops	= &drm_hdmi_overlay_ops,
 	.display_ops	= &drm_hdmi_display_ops,
 };
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_vidi.c b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
index 4400330..15a97ce 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_vidi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_vidi.c
@@ -182,14 +182,13 @@  static void vidi_apply(struct device *subdrv_dev)
 	struct vidi_context *ctx = get_vidi_context(subdrv_dev);
 	struct exynos_drm_manager *mgr = ctx->subdrv.manager;
 	struct exynos_drm_manager_ops *mgr_ops = mgr->ops;
-	struct exynos_drm_overlay_ops *ovl_ops = mgr->overlay_ops;
 	struct vidi_win_data *win_data;
 	int i;
 
 	for (i = 0; i < WINDOWS_NR; i++) {
 		win_data = &ctx->win_data[i];
-		if (win_data->enabled && (ovl_ops && ovl_ops->commit))
-			ovl_ops->commit(subdrv_dev, i);
+		if (win_data->enabled && (mgr_ops && mgr_ops->win_commit))
+			mgr_ops->win_commit(subdrv_dev, i);
 	}
 
 	if (mgr_ops && mgr_ops->commit)
@@ -219,7 +218,7 @@  static int vidi_enable_vblank(struct device *dev)
 	/*
 	 * in case of page flip request, vidi_finish_pageflip function
 	 * will not be called because direct_vblank is true and then
-	 * that function will be called by overlay_ops->commit callback
+	 * that function will be called by manager_ops->win_commit callback
 	 */
 	schedule_work(&ctx->work);
 
@@ -237,14 +236,6 @@  static void vidi_disable_vblank(struct device *dev)
 		ctx->vblank_on = false;
 }
 
-static struct exynos_drm_manager_ops vidi_manager_ops = {
-	.dpms = vidi_dpms,
-	.apply = vidi_apply,
-	.commit = vidi_commit,
-	.enable_vblank = vidi_enable_vblank,
-	.disable_vblank = vidi_disable_vblank,
-};
-
 static void vidi_win_mode_set(struct device *dev,
 			      struct exynos_drm_overlay *overlay)
 {
@@ -341,16 +332,20 @@  static void vidi_win_disable(struct device *dev, int zpos)
 	/* TODO. */
 }
 
-static struct exynos_drm_overlay_ops vidi_overlay_ops = {
-	.mode_set = vidi_win_mode_set,
-	.commit = vidi_win_commit,
-	.disable = vidi_win_disable,
+static struct exynos_drm_manager_ops vidi_manager_ops = {
+	.dpms = vidi_dpms,
+	.apply = vidi_apply,
+	.commit = vidi_commit,
+	.enable_vblank = vidi_enable_vblank,
+	.disable_vblank = vidi_disable_vblank,
+	.win_mode_set = vidi_win_mode_set,
+	.win_commit = vidi_win_commit,
+	.win_disable = vidi_win_disable,
 };
 
 static struct exynos_drm_manager vidi_manager = {
 	.pipe		= -1,
 	.ops		= &vidi_manager_ops,
-	.overlay_ops	= &vidi_overlay_ops,
 	.display_ops	= &vidi_display_ops,
 };
 
diff --git a/drivers/gpu/drm/exynos/exynos_mixer.c b/drivers/gpu/drm/exynos/exynos_mixer.c
index 63bc5f9..9634188 100644
--- a/drivers/gpu/drm/exynos/exynos_mixer.c
+++ b/drivers/gpu/drm/exynos/exynos_mixer.c
@@ -975,8 +975,6 @@  static struct exynos_mixer_ops mixer_ops = {
 	.disable_vblank		= mixer_disable_vblank,
 	.wait_for_vblank	= mixer_wait_for_vblank,
 	.dpms			= mixer_dpms,
-
-	/* overlay */
 	.win_mode_set		= mixer_win_mode_set,
 	.win_commit		= mixer_win_commit,
 	.win_disable		= mixer_win_disable,