From mboxrd@z Thu Jan 1 00:00:00 1970 From: Inki Dae Subject: RE: [PATCH 12/23] drm/exynos: Split manager/display/subdrv Date: Sat, 12 Oct 2013 12:49:02 +0900 Message-ID: <019301cec6fd$fde74220$f9b5c660$%dae@samsung.com> References: <1381451436-10089-1-git-send-email-seanpaul@chromium.org> <1381451436-10089-13-git-send-email-seanpaul@chromium.org> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mailout4.samsung.com (mailout4.samsung.com [203.254.224.34]) by gabe.freedesktop.org (Postfix) with ESMTP id E5960E6EF3 for ; Fri, 11 Oct 2013 20:49:10 -0700 (PDT) Received: from epcpsbgr5.samsung.com (u145.gpu120.samsung.co.kr [203.254.230.145]) by mailout4.samsung.com (Oracle Communications Messaging Server 7u4-24.01 (7.0.4.24.0) 64bit (built Nov 17 2011)) with ESMTP id <0MUJ00EFYELRP770@mailout4.samsung.com> for dri-devel@lists.freedesktop.org; Sat, 12 Oct 2013 12:49:03 +0900 (KST) In-reply-to: Content-language: ko List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org Errors-To: dri-devel-bounces+sf-dri-devel=m.gmane.org@lists.freedesktop.org To: 'Sean Paul' Cc: =?ISO-8859-1?Q?'St=E9phane_Marchesin'?= , 'DRI mailing list' List-Id: dri-devel@lists.freedesktop.org > -----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=E9phane 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 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 =3D 0; > >> > >> - subdrv->manager->dev =3D subdrv->dev; > >> + /* Find possible crtcs for this display */ > >> + list_for_each_entry(manager, &exynos_drm_manager_list, list) > >> + if (manager->type =3D=3D display->type) > >> + possible_crtcs |=3D 1 << manager->pipe; > >> > >> /* create and initialize a encoder for this sub driver. */ > >> - encoder =3D exynos_drm_encoder_create(dev, subdrv->manager, > >> - (1 << MAX_CRTC) - 1); > >> + encoder =3D exynos_drm_encoder_create(dev, display, > possible_crtcs); > >> if (!encoder) { > >> DRM_ERROR("failed to create encoder\n"); > >> return -EFAULT; > >> } > >> - subdrv->encoder =3D encoder; > >> + display->encoder =3D encoder; > >> > >> - if (subdrv->manager->display_ops->type =3D=3D > EXYNOS_DISPLAY_TYPE_LCD) { > >> + if (display->type =3D=3D EXYNOS_DISPLAY_TYPE_LCD) { > >> ret =3D 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 =3D connector; > >> + display->connector =3D connector; > >> > >> return 0; > >> > >> @@ -100,21 +107,6 @@ err_destroy_encoder: > >> return ret; > >> } > >>