Received: by 2002:ac0:a5a7:0:0:0:0:0 with SMTP id m36-v6csp3703464imm; Tue, 17 Jul 2018 08:59:16 -0700 (PDT) X-Google-Smtp-Source: AAOMgpf8gTB9ReEEp5+c1jkAUV3Uzwhzdi3wVTvruFU+uWiH2V6Tqfu062k8wo4+axKJUMPe19EX X-Received: by 2002:a63:62c4:: with SMTP id w187-v6mr2133358pgb.55.1531843156358; Tue, 17 Jul 2018 08:59:16 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1531843156; cv=none; d=google.com; s=arc-20160816; b=YgSEoOVPvVcIfh3QpyqdujQml336YZg6tIwpUOigPhZKqXcu9siG0oLn7OKh2Z533W IZdk4Paeg5zdSrTj0rCggs5Q76vMXz4gXoUjr96ZEUVohS/7/fAR8PyBzvDp+cq7IEAi iiDpQVmCHq36878fIan9U2Pi7aSJAdWwTyVvn2XUh3F1AZ6HtrBO4R+0g19Tsvg5Pfpy v81q20E9oNrfXHSHGTW0UOFUv6OHCP19iXRdOzizV9jR+fDCM61KdEcqigdRXxso0Srl J6+8Ykiadm0GHuvC6jGtQ6T/O3+DwYyfGV/gprax5mbnZ4WszPdCBTk8xipkDjLNxQrp L47g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature:arc-authentication-results; bh=UJ0izCYCfK3sGSuWISfJT8ZXrLuk53sZ5Dpuqj25ATI=; b=hVt/H26fy6nvLlZTDXAfDZx0vOZevpnhzN2wYalNhMR+G8q+xccd8T8N42lRc9hbS5 3AyueEskUvKSSU6rdW07KHoOi69CcuRLpUTe8JY3BJEqRTZcJBs4aElhau01hEzi9Rky 9BE0jsm6YxTlY3tPUMSge0KD0H/EynRTf/UJIKtFVi65d+9rjLPjpiLDOkADA2sOdKGY Ev5RfjH2hEdhafKxvOVvm2PuH2dm8441bblvGqNhbdIDgYRqfLKZ11rLZxzzXHLF9UWy pD4nrZ5y6ZxaC8S4tBzGJ+ONDenv2i4LpLO+fWL4tsai3+A/jIZzTsxzOlu5E/cVilj4 sF7Q== ARC-Authentication-Results: i=1; mx.google.com; dkim=fail header.i=@armlinux.org.uk header.s=pandora-2014 header.b=DawtQ5Vv; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=armlinux.org.uk Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id o3-v6si1112702plk.321.2018.07.17.08.59.00; Tue, 17 Jul 2018 08:59:16 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=fail header.i=@armlinux.org.uk header.s=pandora-2014 header.b=DawtQ5Vv; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=armlinux.org.uk Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1730795AbeGQQbC (ORCPT + 99 others); Tue, 17 Jul 2018 12:31:02 -0400 Received: from pandora.armlinux.org.uk ([78.32.30.218]:53124 "EHLO pandora.armlinux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729741AbeGQQbC (ORCPT ); Tue, 17 Jul 2018 12:31:02 -0400 DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=armlinux.org.uk; s=pandora-2014; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description:Resent-Date: Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID:List-Id: List-Help:List-Unsubscribe:List-Subscribe:List-Post:List-Owner:List-Archive; bh=UJ0izCYCfK3sGSuWISfJT8ZXrLuk53sZ5Dpuqj25ATI=; b=DawtQ5VvOhpwWaj06A6dRnRfI Uwyl/F2yaZ1YGPNXb4EMlDffLqWbaPQpO5OLzTYtzfGeNCTBh8OoR5+Vp356GjN46Oh4OPFEAkd/f qWZ1a46X4bJ6tWyHzu1VzoA8DfSar54bbGLOJjGvpnMIfpbRgnEpQAxkt1FrTqOjnq1Xc=; Received: from n2100.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:4f86]:42981) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.90_1) (envelope-from ) id 1ffSLy-0002uv-5j; Tue, 17 Jul 2018 16:57:30 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.90_1) (envelope-from ) id 1ffSLv-0006f8-Iq; Tue, 17 Jul 2018 16:57:27 +0100 Date: Tue, 17 Jul 2018 16:57:26 +0100 From: Russell King - ARM Linux To: Jyri Sarha Cc: Mark Rutland , Boris Brezillon , Alexandre Belloni , devicetree@vger.kernel.org, David Airlie , Tomi Valkeinen , dri-devel@lists.freedesktop.org, linux-kernel@vger.kernel.org, Rob Herring , Jacopo Mondi , Laurent Pinchart , Peter Rosin , linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v4 7/8] drm/i2c: tda998x: register as a drm bridge Message-ID: <20180717155726.GA25543@n2100.armlinux.org.uk> References: <5d6866d0-75bc-4de8-9b87-4fee5430e9dd@axentia.se> <20180424080833.GJ16141@n2100.armlinux.org.uk> <8448e90a-4562-b564-c160-1b5c67e0f92f@axentia.se> <0cbee3bd-8987-5f2f-519d-f8d1b426f2a3@ti.com> <20180424170625.GL16141@n2100.armlinux.org.uk> <20180424232523.GN16141@n2100.armlinux.org.uk> <7e8cdcf6-c816-1e01-24f1-a526fb0a591f@ti.com> <20180706100346.GV17271@n2100.armlinux.org.uk> <20180706124305.GX17271@n2100.armlinux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20180706124305.GX17271@n2100.armlinux.org.uk> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Ping - any comments from anyone on this idea? On Fri, Jul 06, 2018 at 01:43:05PM +0100, Russell King - ARM Linux wrote: > On Fri, Jul 06, 2018 at 11:03:46AM +0100, Russell King - ARM Linux wrote: > > On Wed, Apr 25, 2018 at 11:01:15PM +0300, Jyri Sarha wrote: > > > Oh yes. But in this case the substandard solution is already there and > > > it is already widely used, despite it being severely broken. I am merely > > > trying to fix the existing substandard solution. > > > > > > I admit that a full integration with component helpers would probably be > > > more elegant solution to the problem, but the amount of work is just too > > > much. The change would impact the way all the master drm drivers pull > > > them selves together. The drivers that already use the component helpers > > > for some internal stuff will add their own challenge. Separate component > > > matching implementations are needed for device-tree and ACPI (are ther > > > more flavors?) etc. I just do not see this happening any time soon (am > > > happy to be wrong about this). > > > > The issue is actually worse than that: > > > > - drivers that are already componentised can't use bridges > > - drivers that use bridges can't use componentised stuff > > > > because bridges don't register themselves with the component helper, > > and the helpers in drm_of.c assume that all graph nodes will be > > components. > > > > The whole thing about whether stuff is componentised or bridge based > > is really getting out of hand, and the push is towards bridge based > > stuff even though that is technically inferior when it comes to being > > able to develop and test (which involves being able to remove and > > re-insert modules.) > > > > Consequently more and more code is being written for bridges, and > > the component helper ignored, and the problems with bridges are > > being ignored. This is not healthy. > > > > The problem is only going to get worse. Someone needs to bite the > > bullet and fix bridges before the problem gets any more out of hand. > > This patch (which is actually two patches locally) allows the component > helper to know what's going on inside the bridge code wrt bridge > availability, and takes the appropriate action at the correct time. > No need for device links or similar, or incompatibilities between > bridges and components. The only requirement is that bridges set the > "device" member of struct drm_bridge to opt-in to this. > > Tested with Armada converted to support bridges, TDA998x as a > componentised bridge, and dumb-vga-dac as a non-componentised bridge: > > root@cubox:~# less /sys/kernel/debug/device_component/display-subsystem > master name status > ------------------------------------------------------------- > display-subsystem bound > > device name status > ------------------------------------------------------------- > port registered > port registered > hdmi-encoder registered > vga-bridge registered > root@cubox:~# dmesg |grep bound > [ 1.921798] armada-drm display-subsystem: bound f1820000.lcd-controller (ops > armada_lcd_ops) > [ 1.931014] armada-drm display-subsystem: bound f1810000.lcd-controller (ops > armada_lcd_ops) > [ 2.069231] armada-drm display-subsystem: bound 1-0070 (ops tda998x_ops) > [ 2.076059] armada-drm display-subsystem: bound vga-bridge (ops dummy_ops) > > Without this, the same DT fails because "vga-bridge" is never added > to the component helpers. > > diff --git a/drivers/base/component.c b/drivers/base/component.c > index 8946dfee4768..b14b3a3655ea 100644 > --- a/drivers/base/component.c > +++ b/drivers/base/component.c > @@ -602,4 +602,32 @@ void component_del(struct device *dev, const struct component_ops *ops) > } > EXPORT_SYMBOL_GPL(component_del); > > +static int component_dummy_bind(struct device *comp, struct device *master, > + void *master_data) > +{ > + return 0; > +} > + > +static void component_dummy_unbind(struct device *comp, struct device *master, > + void *master_data) > +{ > +} > + > +static const struct component_ops dummy_ops = { > + .bind = component_dummy_bind, > + .unbind = component_dummy_unbind, > +}; > + > +int component_mark_available(struct device *dev) > +{ > + return component_add(dev, &dummy_ops); > +} > +EXPORT_SYMBOL_GPL(component_mark_available); > + > +void component_mark_unavailable(struct device *dev) > +{ > + component_del(dev, &dummy_ops); > +} > +EXPORT_SYMBOL_GPL(component_mark_unavailable); > + > MODULE_LICENSE("GPL v2"); > diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c > index 1638bfe9627c..ce3ccd327916 100644 > --- a/drivers/gpu/drm/drm_bridge.c > +++ b/drivers/gpu/drm/drm_bridge.c > @@ -21,6 +21,7 @@ > * DEALINGS IN THE SOFTWARE. > */ > > +#include > #include > #include > #include > @@ -73,6 +74,9 @@ void drm_bridge_add(struct drm_bridge *bridge) > mutex_lock(&bridge_lock); > list_add_tail(&bridge->list, &bridge_list); > mutex_unlock(&bridge_lock); > + > + if (bridge->device) > + WARN_ON(component_mark_available(bridge->device)); > } > EXPORT_SYMBOL(drm_bridge_add); > > @@ -83,6 +87,9 @@ EXPORT_SYMBOL(drm_bridge_add); > */ > void drm_bridge_remove(struct drm_bridge *bridge) > { > + if (bridge->device) > + component_mark_unavailable(bridge->device); > + > mutex_lock(&bridge_lock); > list_del_init(&bridge->list); > mutex_unlock(&bridge_lock); > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h > index 3270fec46979..e863da14d4d9 100644 > --- a/include/drm/drm_bridge.h > +++ b/include/drm/drm_bridge.h > @@ -268,6 +268,7 @@ struct drm_bridge { > struct drm_device *dev; > struct drm_encoder *encoder; > struct drm_bridge *next; > + struct device *device; > #ifdef CONFIG_OF > struct device_node *of_node; > #endif > diff --git a/include/linux/component.h b/include/linux/component.h > index e71fbbbc74e2..a1c824485f54 100644 > --- a/include/linux/component.h > +++ b/include/linux/component.h > @@ -16,6 +16,10 @@ struct component_ops { > int component_add(struct device *, const struct component_ops *); > void component_del(struct device *, const struct component_ops *); > > +/* For subsystems where drivers do not call component_add()/component_del() */ > +int component_mark_available(struct device *dev); > +void component_mark_unavailable(struct device *dev); > + > int component_bind_all(struct device *master, void *master_data); > void component_unbind_all(struct device *master, void *master_data); > > > > -- > RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ > FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up > According to speedtest.net: 13Mbps down 490kbps up -- RMK's Patch system: http://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 13.8Mbps down 630kbps up According to speedtest.net: 13Mbps down 490kbps up