On Fri, Oct 11, 2013 at 11:49 PM, Inki Dae <inki.dae@xxxxxxxxxxx> wrote: > > >> -----Original Message----- >> From: Sean Paul [mailto:seanpaul@xxxxxxxxxxxx] >> 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@xxxxxxxxxxx> 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; >> >> } >> >> > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
