> -----Original Message----- > From: Sean Paul [mailto:seanpaul@xxxxxxxxxxxx] > 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@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, 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; > >> >> } > >> >> > > _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
