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

[Sean Paul]

On Fri, Oct 11, 2013 at 8:42 PM, Inki Dae <inki.dae at 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;
>>  }
>>