From patchwork Wed Mar 5 14:25:33 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Philipp Zabel X-Patchwork-Id: 3775131 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork2.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork2.web.kernel.org (Postfix) with ESMTP id 7CA74BF13A for ; Wed, 5 Mar 2014 14:26:30 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 8CD2C2020E for ; Wed, 5 Mar 2014 14:26:29 +0000 (UTC) Received: from casper.infradead.org (casper.infradead.org [85.118.1.10]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 215B02020A for ; Wed, 5 Mar 2014 14:26:28 +0000 (UTC) Received: from merlin.infradead.org ([2001:4978:20e::2]) by casper.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLClv-0007SP-D0; Wed, 05 Mar 2014 14:26:11 +0000 Received: from localhost ([::1] helo=merlin.infradead.org) by merlin.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLCls-00038G-V9; Wed, 05 Mar 2014 14:26:08 +0000 Received: from metis.ext.pengutronix.de ([2001:6f8:1178:4:290:27ff:fe1d:cc33]) by merlin.infradead.org with esmtps (Exim 4.80.1 #2 (Red Hat Linux)) id 1WLCln-00035Z-TV for linux-arm-kernel@lists.infradead.org; Wed, 05 Mar 2014 14:26:06 +0000 Received: from paszta.hi.pengutronix.de ([2001:6f8:1178:2:96de:80ff:fec2:9969]) by metis.ext.pengutronix.de with esmtp (Exim 4.72) (envelope-from ) id 1WLClG-0003AE-3h; Wed, 05 Mar 2014 15:25:30 +0100 Message-ID: <1394029533.8754.21.camel@paszta.hi.pengutronix.de> Subject: Re: [PATCH v5 01/11] staging: imx-drm-core: Use OF graph to find components and connections between encoder and crtcs From: Philipp Zabel To: Russell King - ARM Linux Date: Wed, 05 Mar 2014 15:25:33 +0100 In-Reply-To: <20140305100516.GV21483@n2100.arm.linux.org.uk> References: <1394011262-16849-1-git-send-email-p.zabel@pengutronix.de> <1394011262-16849-2-git-send-email-p.zabel@pengutronix.de> <20140305100516.GV21483@n2100.arm.linux.org.uk> X-Mailer: Evolution 3.8.5-2+b3 Mime-Version: 1.0 X-SA-Exim-Connect-IP: 2001:6f8:1178:2:96de:80ff:fec2:9969 X-SA-Exim-Mail-From: p.zabel@pengutronix.de X-SA-Exim-Scanned: No (on metis.ext.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-arm-kernel@lists.infradead.org X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140305_092605_340899_A4C95ADC X-CRM114-Status: GOOD ( 28.63 ) X-Spam-Score: 1.1 (+) Cc: devel@driverdev.osuosl.org, devicetree@vger.kernel.org, Philipp Zabel , David Airlie , Greg Kroah-Hartman , dri-devel@lists.freedesktop.org, kernel@pengutronix.de, Grant Likely , Shawn Guo , Fabio Estevam , linux-arm-kernel@lists.infradead.org X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-1.2 required=5.0 tests=BAYES_00,KHOP_BIG_TO_CC, RCVD_IN_DNSWL_MED, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP Am Mittwoch, den 05.03.2014, 10:05 +0000 schrieb Russell King - ARM Linux: > On Wed, Mar 05, 2014 at 10:20:52AM +0100, Philipp Zabel wrote: > > +struct imx_drm_component { > > + struct device_node *of_node; > > + struct list_head list; > > +}; > > + > > The only thing this structure appears to be doing is ensuring that a > single component doesn't get added twice - is that correct? I also think of it as an optimization. Now we scan the whole device graph once in the probe function into a list of needed components that can be walked quickly every time master_ops' .add_components callback is run, instead of having to walk the device tree graph over and over. Functionally, it only protects against duplicate addition. > If so, (and the troublesome problem with the IPU crtcs is now gone) > we can modify the core component code such that it does this: > > if (c->master && c->master != master) > continue; > > if (compare(c->dev, compare_data)) { > if (!c->master) > component_attach_master(master, c); > ret = 0; > break; > } > > which will mean that you don't need to build this list anymore to track > what will be added - though I'd like to think a little more about that > before making that change. Please confirm whether this will eliminate > your list generation. Yes, I can confirm that with this change, I can remove the list, like this (tested on i.MX6S with a single LVDS panel): - int i; + int i, ret; /* * Bind the IPU display interface ports first, so that @@ -660,23 +593,18 @@ static int imx_drm_platform_probe(struct platform_device *pdev) * works as expected. */ for (i = 0; ; i++) { - port = of_parse_phandle(pdev->dev.of_node, "ports", i); + port = of_parse_phandle(master->of_node, "ports", i); if (!port) break; - ret = imx_drm_add_component(&pdev->dev, port); - if (ret < 0) + ret = component_master_add_child(m, compare_of, port); + if (ret) return ret; } - if (i == 0) { - dev_err(&pdev->dev, "missing 'ports' property\n"); - return -ENODEV; - } - /* Then bind all encoders */ for (i = 0; ; i++) { - port = of_parse_phandle(pdev->dev.of_node, "ports", i); + port = of_parse_phandle(master->of_node, "ports", i); if (!port) break; @@ -687,14 +615,44 @@ static int imx_drm_platform_probe(struct platform_device *pdev) continue; } - ret = imx_drm_add_component(&pdev->dev, remote); + ret = component_master_add_child(m, compare_of, remote); of_node_put(remote); - if (ret < 0) + if (ret) return ret; } of_node_put(port); } + return 0; +} + +static int imx_drm_bind(struct device *dev) +{ + return drm_platform_init(&imx_drm_driver, to_platform_device(dev)); +} + +static void imx_drm_unbind(struct device *dev) +{ + drm_put_dev(dev_get_drvdata(dev)); +} + +static const struct component_master_ops imx_drm_ops = { + .add_components = imx_drm_add_components, + .bind = imx_drm_bind, + .unbind = imx_drm_unbind, +}; + +static int imx_drm_platform_probe(struct platform_device *pdev) +{ + struct device_node *port; + int ret; + + port = of_parse_phandle(pdev->dev.of_node, "ports", 0); + if (!port) { + dev_err(&pdev->dev, "missing 'ports' property\n"); + return -ENODEV; + } + ret = dma_set_coherent_mask(&pdev->dev, DMA_BIT_MASK(32)); if (ret) return ret; diff --git a/drivers/staging/imx-drm/imx-drm-core.c b/drivers/staging/imx-drm/imx-drm-core.c index 014e546..f6135b9 100644 --- a/drivers/staging/imx-drm/imx-drm-core.c +++ b/drivers/staging/imx-drm/imx-drm-core.c @@ -32,11 +32,6 @@ struct imx_drm_crtc; -struct imx_drm_component { - struct device_node *of_node; - struct list_head list; -}; - struct imx_drm_device { struct drm_device *drm; struct imx_drm_crtc *crtc[MAX_CRTC]; @@ -587,72 +582,10 @@ static int compare_of(struct device *dev, void *data) return dev->of_node == np; } -static LIST_HEAD(imx_drm_components); - static int imx_drm_add_components(struct device *master, struct master *m) { - struct imx_drm_component *component; - int ret; - - list_for_each_entry(component, &imx_drm_components, list) { - ret = component_master_add_child(m, compare_of, - component->of_node); - if (ret) - return ret; - } - return 0; -} - -static int imx_drm_bind(struct device *dev) -{ - return drm_platform_init(&imx_drm_driver, to_platform_device(dev)); -} - -static void imx_drm_unbind(struct device *dev) -{ - drm_put_dev(dev_get_drvdata(dev)); -} - -static const struct component_master_ops imx_drm_ops = { - .add_components = imx_drm_add_components, - .bind = imx_drm_bind, - .unbind = imx_drm_unbind, -}; - -static struct imx_drm_component *imx_drm_find_component(struct device *dev, - struct device_node *node) -{ - struct imx_drm_component *component; - - list_for_each_entry(component, &imx_drm_components, list) - if (component->of_node == node) - return component; - - return NULL; -} - -static int imx_drm_add_component(struct device *dev, struct device_node *node) -{ - struct imx_drm_component *component; - - if (imx_drm_find_component(dev, node)) - return 0; - - component = devm_kzalloc(dev, sizeof(*component), GFP_KERNEL); - if (!component) - return -ENOMEM; - - component->of_node = node; - list_add_tail(&component->list, &imx_drm_components); - - return 0; -} - -static int imx_drm_platform_probe(struct platform_device *pdev) -{ struct device_node *ep, *port, *remote; - int ret;