diff mbox

[12/23] drm/exynos: Split manager/display/subdrv

Message ID 1381451436-10089-13-git-send-email-seanpaul@chromium.org (mailing list archive)
State New, archived
Headers show

Commit Message

Sean Paul Oct. 11, 2013, 12:30 a.m. UTC
This patch splits display and manager from subdrv. The result is that
crtc functions can directly call into manager callbacks and encoder
functions can directly call into display callbacks. This will allow
us to remove the exynos_drm_hdmi shim and support mixer/hdmi & fimd/dp
with common code.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
---
 drivers/gpu/drm/exynos/exynos_drm_connector.c |  50 ++---
 drivers/gpu/drm/exynos/exynos_drm_core.c      | 181 +++++++++++++-----
 drivers/gpu/drm/exynos/exynos_drm_crtc.c      | 115 +++++++++---
 drivers/gpu/drm/exynos/exynos_drm_crtc.h      |  20 +-
 drivers/gpu/drm/exynos/exynos_drm_drv.c       |  29 +--
 drivers/gpu/drm/exynos/exynos_drm_drv.h       |  87 +++++----
 drivers/gpu/drm/exynos/exynos_drm_encoder.c   | 257 ++++----------------------
 drivers/gpu/drm/exynos/exynos_drm_encoder.h   |  18 +-
 drivers/gpu/drm/exynos/exynos_drm_fb.c        |   4 +-
 drivers/gpu/drm/exynos/exynos_drm_fimd.c      | 153 +++++++--------
 drivers/gpu/drm/exynos/exynos_drm_hdmi.c      | 218 ++++++++++------------
 drivers/gpu/drm/exynos/exynos_drm_hdmi.h      |   2 +
 drivers/gpu/drm/exynos/exynos_drm_plane.c     |  15 +-
 13 files changed, 547 insertions(+), 602 deletions(-)

Comments

대인기/Tizen Platform Lab(SR)/삼성전자 Oct. 12, 2013, 12:42 a.m. UTC | #1
>  static int exynos_drm_create_enc_conn(struct drm_device *dev,
> -                                       struct exynos_drm_subdrv *subdrv)
> +                                       struct exynos_drm_display *display)
>  {
>         struct drm_encoder *encoder;
>         struct drm_connector *connector;
> +       struct exynos_drm_manager *manager;
>         int ret;
> +       unsigned long possible_crtcs = 0;
>
> -       subdrv->manager->dev = subdrv->dev;
> +       /* Find possible crtcs for this display */
> +       list_for_each_entry(manager, &exynos_drm_manager_list, list)
> +               if (manager->type == display->type)
> +                       possible_crtcs |= 1 << manager->pipe;
>
>         /* create and initialize a encoder for this sub driver. */
> -       encoder = exynos_drm_encoder_create(dev, subdrv->manager,
> -                       (1 << MAX_CRTC) - 1);
> +       encoder = exynos_drm_encoder_create(dev, display, possible_crtcs);
>         if (!encoder) {
>                 DRM_ERROR("failed to create encoder\n");
>                 return -EFAULT;
>         }
> -       subdrv->encoder = encoder;
> +       display->encoder = encoder;
>
> -       if (subdrv->manager->display_ops->type == EXYNOS_DISPLAY_TYPE_LCD) {
> +       if (display->type == EXYNOS_DISPLAY_TYPE_LCD) {
>                 ret = exynos_drm_attach_lcd_bridge(dev, encoder)

Plz, _do_not_base_ on top of your lvds bridge patch set. As I
mentioned before, the tricky codes are not good. For this, let's find
a better way after your refactoring patch set are reviewed enough, and
if merged.

>                 if (ret)
>                         return 0;
> @@ -91,7 +98,7 @@ static int exynos_drm_create_enc_conn(struct drm_device *dev,
>                 goto err_destroy_encoder;
>         }
>
> -       subdrv->connector = connector;
> +       display->connector = connector;
>
>         return 0;
>
> @@ -100,21 +107,6 @@ err_destroy_encoder:
>         return ret;
>  }
>
Sean Paul Oct. 12, 2013, 2:06 a.m. UTC | #2
On Fri, Oct 11, 2013 at 8:42 PM, Inki Dae <inki.dae@samsung.com> wrote:
>>  static int exynos_drm_create_enc_conn(struct drm_device *dev,
>> -                                       struct exynos_drm_subdrv *subdrv)
>> +                                       struct exynos_drm_display *display)
>>  {
>>         struct drm_encoder *encoder;
>>         struct drm_connector *connector;
>> +       struct exynos_drm_manager *manager;
>>         int ret;
>> +       unsigned long possible_crtcs = 0;
>>
>> -       subdrv->manager->dev = subdrv->dev;
>> +       /* Find possible crtcs for this display */
>> +       list_for_each_entry(manager, &exynos_drm_manager_list, list)
>> +               if (manager->type == display->type)
>> +                       possible_crtcs |= 1 << manager->pipe;
>>
>>         /* create and initialize a encoder for this sub driver. */
>> -       encoder = exynos_drm_encoder_create(dev, subdrv->manager,
>> -                       (1 << MAX_CRTC) - 1);
>> +       encoder = exynos_drm_encoder_create(dev, display, possible_crtcs);
>>         if (!encoder) {
>>                 DRM_ERROR("failed to create encoder\n");
>>                 return -EFAULT;
>>         }
>> -       subdrv->encoder = encoder;
>> +       display->encoder = encoder;
>>
>> -       if (subdrv->manager->display_ops->type == EXYNOS_DISPLAY_TYPE_LCD) {
>> +       if (display->type == EXYNOS_DISPLAY_TYPE_LCD) {
>>                 ret = exynos_drm_attach_lcd_bridge(dev, encoder)
>
> Plz, _do_not_base_ on top of your lvds bridge patch set. As I
> mentioned before, the tricky codes are not good. For this, let's find
> a better way after your refactoring patch set are reviewed enough, and
> if merged.
>

[+adding back the people you removed from CC]

I'm happy to change how that works, but I've yet to find a better
solution. Instead of just dismissing something as "tricky codes", can
you suggest a concrete solution that is better than what I've got?

I'm also a bit confused as to why you think it's so tricky, it's a
synchronous initialization call to a driver; that's about as simple as
it gets.

Sean



>>                 if (ret)
>>                         return 0;
>> @@ -91,7 +98,7 @@ static int exynos_drm_create_enc_conn(struct drm_device *dev,
>>                 goto err_destroy_encoder;
>>         }
>>
>> -       subdrv->connector = connector;
>> +       display->connector = connector;
>>
>>         return 0;
>>
>> @@ -100,21 +107,6 @@ err_destroy_encoder:
>>         return ret;
>>  }
>>
대인기/Tizen Platform Lab(SR)/삼성전자 Oct. 12, 2013, 3:49 a.m. UTC | #3
> -----Original Message-----
> From: Sean Paul [mailto:seanpaul@chromium.org]
> Sent: Saturday, October 12, 2013 11:07 AM
> To: Inki Dae
> Cc: DRI mailing list; Stéphane Marchesin; Dave Airlie; Tomasz Figa
> Subject: Re: [PATCH 12/23] drm/exynos: Split manager/display/subdrv
> 
> On Fri, Oct 11, 2013 at 8:42 PM, Inki Dae <inki.dae@samsung.com> wrote:
> >>  static int exynos_drm_create_enc_conn(struct drm_device *dev,
> >> -                                       struct exynos_drm_subdrv
*subdrv)
> >> +                                       struct exynos_drm_display
*display)
> >>  {
> >>         struct drm_encoder *encoder;
> >>         struct drm_connector *connector;
> >> +       struct exynos_drm_manager *manager;
> >>         int ret;
> >> +       unsigned long possible_crtcs = 0;
> >>
> >> -       subdrv->manager->dev = subdrv->dev;
> >> +       /* Find possible crtcs for this display */
> >> +       list_for_each_entry(manager, &exynos_drm_manager_list, list)
> >> +               if (manager->type == display->type)
> >> +                       possible_crtcs |= 1 << manager->pipe;
> >>
> >>         /* create and initialize a encoder for this sub driver. */
> >> -       encoder = exynos_drm_encoder_create(dev, subdrv->manager,
> >> -                       (1 << MAX_CRTC) - 1);
> >> +       encoder = exynos_drm_encoder_create(dev, display,
> possible_crtcs);
> >>         if (!encoder) {
> >>                 DRM_ERROR("failed to create encoder\n");
> >>                 return -EFAULT;
> >>         }
> >> -       subdrv->encoder = encoder;
> >> +       display->encoder = encoder;
> >>
> >> -       if (subdrv->manager->display_ops->type ==
> EXYNOS_DISPLAY_TYPE_LCD) {
> >> +       if (display->type == EXYNOS_DISPLAY_TYPE_LCD) {
> >>                 ret = exynos_drm_attach_lcd_bridge(dev, encoder)
> >
> > Plz, _do_not_base_ on top of your lvds bridge patch set. As I
> > mentioned before, the tricky codes are not good. For this, let's find
> > a better way after your refactoring patch set are reviewed enough, and
> > if merged.
> >
> 
> [+adding back the people you removed from CC]
> 

Hm.. I  didn't really remove the people from CC. See the your original
email. :)


> I'm happy to change how that works, but I've yet to find a better
> solution. Instead of just dismissing something as "tricky codes", can
> you suggest a concrete solution that is better than what I've got?
> 

I think I mentioned already at previous email. First of all, let's have a
thorough review to your refactoring patch set, and then let's find a better
way.

I believe that we can find a better way with your refactoring patch set: now
we can implement encoder and connector directly.


> I'm also a bit confused as to why you think it's so tricky, it's a

First, That was your comment, "Right, so this is kind of tricky". And also
it seems like tricky codes to me.

> synchronous initialization call to a driver; that's about as simple as
> it gets.

As you mentioned before, ptn3460 driver is not depended on SoC, and can be
used for other SoC. Let you try to think about lcd class and backlight
class. I think we need a interfacing layer between common driver and
specific something for connecting SoC specific DRM framework.

> 
> Sean
> 
> 
> 
> >>                 if (ret)
> >>                         return 0;
> >> @@ -91,7 +98,7 @@ static int exynos_drm_create_enc_conn(struct
> drm_device *dev,
> >>                 goto err_destroy_encoder;
> >>         }
> >>
> >> -       subdrv->connector = connector;
> >> +       display->connector = connector;
> >>
> >>         return 0;
> >>
> >> @@ -100,21 +107,6 @@ err_destroy_encoder:
> >>         return ret;
> >>  }
> >>
Sean Paul Oct. 12, 2013, 4:19 a.m. UTC | #4
On Fri, Oct 11, 2013 at 11:49 PM, Inki Dae <inki.dae@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Sean Paul [mailto:seanpaul@chromium.org]
>> Sent: Saturday, October 12, 2013 11:07 AM
>> To: Inki Dae
>> Cc: DRI mailing list; Stéphane Marchesin; Dave Airlie; Tomasz Figa
>> Subject: Re: [PATCH 12/23] drm/exynos: Split manager/display/subdrv
>>
>> On Fri, Oct 11, 2013 at 8:42 PM, Inki Dae <inki.dae@samsung.com> wrote:
>> >>  static int exynos_drm_create_enc_conn(struct drm_device *dev,
>> >> -                                       struct exynos_drm_subdrv
> *subdrv)
>> >> +                                       struct exynos_drm_display
> *display)
>> >>  {
>> >>         struct drm_encoder *encoder;
>> >>         struct drm_connector *connector;
>> >> +       struct exynos_drm_manager *manager;
>> >>         int ret;
>> >> +       unsigned long possible_crtcs = 0;
>> >>
>> >> -       subdrv->manager->dev = subdrv->dev;
>> >> +       /* Find possible crtcs for this display */
>> >> +       list_for_each_entry(manager, &exynos_drm_manager_list, list)
>> >> +               if (manager->type == display->type)
>> >> +                       possible_crtcs |= 1 << manager->pipe;
>> >>
>> >>         /* create and initialize a encoder for this sub driver. */
>> >> -       encoder = exynos_drm_encoder_create(dev, subdrv->manager,
>> >> -                       (1 << MAX_CRTC) - 1);
>> >> +       encoder = exynos_drm_encoder_create(dev, display,
>> possible_crtcs);
>> >>         if (!encoder) {
>> >>                 DRM_ERROR("failed to create encoder\n");
>> >>                 return -EFAULT;
>> >>         }
>> >> -       subdrv->encoder = encoder;
>> >> +       display->encoder = encoder;
>> >>
>> >> -       if (subdrv->manager->display_ops->type ==
>> EXYNOS_DISPLAY_TYPE_LCD) {
>> >> +       if (display->type == EXYNOS_DISPLAY_TYPE_LCD) {
>> >>                 ret = exynos_drm_attach_lcd_bridge(dev, encoder)
>> >
>> > Plz, _do_not_base_ on top of your lvds bridge patch set. As I
>> > mentioned before, the tricky codes are not good. For this, let's find
>> > a better way after your refactoring patch set are reviewed enough, and
>> > if merged.
>> >
>>
>> [+adding back the people you removed from CC]
>>
>
> Hm.. I  didn't really remove the people from CC. See the your original
> email. :)
>
>
>> I'm happy to change how that works, but I've yet to find a better
>> solution. Instead of just dismissing something as "tricky codes", can
>> you suggest a concrete solution that is better than what I've got?
>>
>
> I think I mentioned already at previous email. First of all, let's have a
> thorough review to your refactoring patch set, and then let's find a better
> way.
>
> I believe that we can find a better way with your refactoring patch set: now
> we can implement encoder and connector directly.
>

The code will look the exact same, it'll just be in the dp driver
instead of drm_core (since drm_core will not create
encoders/connectors). Adding the ptn driver in the interim allows me
to boot exynos5250-snow with linux-next, so I can test my changes.


>
>> I'm also a bit confused as to why you think it's so tricky, it's a
>
> First, That was your comment, "Right, so this is kind of tricky". And also
> it seems like tricky codes to me.
>

Right, it was a tricky problem (for reasons I've stated multiple
times), but the solution is dead easy to understand.


>> synchronous initialization call to a driver; that's about as simple as
>> it gets.
>
> As you mentioned before, ptn3460 driver is not depended on SoC, and can be
> used for other SoC. Let you try to think about lcd class and backlight
> class. I think we need a interfacing layer between common driver and
> specific something for connecting SoC specific DRM framework.
>

Laurent mentioned that he wanted to come up with a way to model
graphics hardware in dt. He could then build a notifier when the
various pieces were successfully probed, and drm would only load once
all of the pieces were ready.

I think that would be a nice long-term solution, however let's not let
the perfect be the enemy of the good. If you can simplify how this
works, I'm all for it. However, I don't see any other way at the
moment and I don't think it's productive to just wait around for
something better to come along.

Sean


>>
>> Sean
>>
>>
>>
>> >>                 if (ret)
>> >>                         return 0;
>> >> @@ -91,7 +98,7 @@ static int exynos_drm_create_enc_conn(struct
>> drm_device *dev,
>> >>                 goto err_destroy_encoder;
>> >>         }
>> >>
>> >> -       subdrv->connector = connector;
>> >> +       display->connector = connector;
>> >>
>> >>         return 0;
>> >>
>> >> @@ -100,21 +107,6 @@ err_destroy_encoder:
>> >>         return ret;
>> >>  }
>> >>
>
대인기/Tizen Platform Lab(SR)/삼성전자 Oct. 12, 2013, 4:30 a.m. UTC | #5
> -----Original Message-----
> From: Sean Paul [mailto:seanpaul@chromium.org]
> Sent: Saturday, October 12, 2013 1:20 PM
> To: Inki Dae
> Cc: DRI mailing list; Stéphane Marchesin; Dave Airlie; Tomasz Figa
> Subject: Re: [PATCH 12/23] drm/exynos: Split manager/display/subdrv
> 
> On Fri, Oct 11, 2013 at 11:49 PM, Inki Dae <inki.dae@samsung.com> wrote:
> >
> >
> >> -----Original Message-----
> >> From: Sean Paul [mailto:seanpaul@chromium.org]
> >> Sent: Saturday, October 12, 2013 11:07 AM
> >> To: Inki Dae
> >> Cc: DRI mailing list; Stéphane Marchesin; Dave Airlie; Tomasz Figa
> >> Subject: Re: [PATCH 12/23] drm/exynos: Split manager/display/subdrv
> >>
> >> On Fri, Oct 11, 2013 at 8:42 PM, Inki Dae <inki.dae@samsung.com> wrote:
> >> >>  static int exynos_drm_create_enc_conn(struct drm_device *dev,
> >> >> -                                       struct exynos_drm_subdrv
> > *subdrv)
> >> >> +                                       struct exynos_drm_display
> > *display)
> >> >>  {
> >> >>         struct drm_encoder *encoder;
> >> >>         struct drm_connector *connector;
> >> >> +       struct exynos_drm_manager *manager;
> >> >>         int ret;
> >> >> +       unsigned long possible_crtcs = 0;
> >> >>
> >> >> -       subdrv->manager->dev = subdrv->dev;
> >> >> +       /* Find possible crtcs for this display */
> >> >> +       list_for_each_entry(manager, &exynos_drm_manager_list, list)
> >> >> +               if (manager->type == display->type)
> >> >> +                       possible_crtcs |= 1 << manager->pipe;
> >> >>
> >> >>         /* create and initialize a encoder for this sub driver. */
> >> >> -       encoder = exynos_drm_encoder_create(dev, subdrv->manager,
> >> >> -                       (1 << MAX_CRTC) - 1);
> >> >> +       encoder = exynos_drm_encoder_create(dev, display,
> >> possible_crtcs);
> >> >>         if (!encoder) {
> >> >>                 DRM_ERROR("failed to create encoder\n");
> >> >>                 return -EFAULT;
> >> >>         }
> >> >> -       subdrv->encoder = encoder;
> >> >> +       display->encoder = encoder;
> >> >>
> >> >> -       if (subdrv->manager->display_ops->type ==
> >> EXYNOS_DISPLAY_TYPE_LCD) {
> >> >> +       if (display->type == EXYNOS_DISPLAY_TYPE_LCD) {
> >> >>                 ret = exynos_drm_attach_lcd_bridge(dev, encoder)
> >> >
> >> > Plz, _do_not_base_ on top of your lvds bridge patch set. As I
> >> > mentioned before, the tricky codes are not good. For this, let's find
> >> > a better way after your refactoring patch set are reviewed enough,
> and
> >> > if merged.
> >> >
> >>
> >> [+adding back the people you removed from CC]
> >>
> >
> > Hm.. I  didn't really remove the people from CC. See the your original
> > email. :)
> >
> >
> >> I'm happy to change how that works, but I've yet to find a better
> >> solution. Instead of just dismissing something as "tricky codes", can
> >> you suggest a concrete solution that is better than what I've got?
> >>
> >
> > I think I mentioned already at previous email. First of all, let's have
> a
> > thorough review to your refactoring patch set, and then let's find a
> better
> > way.
> >
> > I believe that we can find a better way with your refactoring patch set:
> now
> > we can implement encoder and connector directly.
> >
> 
> The code will look the exact same, it'll just be in the dp driver
> instead of drm_core (since drm_core will not create
> encoders/connectors). Adding the ptn driver in the interim allows me
> to boot exynos5250-snow with linux-next, so I can test my changes.
> 
> 
> >
> >> I'm also a bit confused as to why you think it's so tricky, it's a
> >
> > First, That was your comment, "Right, so this is kind of tricky". And
> also
> > it seems like tricky codes to me.
> >
> 
> Right, it was a tricky problem (for reasons I've stated multiple
> times), but the solution is dead easy to understand.
> 
> 
> >> synchronous initialization call to a driver; that's about as simple as
> >> it gets.
> >
> > As you mentioned before, ptn3460 driver is not depended on SoC, and can
> be
> > used for other SoC. Let you try to think about lcd class and backlight
> > class. I think we need a interfacing layer between common driver and
> > specific something for connecting SoC specific DRM framework.
> >
> 
> Laurent mentioned that he wanted to come up with a way to model
> graphics hardware in dt. He could then build a notifier when the
> various pieces were successfully probed, and drm would only load once
> all of the pieces were ready.
> 
> I think that would be a nice long-term solution, however let's not let
> the perfect be the enemy of the good. If you can simplify how this
> works, I'm all for it. However, I don't see any other way at the
> moment and I don't think it's productive to just wait around for
> something better to come along.
> 

Sean, let's have reviews to your refactoring patch set first. And then I
will merge your lvds bridge patch if we cannot find a better way. That would
not take much time.

> Sean
> 
> 
> >>
> >> Sean
> >>
> >>
> >>
> >> >>                 if (ret)
> >> >>                         return 0;
> >> >> @@ -91,7 +98,7 @@ static int exynos_drm_create_enc_conn(struct
> >> drm_device *dev,
> >> >>                 goto err_destroy_encoder;
> >> >>         }
> >> >>
> >> >> -       subdrv->connector = connector;
> >> >> +       display->connector = connector;
> >> >>
> >> >>         return 0;
> >> >>
> >> >> @@ -100,21 +107,6 @@ err_destroy_encoder:
> >> >>         return ret;
> >> >>  }
> >> >>
> >
대인기/Tizen Platform Lab(SR)/삼성전자 Oct. 14, 2013, 3:24 p.m. UTC | #6
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> index c417c90..ba63c72 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
> @@ -26,24 +26,23 @@
>   * exynos specific encoder structure.
>   *
>   * @drm_encoder: encoder object.
> - * @manager: specific encoder has its own manager to control a hardware
> - *     appropriately and we can access a hardware drawing on this manager.
> + * @display: the display structure that maps to this encoder
>   */
>  struct exynos_drm_encoder {
>         struct drm_crtc                 *old_crtc;
>         struct drm_encoder              drm_encoder;
> -       struct exynos_drm_manager       *manager;
> +       struct exynos_drm_display       *display;
>  };
>
>  static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
>  {
> -       struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
> -       struct exynos_drm_display_ops *display_ops = manager->display_ops;
> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> +       struct exynos_drm_display *display = exynos_encoder->display;
>
>         DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
>
> -       if (display_ops && display_ops->dpms)
> -               display_ops->dpms(manager->ctx, mode);
> +       if (display->ops->dpms)
> +               display->ops->dpms(display->ctx, mode);

It's good to remove apply callback. However, it seems that this patch
has a problem that dma channel of fimd isn't enabled after dpms goes
from off to on. So can you implement win_enable callback of fimd, and
add it to fimd_win_resume function? We should have implemented
win_enable callback.

>  }
>
>  static bool
> @@ -52,15 +51,17 @@ exynos_drm_encoder_mode_fixup(struct drm_encoder *encoder,
>                                struct drm_display_mode *adjusted_mode)
>  {
>         struct drm_device *dev = encoder->dev;
> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> +       struct exynos_drm_display *display = exynos_encoder->display;
>         struct drm_connector *connector;
> -       struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
> -       struct exynos_drm_manager_ops *manager_ops = manager->ops;
>
>         list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
> -               if (connector->encoder == encoder)
> -                       if (manager_ops && manager_ops->mode_fixup)
> -                               manager_ops->mode_fixup(manager->ctx, connector,
> -                                                       mode, adjusted_mode);
> +               if (connector->encoder != encoder)
> +                       continue;
> +
> +               if (display->ops->mode_fixup)
> +                       display->ops->mode_fixup(display->ctx, connector, mode,
> +                                       adjusted_mode);
>         }
>
>         return true;
> @@ -102,8 +103,7 @@ static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder,
>  {
>         struct drm_device *dev = encoder->dev;
>         struct drm_connector *connector;
> -       struct exynos_drm_manager *manager;
> -       struct exynos_drm_manager_ops *manager_ops;
> +       struct exynos_drm_display *display;
>
>         list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
>                 if (connector->encoder == encoder) {
> @@ -123,11 +123,10 @@ static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder,
>                                                 encoder->crtc);
>                         }
>
> -                       manager = exynos_drm_get_manager(encoder);
> -                       manager_ops = manager->ops;
> +                       display = exynos_encoder->display;
>
> -                       if (manager_ops && manager_ops->mode_set)
> -                               manager_ops->mode_set(manager->ctx,
> +                       if (display->ops->mode_set)
> +                               display->ops->mode_set(display->ctx,
>                                                         adjusted_mode);
>
>                         exynos_encoder->old_crtc = encoder->crtc;
> @@ -143,39 +142,15 @@ static void exynos_drm_encoder_prepare(struct drm_encoder *encoder)
>  static void exynos_drm_encoder_commit(struct drm_encoder *encoder)
>  {
>         struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> -       struct exynos_drm_manager *manager = exynos_encoder->manager;
> -       struct exynos_drm_manager_ops *manager_ops = manager->ops;
> -
> -       if (manager_ops && manager_ops->commit)
> -               manager_ops->commit(manager->ctx);
> -}
> +       struct exynos_drm_display *display = exynos_encoder->display;
>
> -void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb)
> -{
> -       struct exynos_drm_encoder *exynos_encoder;
> -       struct exynos_drm_manager_ops *ops;
> -       struct drm_device *dev = fb->dev;
> -       struct drm_encoder *encoder;
> +       if (display->ops->dpms)
> +               display->ops->dpms(display->ctx, DRM_MODE_DPMS_ON);
>
> -       /*
> -        * make sure that overlay data are updated to real hardware
> -        * for all encoders.
> -        */
> -       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
> -               exynos_encoder = to_exynos_encoder(encoder);
> -               ops = exynos_encoder->manager->ops;
> -
> -               /*
> -                * wait for vblank interrupt
> -                * - this makes sure that overlay data are updated to
> -                *      real hardware.
> -                */
> -               if (ops->wait_for_vblank)
> -                       ops->wait_for_vblank(exynos_encoder->manager->dev);
> -       }
> +       if (display->ops->commit)
> +               display->ops->commit(display->ctx);
>  }
>
> -
>  static void exynos_drm_encoder_disable(struct drm_encoder *encoder)
>  {
>         struct drm_plane *plane;
> @@ -201,10 +176,7 @@ static struct drm_encoder_helper_funcs exynos_encoder_helper_funcs = {
>
>  static void exynos_drm_encoder_destroy(struct drm_encoder *encoder)
>  {
> -       struct exynos_drm_encoder *exynos_encoder =
> -               to_exynos_encoder(encoder);
> -
> -       exynos_encoder->manager->pipe = -1;
> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
>
>         drm_encoder_cleanup(encoder);
>         kfree(exynos_encoder);
> @@ -219,13 +191,12 @@ static unsigned int exynos_drm_encoder_clones(struct drm_encoder *encoder)
>         struct drm_encoder *clone;
>         struct drm_device *dev = encoder->dev;
>         struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> -       struct exynos_drm_display_ops *display_ops =
> -                               exynos_encoder->manager->display_ops;
> +       struct exynos_drm_display *display = exynos_encoder->display;
>         unsigned int clone_mask = 0;
>         int cnt = 0;
>
>         list_for_each_entry(clone, &dev->mode_config.encoder_list, head) {
> -               switch (display_ops->type) {
> +               switch (display->type) {
>                 case EXYNOS_DISPLAY_TYPE_LCD:
>                 case EXYNOS_DISPLAY_TYPE_HDMI:
>                 case EXYNOS_DISPLAY_TYPE_VIDI:
> @@ -249,24 +220,20 @@ void exynos_drm_encoder_setup(struct drm_device *dev)
>
>  struct drm_encoder *
>  exynos_drm_encoder_create(struct drm_device *dev,
> -                          struct exynos_drm_manager *manager,
> +                          struct exynos_drm_display *display,
>                            unsigned long possible_crtcs)
>  {
>         struct drm_encoder *encoder;
>         struct exynos_drm_encoder *exynos_encoder;
> -       int ret;
>
> -       if (!manager || !possible_crtcs)
> -               return NULL;
> -
> -       if (!manager->dev)
> +       if (!possible_crtcs)
>                 return NULL;
>
>         exynos_encoder = kzalloc(sizeof(*exynos_encoder), GFP_KERNEL);
>         if (!exynos_encoder)
>                 return NULL;
>
> -       exynos_encoder->manager = manager;
> +       exynos_encoder->display = display;
>         encoder = &exynos_encoder->drm_encoder;
>         encoder->possible_crtcs = possible_crtcs;
>
> @@ -277,174 +244,12 @@ exynos_drm_encoder_create(struct drm_device *dev,
>
>         drm_encoder_helper_add(encoder, &exynos_encoder_helper_funcs);
>
> -       if (manager->ops && manager->ops->initialize) {
> -               ret = manager->ops->initialize(manager->ctx, dev);
> -               if (ret) {
> -                       DRM_ERROR("Manager initialize failed %d\n", ret);
> -                       goto error;
> -               }
> -       }
> -
> -       if (manager->display_ops && manager->display_ops->initialize) {
> -               ret = manager->display_ops->initialize(manager->ctx, dev);
> -               if (ret) {
> -                       DRM_ERROR("Display initialize failed %d\n", ret);
> -                       goto error;
> -               }
> -       }
> -
>         DRM_DEBUG_KMS("encoder has been created\n");
>
>         return encoder;
> -
> -error:
> -       exynos_drm_encoder_destroy(&exynos_encoder->drm_encoder);
> -       return NULL;
>  }
>
> -struct exynos_drm_manager *exynos_drm_get_manager(struct drm_encoder *encoder)
> +struct exynos_drm_display *exynos_drm_get_display(struct drm_encoder *encoder)
>  {
> -       return to_exynos_encoder(encoder)->manager;
> -}
> -
> -void exynos_drm_fn_encoder(struct drm_crtc *crtc, void *data,
> -                           void (*fn)(struct drm_encoder *, void *))
> -{
> -       struct drm_device *dev = crtc->dev;
> -       struct drm_encoder *encoder;
> -       struct exynos_drm_private *private = dev->dev_private;
> -       struct exynos_drm_manager *manager;
> -
> -       list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
> -               /*
> -                * if crtc is detached from encoder, check pipe,
> -                * otherwise check crtc attached to encoder
> -                */
> -               if (!encoder->crtc) {
> -                       manager = to_exynos_encoder(encoder)->manager;
> -                       if (manager->pipe < 0 ||
> -                                       private->crtc[manager->pipe] != crtc)
> -                               continue;
> -               } else {
> -                       if (encoder->crtc != crtc)
> -                               continue;
> -               }
> -
> -               fn(encoder, data);
> -       }
> -}
> -
> -void exynos_drm_enable_vblank(struct drm_encoder *encoder, void *data)
> -{
> -       struct exynos_drm_manager *manager =
> -               to_exynos_encoder(encoder)->manager;
> -       struct exynos_drm_manager_ops *manager_ops = manager->ops;
> -       int crtc = *(int *)data;
> -
> -       if (manager->pipe != crtc)
> -               return;
> -
> -       if (manager_ops->enable_vblank)
> -               manager_ops->enable_vblank(manager->ctx);
> -}
> -
> -void exynos_drm_disable_vblank(struct drm_encoder *encoder, void *data)
> -{
> -       struct exynos_drm_manager *manager =
> -               to_exynos_encoder(encoder)->manager;
> -       struct exynos_drm_manager_ops *manager_ops = manager->ops;
> -       int crtc = *(int *)data;
> -
> -       if (manager->pipe != crtc)
> -               return;
> -
> -       if (manager_ops->disable_vblank)
> -               manager_ops->disable_vblank(manager->ctx);
> -}
> -
> -void exynos_drm_encoder_crtc_dpms(struct drm_encoder *encoder, void *data)
> -{
> -       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
> -       struct exynos_drm_manager *manager = exynos_encoder->manager;
> -       struct exynos_drm_manager_ops *manager_ops = manager->ops;
> -       int mode = *(int *)data;
> -
> -       if (manager_ops && manager_ops->dpms)
> -               manager_ops->dpms(manager->ctx, mode);
> -
> -       /*
> -        * if this condition is ok then it means that the crtc is already
> -        * detached from encoder and last function for detaching is properly
> -        * done, so clear pipe from manager to prevent repeated call.
> -        */
> -       if (mode > DRM_MODE_DPMS_ON) {
> -               if (!encoder->crtc)
> -                       manager->pipe = -1;
> -       }
> -}
> -
> -void exynos_drm_encoder_crtc_pipe(struct drm_encoder *encoder, void *data)
> -{
> -       struct exynos_drm_manager *manager =
> -               to_exynos_encoder(encoder)->manager;
> -       int pipe = *(int *)data;
> -
> -       /*
> -        * when crtc is detached from encoder, this pipe is used
> -        * to select manager operation
> -        */
> -       manager->pipe = pipe;
> -}
> -
> -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_manager_ops *manager_ops = manager->ops;
> -       struct exynos_drm_overlay *overlay = data;
> -
> -       if (manager_ops && manager_ops->win_mode_set)
> -               manager_ops->win_mode_set(manager->ctx, 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_manager_ops *manager_ops = manager->ops;
> -       int zpos = DEFAULT_ZPOS;
> -
> -       if (data)
> -               zpos = *(int *)data;
> -
> -       if (manager_ops && manager_ops->win_commit)
> -               manager_ops->win_commit(manager->ctx, 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_manager_ops *manager_ops = manager->ops;
> -       int zpos = DEFAULT_ZPOS;
> -
> -       if (data)
> -               zpos = *(int *)data;
> -
> -       if (manager_ops && manager_ops->win_enable)
> -               manager_ops->win_enable(manager->ctx, 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_manager_ops *manager_ops = manager->ops;
> -       int zpos = DEFAULT_ZPOS;
> -
> -       if (data)
> -               zpos = *(int *)data;
> -
> -       if (manager_ops && manager_ops->win_disable)
> -               manager_ops->win_disable(manager->ctx, zpos);
> +       return to_exynos_encoder(encoder)->display;
>  }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.h b/drivers/gpu/drm/exynos/exynos_drm_encoder.h
> index 0f3e5e2..b7a1620 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.h
> @@ -18,20 +18,8 @@ struct exynos_drm_manager;
>
>  void exynos_drm_encoder_setup(struct drm_device *dev);
>  struct drm_encoder *exynos_drm_encoder_create(struct drm_device *dev,
> -                                              struct exynos_drm_manager *mgr,
> -                                              unsigned long possible_crtcs);
> -struct exynos_drm_manager *
> -exynos_drm_get_manager(struct drm_encoder *encoder);
> -void exynos_drm_fn_encoder(struct drm_crtc *crtc, void *data,
> -                           void (*fn)(struct drm_encoder *, void *));
> -void exynos_drm_enable_vblank(struct drm_encoder *encoder, void *data);
> -void exynos_drm_disable_vblank(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_crtc_dpms(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_crtc_pipe(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_plane_mode_set(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_plane_commit(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_plane_enable(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_plane_disable(struct drm_encoder *encoder, void *data);
> -void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb);
> +                       struct exynos_drm_display *mgr,
> +                       unsigned long possible_crtcs);
> +struct exynos_drm_display *exynos_drm_get_display(struct drm_encoder *encoder);
>
>  #endif
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> index ea39e0e..c7c08d0 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
> @@ -22,7 +22,7 @@
>  #include "exynos_drm_fb.h"
>  #include "exynos_drm_gem.h"
>  #include "exynos_drm_iommu.h"
> -#include "exynos_drm_encoder.h"
> +#include "exynos_drm_crtc.h"
>
>  #define to_exynos_fb(x)        container_of(x, struct exynos_drm_fb, fb)
>
> @@ -71,7 +71,7 @@ static void exynos_drm_fb_destroy(struct drm_framebuffer *fb)
>         unsigned int i;
>
>         /* make sure that overlay data are updated before relesing fb. */
> -       exynos_drm_encoder_complete_scanout(fb);
> +       exynos_drm_crtc_complete_scanout(fb);
>
>         drm_framebuffer_cleanup(fb);
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> index 838c47d..f3dc808 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
> @@ -105,7 +105,6 @@ struct fimd_win_data {
>  };
>
>  struct fimd_context {
> -       struct exynos_drm_subdrv        subdrv;
>         struct device                   *dev;
>         struct drm_device               *drm_dev;
>         int                             irq;
> @@ -120,6 +119,7 @@ struct fimd_context {
>         u32                             vidcon0;
>         u32                             vidcon1;
>         bool                            suspended;
> +       int                             pipe;
>         struct mutex                    lock;
>         wait_queue_head_t               wait_vsync_queue;
>         atomic_t                        wait_vsync_event;
> @@ -169,12 +169,16 @@ static int fimd_check_mode(void *in_ctx, struct drm_display_mode *mode)
>  }
>
>  static struct exynos_drm_display_ops fimd_display_ops = {
> -       .type = EXYNOS_DISPLAY_TYPE_LCD,
>         .is_connected = fimd_display_is_connected,
>         .get_panel = fimd_get_panel,
>         .check_mode = fimd_check_mode,
>  };
>
> +static struct exynos_drm_display fimd_display = {
> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
> +       .ops = &fimd_display_ops,
> +};
> +
>  static void fimd_win_mode_set(void *in_ctx, struct exynos_drm_overlay *overlay)
>  {
>         struct fimd_context *ctx = in_ctx;
> @@ -481,15 +485,46 @@ static void fimd_win_disable(void *in_ctx, int zpos)
>         win_data->enabled = false;
>  }
>
> -static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
> +static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
> +               int pipe)
>  {
>         struct fimd_context *ctx = in_ctx;
>
>         ctx->drm_dev = drm_dev;
> +       ctx->pipe = pipe;
> +
> +       /*
> +        * enable drm irq mode.
> +        * - with irq_enabled = 1, we can use the vblank feature.
> +        *
> +        * P.S. note that we wouldn't use drm irq handler but
> +        *      just specific driver own one instead because
> +        *      drm framework supports only one irq handler.
> +        */
> +       ctx->drm_dev->irq_enabled = 1;
> +
> +       /*
> +        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
> +        * by drm timer once a current process gives up ownership of
> +        * vblank event.(after drm_vblank_put function is called)
> +        */
> +       drm_dev->vblank_disable_allowed = 1;
> +
> +       /* attach this sub driver to iommu mapping if supported. */
> +       if (is_drm_iommu_supported(ctx->drm_dev))
> +               drm_iommu_attach_device(ctx->drm_dev, ctx->dev);
>
>         return 0;
>  }
>
> +static void fimd_mgr_remove(void *in_ctx)
> +{
> +       struct fimd_context *ctx = in_ctx;
> +
> +       if (is_drm_iommu_supported(ctx->drm_dev))
> +               drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
> +}
> +
>  static void fimd_dpms(void *in_ctx, int mode)
>  {
>         struct fimd_context *ctx = in_ctx;
> @@ -523,24 +558,6 @@ static void fimd_dpms(void *in_ctx, int mode)
>         mutex_unlock(&ctx->lock);
>  }
>
> -static void fimd_apply(void *in_ctx)
> -{
> -       struct fimd_context *ctx = in_ctx;
> -       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(ctx, i);
> -       }
> -
> -       if (mgr_ops && mgr_ops->commit)
> -               mgr_ops->commit(ctx);
> -}
> -
>  static void fimd_commit(void *in_ctx)
>  {
>         struct fimd_context *ctx = in_ctx;
> @@ -597,6 +614,21 @@ static void fimd_commit(void *in_ctx)
>         writel(val, ctx->regs + VIDCON0);
>  }
>
> +static void fimd_apply(void *in_ctx)
> +{
> +       struct fimd_context *ctx = in_ctx;
> +       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)
> +                       fimd_win_commit(ctx, i);
> +       }
> +
> +       fimd_commit(ctx);
> +}
> +
>  static int fimd_enable_vblank(void *in_ctx)
>  {
>         struct fimd_context *ctx = in_ctx;
> @@ -661,6 +693,7 @@ static void fimd_wait_for_vblank(void *in_ctx)
>
>  static struct exynos_drm_manager_ops fimd_manager_ops = {
>         .initialize = fimd_mgr_initialize,
> +       .remove = fimd_mgr_remove,
>         .dpms = fimd_dpms,
>         .apply = fimd_apply,
>         .commit = fimd_commit,
> @@ -673,16 +706,13 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
>  };
>
>  static struct exynos_drm_manager fimd_manager = {
> -       .pipe           = -1,
> -       .ops            = &fimd_manager_ops,
> -       .display_ops    = &fimd_display_ops,
> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
> +       .ops = &fimd_manager_ops,
>  };
>
>  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>  {
>         struct fimd_context *ctx = (struct fimd_context *)dev_id;
> -       struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
> -       struct exynos_drm_manager *manager = subdrv->manager;
>         u32 val;
>
>         val = readl(ctx->regs + VIDINTCON1);
> @@ -692,11 +722,11 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>                 writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
>
>         /* check the crtc is detached already from encoder */
> -       if (manager->pipe < 0 || !ctx->drm_dev)
> +       if (ctx->pipe < 0 || !ctx->drm_dev)
>                 goto out;
>
> -       drm_handle_vblank(ctx->drm_dev, manager->pipe);
> -       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe);
> +       drm_handle_vblank(ctx->drm_dev, ctx->pipe);
> +       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>
>         /* set wait vsync event to zero and wake up queue. */
>         if (atomic_read(&ctx->wait_vsync_event)) {
> @@ -707,39 +737,6 @@ out:
>         return IRQ_HANDLED;
>  }
>
> -static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
> -{
> -       /*
> -        * enable drm irq mode.
> -        * - with irq_enabled = 1, we can use the vblank feature.
> -        *
> -        * P.S. note that we wouldn't use drm irq handler but
> -        *      just specific driver own one instead because
> -        *      drm framework supports only one irq handler.
> -        */
> -       drm_dev->irq_enabled = 1;
> -
> -       /*
> -        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
> -        * by drm timer once a current process gives up ownership of
> -        * vblank event.(after drm_vblank_put function is called)
> -        */
> -       drm_dev->vblank_disable_allowed = 1;
> -
> -       /* attach this sub driver to iommu mapping if supported. */
> -       if (is_drm_iommu_supported(drm_dev))
> -               drm_iommu_attach_device(drm_dev, dev);
> -
> -       return 0;
> -}
> -
> -static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
> -{
> -       /* detach this sub driver from iommu mapping if supported. */
> -       if (is_drm_iommu_supported(drm_dev))
> -               drm_iommu_detach_device(drm_dev, dev);
> -}
> -
>  static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev)
>  {
>         struct videomode *vm = &ctx->panel.vm;
> @@ -825,9 +822,8 @@ static int fimd_clock(struct fimd_context *ctx, bool enable)
>         return 0;
>  }
>
> -static void fimd_window_suspend(struct device *dev)
> +static void fimd_window_suspend(struct fimd_context *ctx)
>  {
> -       struct fimd_context *ctx = get_fimd_context(dev);
>         struct fimd_win_data *win_data;
>         int i;
>
> @@ -839,9 +835,8 @@ static void fimd_window_suspend(struct device *dev)
>         fimd_wait_for_vblank(ctx);
>  }
>
> -static void fimd_window_resume(struct device *dev)
> +static void fimd_window_resume(struct fimd_context *ctx)
>  {
> -       struct fimd_context *ctx = get_fimd_context(dev);
>         struct fimd_win_data *win_data;
>         int i;
>
> @@ -854,7 +849,6 @@ static void fimd_window_resume(struct device *dev)
>
>  static int fimd_activate(struct fimd_context *ctx, bool enable)
>  {
> -       struct device *dev = ctx->subdrv.dev;
>         if (enable) {
>                 int ret;
>
> @@ -866,11 +860,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable)
>
>                 /* if vblank was enabled status, enable it again. */
>                 if (test_and_clear_bit(0, &ctx->irq_flags))
> -                       fimd_enable_vblank(dev);
> +                       fimd_enable_vblank(ctx);
>
> -               fimd_window_resume(dev);
> +               fimd_window_resume(ctx);
>         } else {
> -               fimd_window_suspend(dev);
> +               fimd_window_suspend(ctx);
>
>                 fimd_clock(ctx, false);
>                 ctx->suspended = true;
> @@ -907,7 +901,6 @@ static int fimd_probe(struct platform_device *pdev)
>  {
>         struct device *dev = &pdev->dev;
>         struct fimd_context *ctx;
> -       struct exynos_drm_subdrv *subdrv;
>         struct resource *res;
>         int win;
>         int ret = -EINVAL;
> @@ -952,15 +945,6 @@ static int fimd_probe(struct platform_device *pdev)
>         DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue);
>         atomic_set(&ctx->wait_vsync_event, 0);
>
> -       fimd_manager.ctx = ctx;
> -
> -       subdrv = &ctx->subdrv;
> -
> -       subdrv->dev = dev;
> -       subdrv->manager = &fimd_manager;
> -       subdrv->probe = fimd_subdrv_probe;
> -       subdrv->remove = fimd_subdrv_remove;
> -
>         mutex_init(&ctx->lock);
>
>         platform_set_drvdata(pdev, ctx);
> @@ -971,7 +955,11 @@ static int fimd_probe(struct platform_device *pdev)
>         for (win = 0; win < WINDOWS_NR; win++)
>                 fimd_clear_win(ctx, win);
>
> -       exynos_drm_subdrv_register(subdrv);
> +       fimd_manager.ctx = ctx;
> +       exynos_drm_manager_register(&fimd_manager);
> +
> +       fimd_display.ctx = ctx;
> +       exynos_drm_display_register(&fimd_display);
>
>         return 0;
>  }
> @@ -981,7 +969,8 @@ static int fimd_remove(struct platform_device *pdev)
>         struct device *dev = &pdev->dev;
>         struct fimd_context *ctx = platform_get_drvdata(pdev);
>
> -       exynos_drm_subdrv_unregister(&ctx->subdrv);
> +       exynos_drm_display_unregister(&fimd_display);
> +       exynos_drm_manager_unregister(&fimd_manager);
>
>         if (ctx->suspended)
>                 goto out;
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> index 8173e44..8fc9d6d 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
> @@ -24,14 +24,11 @@
>  #include "exynos_drm_hdmi.h"
>
>  #define to_context(dev)                platform_get_drvdata(to_platform_device(dev))
> -#define to_subdrv(dev)         to_context(dev)
> -#define get_ctx_from_subdrv(subdrv)    container_of(subdrv,\
> -                                       struct drm_hdmi_context, subdrv);
>
>  /* platform device pointer for common drm hdmi device. */
>  static struct platform_device *exynos_drm_hdmi_pdev;
>
> -/* Common hdmi subdrv needs to access the hdmi and mixer though context.
> +/* Common hdmi needs to access the hdmi and mixer though context.
>  * These should be initialied by the repective drivers */
>  static struct exynos_drm_hdmi_context *hdmi_ctx;
>  static struct exynos_drm_hdmi_context *mixer_ctx;
> @@ -41,7 +38,6 @@ static struct exynos_hdmi_ops *hdmi_ops;
>  static struct exynos_mixer_ops *mixer_ops;
>
>  struct drm_hdmi_context {
> -       struct exynos_drm_subdrv        subdrv;
>         struct exynos_drm_hdmi_context  *hdmi_ctx;
>         struct exynos_drm_hdmi_context  *mixer_ctx;
>
> @@ -101,6 +97,12 @@ static int drm_hdmi_display_initialize(void *in_ctx, struct drm_device *drm_dev)
>  {
>         struct drm_hdmi_context *ctx = in_ctx;
>
> +       if (!hdmi_ctx) {
> +               DRM_ERROR("hdmi context not initialized.\n");
> +               return -EINVAL;
> +       }
> +       ctx->hdmi_ctx = hdmi_ctx;
> +
>         if (hdmi_ops && hdmi_ops->initialize)
>                 return hdmi_ops->initialize(ctx->hdmi_ctx->ctx, drm_dev);
>
> @@ -118,6 +120,17 @@ static bool drm_hdmi_is_connected(void *in_ctx)
>         return false;
>  }
>
> +static void drm_hdmi_get_max_resol(void *in_ctx, unsigned int *width,
> +                               unsigned int *height)
> +{
> +       struct drm_hdmi_context *ctx = in_ctx;
> +
> +       DRM_DEBUG_KMS("%s\n", __FILE__);
> +
> +       if (hdmi_ops && hdmi_ops->get_max_resol)
> +               hdmi_ops->get_max_resol(ctx->hdmi_ctx->ctx, width, height);
> +}
> +
>  static struct edid *drm_hdmi_get_edid(void *in_ctx,
>                         struct drm_connector *connector)
>  {
> @@ -129,6 +142,16 @@ static struct edid *drm_hdmi_get_edid(void *in_ctx,
>         return NULL;
>  }
>
> +static void drm_hdmi_mode_set(void *in_ctx, struct drm_display_mode *mode)
> +{
> +       struct drm_hdmi_context *ctx = in_ctx;
> +
> +       DRM_DEBUG_KMS("%s\n", __FILE__);
> +
> +       if (hdmi_ops && hdmi_ops->mode_set)
> +               hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode);
> +}
> +
>  static int drm_hdmi_check_mode(void *in_ctx, struct drm_display_mode *mode)
>  {
>         struct drm_hdmi_context *ctx = in_ctx;
> @@ -151,52 +174,6 @@ static int drm_hdmi_check_mode(void *in_ctx, struct drm_display_mode *mode)
>         return 0;
>  }
>
> -static void drm_hdmi_display_dpms(void *in_ctx, int mode)
> -{
> -       struct drm_hdmi_context *ctx = in_ctx;
> -
> -       if (hdmi_ops && hdmi_ops->dpms)
> -               hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode);
> -}
> -
> -static struct exynos_drm_display_ops drm_hdmi_display_ops = {
> -       .type = EXYNOS_DISPLAY_TYPE_HDMI,
> -       .initialize = drm_hdmi_display_initialize,
> -       .is_connected = drm_hdmi_is_connected,
> -       .get_edid = drm_hdmi_get_edid,
> -       .check_mode = drm_hdmi_check_mode,
> -       .dpms = drm_hdmi_display_dpms,
> -};
> -
> -static int drm_hdmi_enable_vblank(void *in_ctx)
> -{
> -       struct drm_hdmi_context *ctx = in_ctx;
> -       struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
> -       struct exynos_drm_manager *manager = subdrv->manager;
> -
> -       if (mixer_ops && mixer_ops->enable_vblank)
> -               return mixer_ops->enable_vblank(ctx->mixer_ctx->ctx,
> -                                               manager->pipe);
> -
> -       return 0;
> -}
> -
> -static void drm_hdmi_disable_vblank(void *in_ctx)
> -{
> -       struct drm_hdmi_context *ctx = in_ctx;
> -
> -       if (mixer_ops && mixer_ops->disable_vblank)
> -               return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx);
> -}
> -
> -static void drm_hdmi_wait_for_vblank(void *in_ctx)
> -{
> -       struct drm_hdmi_context *ctx = in_ctx;
> -
> -       if (mixer_ops && mixer_ops->wait_for_vblank)
> -               mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx);
> -}
> -
>  static void drm_hdmi_mode_fixup(void *in_ctx, struct drm_connector *connector,
>                                 const struct drm_display_mode *mode,
>                                 struct drm_display_mode *adjusted_mode)
> @@ -241,21 +218,57 @@ static void drm_hdmi_mode_fixup(void *in_ctx, struct drm_connector *connector,
>         }
>  }
>
> -static void drm_hdmi_mode_set(void *in_ctx, void *mode)
> +static void drm_hdmi_display_dpms(void *in_ctx, int mode)
>  {
>         struct drm_hdmi_context *ctx = in_ctx;
>
> -       if (hdmi_ops && hdmi_ops->mode_set)
> -               hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode);
> +       if (hdmi_ops && hdmi_ops->dpms)
> +               hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode);
>  }
>
> -static void drm_hdmi_get_max_resol(void *in_ctx, unsigned int *width,
> -                               unsigned int *height)
> +static struct exynos_drm_display_ops drm_hdmi_display_ops = {
> +       .initialize = drm_hdmi_display_initialize,
> +       .is_connected = drm_hdmi_is_connected,
> +       .get_max_resol = drm_hdmi_get_max_resol,
> +       .get_edid = drm_hdmi_get_edid,
> +       .mode_fixup = drm_hdmi_mode_fixup,
> +       .mode_set = drm_hdmi_mode_set,
> +       .check_mode = drm_hdmi_check_mode,
> +       .dpms = drm_hdmi_display_dpms,
> +};
> +
> +static struct exynos_drm_display hdmi_display = {
> +       .type = EXYNOS_DISPLAY_TYPE_HDMI,
> +       .ops = &drm_hdmi_display_ops,
> +};
> +
> +static int drm_hdmi_enable_vblank(void *in_ctx)
>  {
>         struct drm_hdmi_context *ctx = in_ctx;
>
> -       if (hdmi_ops && hdmi_ops->get_max_resol)
> -               hdmi_ops->get_max_resol(ctx->hdmi_ctx->ctx, width, height);
> +       if (mixer_ops && mixer_ops->enable_vblank)
> +               return mixer_ops->enable_vblank(ctx->mixer_ctx->ctx,
> +                                               ctx->mixer_ctx->pipe);
> +
> +       return 0;
> +}
> +
> +static void drm_hdmi_disable_vblank(void *in_ctx)
> +{
> +       struct drm_hdmi_context *ctx = in_ctx;
> +
> +       DRM_DEBUG_KMS("%s\n", __FILE__);
> +
> +       if (mixer_ops && mixer_ops->disable_vblank)
> +               return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx);
> +}
> +
> +static void drm_hdmi_wait_for_vblank(void *in_ctx)
> +{
> +       struct drm_hdmi_context *ctx = in_ctx;
> +
> +       if (mixer_ops && mixer_ops->wait_for_vblank)
> +               mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx);
>  }
>
>  static void drm_hdmi_commit(void *in_ctx)
> @@ -266,11 +279,19 @@ static void drm_hdmi_commit(void *in_ctx)
>                 hdmi_ops->commit(ctx->hdmi_ctx->ctx);
>  }
>
> -static int drm_hdmi_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
> +static int drm_hdmi_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
> +               int pipe)
>  {
>         struct drm_hdmi_context *ctx = in_ctx;
>         int ret = 0;
>
> +       if (!mixer_ctx) {
> +               DRM_ERROR("mixer context not initialized.\n");
> +               return -EFAULT;
> +       }
> +       ctx->mixer_ctx = mixer_ctx;
> +       ctx->mixer_ctx->pipe = pipe;
> +
>         if (mixer_ops && mixer_ops->initialize)
>                 ret = mixer_ops->initialize(ctx->mixer_ctx->ctx, drm_dev);
>
> @@ -280,6 +301,14 @@ static int drm_hdmi_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
>         return ret;
>  }
>
> +static void drm_hdmi_mgr_remove(void *in_ctx)
> +{
> +       struct drm_hdmi_context *ctx = in_ctx;
> +
> +       if (mixer_ops->iommu_on)
> +               mixer_ops->iommu_on(ctx->mixer_ctx->ctx, false);
> +}
> +
>  static void drm_hdmi_dpms(void *in_ctx, int mode)
>  {
>         struct drm_hdmi_context *ctx = in_ctx;
> @@ -350,14 +379,12 @@ static void drm_mixer_win_disable(void *in_ctx, int zpos)
>
>  static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
>         .initialize = drm_hdmi_mgr_initialize,
> +       .remove = drm_hdmi_mgr_remove,
>         .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,
> @@ -365,82 +392,33 @@ static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
>  };
>
>  static struct exynos_drm_manager hdmi_manager = {
> -       .pipe           = -1,
> -       .ops            = &drm_hdmi_manager_ops,
> -       .display_ops    = &drm_hdmi_display_ops,
> +       .type = EXYNOS_DISPLAY_TYPE_HDMI,
> +       .ops = &drm_hdmi_manager_ops,
>  };
>
> -static int hdmi_subdrv_probe(struct drm_device *drm_dev,
> -               struct device *dev)
> -{
> -       struct exynos_drm_subdrv *subdrv = to_subdrv(dev);
> -       struct drm_hdmi_context *ctx;
> -
> -       if (!hdmi_ctx) {
> -               DRM_ERROR("hdmi context not initialized.\n");
> -               return -EFAULT;
> -       }
> -
> -       if (!mixer_ctx) {
> -               DRM_ERROR("mixer context not initialized.\n");
> -               return -EFAULT;
> -       }
> -
> -       ctx = get_ctx_from_subdrv(subdrv);
> -
> -       if (!ctx) {
> -               DRM_ERROR("no drm hdmi context.\n");
> -               return -EFAULT;
> -       }
> -
> -       ctx->hdmi_ctx = hdmi_ctx;
> -       ctx->mixer_ctx = mixer_ctx;
> -
> -       return 0;
> -}
> -
> -static void hdmi_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
> -{
> -       struct drm_hdmi_context *ctx;
> -       struct exynos_drm_subdrv *subdrv = to_subdrv(dev);
> -
> -       ctx = get_ctx_from_subdrv(subdrv);
> -
> -       if (mixer_ops->iommu_on)
> -               mixer_ops->iommu_on(ctx->mixer_ctx->ctx, false);
> -}
> -
>  static int exynos_drm_hdmi_probe(struct platform_device *pdev)
>  {
> -       struct device *dev = &pdev->dev;
> -       struct exynos_drm_subdrv *subdrv;
>         struct drm_hdmi_context *ctx;
>
> -       ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
> +       ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
>         if (!ctx)
>                 return -ENOMEM;
>
>         hdmi_manager.ctx = ctx;
> +       exynos_drm_manager_register(&hdmi_manager);
>
> -       subdrv = &ctx->subdrv;
> -
> -       subdrv->dev = dev;
> -       subdrv->manager = &hdmi_manager;
> -       subdrv->probe = hdmi_subdrv_probe;
> -       subdrv->remove = hdmi_subdrv_remove;
> +       hdmi_display.ctx = ctx;
> +       exynos_drm_display_register(&hdmi_display);
>
> -       platform_set_drvdata(pdev, subdrv);
> -
> -       exynos_drm_subdrv_register(subdrv);
> +       platform_set_drvdata(pdev, ctx);
>
>         return 0;
>  }
>
>  static int exynos_drm_hdmi_remove(struct platform_device *pdev)
>  {
> -       struct drm_hdmi_context *ctx = platform_get_drvdata(pdev);
> -
> -       exynos_drm_subdrv_unregister(&ctx->subdrv);
> +       exynos_drm_display_unregister(&hdmi_display);
> +       exynos_drm_manager_unregister(&hdmi_manager);
>
>         return 0;
>  }
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> index 923239b..37059ea 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> +++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
> @@ -19,10 +19,12 @@
>   * exynos hdmi common context structure.
>   *
>   * @drm_dev: pointer to drm_device.
> + * @pipe: pipe for mixer
>   * @ctx: pointer to the context of specific device driver.
>   *     this context should be hdmi_context or mixer_context.
>   */
>  struct exynos_drm_hdmi_context {
> +       int                     pipe;
>         void                    *ctx;
>  };
>
> diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> index cff3aed..e0db2b3 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
> @@ -13,7 +13,7 @@
>
>  #include <drm/exynos_drm.h>
>  #include "exynos_drm_drv.h"
> -#include "exynos_drm_encoder.h"
> +#include "exynos_drm_crtc.h"
>  #include "exynos_drm_fb.h"
>  #include "exynos_drm_gem.h"
>  #include "exynos_drm_plane.h"
> @@ -139,7 +139,7 @@ int exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
>                         overlay->crtc_x, overlay->crtc_y,
>                         overlay->crtc_width, overlay->crtc_height);
>
> -       exynos_drm_fn_encoder(crtc, overlay, exynos_drm_encoder_plane_mode_set);
> +       exynos_drm_crtc_plane_mode_set(crtc, overlay);
>
>         return 0;
>  }
> @@ -149,8 +149,7 @@ void exynos_plane_commit(struct drm_plane *plane)
>         struct exynos_plane *exynos_plane = to_exynos_plane(plane);
>         struct exynos_drm_overlay *overlay = &exynos_plane->overlay;
>
> -       exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
> -                       exynos_drm_encoder_plane_commit);
> +       exynos_drm_crtc_plane_commit(plane->crtc, overlay->zpos);
>  }
>
>  void exynos_plane_dpms(struct drm_plane *plane, int mode)
> @@ -162,17 +161,13 @@ void exynos_plane_dpms(struct drm_plane *plane, int mode)
>                 if (exynos_plane->enabled)
>                         return;
>
> -               exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
> -                               exynos_drm_encoder_plane_enable);
> -
> +               exynos_drm_crtc_plane_enable(plane->crtc, overlay->zpos);
>                 exynos_plane->enabled = true;
>         } else {
>                 if (!exynos_plane->enabled)
>                         return;
>
> -               exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
> -                               exynos_drm_encoder_plane_disable);
> -
> +               exynos_drm_crtc_plane_disable(plane->crtc, overlay->zpos);
>                 exynos_plane->enabled = false;
>         }
>  }
> --
> 1.8.4
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
대인기/Tizen Platform Lab(SR)/삼성전자 Oct. 15, 2013, 4:09 a.m. UTC | #7
2013/10/15 Inki Dae <inki.dae@samsung.com>:
>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> index c417c90..ba63c72 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>> @@ -26,24 +26,23 @@
>>   * exynos specific encoder structure.
>>   *
>>   * @drm_encoder: encoder object.
>> - * @manager: specific encoder has its own manager to control a hardware
>> - *     appropriately and we can access a hardware drawing on this manager.
>> + * @display: the display structure that maps to this encoder
>>   */
>>  struct exynos_drm_encoder {
>>         struct drm_crtc                 *old_crtc;
>>         struct drm_encoder              drm_encoder;
>> -       struct exynos_drm_manager       *manager;
>> +       struct exynos_drm_display       *display;
>>  };
>>
>>  static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
>>  {
>> -       struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
>> -       struct exynos_drm_display_ops *display_ops = manager->display_ops;
>> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
>> +       struct exynos_drm_display *display = exynos_encoder->display;
>>
>>         DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
>>
>> -       if (display_ops && display_ops->dpms)
>> -               display_ops->dpms(manager->ctx, mode);
>> +       if (display->ops->dpms)
>> +               display->ops->dpms(display->ctx, mode);
>
> It's good to remove apply callback. However, it seems that this patch
> has a problem that dma channel of fimd isn't enabled after dpms goes
> from off to on. So can you implement win_enable callback of fimd, and
> add it to fimd_win_resume function? We should have implemented
> win_enable callback.
>

Plz, ignore the above comments, and see the below comments.


>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> index 838c47d..f3dc808 100644
>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>> @@ -105,7 +105,6 @@ struct fimd_win_data {
>>  };
>>
>>  struct fimd_context {
>> -       struct exynos_drm_subdrv        subdrv;
>>         struct device                   *dev;
>>         struct drm_device               *drm_dev;
>>         int                             irq;
>> @@ -120,6 +119,7 @@ struct fimd_context {
>>         u32                             vidcon0;
>>         u32                             vidcon1;
>>         bool                            suspended;
>> +       int                             pipe;
>>         struct mutex                    lock;
>>         wait_queue_head_t               wait_vsync_queue;
>>         atomic_t                        wait_vsync_event;
>> @@ -169,12 +169,16 @@ static int fimd_check_mode(void *in_ctx, struct drm_display_mode *mode)
>>  }
>>
>>  static struct exynos_drm_display_ops fimd_display_ops = {
>> -       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>         .is_connected = fimd_display_is_connected,
>>         .get_panel = fimd_get_panel,
>>         .check_mode = fimd_check_mode,
>>  };
>>
>> +static struct exynos_drm_display fimd_display = {
>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>> +       .ops = &fimd_display_ops,
>> +};
>> +
>>  static void fimd_win_mode_set(void *in_ctx, struct exynos_drm_overlay *overlay)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -481,15 +485,46 @@ static void fimd_win_disable(void *in_ctx, int zpos)
>>         win_data->enabled = false;
>>  }
>>
>> -static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
>> +static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
>> +               int pipe)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>>
>>         ctx->drm_dev = drm_dev;
>> +       ctx->pipe = pipe;
>> +
>> +       /*
>> +        * enable drm irq mode.
>> +        * - with irq_enabled = 1, we can use the vblank feature.
>> +        *
>> +        * P.S. note that we wouldn't use drm irq handler but
>> +        *      just specific driver own one instead because
>> +        *      drm framework supports only one irq handler.
>> +        */
>> +       ctx->drm_dev->irq_enabled = 1;
>> +
>> +       /*
>> +        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>> +        * by drm timer once a current process gives up ownership of
>> +        * vblank event.(after drm_vblank_put function is called)
>> +        */
>> +       drm_dev->vblank_disable_allowed = 1;
>> +
>> +       /* attach this sub driver to iommu mapping if supported. */
>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>> +               drm_iommu_attach_device(ctx->drm_dev, ctx->dev);
>>
>>         return 0;
>>  }
>>
>> +static void fimd_mgr_remove(void *in_ctx)
>> +{
>> +       struct fimd_context *ctx = in_ctx;
>> +
>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>> +               drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>> +}
>> +
>>  static void fimd_dpms(void *in_ctx, int mode)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -523,24 +558,6 @@ static void fimd_dpms(void *in_ctx, int mode)
>>         mutex_unlock(&ctx->lock);
>>  }
>>
>> -static void fimd_apply(void *in_ctx)
>> -{
>> -       struct fimd_context *ctx = in_ctx;
>> -       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(ctx, i);
>> -       }
>> -
>> -       if (mgr_ops && mgr_ops->commit)
>> -               mgr_ops->commit(ctx);
>> -}
>> -
>>  static void fimd_commit(void *in_ctx)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -597,6 +614,21 @@ static void fimd_commit(void *in_ctx)
>>         writel(val, ctx->regs + VIDCON0);
>>  }
>>
>> +static void fimd_apply(void *in_ctx)
>> +{
>> +       struct fimd_context *ctx = in_ctx;
>> +       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)
>> +                       fimd_win_commit(ctx, i);

Implement fimd_win_enable function and call it at here,

                      if (win_data->enabled) {
                             fimd_win_commit(ctx,i);
                             fimd_win_enable(ctx, i);  <- here
                      }

And, the below codes, for enable overlay dma channel, should be
removed from fimd_win_commit function,

    /* wincon */
    val = readl(ctx->regs + WINCON(win));
    val |= WINCONx_ENWIN;
    writel(val, ctx->regs + WINCON(win));


And, with [PATCH v2] drm/exynos: fimd: clean up pm suspend/resume, it
must work well.

>> +       }
>> +
>> +       fimd_commit(ctx);
>> +}
>> +
>>  static int fimd_enable_vblank(void *in_ctx)
>>  {
>>         struct fimd_context *ctx = in_ctx;
>> @@ -661,6 +693,7 @@ static void fimd_wait_for_vblank(void *in_ctx)
>>
>>  static struct exynos_drm_manager_ops fimd_manager_ops = {
>>         .initialize = fimd_mgr_initialize,
>> +       .remove = fimd_mgr_remove,
>>         .dpms = fimd_dpms,
>>         .apply = fimd_apply,
>>         .commit = fimd_commit,
>> @@ -673,16 +706,13 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
>>  };
>>
>>  static struct exynos_drm_manager fimd_manager = {
>> -       .pipe           = -1,
>> -       .ops            = &fimd_manager_ops,
>> -       .display_ops    = &fimd_display_ops,
>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>> +       .ops = &fimd_manager_ops,
>>  };
>>
>>  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>  {
>>         struct fimd_context *ctx = (struct fimd_context *)dev_id;
>> -       struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
>> -       struct exynos_drm_manager *manager = subdrv->manager;
>>         u32 val;
>>
>>         val = readl(ctx->regs + VIDINTCON1);
>> @@ -692,11 +722,11 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>                 writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
>>
>>         /* check the crtc is detached already from encoder */
>> -       if (manager->pipe < 0 || !ctx->drm_dev)
>> +       if (ctx->pipe < 0 || !ctx->drm_dev)
>>                 goto out;
>>
>> -       drm_handle_vblank(ctx->drm_dev, manager->pipe);
>> -       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe);
>> +       drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>> +       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>>
>>         /* set wait vsync event to zero and wake up queue. */
>>         if (atomic_read(&ctx->wait_vsync_event)) {
>> @@ -707,39 +737,6 @@ out:
>>         return IRQ_HANDLED;
>>  }
>>
>> -static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>> -{
>> -       /*
>> -        * enable drm irq mode.
>> -        * - with irq_enabled = 1, we can use the vblank feature.
>> -        *
>> -        * P.S. note that we wouldn't use drm irq handler but
>> -        *      just specific driver own one instead because
>> -        *      drm framework supports only one irq handler.
>> -        */
>> -       drm_dev->irq_enabled = 1;
>> -
>> -       /*
>> -        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>> -        * by drm timer once a current process gives up ownership of
>> -        * vblank event.(after drm_vblank_put function is called)
>> -        */
>> -       drm_dev->vblank_disable_allowed = 1;
>> -
>> -       /* attach this sub driver to iommu mapping if supported. */
>> -       if (is_drm_iommu_supported(drm_dev))
>> -               drm_iommu_attach_device(drm_dev, dev);
>> -
>> -       return 0;
>> -}
>> -
>> -static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
>> -{
>> -       /* detach this sub driver from iommu mapping if supported. */
>> -       if (is_drm_iommu_supported(drm_dev))
>> -               drm_iommu_detach_device(drm_dev, dev);
>> -}
>> -
>>  static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev)
>>  {
>>         struct videomode *vm = &ctx->panel.vm;
>> @@ -825,9 +822,8 @@ static int fimd_clock(struct fimd_context *ctx, bool enable)
>>         return 0;
>>  }
>>
>> -static void fimd_window_suspend(struct device *dev)
>> +static void fimd_window_suspend(struct fimd_context *ctx)
>>  {
>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>         struct fimd_win_data *win_data;
>>         int i;
>>
>> @@ -839,9 +835,8 @@ static void fimd_window_suspend(struct device *dev)
>>         fimd_wait_for_vblank(ctx);
>>  }
>>
>> -static void fimd_window_resume(struct device *dev)
>> +static void fimd_window_resume(struct fimd_context *ctx)
>>  {
>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>         struct fimd_win_data *win_data;
>>         int i;
>>
>> @@ -854,7 +849,6 @@ static void fimd_window_resume(struct device *dev)
>>
>>  static int fimd_activate(struct fimd_context *ctx, bool enable)
>>  {
>> -       struct device *dev = ctx->subdrv.dev;
>>         if (enable) {
>>                 int ret;
>>
>> @@ -866,11 +860,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable)
>>
>>                 /* if vblank was enabled status, enable it again. */
>>                 if (test_and_clear_bit(0, &ctx->irq_flags))
>> -                       fimd_enable_vblank(dev);
>> +                       fimd_enable_vblank(ctx);
>>
>> -               fimd_window_resume(dev);
>> +               fimd_window_resume(ctx);
>>         } else {
>> -               fimd_window_suspend(dev);
>> +               fimd_window_suspend(ctx);
>>
>>                 fimd_clock(ctx, false);
>>                 ctx->suspended = true;
>> @@ -907,7 +901,6 @@ static int fimd_probe(struct platform_device *pdev)
>>  {
>>         struct device *dev = &pdev->dev;
>>         struct fimd_context *ctx;
>> -       struct exynos_drm_subdrv *subdrv;
>>         struct resource *res;
>>         int win;
>>         int ret = -EINVAL;
>> @@ -952,15 +945,6 @@ static int fimd_probe(struct platform_device *pdev)
>>         DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue);
>>         atomic_set(&ctx->wait_vsync_event, 0);
>>
>> -       fimd_manager.ctx = ctx;
>> -
>> -       subdrv = &ctx->subdrv;
>> -
>> -       subdrv->dev = dev;
>> -       subdrv->manager = &fimd_manager;
>> -       subdrv->probe = fimd_subdrv_probe;
>> -       subdrv->remove = fimd_subdrv_remove;
>> -
>>         mutex_init(&ctx->lock);
>>
>>         platform_set_drvdata(pdev, ctx);
>> @@ -971,7 +955,11 @@ static int fimd_probe(struct platform_device *pdev)
>>         for (win = 0; win < WINDOWS_NR; win++)
>>                 fimd_clear_win(ctx, win);
>>
>> -       exynos_drm_subdrv_register(subdrv);
>> +       fimd_manager.ctx = ctx;
>> +       exynos_drm_manager_register(&fimd_manager);
>> +
>> +       fimd_display.ctx = ctx;
>> +       exynos_drm_display_register(&fimd_display);
>>
>>         return 0;
>>  }
>> @@ -981,7 +969,8 @@ static int fimd_remove(struct platform_device *pdev)
>>         struct device *dev = &pdev->dev;
>>         struct fimd_context *ctx = platform_get_drvdata(pdev);
>>
>> -       exynos_drm_subdrv_unregister(&ctx->subdrv);
>> +       exynos_drm_display_unregister(&fimd_display);
>> +       exynos_drm_manager_unregister(&fimd_manager);
>>
>>         if (ctx->suspended)
>>                 goto out;
Sean Paul Oct. 15, 2013, 6:12 p.m. UTC | #8
On Tue, Oct 15, 2013 at 12:09 AM, Inki Dae <inki.dae@samsung.com> wrote:
> 2013/10/15 Inki Dae <inki.dae@samsung.com>:
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>> index c417c90..ba63c72 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>> @@ -26,24 +26,23 @@
>>>   * exynos specific encoder structure.
>>>   *
>>>   * @drm_encoder: encoder object.
>>> - * @manager: specific encoder has its own manager to control a hardware
>>> - *     appropriately and we can access a hardware drawing on this manager.
>>> + * @display: the display structure that maps to this encoder
>>>   */
>>>  struct exynos_drm_encoder {
>>>         struct drm_crtc                 *old_crtc;
>>>         struct drm_encoder              drm_encoder;
>>> -       struct exynos_drm_manager       *manager;
>>> +       struct exynos_drm_display       *display;
>>>  };
>>>
>>>  static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
>>>  {
>>> -       struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
>>> -       struct exynos_drm_display_ops *display_ops = manager->display_ops;
>>> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
>>> +       struct exynos_drm_display *display = exynos_encoder->display;
>>>
>>>         DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
>>>
>>> -       if (display_ops && display_ops->dpms)
>>> -               display_ops->dpms(manager->ctx, mode);
>>> +       if (display->ops->dpms)
>>> +               display->ops->dpms(display->ctx, mode);
>>
>> It's good to remove apply callback. However, it seems that this patch
>> has a problem that dma channel of fimd isn't enabled after dpms goes
>> from off to on. So can you implement win_enable callback of fimd, and
>> add it to fimd_win_resume function? We should have implemented
>> win_enable callback.
>>
>
> Plz, ignore the above comments, and see the below comments.
>
>
>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> index 838c47d..f3dc808 100644
>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>> @@ -105,7 +105,6 @@ struct fimd_win_data {
>>>  };
>>>
>>>  struct fimd_context {
>>> -       struct exynos_drm_subdrv        subdrv;
>>>         struct device                   *dev;
>>>         struct drm_device               *drm_dev;
>>>         int                             irq;
>>> @@ -120,6 +119,7 @@ struct fimd_context {
>>>         u32                             vidcon0;
>>>         u32                             vidcon1;
>>>         bool                            suspended;
>>> +       int                             pipe;
>>>         struct mutex                    lock;
>>>         wait_queue_head_t               wait_vsync_queue;
>>>         atomic_t                        wait_vsync_event;
>>> @@ -169,12 +169,16 @@ static int fimd_check_mode(void *in_ctx, struct drm_display_mode *mode)
>>>  }
>>>
>>>  static struct exynos_drm_display_ops fimd_display_ops = {
>>> -       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>>         .is_connected = fimd_display_is_connected,
>>>         .get_panel = fimd_get_panel,
>>>         .check_mode = fimd_check_mode,
>>>  };
>>>
>>> +static struct exynos_drm_display fimd_display = {
>>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>> +       .ops = &fimd_display_ops,
>>> +};
>>> +
>>>  static void fimd_win_mode_set(void *in_ctx, struct exynos_drm_overlay *overlay)
>>>  {
>>>         struct fimd_context *ctx = in_ctx;
>>> @@ -481,15 +485,46 @@ static void fimd_win_disable(void *in_ctx, int zpos)
>>>         win_data->enabled = false;
>>>  }
>>>
>>> -static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
>>> +static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
>>> +               int pipe)
>>>  {
>>>         struct fimd_context *ctx = in_ctx;
>>>
>>>         ctx->drm_dev = drm_dev;
>>> +       ctx->pipe = pipe;
>>> +
>>> +       /*
>>> +        * enable drm irq mode.
>>> +        * - with irq_enabled = 1, we can use the vblank feature.
>>> +        *
>>> +        * P.S. note that we wouldn't use drm irq handler but
>>> +        *      just specific driver own one instead because
>>> +        *      drm framework supports only one irq handler.
>>> +        */
>>> +       ctx->drm_dev->irq_enabled = 1;
>>> +
>>> +       /*
>>> +        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>>> +        * by drm timer once a current process gives up ownership of
>>> +        * vblank event.(after drm_vblank_put function is called)
>>> +        */
>>> +       drm_dev->vblank_disable_allowed = 1;
>>> +
>>> +       /* attach this sub driver to iommu mapping if supported. */
>>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>>> +               drm_iommu_attach_device(ctx->drm_dev, ctx->dev);
>>>
>>>         return 0;
>>>  }
>>>
>>> +static void fimd_mgr_remove(void *in_ctx)
>>> +{
>>> +       struct fimd_context *ctx = in_ctx;
>>> +
>>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>>> +               drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>>> +}
>>> +
>>>  static void fimd_dpms(void *in_ctx, int mode)
>>>  {
>>>         struct fimd_context *ctx = in_ctx;
>>> @@ -523,24 +558,6 @@ static void fimd_dpms(void *in_ctx, int mode)
>>>         mutex_unlock(&ctx->lock);
>>>  }
>>>
>>> -static void fimd_apply(void *in_ctx)
>>> -{
>>> -       struct fimd_context *ctx = in_ctx;
>>> -       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(ctx, i);
>>> -       }
>>> -
>>> -       if (mgr_ops && mgr_ops->commit)
>>> -               mgr_ops->commit(ctx);
>>> -}
>>> -
>>>  static void fimd_commit(void *in_ctx)
>>>  {
>>>         struct fimd_context *ctx = in_ctx;
>>> @@ -597,6 +614,21 @@ static void fimd_commit(void *in_ctx)
>>>         writel(val, ctx->regs + VIDCON0);
>>>  }
>>>
>>> +static void fimd_apply(void *in_ctx)
>>> +{
>>> +       struct fimd_context *ctx = in_ctx;
>>> +       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)
>>> +                       fimd_win_commit(ctx, i);
>
> Implement fimd_win_enable function and call it at here,
>
>                       if (win_data->enabled) {
>                              fimd_win_commit(ctx,i);
>                              fimd_win_enable(ctx, i);  <- here
>                       }
>
> And, the below codes, for enable overlay dma channel, should be
> removed from fimd_win_commit function,
>
>     /* wincon */
>     val = readl(ctx->regs + WINCON(win));
>     val |= WINCONx_ENWIN;
>     writel(val, ctx->regs + WINCON(win));
>

I'm not sure I follow why you want this done here. It seems unrelated
to this patch.

>
> And, with [PATCH v2] drm/exynos: fimd: clean up pm suspend/resume, it
> must work well.


I've got a patch to remove all of the
suspend/resume/pm_suspend/pm_resume stuff from all of the individual
drivers and do everything through dpms. This will eliminate the error
modes where the dpms state in the upper layers get out of sync with
the actual state of the hardware. It's also more consistent with other
drm drivers. I'll post it after this set.

Sean

>
>>> +       }
>>> +
>>> +       fimd_commit(ctx);
>>> +}
>>> +
>>>  static int fimd_enable_vblank(void *in_ctx)
>>>  {
>>>         struct fimd_context *ctx = in_ctx;
>>> @@ -661,6 +693,7 @@ static void fimd_wait_for_vblank(void *in_ctx)
>>>
>>>  static struct exynos_drm_manager_ops fimd_manager_ops = {
>>>         .initialize = fimd_mgr_initialize,
>>> +       .remove = fimd_mgr_remove,
>>>         .dpms = fimd_dpms,
>>>         .apply = fimd_apply,
>>>         .commit = fimd_commit,
>>> @@ -673,16 +706,13 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
>>>  };
>>>
>>>  static struct exynos_drm_manager fimd_manager = {
>>> -       .pipe           = -1,
>>> -       .ops            = &fimd_manager_ops,
>>> -       .display_ops    = &fimd_display_ops,
>>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>> +       .ops = &fimd_manager_ops,
>>>  };
>>>
>>>  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>>  {
>>>         struct fimd_context *ctx = (struct fimd_context *)dev_id;
>>> -       struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
>>> -       struct exynos_drm_manager *manager = subdrv->manager;
>>>         u32 val;
>>>
>>>         val = readl(ctx->regs + VIDINTCON1);
>>> @@ -692,11 +722,11 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>>                 writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
>>>
>>>         /* check the crtc is detached already from encoder */
>>> -       if (manager->pipe < 0 || !ctx->drm_dev)
>>> +       if (ctx->pipe < 0 || !ctx->drm_dev)
>>>                 goto out;
>>>
>>> -       drm_handle_vblank(ctx->drm_dev, manager->pipe);
>>> -       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe);
>>> +       drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>>> +       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>>>
>>>         /* set wait vsync event to zero and wake up queue. */
>>>         if (atomic_read(&ctx->wait_vsync_event)) {
>>> @@ -707,39 +737,6 @@ out:
>>>         return IRQ_HANDLED;
>>>  }
>>>
>>> -static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>>> -{
>>> -       /*
>>> -        * enable drm irq mode.
>>> -        * - with irq_enabled = 1, we can use the vblank feature.
>>> -        *
>>> -        * P.S. note that we wouldn't use drm irq handler but
>>> -        *      just specific driver own one instead because
>>> -        *      drm framework supports only one irq handler.
>>> -        */
>>> -       drm_dev->irq_enabled = 1;
>>> -
>>> -       /*
>>> -        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>>> -        * by drm timer once a current process gives up ownership of
>>> -        * vblank event.(after drm_vblank_put function is called)
>>> -        */
>>> -       drm_dev->vblank_disable_allowed = 1;
>>> -
>>> -       /* attach this sub driver to iommu mapping if supported. */
>>> -       if (is_drm_iommu_supported(drm_dev))
>>> -               drm_iommu_attach_device(drm_dev, dev);
>>> -
>>> -       return 0;
>>> -}
>>> -
>>> -static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
>>> -{
>>> -       /* detach this sub driver from iommu mapping if supported. */
>>> -       if (is_drm_iommu_supported(drm_dev))
>>> -               drm_iommu_detach_device(drm_dev, dev);
>>> -}
>>> -
>>>  static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev)
>>>  {
>>>         struct videomode *vm = &ctx->panel.vm;
>>> @@ -825,9 +822,8 @@ static int fimd_clock(struct fimd_context *ctx, bool enable)
>>>         return 0;
>>>  }
>>>
>>> -static void fimd_window_suspend(struct device *dev)
>>> +static void fimd_window_suspend(struct fimd_context *ctx)
>>>  {
>>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>>         struct fimd_win_data *win_data;
>>>         int i;
>>>
>>> @@ -839,9 +835,8 @@ static void fimd_window_suspend(struct device *dev)
>>>         fimd_wait_for_vblank(ctx);
>>>  }
>>>
>>> -static void fimd_window_resume(struct device *dev)
>>> +static void fimd_window_resume(struct fimd_context *ctx)
>>>  {
>>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>>         struct fimd_win_data *win_data;
>>>         int i;
>>>
>>> @@ -854,7 +849,6 @@ static void fimd_window_resume(struct device *dev)
>>>
>>>  static int fimd_activate(struct fimd_context *ctx, bool enable)
>>>  {
>>> -       struct device *dev = ctx->subdrv.dev;
>>>         if (enable) {
>>>                 int ret;
>>>
>>> @@ -866,11 +860,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable)
>>>
>>>                 /* if vblank was enabled status, enable it again. */
>>>                 if (test_and_clear_bit(0, &ctx->irq_flags))
>>> -                       fimd_enable_vblank(dev);
>>> +                       fimd_enable_vblank(ctx);
>>>
>>> -               fimd_window_resume(dev);
>>> +               fimd_window_resume(ctx);
>>>         } else {
>>> -               fimd_window_suspend(dev);
>>> +               fimd_window_suspend(ctx);
>>>
>>>                 fimd_clock(ctx, false);
>>>                 ctx->suspended = true;
>>> @@ -907,7 +901,6 @@ static int fimd_probe(struct platform_device *pdev)
>>>  {
>>>         struct device *dev = &pdev->dev;
>>>         struct fimd_context *ctx;
>>> -       struct exynos_drm_subdrv *subdrv;
>>>         struct resource *res;
>>>         int win;
>>>         int ret = -EINVAL;
>>> @@ -952,15 +945,6 @@ static int fimd_probe(struct platform_device *pdev)
>>>         DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue);
>>>         atomic_set(&ctx->wait_vsync_event, 0);
>>>
>>> -       fimd_manager.ctx = ctx;
>>> -
>>> -       subdrv = &ctx->subdrv;
>>> -
>>> -       subdrv->dev = dev;
>>> -       subdrv->manager = &fimd_manager;
>>> -       subdrv->probe = fimd_subdrv_probe;
>>> -       subdrv->remove = fimd_subdrv_remove;
>>> -
>>>         mutex_init(&ctx->lock);
>>>
>>>         platform_set_drvdata(pdev, ctx);
>>> @@ -971,7 +955,11 @@ static int fimd_probe(struct platform_device *pdev)
>>>         for (win = 0; win < WINDOWS_NR; win++)
>>>                 fimd_clear_win(ctx, win);
>>>
>>> -       exynos_drm_subdrv_register(subdrv);
>>> +       fimd_manager.ctx = ctx;
>>> +       exynos_drm_manager_register(&fimd_manager);
>>> +
>>> +       fimd_display.ctx = ctx;
>>> +       exynos_drm_display_register(&fimd_display);
>>>
>>>         return 0;
>>>  }
>>> @@ -981,7 +969,8 @@ static int fimd_remove(struct platform_device *pdev)
>>>         struct device *dev = &pdev->dev;
>>>         struct fimd_context *ctx = platform_get_drvdata(pdev);
>>>
>>> -       exynos_drm_subdrv_unregister(&ctx->subdrv);
>>> +       exynos_drm_display_unregister(&fimd_display);
>>> +       exynos_drm_manager_unregister(&fimd_manager);
>>>
>>>         if (ctx->suspended)
>>>                 goto out;
대인기/Tizen Platform Lab(SR)/삼성전자 Oct. 16, 2013, 2:17 a.m. UTC | #9
2013/10/16 Sean Paul <seanpaul@chromium.org>:
> On Tue, Oct 15, 2013 at 12:09 AM, Inki Dae <inki.dae@samsung.com> wrote:
>> 2013/10/15 Inki Dae <inki.dae@samsung.com>:
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>>> index c417c90..ba63c72 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
>>>> @@ -26,24 +26,23 @@
>>>>   * exynos specific encoder structure.
>>>>   *
>>>>   * @drm_encoder: encoder object.
>>>> - * @manager: specific encoder has its own manager to control a hardware
>>>> - *     appropriately and we can access a hardware drawing on this manager.
>>>> + * @display: the display structure that maps to this encoder
>>>>   */
>>>>  struct exynos_drm_encoder {
>>>>         struct drm_crtc                 *old_crtc;
>>>>         struct drm_encoder              drm_encoder;
>>>> -       struct exynos_drm_manager       *manager;
>>>> +       struct exynos_drm_display       *display;
>>>>  };
>>>>
>>>>  static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
>>>>  {
>>>> -       struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
>>>> -       struct exynos_drm_display_ops *display_ops = manager->display_ops;
>>>> +       struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
>>>> +       struct exynos_drm_display *display = exynos_encoder->display;
>>>>
>>>>         DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
>>>>
>>>> -       if (display_ops && display_ops->dpms)
>>>> -               display_ops->dpms(manager->ctx, mode);
>>>> +       if (display->ops->dpms)
>>>> +               display->ops->dpms(display->ctx, mode);
>>>
>>> It's good to remove apply callback. However, it seems that this patch
>>> has a problem that dma channel of fimd isn't enabled after dpms goes
>>> from off to on. So can you implement win_enable callback of fimd, and
>>> add it to fimd_win_resume function? We should have implemented
>>> win_enable callback.
>>>
>>
>> Plz, ignore the above comments, and see the below comments.
>>
>>
>>>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>>> index 838c47d..f3dc808 100644
>>>> --- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>>> +++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
>>>> @@ -105,7 +105,6 @@ struct fimd_win_data {
>>>>  };
>>>>
>>>>  struct fimd_context {
>>>> -       struct exynos_drm_subdrv        subdrv;
>>>>         struct device                   *dev;
>>>>         struct drm_device               *drm_dev;
>>>>         int                             irq;
>>>> @@ -120,6 +119,7 @@ struct fimd_context {
>>>>         u32                             vidcon0;
>>>>         u32                             vidcon1;
>>>>         bool                            suspended;
>>>> +       int                             pipe;
>>>>         struct mutex                    lock;
>>>>         wait_queue_head_t               wait_vsync_queue;
>>>>         atomic_t                        wait_vsync_event;
>>>> @@ -169,12 +169,16 @@ static int fimd_check_mode(void *in_ctx, struct drm_display_mode *mode)
>>>>  }
>>>>
>>>>  static struct exynos_drm_display_ops fimd_display_ops = {
>>>> -       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>>>         .is_connected = fimd_display_is_connected,
>>>>         .get_panel = fimd_get_panel,
>>>>         .check_mode = fimd_check_mode,
>>>>  };
>>>>
>>>> +static struct exynos_drm_display fimd_display = {
>>>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>>> +       .ops = &fimd_display_ops,
>>>> +};
>>>> +
>>>>  static void fimd_win_mode_set(void *in_ctx, struct exynos_drm_overlay *overlay)
>>>>  {
>>>>         struct fimd_context *ctx = in_ctx;
>>>> @@ -481,15 +485,46 @@ static void fimd_win_disable(void *in_ctx, int zpos)
>>>>         win_data->enabled = false;
>>>>  }
>>>>
>>>> -static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
>>>> +static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
>>>> +               int pipe)
>>>>  {
>>>>         struct fimd_context *ctx = in_ctx;
>>>>
>>>>         ctx->drm_dev = drm_dev;
>>>> +       ctx->pipe = pipe;
>>>> +
>>>> +       /*
>>>> +        * enable drm irq mode.
>>>> +        * - with irq_enabled = 1, we can use the vblank feature.
>>>> +        *
>>>> +        * P.S. note that we wouldn't use drm irq handler but
>>>> +        *      just specific driver own one instead because
>>>> +        *      drm framework supports only one irq handler.
>>>> +        */
>>>> +       ctx->drm_dev->irq_enabled = 1;
>>>> +
>>>> +       /*
>>>> +        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>>>> +        * by drm timer once a current process gives up ownership of
>>>> +        * vblank event.(after drm_vblank_put function is called)
>>>> +        */
>>>> +       drm_dev->vblank_disable_allowed = 1;
>>>> +
>>>> +       /* attach this sub driver to iommu mapping if supported. */
>>>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>>>> +               drm_iommu_attach_device(ctx->drm_dev, ctx->dev);
>>>>
>>>>         return 0;
>>>>  }
>>>>
>>>> +static void fimd_mgr_remove(void *in_ctx)
>>>> +{
>>>> +       struct fimd_context *ctx = in_ctx;
>>>> +
>>>> +       if (is_drm_iommu_supported(ctx->drm_dev))
>>>> +               drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
>>>> +}
>>>> +
>>>>  static void fimd_dpms(void *in_ctx, int mode)
>>>>  {
>>>>         struct fimd_context *ctx = in_ctx;
>>>> @@ -523,24 +558,6 @@ static void fimd_dpms(void *in_ctx, int mode)
>>>>         mutex_unlock(&ctx->lock);
>>>>  }
>>>>
>>>> -static void fimd_apply(void *in_ctx)
>>>> -{
>>>> -       struct fimd_context *ctx = in_ctx;
>>>> -       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(ctx, i);
>>>> -       }
>>>> -
>>>> -       if (mgr_ops && mgr_ops->commit)
>>>> -               mgr_ops->commit(ctx);
>>>> -}
>>>> -
>>>>  static void fimd_commit(void *in_ctx)
>>>>  {
>>>>         struct fimd_context *ctx = in_ctx;
>>>> @@ -597,6 +614,21 @@ static void fimd_commit(void *in_ctx)
>>>>         writel(val, ctx->regs + VIDCON0);
>>>>  }
>>>>
>>>> +static void fimd_apply(void *in_ctx)
>>>> +{
>>>> +       struct fimd_context *ctx = in_ctx;
>>>> +       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)
>>>> +                       fimd_win_commit(ctx, i);
>>
>> Implement fimd_win_enable function and call it at here,
>>
>>                       if (win_data->enabled) {
>>                              fimd_win_commit(ctx,i);
>>                              fimd_win_enable(ctx, i);  <- here
>>                       }
>>
>> And, the below codes, for enable overlay dma channel, should be
>> removed from fimd_win_commit function,
>>
>>     /* wincon */
>>     val = readl(ctx->regs + WINCON(win));
>>     val |= WINCONx_ENWIN;
>>     writel(val, ctx->regs + WINCON(win));
>>
>
> I'm not sure I follow why you want this done here. It seems unrelated
> to this patch.
>

Yes, it's unrelated to this patch. I mean additional work.

>>
>> And, with [PATCH v2] drm/exynos: fimd: clean up pm suspend/resume, it
>> must work well.
>
>
> I've got a patch to remove all of the
> suspend/resume/pm_suspend/pm_resume stuff from all of the individual
> drivers and do everything through dpms. This will eliminate the error
> modes where the dpms state in the upper layers get out of sync with
> the actual state of the hardware. It's also more consistent with other
> drm drivers. I'll post it after this set.
>

It seems like more clear  if we _can_do_ so. However, can the dpms be
called when entering sleep? And I think this way - removing all pm
related stuff from device drivers - is not reasonable, and we'd need
more discussion about that with pm people. So add the related patch
set at top of your re-factoring patch set if you want to do so.

Thanks,
Inki Dae

> Sean
>
>>
>>>> +       }
>>>> +
>>>> +       fimd_commit(ctx);
>>>> +}
>>>> +
>>>>  static int fimd_enable_vblank(void *in_ctx)
>>>>  {
>>>>         struct fimd_context *ctx = in_ctx;
>>>> @@ -661,6 +693,7 @@ static void fimd_wait_for_vblank(void *in_ctx)
>>>>
>>>>  static struct exynos_drm_manager_ops fimd_manager_ops = {
>>>>         .initialize = fimd_mgr_initialize,
>>>> +       .remove = fimd_mgr_remove,
>>>>         .dpms = fimd_dpms,
>>>>         .apply = fimd_apply,
>>>>         .commit = fimd_commit,
>>>> @@ -673,16 +706,13 @@ static struct exynos_drm_manager_ops fimd_manager_ops = {
>>>>  };
>>>>
>>>>  static struct exynos_drm_manager fimd_manager = {
>>>> -       .pipe           = -1,
>>>> -       .ops            = &fimd_manager_ops,
>>>> -       .display_ops    = &fimd_display_ops,
>>>> +       .type = EXYNOS_DISPLAY_TYPE_LCD,
>>>> +       .ops = &fimd_manager_ops,
>>>>  };
>>>>
>>>>  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>>>  {
>>>>         struct fimd_context *ctx = (struct fimd_context *)dev_id;
>>>> -       struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
>>>> -       struct exynos_drm_manager *manager = subdrv->manager;
>>>>         u32 val;
>>>>
>>>>         val = readl(ctx->regs + VIDINTCON1);
>>>> @@ -692,11 +722,11 @@ static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
>>>>                 writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
>>>>
>>>>         /* check the crtc is detached already from encoder */
>>>> -       if (manager->pipe < 0 || !ctx->drm_dev)
>>>> +       if (ctx->pipe < 0 || !ctx->drm_dev)
>>>>                 goto out;
>>>>
>>>> -       drm_handle_vblank(ctx->drm_dev, manager->pipe);
>>>> -       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe);
>>>> +       drm_handle_vblank(ctx->drm_dev, ctx->pipe);
>>>> +       exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
>>>>
>>>>         /* set wait vsync event to zero and wake up queue. */
>>>>         if (atomic_read(&ctx->wait_vsync_event)) {
>>>> @@ -707,39 +737,6 @@ out:
>>>>         return IRQ_HANDLED;
>>>>  }
>>>>
>>>> -static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
>>>> -{
>>>> -       /*
>>>> -        * enable drm irq mode.
>>>> -        * - with irq_enabled = 1, we can use the vblank feature.
>>>> -        *
>>>> -        * P.S. note that we wouldn't use drm irq handler but
>>>> -        *      just specific driver own one instead because
>>>> -        *      drm framework supports only one irq handler.
>>>> -        */
>>>> -       drm_dev->irq_enabled = 1;
>>>> -
>>>> -       /*
>>>> -        * with vblank_disable_allowed = 1, vblank interrupt will be disabled
>>>> -        * by drm timer once a current process gives up ownership of
>>>> -        * vblank event.(after drm_vblank_put function is called)
>>>> -        */
>>>> -       drm_dev->vblank_disable_allowed = 1;
>>>> -
>>>> -       /* attach this sub driver to iommu mapping if supported. */
>>>> -       if (is_drm_iommu_supported(drm_dev))
>>>> -               drm_iommu_attach_device(drm_dev, dev);
>>>> -
>>>> -       return 0;
>>>> -}
>>>> -
>>>> -static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
>>>> -{
>>>> -       /* detach this sub driver from iommu mapping if supported. */
>>>> -       if (is_drm_iommu_supported(drm_dev))
>>>> -               drm_iommu_detach_device(drm_dev, dev);
>>>> -}
>>>> -
>>>>  static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev)
>>>>  {
>>>>         struct videomode *vm = &ctx->panel.vm;
>>>> @@ -825,9 +822,8 @@ static int fimd_clock(struct fimd_context *ctx, bool enable)
>>>>         return 0;
>>>>  }
>>>>
>>>> -static void fimd_window_suspend(struct device *dev)
>>>> +static void fimd_window_suspend(struct fimd_context *ctx)
>>>>  {
>>>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>>>         struct fimd_win_data *win_data;
>>>>         int i;
>>>>
>>>> @@ -839,9 +835,8 @@ static void fimd_window_suspend(struct device *dev)
>>>>         fimd_wait_for_vblank(ctx);
>>>>  }
>>>>
>>>> -static void fimd_window_resume(struct device *dev)
>>>> +static void fimd_window_resume(struct fimd_context *ctx)
>>>>  {
>>>> -       struct fimd_context *ctx = get_fimd_context(dev);
>>>>         struct fimd_win_data *win_data;
>>>>         int i;
>>>>
>>>> @@ -854,7 +849,6 @@ static void fimd_window_resume(struct device *dev)
>>>>
>>>>  static int fimd_activate(struct fimd_context *ctx, bool enable)
>>>>  {
>>>> -       struct device *dev = ctx->subdrv.dev;
>>>>         if (enable) {
>>>>                 int ret;
>>>>
>>>> @@ -866,11 +860,11 @@ static int fimd_activate(struct fimd_context *ctx, bool enable)
>>>>
>>>>                 /* if vblank was enabled status, enable it again. */
>>>>                 if (test_and_clear_bit(0, &ctx->irq_flags))
>>>> -                       fimd_enable_vblank(dev);
>>>> +                       fimd_enable_vblank(ctx);
>>>>
>>>> -               fimd_window_resume(dev);
>>>> +               fimd_window_resume(ctx);
>>>>         } else {
>>>> -               fimd_window_suspend(dev);
>>>> +               fimd_window_suspend(ctx);
>>>>
>>>>                 fimd_clock(ctx, false);
>>>>                 ctx->suspended = true;
>>>> @@ -907,7 +901,6 @@ static int fimd_probe(struct platform_device *pdev)
>>>>  {
>>>>         struct device *dev = &pdev->dev;
>>>>         struct fimd_context *ctx;
>>>> -       struct exynos_drm_subdrv *subdrv;
>>>>         struct resource *res;
>>>>         int win;
>>>>         int ret = -EINVAL;
>>>> @@ -952,15 +945,6 @@ static int fimd_probe(struct platform_device *pdev)
>>>>         DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue);
>>>>         atomic_set(&ctx->wait_vsync_event, 0);
>>>>
>>>> -       fimd_manager.ctx = ctx;
>>>> -
>>>> -       subdrv = &ctx->subdrv;
>>>> -
>>>> -       subdrv->dev = dev;
>>>> -       subdrv->manager = &fimd_manager;
>>>> -       subdrv->probe = fimd_subdrv_probe;
>>>> -       subdrv->remove = fimd_subdrv_remove;
>>>> -
>>>>         mutex_init(&ctx->lock);
>>>>
>>>>         platform_set_drvdata(pdev, ctx);
>>>> @@ -971,7 +955,11 @@ static int fimd_probe(struct platform_device *pdev)
>>>>         for (win = 0; win < WINDOWS_NR; win++)
>>>>                 fimd_clear_win(ctx, win);
>>>>
>>>> -       exynos_drm_subdrv_register(subdrv);
>>>> +       fimd_manager.ctx = ctx;
>>>> +       exynos_drm_manager_register(&fimd_manager);
>>>> +
>>>> +       fimd_display.ctx = ctx;
>>>> +       exynos_drm_display_register(&fimd_display);
>>>>
>>>>         return 0;
>>>>  }
>>>> @@ -981,7 +969,8 @@ static int fimd_remove(struct platform_device *pdev)
>>>>         struct device *dev = &pdev->dev;
>>>>         struct fimd_context *ctx = platform_get_drvdata(pdev);
>>>>
>>>> -       exynos_drm_subdrv_unregister(&ctx->subdrv);
>>>> +       exynos_drm_display_unregister(&fimd_display);
>>>> +       exynos_drm_manager_unregister(&fimd_manager);
>>>>
>>>>         if (ctx->suspended)
>>>>                 goto out;
> _______________________________________________
> 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_connector.c b/drivers/gpu/drm/exynos/exynos_drm_connector.c
index 2a5639a..782a9e1 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_connector.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_connector.c
@@ -23,26 +23,20 @@ 
 				drm_connector)
 
 struct exynos_drm_connector {
-	struct drm_connector	drm_connector;
-	uint32_t		encoder_id;
-	struct exynos_drm_manager *manager;
+	struct drm_connector		drm_connector;
+	uint32_t			encoder_id;
+	struct exynos_drm_display	*display;
 };
 
 static int exynos_drm_connector_get_modes(struct drm_connector *connector)
 {
 	struct exynos_drm_connector *exynos_connector =
 					to_exynos_connector(connector);
-	struct exynos_drm_manager *manager = exynos_connector->manager;
-	struct exynos_drm_display_ops *display_ops = manager->display_ops;
+	struct exynos_drm_display *display = exynos_connector->display;
 	struct edid *edid = NULL;
 	unsigned int count = 0;
 	int ret;
 
-	if (!display_ops) {
-		DRM_DEBUG_KMS("display_ops is null.\n");
-		return 0;
-	}
-
 	/*
 	 * if get_edid() exists then get_edid() callback of hdmi side
 	 * is called to get edid data through i2c interface else
@@ -51,8 +45,8 @@  static int exynos_drm_connector_get_modes(struct drm_connector *connector)
 	 * P.S. in case of lcd panel, count is always 1 if success
 	 * because lcd panel has only one mode.
 	 */
-	if (display_ops->get_edid) {
-		edid = display_ops->get_edid(manager->ctx, connector);
+	if (display->ops->get_edid) {
+		edid = display->ops->get_edid(display->ctx, connector);
 		if (IS_ERR_OR_NULL(edid)) {
 			ret = PTR_ERR(edid);
 			edid = NULL;
@@ -75,8 +69,8 @@  static int exynos_drm_connector_get_modes(struct drm_connector *connector)
 			return 0;
 		}
 
-		if (display_ops->get_panel)
-			panel = display_ops->get_panel(manager->ctx);
+		if (display->ops->get_panel)
+			panel = display->ops->get_panel(display->ctx);
 		else {
 			drm_mode_destroy(connector->dev, mode);
 			return 0;
@@ -105,14 +99,13 @@  static int exynos_drm_connector_mode_valid(struct drm_connector *connector,
 {
 	struct exynos_drm_connector *exynos_connector =
 					to_exynos_connector(connector);
-	struct exynos_drm_manager *manager = exynos_connector->manager;
-	struct exynos_drm_display_ops *display_ops = manager->display_ops;
+	struct exynos_drm_display *display = exynos_connector->display;
 	int ret = MODE_BAD;
 
 	DRM_DEBUG_KMS("%s\n", __FILE__);
 
-	if (display_ops && display_ops->check_mode)
-		if (!display_ops->check_mode(manager->ctx, mode))
+	if (display->ops->check_mode)
+		if (!display->ops->check_mode(display->ctx, mode))
 			ret = MODE_OK;
 
 	return ret;
@@ -151,8 +144,7 @@  static int exynos_drm_connector_fill_modes(struct drm_connector *connector,
 {
 	struct exynos_drm_connector *exynos_connector =
 					to_exynos_connector(connector);
-	struct exynos_drm_manager *manager = exynos_connector->manager;
-	struct exynos_drm_manager_ops *ops = manager->ops;
+	struct exynos_drm_display *display = exynos_connector->display;
 	unsigned int width, height;
 
 	width = max_width;
@@ -162,8 +154,8 @@  static int exynos_drm_connector_fill_modes(struct drm_connector *connector,
 	 * if specific driver want to find desired_mode using maxmum
 	 * resolution then get max width and height from that driver.
 	 */
-	if (ops && ops->get_max_resol)
-		ops->get_max_resol(manager->ctx, &width, &height);
+	if (display->ops->get_max_resol)
+		display->ops->get_max_resol(display->ctx, &width, &height);
 
 	return drm_helper_probe_single_connector_modes(connector, width,
 							height);
@@ -175,13 +167,11 @@  exynos_drm_connector_detect(struct drm_connector *connector, bool force)
 {
 	struct exynos_drm_connector *exynos_connector =
 					to_exynos_connector(connector);
-	struct exynos_drm_manager *manager = exynos_connector->manager;
-	struct exynos_drm_display_ops *display_ops =
-					manager->display_ops;
+	struct exynos_drm_display *display = exynos_connector->display;
 	enum drm_connector_status status = connector_status_disconnected;
 
-	if (display_ops && display_ops->is_connected) {
-		if (display_ops->is_connected(manager->ctx))
+	if (display->ops->is_connected) {
+		if (display->ops->is_connected(display->ctx))
 			status = connector_status_connected;
 		else
 			status = connector_status_disconnected;
@@ -211,7 +201,7 @@  struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
 						   struct drm_encoder *encoder)
 {
 	struct exynos_drm_connector *exynos_connector;
-	struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
+	struct exynos_drm_display *display = exynos_drm_get_display(encoder);
 	struct drm_connector *connector;
 	int type;
 	int err;
@@ -222,7 +212,7 @@  struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
 
 	connector = &exynos_connector->drm_connector;
 
-	switch (manager->display_ops->type) {
+	switch (display->type) {
 	case EXYNOS_DISPLAY_TYPE_HDMI:
 		type = DRM_MODE_CONNECTOR_HDMIA;
 		connector->interlace_allowed = true;
@@ -245,7 +235,7 @@  struct drm_connector *exynos_drm_connector_create(struct drm_device *dev,
 		goto err_connector;
 
 	exynos_connector->encoder_id = encoder->base.id;
-	exynos_connector->manager = manager;
+	exynos_connector->display = display;
 	connector->dpms = DRM_MODE_DPMS_OFF;
 	connector->encoder = encoder;
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c
index 08ca4f9..12e60f2 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_core.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_core.c
@@ -16,11 +16,14 @@ 
 #include <drm/drmP.h>
 #include <drm/bridge/ptn3460.h>
 #include "exynos_drm_drv.h"
+#include "exynos_drm_crtc.h"
 #include "exynos_drm_encoder.h"
 #include "exynos_drm_connector.h"
 #include "exynos_drm_fbdev.h"
 
 static LIST_HEAD(exynos_drm_subdrv_list);
+static LIST_HEAD(exynos_drm_manager_list);
+static LIST_HEAD(exynos_drm_display_list);
 
 struct bridge_init {
 	struct i2c_client *client;
@@ -57,24 +60,28 @@  static int exynos_drm_attach_lcd_bridge(struct drm_device *dev,
 }
 
 static int exynos_drm_create_enc_conn(struct drm_device *dev,
-					struct exynos_drm_subdrv *subdrv)
+					struct exynos_drm_display *display)
 {
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
+	struct exynos_drm_manager *manager;
 	int ret;
+	unsigned long possible_crtcs = 0;
 
-	subdrv->manager->dev = subdrv->dev;
+	/* Find possible crtcs for this display */
+	list_for_each_entry(manager, &exynos_drm_manager_list, list)
+		if (manager->type == display->type)
+			possible_crtcs |= 1 << manager->pipe;
 
 	/* create and initialize a encoder for this sub driver. */
-	encoder = exynos_drm_encoder_create(dev, subdrv->manager,
-			(1 << MAX_CRTC) - 1);
+	encoder = exynos_drm_encoder_create(dev, display, possible_crtcs);
 	if (!encoder) {
 		DRM_ERROR("failed to create encoder\n");
 		return -EFAULT;
 	}
-	subdrv->encoder = encoder;
+	display->encoder = encoder;
 
-	if (subdrv->manager->display_ops->type == EXYNOS_DISPLAY_TYPE_LCD) {
+	if (display->type == EXYNOS_DISPLAY_TYPE_LCD) {
 		ret = exynos_drm_attach_lcd_bridge(dev, encoder);
 		if (ret)
 			return 0;
@@ -91,7 +98,7 @@  static int exynos_drm_create_enc_conn(struct drm_device *dev,
 		goto err_destroy_encoder;
 	}
 
-	subdrv->connector = connector;
+	display->connector = connector;
 
 	return 0;
 
@@ -100,21 +107,6 @@  err_destroy_encoder:
 	return ret;
 }
 
-static void exynos_drm_destroy_enc_conn(struct exynos_drm_subdrv *subdrv)
-{
-	if (subdrv->encoder) {
-		struct drm_encoder *encoder = subdrv->encoder;
-		encoder->funcs->destroy(encoder);
-		subdrv->encoder = NULL;
-	}
-
-	if (subdrv->connector) {
-		struct drm_connector *connector = subdrv->connector;
-		connector->funcs->destroy(connector);
-		subdrv->connector = NULL;
-	}
-}
-
 static int exynos_drm_subdrv_probe(struct drm_device *dev,
 					struct exynos_drm_subdrv *subdrv)
 {
@@ -146,10 +138,98 @@  static void exynos_drm_subdrv_remove(struct drm_device *dev,
 		subdrv->remove(dev, subdrv->dev);
 }
 
+int exynos_drm_initialize_managers(struct drm_device *dev)
+{
+	struct exynos_drm_manager *manager, *n;
+	int ret, pipe = 0;
+
+	list_for_each_entry(manager, &exynos_drm_manager_list, list) {
+		if (manager->ops->initialize) {
+			ret = manager->ops->initialize(manager->ctx, dev, pipe);
+			if (ret) {
+				DRM_ERROR("Mgr init [%d] failed with %d\n",
+						manager->type, ret);
+				goto err;
+			}
+		}
+
+		manager->drm_dev = dev;
+		manager->pipe = pipe++;
+
+		ret = exynos_drm_crtc_create(manager);
+		if (ret) {
+			DRM_ERROR("CRTC create [%d] failed with %d\n",
+					manager->type, ret);
+			goto err;
+		}
+	}
+	return 0;
+
+err:
+	list_for_each_entry_safe(manager, n, &exynos_drm_manager_list, list) {
+		if (pipe-- > 0)
+			exynos_drm_manager_unregister(manager);
+		else
+			list_del(&manager->list);
+	}
+	return ret;
+}
+
+void exynos_drm_remove_managers(struct drm_device *dev)
+{
+	struct exynos_drm_manager *manager, *n;
+
+	list_for_each_entry_safe(manager, n, &exynos_drm_manager_list, list)
+		exynos_drm_manager_unregister(manager);
+}
+
+int exynos_drm_initialize_displays(struct drm_device *dev)
+{
+	struct exynos_drm_display *display, *n;
+	int ret, initialized = 0;
+
+	list_for_each_entry(display, &exynos_drm_display_list, list) {
+		if (display->ops->initialize) {
+			ret = display->ops->initialize(display->ctx, dev);
+			if (ret) {
+				DRM_ERROR("Display init [%d] failed with %d\n",
+						display->type, ret);
+				goto err;
+			}
+		}
+
+		initialized++;
+
+		ret = exynos_drm_create_enc_conn(dev, display);
+		if (ret) {
+			DRM_ERROR("Encoder create [%d] failed with %d\n",
+					display->type, ret);
+			goto err;
+		}
+	}
+	return 0;
+
+err:
+	list_for_each_entry_safe(display, n, &exynos_drm_display_list, list) {
+		if (initialized-- > 0)
+			exynos_drm_display_unregister(display);
+		else
+			list_del(&display->list);
+	}
+	return ret;
+}
+
+void exynos_drm_remove_displays(struct drm_device *dev)
+{
+	struct exynos_drm_display *display, *n;
+
+	list_for_each_entry_safe(display, n, &exynos_drm_display_list, list)
+		exynos_drm_display_unregister(display);
+}
+
 int exynos_drm_device_register(struct drm_device *dev)
 {
 	struct exynos_drm_subdrv *subdrv, *n;
-	unsigned int fine_cnt = 0;
 	int err;
 
 	if (!dev)
@@ -162,30 +242,8 @@  int exynos_drm_device_register(struct drm_device *dev)
 			list_del(&subdrv->list);
 			continue;
 		}
-
-		/*
-		 * if manager is null then it means that this sub driver
-		 * doesn't need encoder and connector.
-		 */
-		if (!subdrv->manager) {
-			fine_cnt++;
-			continue;
-		}
-
-		err = exynos_drm_create_enc_conn(dev, subdrv);
-		if (err) {
-			DRM_DEBUG("failed to create encoder and connector.\n");
-			exynos_drm_subdrv_remove(dev, subdrv);
-			list_del(&subdrv->list);
-			continue;
-		}
-
-		fine_cnt++;
 	}
 
-	if (!fine_cnt)
-		return -EINVAL;
-
 	return 0;
 }
 EXPORT_SYMBOL_GPL(exynos_drm_device_register);
@@ -201,13 +259,44 @@  int exynos_drm_device_unregister(struct drm_device *dev)
 
 	list_for_each_entry(subdrv, &exynos_drm_subdrv_list, list) {
 		exynos_drm_subdrv_remove(dev, subdrv);
-		exynos_drm_destroy_enc_conn(subdrv);
 	}
 
 	return 0;
 }
 EXPORT_SYMBOL_GPL(exynos_drm_device_unregister);
 
+int exynos_drm_manager_register(struct exynos_drm_manager *manager)
+{
+	BUG_ON(!manager->ops);
+	list_add_tail(&manager->list, &exynos_drm_manager_list);
+	return 0;
+}
+
+int exynos_drm_manager_unregister(struct exynos_drm_manager *manager)
+{
+	if (manager->ops->remove)
+		manager->ops->remove(manager->ctx);
+
+	list_del(&manager->list);
+	return 0;
+}
+
+int exynos_drm_display_register(struct exynos_drm_display *display)
+{
+	BUG_ON(!display->ops);
+	list_add_tail(&display->list, &exynos_drm_display_list);
+	return 0;
+}
+
+int exynos_drm_display_unregister(struct exynos_drm_display *display)
+{
+	if (display->ops->remove)
+		display->ops->remove(display->ctx);
+
+	list_del(&display->list);
+	return 0;
+}
+
 int exynos_drm_subdrv_register(struct exynos_drm_subdrv *subdrv)
 {
 	if (!subdrv)
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.c b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
index ebc0150..9f6ada4 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.c
@@ -33,6 +33,7 @@  enum exynos_crtc_mode {
  *
  * @drm_crtc: crtc object.
  * @drm_plane: pointer of private plane object for this crtc
+ * @manager: the manager associated with this crtc
  * @pipe: a crtc index created at load() with a new crtc object creation
  *	and the crtc object would be set to private->crtc array
  *	to get a crtc object corresponding to this pipe from private->crtc
@@ -46,6 +47,7 @@  enum exynos_crtc_mode {
 struct exynos_drm_crtc {
 	struct drm_crtc			drm_crtc;
 	struct drm_plane		*plane;
+	struct exynos_drm_manager	*manager;
 	unsigned int			pipe;
 	unsigned int			dpms;
 	enum exynos_crtc_mode		mode;
@@ -56,6 +58,7 @@  struct exynos_drm_crtc {
 static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+	struct exynos_drm_manager *manager = exynos_crtc->manager;
 
 	DRM_DEBUG_KMS("crtc[%d] mode[%d]\n", crtc->base.id, mode);
 
@@ -71,7 +74,9 @@  static void exynos_drm_crtc_dpms(struct drm_crtc *crtc, int mode)
 		drm_vblank_off(crtc->dev, exynos_crtc->pipe);
 	}
 
-	exynos_drm_fn_encoder(crtc, &mode, exynos_drm_encoder_crtc_dpms);
+	if (manager->ops->dpms)
+		manager->ops->dpms(manager->ctx, mode);
+
 	exynos_crtc->dpms = mode;
 }
 
@@ -83,9 +88,15 @@  static void exynos_drm_crtc_prepare(struct drm_crtc *crtc)
 static void exynos_drm_crtc_commit(struct drm_crtc *crtc)
 {
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(crtc);
+	struct exynos_drm_manager *manager = exynos_crtc->manager;
 
 	exynos_drm_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
+
 	exynos_plane_commit(exynos_crtc->plane);
+
+	if (manager->ops->commit)
+		manager->ops->commit(manager->ctx);
+
 	exynos_plane_dpms(exynos_crtc->plane, DRM_MODE_DPMS_ON);
 }
 
@@ -107,7 +118,6 @@  exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	struct drm_plane *plane = exynos_crtc->plane;
 	unsigned int crtc_w;
 	unsigned int crtc_h;
-	int pipe = exynos_crtc->pipe;
 	int ret;
 
 	/*
@@ -127,8 +137,6 @@  exynos_drm_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
 	plane->crtc = crtc;
 	plane->fb = crtc->fb;
 
-	exynos_drm_fn_encoder(crtc, &pipe, exynos_drm_encoder_crtc_pipe);
-
 	return 0;
 }
 
@@ -318,21 +326,24 @@  static void exynos_drm_crtc_attach_mode_property(struct drm_crtc *crtc)
 	drm_object_attach_property(&crtc->base, prop, 0);
 }
 
-int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr)
+int exynos_drm_crtc_create(struct exynos_drm_manager *manager)
 {
 	struct exynos_drm_crtc *exynos_crtc;
-	struct exynos_drm_private *private = dev->dev_private;
+	struct exynos_drm_private *private = manager->drm_dev->dev_private;
 	struct drm_crtc *crtc;
 
 	exynos_crtc = kzalloc(sizeof(*exynos_crtc), GFP_KERNEL);
 	if (!exynos_crtc)
 		return -ENOMEM;
 
-	exynos_crtc->pipe = nr;
-	exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
 	init_waitqueue_head(&exynos_crtc->pending_flip_queue);
 	atomic_set(&exynos_crtc->pending_flip, 0);
-	exynos_crtc->plane = exynos_plane_init(dev, 1 << nr, true);
+
+	exynos_crtc->dpms = DRM_MODE_DPMS_OFF;
+	exynos_crtc->manager = manager;
+	exynos_crtc->pipe = manager->pipe;
+	exynos_crtc->plane = exynos_plane_init(manager->drm_dev,
+				1 << manager->pipe, true);
 	if (!exynos_crtc->plane) {
 		kfree(exynos_crtc);
 		return -ENOMEM;
@@ -340,9 +351,9 @@  int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr)
 
 	crtc = &exynos_crtc->drm_crtc;
 
-	private->crtc[nr] = crtc;
+	private->crtc[manager->pipe] = crtc;
 
-	drm_crtc_init(dev, crtc, &exynos_crtc_funcs);
+	drm_crtc_init(manager->drm_dev, crtc, &exynos_crtc_funcs);
 	drm_crtc_helper_add(crtc, &exynos_crtc_helper_funcs);
 
 	exynos_drm_crtc_attach_mode_property(crtc);
@@ -350,39 +361,41 @@  int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr)
 	return 0;
 }
 
-int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int crtc)
+int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe)
 {
 	struct exynos_drm_private *private = dev->dev_private;
 	struct exynos_drm_crtc *exynos_crtc =
-		to_exynos_crtc(private->crtc[crtc]);
+		to_exynos_crtc(private->crtc[pipe]);
+	struct exynos_drm_manager *manager = exynos_crtc->manager;
 
 	if (exynos_crtc->dpms != DRM_MODE_DPMS_ON)
 		return -EPERM;
 
-	exynos_drm_fn_encoder(private->crtc[crtc], &crtc,
-			exynos_drm_enable_vblank);
+	if (manager->ops->enable_vblank)
+		manager->ops->enable_vblank(manager->ctx);
 
 	return 0;
 }
 
-void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int crtc)
+void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe)
 {
 	struct exynos_drm_private *private = dev->dev_private;
 	struct exynos_drm_crtc *exynos_crtc =
-		to_exynos_crtc(private->crtc[crtc]);
+		to_exynos_crtc(private->crtc[pipe]);
+	struct exynos_drm_manager *manager = exynos_crtc->manager;
 
 	if (exynos_crtc->dpms != DRM_MODE_DPMS_ON)
 		return;
 
-	exynos_drm_fn_encoder(private->crtc[crtc], &crtc,
-			exynos_drm_disable_vblank);
+	if (manager->ops->disable_vblank)
+		manager->ops->disable_vblank(manager->ctx);
 }
 
-void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int crtc)
+void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe)
 {
 	struct exynos_drm_private *dev_priv = dev->dev_private;
 	struct drm_pending_vblank_event *e, *t;
-	struct drm_crtc *drm_crtc = dev_priv->crtc[crtc];
+	struct drm_crtc *drm_crtc = dev_priv->crtc[pipe];
 	struct exynos_drm_crtc *exynos_crtc = to_exynos_crtc(drm_crtc);
 	unsigned long flags;
 
@@ -391,15 +404,71 @@  void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int crtc)
 	list_for_each_entry_safe(e, t, &dev_priv->pageflip_event_list,
 			base.link) {
 		/* if event's pipe isn't same as crtc then ignore it. */
-		if (crtc != e->pipe)
+		if (pipe != e->pipe)
 			continue;
 
 		list_del(&e->base.link);
 		drm_send_vblank_event(dev, -1, e);
-		drm_vblank_put(dev, crtc);
+		drm_vblank_put(dev, pipe);
 		atomic_set(&exynos_crtc->pending_flip, 0);
 		wake_up(&exynos_crtc->pending_flip_queue);
 	}
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
+
+void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc,
+			struct exynos_drm_overlay *overlay)
+{
+	struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager;
+
+	if (manager->ops->win_mode_set)
+		manager->ops->win_mode_set(manager->ctx, overlay);
+}
+
+void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos)
+{
+	struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager;
+
+	if (manager->ops->win_commit)
+		manager->ops->win_commit(manager->ctx, zpos);
+}
+
+void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos)
+{
+	struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager;
+
+	if (manager->ops->win_enable)
+		manager->ops->win_enable(manager->ctx, zpos);
+}
+
+void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos)
+{
+	struct exynos_drm_manager *manager = to_exynos_crtc(crtc)->manager;
+
+	if (manager->ops->win_disable)
+		manager->ops->win_disable(manager->ctx, zpos);
+}
+
+void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb)
+{
+	struct exynos_drm_manager *manager;
+	struct drm_device *dev = fb->dev;
+	struct drm_crtc *crtc;
+
+	/*
+	 * make sure that overlay data are updated to real hardware
+	 * for all encoders.
+	 */
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head) {
+		manager = to_exynos_crtc(crtc)->manager;
+
+		/*
+		 * wait for vblank interrupt
+		 * - this makes sure that overlay data are updated to
+		 *	real hardware.
+		 */
+		if (manager->ops->wait_for_vblank)
+			manager->ops->wait_for_vblank(manager->ctx);
+	}
+}
diff --git a/drivers/gpu/drm/exynos/exynos_drm_crtc.h b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
index 3e197e6..c27b66c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_crtc.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_crtc.h
@@ -15,9 +15,21 @@ 
 #ifndef _EXYNOS_DRM_CRTC_H_
 #define _EXYNOS_DRM_CRTC_H_
 
-int exynos_drm_crtc_create(struct drm_device *dev, unsigned int nr);
-int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int crtc);
-void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int crtc);
-void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int crtc);
+struct drm_device;
+struct drm_crtc;
+struct exynos_drm_manager;
+struct exynos_drm_overlay;
+
+int exynos_drm_crtc_create(struct exynos_drm_manager *manager);
+int exynos_drm_crtc_enable_vblank(struct drm_device *dev, int pipe);
+void exynos_drm_crtc_disable_vblank(struct drm_device *dev, int pipe);
+void exynos_drm_crtc_finish_pageflip(struct drm_device *dev, int pipe);
+void exynos_drm_crtc_complete_scanout(struct drm_framebuffer *fb);
+
+void exynos_drm_crtc_plane_mode_set(struct drm_crtc *crtc,
+			struct exynos_drm_overlay *overlay);
+void exynos_drm_crtc_plane_commit(struct drm_crtc *crtc, int zpos);
+void exynos_drm_crtc_plane_enable(struct drm_crtc *crtc, int zpos);
+void exynos_drm_crtc_plane_disable(struct drm_crtc *crtc, int zpos);
 
 #endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 617748e..250903c 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -72,15 +72,9 @@  static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 
 	exynos_drm_mode_config_init(dev);
 
-	/*
-	 * EXYNOS4 is enough to have two CRTCs and each crtc would be used
-	 * without dependency of hardware.
-	 */
-	for (nr = 0; nr < MAX_CRTC; nr++) {
-		ret = exynos_drm_crtc_create(dev, nr);
-		if (ret)
-			goto err_release_iommu_mapping;
-	}
+	ret = exynos_drm_initialize_managers(dev);
+	if (ret)
+		goto err_mode_config_cleanup;
 
 	for (nr = 0; nr < MAX_PLANE; nr++) {
 		struct drm_plane *plane;
@@ -88,12 +82,16 @@  static int exynos_drm_load(struct drm_device *dev, unsigned long flags)
 
 		plane = exynos_plane_init(dev, possible_crtcs, false);
 		if (!plane)
-			goto err_release_iommu_mapping;
+			goto err_manager_cleanup;
 	}
 
+	ret = exynos_drm_initialize_displays(dev);
+	if (ret)
+		goto err_manager_cleanup;
+
 	ret = drm_vblank_init(dev, MAX_CRTC);
 	if (ret)
-		goto err_release_iommu_mapping;
+		goto err_display_cleanup;
 
 	/*
 	 * probe sub drivers such as display controller and hdmi driver,
@@ -125,7 +123,12 @@  err_drm_device:
 	exynos_drm_device_unregister(dev);
 err_vblank:
 	drm_vblank_cleanup(dev);
-err_release_iommu_mapping:
+err_display_cleanup:
+	exynos_drm_remove_displays(dev);
+err_manager_cleanup:
+	exynos_drm_remove_managers(dev);
+err_mode_config_cleanup:
+	drm_mode_config_cleanup(dev);
 	drm_release_iommu_mapping(dev);
 err_crtc:
 	drm_mode_config_cleanup(dev);
@@ -140,6 +143,8 @@  static int exynos_drm_unload(struct drm_device *dev)
 	exynos_drm_device_unregister(dev);
 	drm_vblank_cleanup(dev);
 	drm_kms_helper_poll_fini(dev);
+	exynos_drm_remove_displays(dev);
+	exynos_drm_remove_managers(dev);
 	drm_mode_config_cleanup(dev);
 
 	drm_release_iommu_mapping(dev);
diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.h b/drivers/gpu/drm/exynos/exynos_drm_drv.h
index be3d352..e82234e 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.h
@@ -122,34 +122,62 @@  struct exynos_drm_overlay {
  * Exynos DRM Display Structure.
  *	- this structure is common to analog tv, digital tv and lcd panel.
  *
- * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI.
  * @initialize: initializes the display with drm_dev
+ * @remove: cleans up the display for removal
  * @is_connected: check for that display is connected or not.
+ * @get_max_resol: get maximum resolution to specific hardware.
  * @get_edid: get edid modes from display driver.
  * @get_panel: get panel object from display driver.
+ * @mode_fixup: fix mode data comparing to hw specific display mode.
+ * @mode_set: convert drm_display_mode to hw specific display mode and
+ *	      would be called by encoder->mode_set().
  * @check_mode: check if mode is valid or not.
  * @dpms: display device on or off.
+ * @commit: apply changes to hw
  */
 struct exynos_drm_display_ops {
-	enum exynos_drm_output_type type;
 	int (*initialize)(void *ctx, struct drm_device *drm_dev);
+	void (*remove)(void *ctx);
 	bool (*is_connected)(void *ctx);
+	void (*get_max_resol)(void *ctx, unsigned int *width,
+				unsigned int *height);
 	struct edid *(*get_edid)(void *ctx, struct drm_connector *connector);
 	void *(*get_panel)(void *ctx);
+	void (*mode_fixup)(void *ctx, struct drm_connector *connector,
+				const struct drm_display_mode *mode,
+				struct drm_display_mode *adjusted_mode);
+	void (*mode_set)(void *ctx, struct drm_display_mode *mode);
 	int (*check_mode)(void *ctx, struct drm_display_mode *mode);
 	void (*dpms)(void *ctx, int mode);
+	void (*commit)(void *ctx);
+};
+
+/*
+ * Exynos drm display structure, maps 1:1 with an encoder/connector
+ *
+ * @list: the list entry for this manager
+ * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI.
+ * @encoder: encoder object this display maps to
+ * @connector: connector object this display maps to
+ * @ops: pointer to callbacks for exynos drm specific functionality
+ * @ctx: A pointer to the display's implementation specific context
+ */
+struct exynos_drm_display {
+	struct list_head list;
+	enum exynos_drm_output_type type;
+	struct drm_encoder *encoder;
+	struct drm_connector *connector;
+	struct exynos_drm_display_ops *ops;
+	void *ctx;
 };
 
 /*
  * Exynos drm manager ops
  *
  * @initialize: initializes the manager with drm_dev
+ * @remove: cleans up the manager for removal
  * @dpms: control device power.
  * @apply: set timing, vblank and overlay data to registers.
- * @mode_fixup: fix mode data comparing to hw specific display mode.
- * @mode_set: convert drm_display_mode to hw specific display mode and
- *	      would be called by encoder->mode_set().
- * @get_max_resol: get maximum resolution to specific hardware.
  * @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.
@@ -161,15 +189,10 @@  struct exynos_drm_display_ops {
  * @win_disable: disable hardware specific overlay.
  */
 struct exynos_drm_manager_ops {
-	int (*initialize)(void *ctx, struct drm_device *drm_dev);
+	int (*initialize)(void *ctx, struct drm_device *drm_dev, int pipe);
+	void (*remove)(void *ctx);
 	void (*dpms)(void *ctx, int mode);
 	void (*apply)(void *ctx);
-	void (*mode_fixup)(void *ctx, struct drm_connector *connector,
-				const struct drm_display_mode *mode,
-				struct drm_display_mode *adjusted_mode);
-	void (*mode_set)(void *ctx, void *mode);
-	void (*get_max_resol)(void *ctx, unsigned int *width,
-				unsigned int *height);
 	void (*commit)(void *ctx);
 	int (*enable_vblank)(void *ctx);
 	void (*disable_vblank)(void *ctx);
@@ -181,25 +204,21 @@  struct exynos_drm_manager_ops {
 };
 
 /*
- * Exynos drm common manager structure.
+ * Exynos drm common manager structure, maps 1:1 with a crtc
  *
- * @dev: pointer to device object for subdrv device driver.
- *	sub drivers such as display controller or hdmi driver,
- *	have their own device object.
- * @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.
- * @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
- *	analog tv, digital tv and lcd panel and also get timing data for them.
+ * @list: the list entry for this manager
+ * @type: one of EXYNOS_DISPLAY_TYPE_LCD and HDMI.
+ * @drm_dev: pointer to the drm device
+ * @pipe: the pipe number for this crtc/manager
+ * @ops: pointer to callbacks for exynos drm specific functionality
  * @ctx: A pointer to the manager's implementation specific context
  */
 struct exynos_drm_manager {
-	struct device *dev;
+	struct list_head list;
+	enum exynos_drm_output_type type;
+	struct drm_device *drm_dev;
 	int pipe;
 	struct exynos_drm_manager_ops *ops;
-	struct exynos_drm_display_ops *display_ops;
 	void *ctx;
 };
 
@@ -264,14 +283,11 @@  struct exynos_drm_private {
  *	by probe callback.
  * @open: this would be called with drm device file open.
  * @close: this would be called with drm device file close.
- * @encoder: encoder object owned by this sub driver.
- * @connector: connector object owned by this sub driver.
  */
 struct exynos_drm_subdrv {
 	struct list_head list;
 	struct device *dev;
 	struct drm_device *drm_dev;
-	struct exynos_drm_manager *manager;
 
 	int (*probe)(struct drm_device *drm_dev, struct device *dev);
 	void (*remove)(struct drm_device *drm_dev, struct device *dev);
@@ -279,9 +295,6 @@  struct exynos_drm_subdrv {
 			struct drm_file *file);
 	void (*close)(struct drm_device *drm_dev, struct device *dev,
 			struct drm_file *file);
-
-	struct drm_encoder *encoder;
-	struct drm_connector *connector;
 };
 
 /*
@@ -296,6 +309,16 @@  int exynos_drm_device_register(struct drm_device *dev);
  */
 int exynos_drm_device_unregister(struct drm_device *dev);
 
+int exynos_drm_initialize_managers(struct drm_device *dev);
+void exynos_drm_remove_managers(struct drm_device *dev);
+int exynos_drm_initialize_displays(struct drm_device *dev);
+void exynos_drm_remove_displays(struct drm_device *dev);
+
+int exynos_drm_manager_register(struct exynos_drm_manager *manager);
+int exynos_drm_manager_unregister(struct exynos_drm_manager *manager);
+int exynos_drm_display_register(struct exynos_drm_display *display);
+int exynos_drm_display_unregister(struct exynos_drm_display *display);
+
 /*
  * this function would be called by sub drivers such as display controller
  * or hdmi driver to register this sub driver object to exynos drm driver
diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.c b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
index c417c90..ba63c72 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_encoder.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.c
@@ -26,24 +26,23 @@ 
  * exynos specific encoder structure.
  *
  * @drm_encoder: encoder object.
- * @manager: specific encoder has its own manager to control a hardware
- *	appropriately and we can access a hardware drawing on this manager.
+ * @display: the display structure that maps to this encoder
  */
 struct exynos_drm_encoder {
 	struct drm_crtc			*old_crtc;
 	struct drm_encoder		drm_encoder;
-	struct exynos_drm_manager	*manager;
+	struct exynos_drm_display	*display;
 };
 
 static void exynos_drm_encoder_dpms(struct drm_encoder *encoder, int mode)
 {
-	struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
-	struct exynos_drm_display_ops *display_ops = manager->display_ops;
+	struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
+	struct exynos_drm_display *display = exynos_encoder->display;
 
 	DRM_DEBUG_KMS("encoder dpms: %d\n", mode);
 
-	if (display_ops && display_ops->dpms)
-		display_ops->dpms(manager->ctx, mode);
+	if (display->ops->dpms)
+		display->ops->dpms(display->ctx, mode);
 }
 
 static bool
@@ -52,15 +51,17 @@  exynos_drm_encoder_mode_fixup(struct drm_encoder *encoder,
 			       struct drm_display_mode *adjusted_mode)
 {
 	struct drm_device *dev = encoder->dev;
+	struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
+	struct exynos_drm_display *display = exynos_encoder->display;
 	struct drm_connector *connector;
-	struct exynos_drm_manager *manager = exynos_drm_get_manager(encoder);
-	struct exynos_drm_manager_ops *manager_ops = manager->ops;
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		if (connector->encoder == encoder)
-			if (manager_ops && manager_ops->mode_fixup)
-				manager_ops->mode_fixup(manager->ctx, connector,
-							mode, adjusted_mode);
+		if (connector->encoder != encoder)
+			continue;
+
+		if (display->ops->mode_fixup)
+			display->ops->mode_fixup(display->ctx, connector, mode,
+					adjusted_mode);
 	}
 
 	return true;
@@ -102,8 +103,7 @@  static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder,
 {
 	struct drm_device *dev = encoder->dev;
 	struct drm_connector *connector;
-	struct exynos_drm_manager *manager;
-	struct exynos_drm_manager_ops *manager_ops;
+	struct exynos_drm_display *display;
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		if (connector->encoder == encoder) {
@@ -123,11 +123,10 @@  static void exynos_drm_encoder_mode_set(struct drm_encoder *encoder,
 						encoder->crtc);
 			}
 
-			manager = exynos_drm_get_manager(encoder);
-			manager_ops = manager->ops;
+			display = exynos_encoder->display;
 
-			if (manager_ops && manager_ops->mode_set)
-				manager_ops->mode_set(manager->ctx,
+			if (display->ops->mode_set)
+				display->ops->mode_set(display->ctx,
 							adjusted_mode);
 
 			exynos_encoder->old_crtc = encoder->crtc;
@@ -143,39 +142,15 @@  static void exynos_drm_encoder_prepare(struct drm_encoder *encoder)
 static void exynos_drm_encoder_commit(struct drm_encoder *encoder)
 {
 	struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
-	struct exynos_drm_manager *manager = exynos_encoder->manager;
-	struct exynos_drm_manager_ops *manager_ops = manager->ops;
-
-	if (manager_ops && manager_ops->commit)
-		manager_ops->commit(manager->ctx);
-}
+	struct exynos_drm_display *display = exynos_encoder->display;
 
-void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb)
-{
-	struct exynos_drm_encoder *exynos_encoder;
-	struct exynos_drm_manager_ops *ops;
-	struct drm_device *dev = fb->dev;
-	struct drm_encoder *encoder;
+	if (display->ops->dpms)
+		display->ops->dpms(display->ctx, DRM_MODE_DPMS_ON);
 
-	/*
-	 * make sure that overlay data are updated to real hardware
-	 * for all encoders.
-	 */
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
-		exynos_encoder = to_exynos_encoder(encoder);
-		ops = exynos_encoder->manager->ops;
-
-		/*
-		 * wait for vblank interrupt
-		 * - this makes sure that overlay data are updated to
-		 *	real hardware.
-		 */
-		if (ops->wait_for_vblank)
-			ops->wait_for_vblank(exynos_encoder->manager->dev);
-	}
+	if (display->ops->commit)
+		display->ops->commit(display->ctx);
 }
 
-
 static void exynos_drm_encoder_disable(struct drm_encoder *encoder)
 {
 	struct drm_plane *plane;
@@ -201,10 +176,7 @@  static struct drm_encoder_helper_funcs exynos_encoder_helper_funcs = {
 
 static void exynos_drm_encoder_destroy(struct drm_encoder *encoder)
 {
-	struct exynos_drm_encoder *exynos_encoder =
-		to_exynos_encoder(encoder);
-
-	exynos_encoder->manager->pipe = -1;
+	struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
 
 	drm_encoder_cleanup(encoder);
 	kfree(exynos_encoder);
@@ -219,13 +191,12 @@  static unsigned int exynos_drm_encoder_clones(struct drm_encoder *encoder)
 	struct drm_encoder *clone;
 	struct drm_device *dev = encoder->dev;
 	struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
-	struct exynos_drm_display_ops *display_ops =
-				exynos_encoder->manager->display_ops;
+	struct exynos_drm_display *display = exynos_encoder->display;
 	unsigned int clone_mask = 0;
 	int cnt = 0;
 
 	list_for_each_entry(clone, &dev->mode_config.encoder_list, head) {
-		switch (display_ops->type) {
+		switch (display->type) {
 		case EXYNOS_DISPLAY_TYPE_LCD:
 		case EXYNOS_DISPLAY_TYPE_HDMI:
 		case EXYNOS_DISPLAY_TYPE_VIDI:
@@ -249,24 +220,20 @@  void exynos_drm_encoder_setup(struct drm_device *dev)
 
 struct drm_encoder *
 exynos_drm_encoder_create(struct drm_device *dev,
-			   struct exynos_drm_manager *manager,
+			   struct exynos_drm_display *display,
 			   unsigned long possible_crtcs)
 {
 	struct drm_encoder *encoder;
 	struct exynos_drm_encoder *exynos_encoder;
-	int ret;
 
-	if (!manager || !possible_crtcs)
-		return NULL;
-
-	if (!manager->dev)
+	if (!possible_crtcs)
 		return NULL;
 
 	exynos_encoder = kzalloc(sizeof(*exynos_encoder), GFP_KERNEL);
 	if (!exynos_encoder)
 		return NULL;
 
-	exynos_encoder->manager = manager;
+	exynos_encoder->display = display;
 	encoder = &exynos_encoder->drm_encoder;
 	encoder->possible_crtcs = possible_crtcs;
 
@@ -277,174 +244,12 @@  exynos_drm_encoder_create(struct drm_device *dev,
 
 	drm_encoder_helper_add(encoder, &exynos_encoder_helper_funcs);
 
-	if (manager->ops && manager->ops->initialize) {
-		ret = manager->ops->initialize(manager->ctx, dev);
-		if (ret) {
-			DRM_ERROR("Manager initialize failed %d\n", ret);
-			goto error;
-		}
-	}
-
-	if (manager->display_ops && manager->display_ops->initialize) {
-		ret = manager->display_ops->initialize(manager->ctx, dev);
-		if (ret) {
-			DRM_ERROR("Display initialize failed %d\n", ret);
-			goto error;
-		}
-	}
-
 	DRM_DEBUG_KMS("encoder has been created\n");
 
 	return encoder;
-
-error:
-	exynos_drm_encoder_destroy(&exynos_encoder->drm_encoder);
-	return NULL;
 }
 
-struct exynos_drm_manager *exynos_drm_get_manager(struct drm_encoder *encoder)
+struct exynos_drm_display *exynos_drm_get_display(struct drm_encoder *encoder)
 {
-	return to_exynos_encoder(encoder)->manager;
-}
-
-void exynos_drm_fn_encoder(struct drm_crtc *crtc, void *data,
-			    void (*fn)(struct drm_encoder *, void *))
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_encoder *encoder;
-	struct exynos_drm_private *private = dev->dev_private;
-	struct exynos_drm_manager *manager;
-
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
-		/*
-		 * if crtc is detached from encoder, check pipe,
-		 * otherwise check crtc attached to encoder
-		 */
-		if (!encoder->crtc) {
-			manager = to_exynos_encoder(encoder)->manager;
-			if (manager->pipe < 0 ||
-					private->crtc[manager->pipe] != crtc)
-				continue;
-		} else {
-			if (encoder->crtc != crtc)
-				continue;
-		}
-
-		fn(encoder, data);
-	}
-}
-
-void exynos_drm_enable_vblank(struct drm_encoder *encoder, void *data)
-{
-	struct exynos_drm_manager *manager =
-		to_exynos_encoder(encoder)->manager;
-	struct exynos_drm_manager_ops *manager_ops = manager->ops;
-	int crtc = *(int *)data;
-
-	if (manager->pipe != crtc)
-		return;
-
-	if (manager_ops->enable_vblank)
-		manager_ops->enable_vblank(manager->ctx);
-}
-
-void exynos_drm_disable_vblank(struct drm_encoder *encoder, void *data)
-{
-	struct exynos_drm_manager *manager =
-		to_exynos_encoder(encoder)->manager;
-	struct exynos_drm_manager_ops *manager_ops = manager->ops;
-	int crtc = *(int *)data;
-
-	if (manager->pipe != crtc)
-		return;
-
-	if (manager_ops->disable_vblank)
-		manager_ops->disable_vblank(manager->ctx);
-}
-
-void exynos_drm_encoder_crtc_dpms(struct drm_encoder *encoder, void *data)
-{
-	struct exynos_drm_encoder *exynos_encoder = to_exynos_encoder(encoder);
-	struct exynos_drm_manager *manager = exynos_encoder->manager;
-	struct exynos_drm_manager_ops *manager_ops = manager->ops;
-	int mode = *(int *)data;
-
-	if (manager_ops && manager_ops->dpms)
-		manager_ops->dpms(manager->ctx, mode);
-
-	/*
-	 * if this condition is ok then it means that the crtc is already
-	 * detached from encoder and last function for detaching is properly
-	 * done, so clear pipe from manager to prevent repeated call.
-	 */
-	if (mode > DRM_MODE_DPMS_ON) {
-		if (!encoder->crtc)
-			manager->pipe = -1;
-	}
-}
-
-void exynos_drm_encoder_crtc_pipe(struct drm_encoder *encoder, void *data)
-{
-	struct exynos_drm_manager *manager =
-		to_exynos_encoder(encoder)->manager;
-	int pipe = *(int *)data;
-
-	/*
-	 * when crtc is detached from encoder, this pipe is used
-	 * to select manager operation
-	 */
-	manager->pipe = pipe;
-}
-
-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_manager_ops *manager_ops = manager->ops;
-	struct exynos_drm_overlay *overlay = data;
-
-	if (manager_ops && manager_ops->win_mode_set)
-		manager_ops->win_mode_set(manager->ctx, 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_manager_ops *manager_ops = manager->ops;
-	int zpos = DEFAULT_ZPOS;
-
-	if (data)
-		zpos = *(int *)data;
-
-	if (manager_ops && manager_ops->win_commit)
-		manager_ops->win_commit(manager->ctx, 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_manager_ops *manager_ops = manager->ops;
-	int zpos = DEFAULT_ZPOS;
-
-	if (data)
-		zpos = *(int *)data;
-
-	if (manager_ops && manager_ops->win_enable)
-		manager_ops->win_enable(manager->ctx, 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_manager_ops *manager_ops = manager->ops;
-	int zpos = DEFAULT_ZPOS;
-
-	if (data)
-		zpos = *(int *)data;
-
-	if (manager_ops && manager_ops->win_disable)
-		manager_ops->win_disable(manager->ctx, zpos);
+	return to_exynos_encoder(encoder)->display;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_encoder.h b/drivers/gpu/drm/exynos/exynos_drm_encoder.h
index 0f3e5e2..b7a1620 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_encoder.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_encoder.h
@@ -18,20 +18,8 @@  struct exynos_drm_manager;
 
 void exynos_drm_encoder_setup(struct drm_device *dev);
 struct drm_encoder *exynos_drm_encoder_create(struct drm_device *dev,
-					       struct exynos_drm_manager *mgr,
-					       unsigned long possible_crtcs);
-struct exynos_drm_manager *
-exynos_drm_get_manager(struct drm_encoder *encoder);
-void exynos_drm_fn_encoder(struct drm_crtc *crtc, void *data,
-			    void (*fn)(struct drm_encoder *, void *));
-void exynos_drm_enable_vblank(struct drm_encoder *encoder, void *data);
-void exynos_drm_disable_vblank(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_crtc_dpms(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_crtc_pipe(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_plane_mode_set(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_plane_commit(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_plane_enable(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_plane_disable(struct drm_encoder *encoder, void *data);
-void exynos_drm_encoder_complete_scanout(struct drm_framebuffer *fb);
+			struct exynos_drm_display *mgr,
+			unsigned long possible_crtcs);
+struct exynos_drm_display *exynos_drm_get_display(struct drm_encoder *encoder);
 
 #endif
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fb.c b/drivers/gpu/drm/exynos/exynos_drm_fb.c
index ea39e0e..c7c08d0 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fb.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fb.c
@@ -22,7 +22,7 @@ 
 #include "exynos_drm_fb.h"
 #include "exynos_drm_gem.h"
 #include "exynos_drm_iommu.h"
-#include "exynos_drm_encoder.h"
+#include "exynos_drm_crtc.h"
 
 #define to_exynos_fb(x)	container_of(x, struct exynos_drm_fb, fb)
 
@@ -71,7 +71,7 @@  static void exynos_drm_fb_destroy(struct drm_framebuffer *fb)
 	unsigned int i;
 
 	/* make sure that overlay data are updated before relesing fb. */
-	exynos_drm_encoder_complete_scanout(fb);
+	exynos_drm_crtc_complete_scanout(fb);
 
 	drm_framebuffer_cleanup(fb);
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_fimd.c b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
index 838c47d..f3dc808 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_fimd.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_fimd.c
@@ -105,7 +105,6 @@  struct fimd_win_data {
 };
 
 struct fimd_context {
-	struct exynos_drm_subdrv	subdrv;
 	struct device			*dev;
 	struct drm_device		*drm_dev;
 	int				irq;
@@ -120,6 +119,7 @@  struct fimd_context {
 	u32				vidcon0;
 	u32				vidcon1;
 	bool				suspended;
+	int				pipe;
 	struct mutex			lock;
 	wait_queue_head_t		wait_vsync_queue;
 	atomic_t			wait_vsync_event;
@@ -169,12 +169,16 @@  static int fimd_check_mode(void *in_ctx, struct drm_display_mode *mode)
 }
 
 static struct exynos_drm_display_ops fimd_display_ops = {
-	.type = EXYNOS_DISPLAY_TYPE_LCD,
 	.is_connected = fimd_display_is_connected,
 	.get_panel = fimd_get_panel,
 	.check_mode = fimd_check_mode,
 };
 
+static struct exynos_drm_display fimd_display = {
+	.type = EXYNOS_DISPLAY_TYPE_LCD,
+	.ops = &fimd_display_ops,
+};
+
 static void fimd_win_mode_set(void *in_ctx, struct exynos_drm_overlay *overlay)
 {
 	struct fimd_context *ctx = in_ctx;
@@ -481,15 +485,46 @@  static void fimd_win_disable(void *in_ctx, int zpos)
 	win_data->enabled = false;
 }
 
-static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
+static int fimd_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
+		int pipe)
 {
 	struct fimd_context *ctx = in_ctx;
 
 	ctx->drm_dev = drm_dev;
+	ctx->pipe = pipe;
+
+	/*
+	 * enable drm irq mode.
+	 * - with irq_enabled = 1, we can use the vblank feature.
+	 *
+	 * P.S. note that we wouldn't use drm irq handler but
+	 *	just specific driver own one instead because
+	 *	drm framework supports only one irq handler.
+	 */
+	ctx->drm_dev->irq_enabled = 1;
+
+	/*
+	 * with vblank_disable_allowed = 1, vblank interrupt will be disabled
+	 * by drm timer once a current process gives up ownership of
+	 * vblank event.(after drm_vblank_put function is called)
+	 */
+	drm_dev->vblank_disable_allowed = 1;
+
+	/* attach this sub driver to iommu mapping if supported. */
+	if (is_drm_iommu_supported(ctx->drm_dev))
+		drm_iommu_attach_device(ctx->drm_dev, ctx->dev);
 
 	return 0;
 }
 
+static void fimd_mgr_remove(void *in_ctx)
+{
+	struct fimd_context *ctx = in_ctx;
+
+	if (is_drm_iommu_supported(ctx->drm_dev))
+		drm_iommu_detach_device(ctx->drm_dev, ctx->dev);
+}
+
 static void fimd_dpms(void *in_ctx, int mode)
 {
 	struct fimd_context *ctx = in_ctx;
@@ -523,24 +558,6 @@  static void fimd_dpms(void *in_ctx, int mode)
 	mutex_unlock(&ctx->lock);
 }
 
-static void fimd_apply(void *in_ctx)
-{
-	struct fimd_context *ctx = in_ctx;
-	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(ctx, i);
-	}
-
-	if (mgr_ops && mgr_ops->commit)
-		mgr_ops->commit(ctx);
-}
-
 static void fimd_commit(void *in_ctx)
 {
 	struct fimd_context *ctx = in_ctx;
@@ -597,6 +614,21 @@  static void fimd_commit(void *in_ctx)
 	writel(val, ctx->regs + VIDCON0);
 }
 
+static void fimd_apply(void *in_ctx)
+{
+	struct fimd_context *ctx = in_ctx;
+	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)
+			fimd_win_commit(ctx, i);
+	}
+
+	fimd_commit(ctx);
+}
+
 static int fimd_enable_vblank(void *in_ctx)
 {
 	struct fimd_context *ctx = in_ctx;
@@ -661,6 +693,7 @@  static void fimd_wait_for_vblank(void *in_ctx)
 
 static struct exynos_drm_manager_ops fimd_manager_ops = {
 	.initialize = fimd_mgr_initialize,
+	.remove = fimd_mgr_remove,
 	.dpms = fimd_dpms,
 	.apply = fimd_apply,
 	.commit = fimd_commit,
@@ -673,16 +706,13 @@  static struct exynos_drm_manager_ops fimd_manager_ops = {
 };
 
 static struct exynos_drm_manager fimd_manager = {
-	.pipe		= -1,
-	.ops		= &fimd_manager_ops,
-	.display_ops	= &fimd_display_ops,
+	.type = EXYNOS_DISPLAY_TYPE_LCD,
+	.ops = &fimd_manager_ops,
 };
 
 static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
 {
 	struct fimd_context *ctx = (struct fimd_context *)dev_id;
-	struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
-	struct exynos_drm_manager *manager = subdrv->manager;
 	u32 val;
 
 	val = readl(ctx->regs + VIDINTCON1);
@@ -692,11 +722,11 @@  static irqreturn_t fimd_irq_handler(int irq, void *dev_id)
 		writel(VIDINTCON1_INT_FRAME, ctx->regs + VIDINTCON1);
 
 	/* check the crtc is detached already from encoder */
-	if (manager->pipe < 0 || !ctx->drm_dev)
+	if (ctx->pipe < 0 || !ctx->drm_dev)
 		goto out;
 
-	drm_handle_vblank(ctx->drm_dev, manager->pipe);
-	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, manager->pipe);
+	drm_handle_vblank(ctx->drm_dev, ctx->pipe);
+	exynos_drm_crtc_finish_pageflip(ctx->drm_dev, ctx->pipe);
 
 	/* set wait vsync event to zero and wake up queue. */
 	if (atomic_read(&ctx->wait_vsync_event)) {
@@ -707,39 +737,6 @@  out:
 	return IRQ_HANDLED;
 }
 
-static int fimd_subdrv_probe(struct drm_device *drm_dev, struct device *dev)
-{
-	/*
-	 * enable drm irq mode.
-	 * - with irq_enabled = 1, we can use the vblank feature.
-	 *
-	 * P.S. note that we wouldn't use drm irq handler but
-	 *	just specific driver own one instead because
-	 *	drm framework supports only one irq handler.
-	 */
-	drm_dev->irq_enabled = 1;
-
-	/*
-	 * with vblank_disable_allowed = 1, vblank interrupt will be disabled
-	 * by drm timer once a current process gives up ownership of
-	 * vblank event.(after drm_vblank_put function is called)
-	 */
-	drm_dev->vblank_disable_allowed = 1;
-
-	/* attach this sub driver to iommu mapping if supported. */
-	if (is_drm_iommu_supported(drm_dev))
-		drm_iommu_attach_device(drm_dev, dev);
-
-	return 0;
-}
-
-static void fimd_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
-{
-	/* detach this sub driver from iommu mapping if supported. */
-	if (is_drm_iommu_supported(drm_dev))
-		drm_iommu_detach_device(drm_dev, dev);
-}
-
 static int fimd_configure_clocks(struct fimd_context *ctx, struct device *dev)
 {
 	struct videomode *vm = &ctx->panel.vm;
@@ -825,9 +822,8 @@  static int fimd_clock(struct fimd_context *ctx, bool enable)
 	return 0;
 }
 
-static void fimd_window_suspend(struct device *dev)
+static void fimd_window_suspend(struct fimd_context *ctx)
 {
-	struct fimd_context *ctx = get_fimd_context(dev);
 	struct fimd_win_data *win_data;
 	int i;
 
@@ -839,9 +835,8 @@  static void fimd_window_suspend(struct device *dev)
 	fimd_wait_for_vblank(ctx);
 }
 
-static void fimd_window_resume(struct device *dev)
+static void fimd_window_resume(struct fimd_context *ctx)
 {
-	struct fimd_context *ctx = get_fimd_context(dev);
 	struct fimd_win_data *win_data;
 	int i;
 
@@ -854,7 +849,6 @@  static void fimd_window_resume(struct device *dev)
 
 static int fimd_activate(struct fimd_context *ctx, bool enable)
 {
-	struct device *dev = ctx->subdrv.dev;
 	if (enable) {
 		int ret;
 
@@ -866,11 +860,11 @@  static int fimd_activate(struct fimd_context *ctx, bool enable)
 
 		/* if vblank was enabled status, enable it again. */
 		if (test_and_clear_bit(0, &ctx->irq_flags))
-			fimd_enable_vblank(dev);
+			fimd_enable_vblank(ctx);
 
-		fimd_window_resume(dev);
+		fimd_window_resume(ctx);
 	} else {
-		fimd_window_suspend(dev);
+		fimd_window_suspend(ctx);
 
 		fimd_clock(ctx, false);
 		ctx->suspended = true;
@@ -907,7 +901,6 @@  static int fimd_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
 	struct fimd_context *ctx;
-	struct exynos_drm_subdrv *subdrv;
 	struct resource *res;
 	int win;
 	int ret = -EINVAL;
@@ -952,15 +945,6 @@  static int fimd_probe(struct platform_device *pdev)
 	DRM_INIT_WAITQUEUE(&ctx->wait_vsync_queue);
 	atomic_set(&ctx->wait_vsync_event, 0);
 
-	fimd_manager.ctx = ctx;
-
-	subdrv = &ctx->subdrv;
-
-	subdrv->dev = dev;
-	subdrv->manager = &fimd_manager;
-	subdrv->probe = fimd_subdrv_probe;
-	subdrv->remove = fimd_subdrv_remove;
-
 	mutex_init(&ctx->lock);
 
 	platform_set_drvdata(pdev, ctx);
@@ -971,7 +955,11 @@  static int fimd_probe(struct platform_device *pdev)
 	for (win = 0; win < WINDOWS_NR; win++)
 		fimd_clear_win(ctx, win);
 
-	exynos_drm_subdrv_register(subdrv);
+	fimd_manager.ctx = ctx;
+	exynos_drm_manager_register(&fimd_manager);
+
+	fimd_display.ctx = ctx;
+	exynos_drm_display_register(&fimd_display);
 
 	return 0;
 }
@@ -981,7 +969,8 @@  static int fimd_remove(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct fimd_context *ctx = platform_get_drvdata(pdev);
 
-	exynos_drm_subdrv_unregister(&ctx->subdrv);
+	exynos_drm_display_unregister(&fimd_display);
+	exynos_drm_manager_unregister(&fimd_manager);
 
 	if (ctx->suspended)
 		goto out;
diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
index 8173e44..8fc9d6d 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.c
@@ -24,14 +24,11 @@ 
 #include "exynos_drm_hdmi.h"
 
 #define to_context(dev)		platform_get_drvdata(to_platform_device(dev))
-#define to_subdrv(dev)		to_context(dev)
-#define get_ctx_from_subdrv(subdrv)	container_of(subdrv,\
-					struct drm_hdmi_context, subdrv);
 
 /* platform device pointer for common drm hdmi device. */
 static struct platform_device *exynos_drm_hdmi_pdev;
 
-/* Common hdmi subdrv needs to access the hdmi and mixer though context.
+/* Common hdmi needs to access the hdmi and mixer though context.
 * These should be initialied by the repective drivers */
 static struct exynos_drm_hdmi_context *hdmi_ctx;
 static struct exynos_drm_hdmi_context *mixer_ctx;
@@ -41,7 +38,6 @@  static struct exynos_hdmi_ops *hdmi_ops;
 static struct exynos_mixer_ops *mixer_ops;
 
 struct drm_hdmi_context {
-	struct exynos_drm_subdrv	subdrv;
 	struct exynos_drm_hdmi_context	*hdmi_ctx;
 	struct exynos_drm_hdmi_context	*mixer_ctx;
 
@@ -101,6 +97,12 @@  static int drm_hdmi_display_initialize(void *in_ctx, struct drm_device *drm_dev)
 {
 	struct drm_hdmi_context *ctx = in_ctx;
 
+	if (!hdmi_ctx) {
+		DRM_ERROR("hdmi context not initialized.\n");
+		return -EINVAL;
+	}
+	ctx->hdmi_ctx = hdmi_ctx;
+
 	if (hdmi_ops && hdmi_ops->initialize)
 		return hdmi_ops->initialize(ctx->hdmi_ctx->ctx, drm_dev);
 
@@ -118,6 +120,17 @@  static bool drm_hdmi_is_connected(void *in_ctx)
 	return false;
 }
 
+static void drm_hdmi_get_max_resol(void *in_ctx, unsigned int *width,
+				unsigned int *height)
+{
+	struct drm_hdmi_context *ctx = in_ctx;
+
+	DRM_DEBUG_KMS("%s\n", __FILE__);
+
+	if (hdmi_ops && hdmi_ops->get_max_resol)
+		hdmi_ops->get_max_resol(ctx->hdmi_ctx->ctx, width, height);
+}
+
 static struct edid *drm_hdmi_get_edid(void *in_ctx,
 			struct drm_connector *connector)
 {
@@ -129,6 +142,16 @@  static struct edid *drm_hdmi_get_edid(void *in_ctx,
 	return NULL;
 }
 
+static void drm_hdmi_mode_set(void *in_ctx, struct drm_display_mode *mode)
+{
+	struct drm_hdmi_context *ctx = in_ctx;
+
+	DRM_DEBUG_KMS("%s\n", __FILE__);
+
+	if (hdmi_ops && hdmi_ops->mode_set)
+		hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode);
+}
+
 static int drm_hdmi_check_mode(void *in_ctx, struct drm_display_mode *mode)
 {
 	struct drm_hdmi_context *ctx = in_ctx;
@@ -151,52 +174,6 @@  static int drm_hdmi_check_mode(void *in_ctx, struct drm_display_mode *mode)
 	return 0;
 }
 
-static void drm_hdmi_display_dpms(void *in_ctx, int mode)
-{
-	struct drm_hdmi_context *ctx = in_ctx;
-
-	if (hdmi_ops && hdmi_ops->dpms)
-		hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode);
-}
-
-static struct exynos_drm_display_ops drm_hdmi_display_ops = {
-	.type = EXYNOS_DISPLAY_TYPE_HDMI,
-	.initialize = drm_hdmi_display_initialize,
-	.is_connected = drm_hdmi_is_connected,
-	.get_edid = drm_hdmi_get_edid,
-	.check_mode = drm_hdmi_check_mode,
-	.dpms = drm_hdmi_display_dpms,
-};
-
-static int drm_hdmi_enable_vblank(void *in_ctx)
-{
-	struct drm_hdmi_context *ctx = in_ctx;
-	struct exynos_drm_subdrv *subdrv = &ctx->subdrv;
-	struct exynos_drm_manager *manager = subdrv->manager;
-
-	if (mixer_ops && mixer_ops->enable_vblank)
-		return mixer_ops->enable_vblank(ctx->mixer_ctx->ctx,
-						manager->pipe);
-
-	return 0;
-}
-
-static void drm_hdmi_disable_vblank(void *in_ctx)
-{
-	struct drm_hdmi_context *ctx = in_ctx;
-
-	if (mixer_ops && mixer_ops->disable_vblank)
-		return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx);
-}
-
-static void drm_hdmi_wait_for_vblank(void *in_ctx)
-{
-	struct drm_hdmi_context *ctx = in_ctx;
-
-	if (mixer_ops && mixer_ops->wait_for_vblank)
-		mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx);
-}
-
 static void drm_hdmi_mode_fixup(void *in_ctx, struct drm_connector *connector,
 				const struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -241,21 +218,57 @@  static void drm_hdmi_mode_fixup(void *in_ctx, struct drm_connector *connector,
 	}
 }
 
-static void drm_hdmi_mode_set(void *in_ctx, void *mode)
+static void drm_hdmi_display_dpms(void *in_ctx, int mode)
 {
 	struct drm_hdmi_context *ctx = in_ctx;
 
-	if (hdmi_ops && hdmi_ops->mode_set)
-		hdmi_ops->mode_set(ctx->hdmi_ctx->ctx, mode);
+	if (hdmi_ops && hdmi_ops->dpms)
+		hdmi_ops->dpms(ctx->hdmi_ctx->ctx, mode);
 }
 
-static void drm_hdmi_get_max_resol(void *in_ctx, unsigned int *width,
-				unsigned int *height)
+static struct exynos_drm_display_ops drm_hdmi_display_ops = {
+	.initialize = drm_hdmi_display_initialize,
+	.is_connected = drm_hdmi_is_connected,
+	.get_max_resol = drm_hdmi_get_max_resol,
+	.get_edid = drm_hdmi_get_edid,
+	.mode_fixup = drm_hdmi_mode_fixup,
+	.mode_set = drm_hdmi_mode_set,
+	.check_mode = drm_hdmi_check_mode,
+	.dpms = drm_hdmi_display_dpms,
+};
+
+static struct exynos_drm_display hdmi_display = {
+	.type = EXYNOS_DISPLAY_TYPE_HDMI,
+	.ops = &drm_hdmi_display_ops,
+};
+
+static int drm_hdmi_enable_vblank(void *in_ctx)
 {
 	struct drm_hdmi_context *ctx = in_ctx;
 
-	if (hdmi_ops && hdmi_ops->get_max_resol)
-		hdmi_ops->get_max_resol(ctx->hdmi_ctx->ctx, width, height);
+	if (mixer_ops && mixer_ops->enable_vblank)
+		return mixer_ops->enable_vblank(ctx->mixer_ctx->ctx,
+						ctx->mixer_ctx->pipe);
+
+	return 0;
+}
+
+static void drm_hdmi_disable_vblank(void *in_ctx)
+{
+	struct drm_hdmi_context *ctx = in_ctx;
+
+	DRM_DEBUG_KMS("%s\n", __FILE__);
+
+	if (mixer_ops && mixer_ops->disable_vblank)
+		return mixer_ops->disable_vblank(ctx->mixer_ctx->ctx);
+}
+
+static void drm_hdmi_wait_for_vblank(void *in_ctx)
+{
+	struct drm_hdmi_context *ctx = in_ctx;
+
+	if (mixer_ops && mixer_ops->wait_for_vblank)
+		mixer_ops->wait_for_vblank(ctx->mixer_ctx->ctx);
 }
 
 static void drm_hdmi_commit(void *in_ctx)
@@ -266,11 +279,19 @@  static void drm_hdmi_commit(void *in_ctx)
 		hdmi_ops->commit(ctx->hdmi_ctx->ctx);
 }
 
-static int drm_hdmi_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
+static int drm_hdmi_mgr_initialize(void *in_ctx, struct drm_device *drm_dev,
+		int pipe)
 {
 	struct drm_hdmi_context *ctx = in_ctx;
 	int ret = 0;
 
+	if (!mixer_ctx) {
+		DRM_ERROR("mixer context not initialized.\n");
+		return -EFAULT;
+	}
+	ctx->mixer_ctx = mixer_ctx;
+	ctx->mixer_ctx->pipe = pipe;
+
 	if (mixer_ops && mixer_ops->initialize)
 		ret = mixer_ops->initialize(ctx->mixer_ctx->ctx, drm_dev);
 
@@ -280,6 +301,14 @@  static int drm_hdmi_mgr_initialize(void *in_ctx, struct drm_device *drm_dev)
 	return ret;
 }
 
+static void drm_hdmi_mgr_remove(void *in_ctx)
+{
+	struct drm_hdmi_context *ctx = in_ctx;
+
+	if (mixer_ops->iommu_on)
+		mixer_ops->iommu_on(ctx->mixer_ctx->ctx, false);
+}
+
 static void drm_hdmi_dpms(void *in_ctx, int mode)
 {
 	struct drm_hdmi_context *ctx = in_ctx;
@@ -350,14 +379,12 @@  static void drm_mixer_win_disable(void *in_ctx, int zpos)
 
 static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
 	.initialize = drm_hdmi_mgr_initialize,
+	.remove = drm_hdmi_mgr_remove,
 	.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,
@@ -365,82 +392,33 @@  static struct exynos_drm_manager_ops drm_hdmi_manager_ops = {
 };
 
 static struct exynos_drm_manager hdmi_manager = {
-	.pipe		= -1,
-	.ops		= &drm_hdmi_manager_ops,
-	.display_ops	= &drm_hdmi_display_ops,
+	.type = EXYNOS_DISPLAY_TYPE_HDMI,
+	.ops = &drm_hdmi_manager_ops,
 };
 
-static int hdmi_subdrv_probe(struct drm_device *drm_dev,
-		struct device *dev)
-{
-	struct exynos_drm_subdrv *subdrv = to_subdrv(dev);
-	struct drm_hdmi_context *ctx;
-
-	if (!hdmi_ctx) {
-		DRM_ERROR("hdmi context not initialized.\n");
-		return -EFAULT;
-	}
-
-	if (!mixer_ctx) {
-		DRM_ERROR("mixer context not initialized.\n");
-		return -EFAULT;
-	}
-
-	ctx = get_ctx_from_subdrv(subdrv);
-
-	if (!ctx) {
-		DRM_ERROR("no drm hdmi context.\n");
-		return -EFAULT;
-	}
-
-	ctx->hdmi_ctx = hdmi_ctx;
-	ctx->mixer_ctx = mixer_ctx;
-
-	return 0;
-}
-
-static void hdmi_subdrv_remove(struct drm_device *drm_dev, struct device *dev)
-{
-	struct drm_hdmi_context *ctx;
-	struct exynos_drm_subdrv *subdrv = to_subdrv(dev);
-
-	ctx = get_ctx_from_subdrv(subdrv);
-
-	if (mixer_ops->iommu_on)
-		mixer_ops->iommu_on(ctx->mixer_ctx->ctx, false);
-}
-
 static int exynos_drm_hdmi_probe(struct platform_device *pdev)
 {
-	struct device *dev = &pdev->dev;
-	struct exynos_drm_subdrv *subdrv;
 	struct drm_hdmi_context *ctx;
 
-	ctx = devm_kzalloc(dev, sizeof(*ctx), GFP_KERNEL);
+	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_KERNEL);
 	if (!ctx)
 		return -ENOMEM;
 
 	hdmi_manager.ctx = ctx;
+	exynos_drm_manager_register(&hdmi_manager);
 
-	subdrv = &ctx->subdrv;
-
-	subdrv->dev = dev;
-	subdrv->manager = &hdmi_manager;
-	subdrv->probe = hdmi_subdrv_probe;
-	subdrv->remove = hdmi_subdrv_remove;
+	hdmi_display.ctx = ctx;
+	exynos_drm_display_register(&hdmi_display);
 
-	platform_set_drvdata(pdev, subdrv);
-
-	exynos_drm_subdrv_register(subdrv);
+	platform_set_drvdata(pdev, ctx);
 
 	return 0;
 }
 
 static int exynos_drm_hdmi_remove(struct platform_device *pdev)
 {
-	struct drm_hdmi_context *ctx = platform_get_drvdata(pdev);
-
-	exynos_drm_subdrv_unregister(&ctx->subdrv);
+	exynos_drm_display_unregister(&hdmi_display);
+	exynos_drm_manager_unregister(&hdmi_manager);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
index 923239b..37059ea 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
+++ b/drivers/gpu/drm/exynos/exynos_drm_hdmi.h
@@ -19,10 +19,12 @@ 
  * exynos hdmi common context structure.
  *
  * @drm_dev: pointer to drm_device.
+ * @pipe: pipe for mixer
  * @ctx: pointer to the context of specific device driver.
  *	this context should be hdmi_context or mixer_context.
  */
 struct exynos_drm_hdmi_context {
+	int			pipe;
 	void			*ctx;
 };
 
diff --git a/drivers/gpu/drm/exynos/exynos_drm_plane.c b/drivers/gpu/drm/exynos/exynos_drm_plane.c
index cff3aed..e0db2b3 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_plane.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_plane.c
@@ -13,7 +13,7 @@ 
 
 #include <drm/exynos_drm.h>
 #include "exynos_drm_drv.h"
-#include "exynos_drm_encoder.h"
+#include "exynos_drm_crtc.h"
 #include "exynos_drm_fb.h"
 #include "exynos_drm_gem.h"
 #include "exynos_drm_plane.h"
@@ -139,7 +139,7 @@  int exynos_plane_mode_set(struct drm_plane *plane, struct drm_crtc *crtc,
 			overlay->crtc_x, overlay->crtc_y,
 			overlay->crtc_width, overlay->crtc_height);
 
-	exynos_drm_fn_encoder(crtc, overlay, exynos_drm_encoder_plane_mode_set);
+	exynos_drm_crtc_plane_mode_set(crtc, overlay);
 
 	return 0;
 }
@@ -149,8 +149,7 @@  void exynos_plane_commit(struct drm_plane *plane)
 	struct exynos_plane *exynos_plane = to_exynos_plane(plane);
 	struct exynos_drm_overlay *overlay = &exynos_plane->overlay;
 
-	exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
-			exynos_drm_encoder_plane_commit);
+	exynos_drm_crtc_plane_commit(plane->crtc, overlay->zpos);
 }
 
 void exynos_plane_dpms(struct drm_plane *plane, int mode)
@@ -162,17 +161,13 @@  void exynos_plane_dpms(struct drm_plane *plane, int mode)
 		if (exynos_plane->enabled)
 			return;
 
-		exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
-				exynos_drm_encoder_plane_enable);
-
+		exynos_drm_crtc_plane_enable(plane->crtc, overlay->zpos);
 		exynos_plane->enabled = true;
 	} else {
 		if (!exynos_plane->enabled)
 			return;
 
-		exynos_drm_fn_encoder(plane->crtc, &overlay->zpos,
-				exynos_drm_encoder_plane_disable);
-
+		exynos_drm_crtc_plane_disable(plane->crtc, overlay->zpos);
 		exynos_plane->enabled = false;
 	}
 }