Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750982Ab3JDESq (ORCPT ); Fri, 4 Oct 2013 00:18:46 -0400 Received: from mail-la0-f41.google.com ([209.85.215.41]:35957 "EHLO mail-la0-f41.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750728Ab3JDESn (ORCPT ); Fri, 4 Oct 2013 00:18:43 -0400 MIME-Version: 1.0 In-Reply-To: References: <1380670860-17621-1-git-send-email-seanpaul@chromium.org> <1380839303-4834-1-git-send-email-seanpaul@chromium.org> <1380839303-4834-5-git-send-email-seanpaul@chromium.org> Date: Fri, 4 Oct 2013 13:18:41 +0900 X-Google-Sender-Auth: GnEtxK4XpK7MrOMxYXZsehJhfaM Message-ID: Subject: Re: [PATCH v2 4/5] drm/exynos: Initialize ptn3460 if present From: Inki Dae To: Sean Paul Cc: Kukjin Kim , DRI mailing list , "linux-samsung-soc@vger.kernel.org" , "linux-arm-kernel@lists.infradead.org" , "linux-kernel@vger.kernel.org" , "linux-doc@vger.kernel.org" , "devicetree@vger.kernel.org" , Dave Airlie Content-Type: text/plain; charset=ISO-8859-1 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 5772 Lines: 152 2013/10/4 Sean Paul : > On Thu, Oct 3, 2013 at 10:29 PM, Inki Dae wrote: >> 2013/10/4 Sean Paul : >>> This patch adds code to look for the ptn3460 in the device tree file on >>> exynos initialization. If ptn node is found, the driver will initialize >>> the ptn3460 driver and skip creating a DP connector (since the bridge >>> driver will register its own connector). >>> >>> Signed-off-by: Sean Paul >>> --- >>> >>> v2: >>> - Changed include from of_i2c.h to i2c.h >>> - Changed of_find_by_name to of_find_compatible >>> >>> drivers/gpu/drm/exynos/exynos_drm_core.c | 44 +++++++++++++++++++++++++++++++- >>> 1 file changed, 43 insertions(+), 1 deletion(-) >>> >>> diff --git a/drivers/gpu/drm/exynos/exynos_drm_core.c b/drivers/gpu/drm/exynos/exynos_drm_core.c >>> index 1bef6dc..08ca4f9 100644 >>> --- a/drivers/gpu/drm/exynos/exynos_drm_core.c >>> +++ b/drivers/gpu/drm/exynos/exynos_drm_core.c >>> @@ -12,7 +12,9 @@ >>> * option) any later version. >>> */ >>> >>> +#include >>> #include >>> +#include >>> #include "exynos_drm_drv.h" >>> #include "exynos_drm_encoder.h" >>> #include "exynos_drm_connector.h" >>> @@ -20,6 +22,40 @@ >>> >>> static LIST_HEAD(exynos_drm_subdrv_list); >>> >>> +struct bridge_init { >>> + struct i2c_client *client; >>> + struct device_node *node; >>> +}; >>> + >>> +static bool find_bridge(const char *compat, struct bridge_init *bridge) >>> +{ >>> + bridge->client = NULL; >>> + bridge->node = of_find_compatible_node(NULL, NULL, compat); >> >> Then, shouldn't the lvds-bridge driver be implemented as i2c driver so >> that such tricky isn't needed? Is there any reason you use such tricky >> codes? >> > > This is what I was explaining earlier today. If the bridge driver is > just an i2c driver, it won't get a pointer to drm_device, and can't > register itself as a drm_bridge or drm_connector. To solve this, you > need the ptn3460_init callback. > No, I think you can use sub driver. how about registering to exynos drm core that driver using exynos_drm_subdrv_register function after probed? Is there something I am missing? > If it's an i2c driver with the ptn3460_init callback, where it parses > the dt in probe, then you have a race between the ptn probe and the > ptn init/drm hooks. ie: it's possible drm will initialize and call > disable/post_disable/pre_enable/enable before things have been probed. And also, exynos drm core will call a probe callback of the sub driver after _drm is initialized_. Is there something I am missing? > To solve this, you need to use some global state and/or locking. > > So, to summarize. We can have this of_find_compatible with a init > callback, or we can have an i2c driver + global state/locking + init > callback. I chose the former, since it seemed easier. > > If you have a better way to achieve this, I'm definitely interested, > but I saw this as the lesser of two evils. > > Sean > >> I think you need to implement the lvds-bridge driver as i2c driver, >> and then consider how to take care of this. I mean let's find a better >> way how we could take care of the i2c based driver in exynos drm >> framework or driver. In all cases, such tricky codes are not good. >> >>> + if (!bridge->node) >>> + return false; >>> + >>> + bridge->client = of_find_i2c_device_by_node(bridge->node); >>> + if (!bridge->client) >>> + return false; >>> + >>> + return true; >>> +} >>> + >>> +/* returns the number of bridges attached */ >>> +static int exynos_drm_attach_lcd_bridge(struct drm_device *dev, >>> + struct drm_encoder *encoder) >>> +{ >>> + struct bridge_init bridge; >>> + int ret; >>> + >>> + if (find_bridge("nxp,ptn3460", &bridge)) { >>> + ret = ptn3460_init(dev, encoder, bridge.client, bridge.node); >>> + if (!ret) >>> + return 1; >>> + } >>> + return 0; >>> +} >>> + >>> static int exynos_drm_create_enc_conn(struct drm_device *dev, >>> struct exynos_drm_subdrv *subdrv) >>> { >>> @@ -36,6 +72,13 @@ static int exynos_drm_create_enc_conn(struct drm_device *dev, >>> DRM_ERROR("failed to create encoder\n"); >>> return -EFAULT; >>> } >>> + subdrv->encoder = encoder; >>> + >>> + if (subdrv->manager->display_ops->type == EXYNOS_DISPLAY_TYPE_LCD) { >>> + ret = exynos_drm_attach_lcd_bridge(dev, encoder); >>> + if (ret) >>> + return 0; >>> + } >>> >>> /* >>> * create and initialize a connector for this sub driver and >>> @@ -48,7 +91,6 @@ static int exynos_drm_create_enc_conn(struct drm_device *dev, >>> goto err_destroy_encoder; >>> } >>> >>> - subdrv->encoder = encoder; >>> subdrv->connector = connector; >>> >>> return 0; >>> -- >>> 1.8.4 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/