Re: [PATCH v3 12/32] drm/exynos: Split manager/display/subdrv

[Sean Paul <seanpaul@chromium.org>]

On Thu, Oct 31, 2013 at 6:30 AM, Inki Dae <inki.dae@samsung.com> wrote:
>
>
>> -----Original Message-----
>> From: Sean Paul [mailto:seanpaul@chromium.org]
>> Sent: Wednesday, October 30, 2013 1:13 AM
>> To: dri-devel@lists.freedesktop.org; inki.dae@samsung.com
>> Cc: airlied@linux.ie; tomasz.figa@gmail.com; marcheu@chromium.org; Sean
>> Paul
>> Subject: [PATCH v3 12/32] drm/exynos: Split manager/display/subdrv
>>
>> 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>
>> ---
>>
>> Changes in v2:
>>       - Pass display into display_ops instead of context
>> Changes in v3:
>>       - Changed vidi args to exynos_drm_display instead of void
>>       - Moved exynos_drm_hdmi.c removal into next patch
>>
>>  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       | 106 +++++++----
>>  drivers/gpu/drm/exynos/exynos_drm_encoder.c   | 258
> ++++------------------
>> ----
>>  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      | 161 ++++++++--------
>>  drivers/gpu/drm/exynos/exynos_drm_hdmi.c      | 211 +++++++++------------
>>  drivers/gpu/drm/exynos/exynos_drm_hdmi.h      |   2 +
>>  drivers/gpu/drm/exynos/exynos_drm_plane.c     |  15 +-
>>  drivers/gpu/drm/exynos/exynos_drm_vidi.c      | 129 ++++++-------
>>  14 files changed, 615 insertions(+), 684 deletions(-)
>>
>>  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);
>
> The lvds bridge related codes need more reviews

I think everyone agrees that there need not be a 1:1 binding to driver
relationship, so what is left to discuss?

Sean



> so I'll merge this patch
> manually excepting the lvds bridge related codes. Let's discuss lvds bridge
> later.
>
> Thanks,
> Inki Dae
>
>>               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;
>