From patchwork Fri Apr 20 12:00:16 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Russell King (Oracle)" X-Patchwork-Id: 10352499 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 31A326023A for ; Fri, 20 Apr 2018 12:01:14 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1C4B628405 for ; Fri, 20 Apr 2018 12:01:14 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1038F286F7; Fri, 20 Apr 2018 12:01:14 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-2.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,MAILING_LIST_MULTI autolearn=unavailable version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 46E6928405 for ; Fri, 20 Apr 2018 12:01:13 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20170209; h=Sender: Content-Transfer-Encoding:Content-Type:Cc:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=NB/68CJba0AbNZHVR043z0NN50a8HpqakjcYMd3Alfs=; b=dnpHfp62CTuNO9 oN0squIVqS1ITlOYQ779+FhBq9h4yuzRCssJMmWRsA3Q6Bx7Owdls1IqmHBy+yj6e571q8R8JayE0 /fs9cFij27ASQBWDtdK1bx8ZkRUk6TAVdFnpZmX0b4jERdfAVWWe8B6swkThXB6+Wuag53OuqNJG5 PUWiroqwKKyFu0MiJDNCSTmwAK4nDQSXH9b9/nHlKNck+gj28cF7QZoFr/9/LHW5dBO1yxFPweyxP /IZq6LQIueEbJrIRdye09N+NkuP+CM31VgxWg+wZjqkV+4FnAwSqxrgPzszYNvuYhSLKZVwAxNRvT shsd/ABr86X9K+jl+CAA==; Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.90_1 #2 (Red Hat Linux)) id 1f9Uiu-0001vA-9S; Fri, 20 Apr 2018 12:01:04 +0000 Received: from pandora.armlinux.org.uk ([2001:4d48:ad52:3201:214:fdff:fe10:1be6]) by bombadil.infradead.org with esmtps (Exim 4.90_1 #2 (Red Hat Linux)) id 1f9Uip-0001tA-GA for linux-arm-kernel@lists.infradead.org; Fri, 20 Apr 2018 12:01:02 +0000 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=8pP/lfv/ZEkA5sV92T/tr4xNKPPyDPcMAn3aerxXYrU=; b=MWiQt8Wutb5IdAObYx1QxVvn/ 2YjO6Wu3Pv4cnTlQy9kuztRzDnYs8rjLbyExO8jcc+mPXbYDwwBc0rMKZ9dkDJeCmMjxsJ+ADY6UK Y0mmJZaaMpeO0/YwNMjgGew7wZegzdB97gB2skVR3L7TqYiyPci9WXmYOXGdzLfHvYJew=; Received: from n2100.armlinux.org.uk ([2002:4e20:1eda:1:214:fdff:fe10:4f86]:45595) by pandora.armlinux.org.uk with esmtpsa (TLSv1.2:ECDHE-RSA-AES128-GCM-SHA256:128) (Exim 4.90_1) (envelope-from ) id 1f9UiD-0005CE-Fg; Fri, 20 Apr 2018 13:00:21 +0100 Received: from linux by n2100.armlinux.org.uk with local (Exim 4.90_1) (envelope-from ) id 1f9Ui9-0000xk-Tc; Fri, 20 Apr 2018 13:00:18 +0100 Date: Fri, 20 Apr 2018 13:00:16 +0100 From: Russell King - ARM Linux To: Peter Rosin Subject: Re: [PATCH v3 7/7] drm/i2c: tda998x: register as a drm bridge Message-ID: <20180420120015.GY16141@n2100.armlinux.org.uk> References: <20180419162751.25223-1-peda@axentia.se> <20180419162751.25223-8-peda@axentia.se> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20180419162751.25223-8-peda@axentia.se> User-Agent: Mutt/1.5.23 (2014-03-12) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20180420_050059_981490_7EF05676 X-CRM114-Status: GOOD ( 30.82 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Mark Rutland , Boris Brezillon , Alexandre Belloni , devicetree@vger.kernel.org, David Airlie , Gustavo Padovan , linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org, Rob Herring , Sean Paul , Laurent Pinchart , Daniel Vetter , Jacopo Mondi , linux-arm-kernel@lists.infradead.org Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Apr 19, 2018 at 06:27:51PM +0200, Peter Rosin wrote: > This makes this driver work with all(?) drivers that are not > componentized and instead expect to connect to a panel/bridge. That > said, the only one tested is atmel_hlcdc. > > This hooks the relevant work function previously called by the encoder > and the component also to the bridge, since the encoder goes away when > connecting to the bridge interface of the driver and the equivalent of > bind/unbind of the component is handled by bridge attach/detach. > > The lifetime requirements of a bridge and a component are slightly > different, which is the reason for struct tda998x_bridge. As we are talking about bridge stuff, the patch below is what I've had for a while converting Armada to be able to use a bridge-based tda998x. As you can see, it's far from satisfactory at the moment. Specifically: 1) it assumes all bridges are TMDS bridges, because as far as I can see, there's no way to know any different. 2) there's no way to really know whether a missing bridge is because we should be using the component helpers or that the bridge device doesn't exist yet. diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c index 262409cae8bf..854d74466dec 100644 --- a/drivers/gpu/drm/armada/armada_drv.c +++ b/drivers/gpu/drm/armada/armada_drv.c @@ -20,6 +20,127 @@ #include #include "armada_ioctlP.h" +static const struct drm_encoder_helper_funcs dummy_encoder_helper_funcs = { +}; + +static const struct drm_encoder_funcs dummy_encoder_funcs = { + .destroy = drm_encoder_cleanup, +}; + +static int dummy_encoder_add(struct device *dev, struct drm_device *drm, + struct drm_bridge *bridge, int type, u32 crtcs) +{ + struct drm_encoder *enc; + int ret; + + enc = devm_kzalloc(dev, sizeof(*enc), GFP_KERNEL); + if (!enc) + return -ENOMEM; + + drm_encoder_helper_add(enc, &dummy_encoder_helper_funcs); + ret = drm_encoder_init(drm, enc, &dummy_encoder_funcs, type, NULL); + if (ret) + return ret; + + enc->possible_crtcs = crtcs; + enc->bridge = bridge; + bridge->encoder = enc; + + ret = drm_bridge_attach(enc, bridge, NULL); + if (ret) + dev_err(dev, "drm_bridge_attach() failed: %d\n", ret); + + return ret; +} + +static int hack_of_add_crtc_encoders(struct drm_device *drm, + struct device_node *port) +{ + struct device *dev = drm->dev; + struct device_node *ep, *remote; + struct drm_bridge *bridge; + u32 crtcs; + int ret; + + for_each_child_of_node(port, ep) { + remote = of_graph_get_remote_port_parent(ep); + if (!remote || !of_device_is_available(remote) || + !of_device_is_available(remote->parent)) { + of_node_put(remote); + continue; + } + + bridge = of_drm_find_bridge(remote); +dev_info(dev, "found remote %s => bridge %p\n", of_node_full_name(remote), bridge); + if (!bridge) { + of_node_put(remote); + continue; + } + + crtcs = drm_of_find_possible_crtcs(drm, remote); + /* If no CRTCs were found, fall back to our old behaviour */ + if (crtcs == 0) { + dev_warn(dev, "Falling back to first CRTC\n"); + crtcs = 1 << 0; + } + + ret = dummy_encoder_add(dev, drm, bridge, DRM_MODE_ENCODER_TMDS, + crtcs); + if (ret) { + dev_err(dev, "drm_bridge_attach() failed: %d\n", ret); + of_node_put(ep); + return ret; + } + } + + return 0; +} + +static int hack_create_encoders(struct drm_device *drm) +{ + struct device *dev = drm->dev; + struct device_node *port = NULL; + int i, ret = 0; + + if (dev->of_node) { + for (i = 0; ; i++) { + port = of_parse_phandle(dev->of_node, "ports", i); + if (!port) + break; + + if (of_device_is_available(port->parent)) + ret = hack_of_add_crtc_encoders(drm, port); + + of_node_put(port); + + if (ret) + break; + } + } else if (dev->platform_data) { + const char **devices = dev->platform_data; + struct device *d; + + for (i = 0; devices[i]; i++) { + d = bus_find_device_by_name(&platform_bus_type, NULL, + devices[i]); + if (d && d->of_node) { + for_each_child_of_node(d->of_node, port) { + ret = hack_of_add_crtc_encoders(drm, port); + if (ret) { + of_node_put(port); + break; + } + } + } + put_device(d); + if (ret) + break; + } + } + + return ret; +} + static void armada_drm_unref_work(struct work_struct *work) { struct armada_private *priv = @@ -153,6 +274,10 @@ static int armada_drm_bind(struct device *dev) if (ret) goto err_kms; + ret = hack_create_encoders(&priv->drm); + if (ret) + goto err_comp; + ret = drm_vblank_init(&priv->drm, priv->drm.mode_config.num_crtc); if (ret) goto err_comp; diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c b/drivers/gpu/drm/i2c/tda998x_drv.c index b78c0627e7cf..feb6debb1563 100644 --- a/drivers/gpu/drm/i2c/tda998x_drv.c +++ b/drivers/gpu/drm/i2c/tda998x_drv.c @@ -68,7 +68,7 @@ struct tda998x_priv { wait_queue_head_t edid_delay_waitq; bool edid_delay_active; - struct drm_encoder encoder; + struct drm_bridge bridge; struct drm_connector connector; struct tda998x_audio_port audio_port[2]; @@ -80,8 +80,8 @@ struct tda998x_priv { #define conn_to_tda998x_priv(x) \ container_of(x, struct tda998x_priv, connector) -#define enc_to_tda998x_priv(x) \ - container_of(x, struct tda998x_priv, encoder) +#define bridge_to_tda998x_priv(x) \ + container_of(x, struct tda998x_priv, bridge) /* The TDA9988 series of devices use a paged register scheme.. to simplify * things we encode the page # in upper bits of the register #. To read/ @@ -753,7 +753,7 @@ static void tda998x_detect_work(struct work_struct *work) { struct tda998x_priv *priv = container_of(work, struct tda998x_priv, detect_work); - struct drm_device *dev = priv->encoder.dev; + struct drm_device *dev = priv->connector.dev; if (dev) drm_kms_helper_hotplug_event(dev); @@ -1262,7 +1262,7 @@ tda998x_connector_best_encoder(struct drm_connector *connector) { struct tda998x_priv *priv = conn_to_tda998x_priv(connector); - return &priv->encoder; + return priv->bridge.encoder; } static @@ -1292,36 +1292,27 @@ static int tda998x_connector_init(struct tda998x_priv *priv, if (ret) return ret; - drm_mode_connector_attach_encoder(&priv->connector, &priv->encoder); + drm_mode_connector_attach_encoder(&priv->connector, + priv->bridge.encoder); return 0; } -/* DRM encoder functions */ +/* DRM bridge functions */ -static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) +static int tda998x_bridge_attach(struct drm_bridge *bridge) { - struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); - bool on; + struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); + struct drm_device *drm = bridge->dev; - /* we only care about on or off: */ - on = mode == DRM_MODE_DPMS_ON; - - if (on == priv->is_on) - return; + return tda998x_connector_init(priv, drm); +} - if (on) { - /* enable video ports, audio will be enabled later */ - reg_write(priv, REG_ENA_VP_0, 0xff); - reg_write(priv, REG_ENA_VP_1, 0xff); - reg_write(priv, REG_ENA_VP_2, 0xff); - /* set muxing after enabling ports: */ - reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0); - reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1); - reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2); +static void tda998x_bridge_disable(struct drm_bridge *bridge) +{ + struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); - priv->is_on = true; - } else { + if (priv->is_on) { /* disable video ports */ reg_write(priv, REG_ENA_VP_0, 0x00); reg_write(priv, REG_ENA_VP_1, 0x00); @@ -1332,11 +1323,11 @@ static void tda998x_encoder_dpms(struct drm_encoder *encoder, int mode) } static void -tda998x_encoder_mode_set(struct drm_encoder *encoder, - struct drm_display_mode *mode, - struct drm_display_mode *adjusted_mode) +tda998x_bridge_mode_set(struct drm_bridge *bridge, + struct drm_display_mode *mode, + struct drm_display_mode *adjusted_mode) { - struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); + struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); u16 ref_pix, ref_line, n_pix, n_line; u16 hs_pix_s, hs_pix_e; u16 vs1_pix_s, vs1_pix_e, vs1_line_s, vs1_line_e; @@ -1543,6 +1534,31 @@ tda998x_encoder_mode_set(struct drm_encoder *encoder, mutex_unlock(&priv->audio_mutex); } +static void tda998x_bridge_enable(struct drm_bridge *bridge) +{ + struct tda998x_priv *priv = bridge_to_tda998x_priv(bridge); + + if (!priv->is_on) { + /* enable video ports, audio will be enabled later */ + reg_write(priv, REG_ENA_VP_0, 0xff); + reg_write(priv, REG_ENA_VP_1, 0xff); + reg_write(priv, REG_ENA_VP_2, 0xff); + /* set muxing after enabling ports: */ + reg_write(priv, REG_VIP_CNTRL_0, priv->vip_cntrl_0); + reg_write(priv, REG_VIP_CNTRL_1, priv->vip_cntrl_1); + reg_write(priv, REG_VIP_CNTRL_2, priv->vip_cntrl_2); + + priv->is_on = true; + } +} + +static const struct drm_bridge_funcs tda998x_bridge_funcs = { + .attach = tda998x_bridge_attach, + .disable = tda998x_bridge_disable, + .mode_set = tda998x_bridge_mode_set, + .enable = tda998x_bridge_enable, +}; + static void tda998x_destroy(struct tda998x_priv *priv) { /* disable all IRQs and free the IRQ handler */ @@ -1796,35 +1812,6 @@ static int tda998x_create(struct i2c_client *client, struct tda998x_priv *priv) return ret; } -static void tda998x_encoder_prepare(struct drm_encoder *encoder) -{ - tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_OFF); -} - -static void tda998x_encoder_commit(struct drm_encoder *encoder) -{ - tda998x_encoder_dpms(encoder, DRM_MODE_DPMS_ON); -} - -static const struct drm_encoder_helper_funcs tda998x_encoder_helper_funcs = { - .dpms = tda998x_encoder_dpms, - .prepare = tda998x_encoder_prepare, - .commit = tda998x_encoder_commit, - .mode_set = tda998x_encoder_mode_set, -}; - -static void tda998x_encoder_destroy(struct drm_encoder *encoder) -{ - struct tda998x_priv *priv = enc_to_tda998x_priv(encoder); - - tda998x_destroy(priv); - drm_encoder_cleanup(encoder); -} - -static const struct drm_encoder_funcs tda998x_encoder_funcs = { - .destroy = tda998x_encoder_destroy, -}; - static void tda998x_set_config(struct tda998x_priv *priv, const struct tda998x_encoder_params *p) { @@ -1882,9 +1869,7 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) { struct tda998x_encoder_params *params = dev->platform_data; struct i2c_client *client = to_i2c_client(dev); - struct drm_device *drm = data; struct tda998x_priv *priv; - u32 crtcs = 0; int ret; priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL); @@ -1893,16 +1878,6 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) dev_set_drvdata(dev, priv); - if (dev->of_node) - crtcs = drm_of_find_possible_crtcs(drm, dev->of_node); - - /* If no CRTCs were found, fall back to our old behaviour */ - if (crtcs == 0) { - dev_warn(dev, "Falling back to first CRTC\n"); - crtcs = 1 << 0; - } - - priv->encoder.possible_crtcs = crtcs; priv->audio_params.config = BIT(2); priv->audio_params.format = AFMT_SPDIF; priv->audio_params.sample_rate = 44100; @@ -1925,23 +1900,12 @@ static int tda998x_bind(struct device *dev, struct device *master, void *data) if (!dev->of_node && params) tda998x_set_config(priv, params); - drm_encoder_helper_add(&priv->encoder, &tda998x_encoder_helper_funcs); - ret = drm_encoder_init(drm, &priv->encoder, &tda998x_encoder_funcs, - DRM_MODE_ENCODER_TMDS, NULL); - if (ret) - goto err_encoder; + priv->bridge.funcs = &tda998x_bridge_funcs; + priv->bridge.of_node = dev->of_node; - ret = tda998x_connector_init(priv, drm); - if (ret) - goto err_connector; + drm_bridge_add(&priv->bridge); return 0; - -err_connector: - drm_encoder_cleanup(&priv->encoder); -err_encoder: - tda998x_destroy(priv); - return ret; } static void tda998x_unbind(struct device *dev, struct device *master, @@ -1950,7 +1914,7 @@ static void tda998x_unbind(struct device *dev, struct device *master, struct tda998x_priv *priv = dev_get_drvdata(dev); drm_connector_cleanup(&priv->connector); - drm_encoder_cleanup(&priv->encoder); + drm_bridge_remove(&priv->bridge); tda998x_destroy(priv); }