| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CANMq1KBYJLqAahTq3xddr2Y05KMNqb=K00uhw6KHHaOWFPuaoA@mail.gmail.com>
Date: Wed, 6 Jul 2016 10:10:13 +0800
From: Nicolas Boichat <drinkcat@...omium.org>
To: Archit Taneja <architt@...eaurora.org>
Cc: dri-devel@...ts.freedesktop.org,
Thierry Reding <treding@...dia.com>,
Russell King <rmk+kernel@....linux.org.uk>,
linux-kernel@...r.kernel.org,
Philipp Zabel <p.zabel@...gutronix.de>,
Stéphane Marchesin <marcheu@...omium.org>
Subject: Re: [RFC PATCH 3/4] drm: bridge: Generic GPIO mux driver
(whoops, apologies for the CHROMIUM tag in the subject line...)
On Tue, Jun 28, 2016 at 4:52 PM, Archit Taneja <architt@...eaurora.org> wrote:
>
>
> On 06/27/2016 01:18 PM, Nicolas Boichat wrote:
>>
>> This driver supports single input, 2 output display mux (e.g.
>> HDMI mux), that provides its status via a GPIO.
>
>
> This might not work if we had a 2 or more bridges connected
> one after the other at the output ports. It would be nicer
> if the interrupt handler directly modified the drm_bridge's
> next pointer rather than managing things by its own.
I don't think we can directly change ->next as different functions
need different handling. For example, pre_enable should be called on
all possible downstream bridges, as we cannot know in advance which
bridge should be ready. So does attach, but I think that's easier to
solve.
> That being said, the bridge chains (by setting the next
> pointers) aren't expected to change after they are set up.
> In order to use make this a truly generic driver, we'd
> probably need to add some sort of locking for the entire
> bridge chain to make sure we don't change things in the
> middle of a modeset. We don't really need to add this
> functionality unless there are many more platforms like
> these have non-static muxes in the display chain.
Agree with that. Also, since our downstream bridges (HDMI connector,
so no bridge, and anx7688) don't have enable/disable callbacks, we are
actually using a simpler version of this driver that just leaves all
downstream bridges enabled.
> It would be better if this driver was considered to be
> used only for the mux hardware that's used on the board.
>
> Archit
>
>
>>
>> Signed-off-by: Nicolas Boichat <drinkcat@...omium.org>
>> ---
>> drivers/gpu/drm/bridge/Kconfig | 11 +
>> drivers/gpu/drm/bridge/Makefile | 1 +
>> drivers/gpu/drm/bridge/generic-gpio-mux.c | 347
>> ++++++++++++++++++++++++++++++
>> 3 files changed, 359 insertions(+)
>> create mode 100644 drivers/gpu/drm/bridge/generic-gpio-mux.c
>>
>> diff --git a/drivers/gpu/drm/bridge/Kconfig
>> b/drivers/gpu/drm/bridge/Kconfig
>> index da489f0..f1f6fc6 100644
>> --- a/drivers/gpu/drm/bridge/Kconfig
>> +++ b/drivers/gpu/drm/bridge/Kconfig
>> @@ -41,6 +41,17 @@ config DRM_DW_HDMI_AHB_AUDIO
>> Designware HDMI block. This is used in conjunction with
>> the i.MX6 HDMI driver.
>>
>> +config DRM_GENERIC_GPIO_MUX
>> + tristate "Generic GPIO-controlled mux"
>> + depends on DRM
>> + depends on OF
>> + select DRM_KMS_HELPER
>> + ---help---
>> + This bridge driver models a GPIO-controlled display mux with one
>> + input, 2 outputs (e.g. an HDMI mux). The hardware decides which
>> output
>> + is active, reports it as a GPIO, and the driver redirects calls
>> to the
>> + appropriate downstream bridge (if any).
>> +
>> config DRM_NXP_PTN3460
>> tristate "NXP PTN3460 DP/LVDS bridge"
>> depends on OF
>> diff --git a/drivers/gpu/drm/bridge/Makefile
>> b/drivers/gpu/drm/bridge/Makefile
>> index 4846465..cb97274fd 100644
>> --- a/drivers/gpu/drm/bridge/Makefile
>> +++ b/drivers/gpu/drm/bridge/Makefile
>> @@ -4,6 +4,7 @@ obj-$(CONFIG_DRM_ANALOGIX_ANX7688) += analogix-anx7688.o
>> obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
>> obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>> obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>> +obj-$(CONFIG_DRM_GENERIC_GPIO_MUX) += generic-gpio-mux.o
>> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
>> obj-$(CONFIG_DRM_SII902X) += sii902x.o
>> diff --git a/drivers/gpu/drm/bridge/generic-gpio-mux.c
>> b/drivers/gpu/drm/bridge/generic-gpio-mux.c
>> new file mode 100644
>> index 0000000..d3367e2
>> --- /dev/null
>> +++ b/drivers/gpu/drm/bridge/generic-gpio-mux.c
>> @@ -0,0 +1,347 @@
>> +/*
>> + * ANX7688 HDMI->DP bridge driver
>> + *
>> + * Copyright (C) 2016 Google, Inc.
>> + *
>> + * This software is licensed under the terms of the GNU General Public
>> + * License version 2, as published by the Free Software Foundation, and
>> + * may be copied, distributed, and modified under those terms.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/gpio.h>
>> +#include <linux/interrupt.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_gpio.h>
>> +#include <linux/of_graph.h>
>> +#include <drm/drm_crtc.h>
>> +#include <drm/drm_crtc_helper.h>
>> +
>> +struct gpio_display_mux {
>> + struct device *dev;
>> +
>> + struct gpio_desc *gpiod_detect;
>> + int detect_irq;
>> +
>> + struct drm_bridge bridge;
>> +
>> + struct drm_bridge *next[2];
>> +
>> + struct mutex lock;
>> + int active;
>> + bool enabled;
>> +};
>> +
>> +static inline struct gpio_display_mux *bridge_to_gpio_display_mux(
>> + struct drm_bridge *bridge)
>> +{
>> + return container_of(bridge, struct gpio_display_mux, bridge);
>> +}
>> +
>> +static irqreturn_t gpio_display_mux_det_threaded_handler(int unused, void
>> *data)
>> +{
>> + struct gpio_display_mux *gpio_display_mux = data;
>> + struct drm_bridge *next;
>> + int active;
>> +
>> + active = gpiod_get_value(gpio_display_mux->gpiod_detect);
>> +
>> + dev_dbg(gpio_display_mux->dev, "Interrupt %d!\n", active);
>> +
>> + if (active == gpio_display_mux->active)
>> + return IRQ_HANDLED;
>> +
>> + /* Disable previous bridge */
>> + mutex_lock(&gpio_display_mux->lock);
>> + if (gpio_display_mux->enabled) {
>> + next = gpio_display_mux->next[gpio_display_mux->active];
>> + if (next && next->funcs->disable)
>> + next->funcs->disable(next);
>> + }
>> + mutex_unlock(&gpio_display_mux->lock);
>> +
>> + if (gpio_display_mux->bridge.dev)
>> +
>> drm_kms_helper_hotplug_event(gpio_display_mux->bridge.dev);
>> +
>> + /* Enable current bridge */
>> + mutex_lock(&gpio_display_mux->lock);
>> + if (gpio_display_mux->enabled) {
>> + next = gpio_display_mux->next[active];
>> + if (next && next->funcs->enable)
>> + next->funcs->enable(next);
>> + }
>> + gpio_display_mux->active = active;
>> + mutex_unlock(&gpio_display_mux->lock);
>> +
>> + return IRQ_HANDLED;
>> +}
>> +
>> +static int gpio_display_mux_attach(struct drm_bridge *bridge)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(gpio_display_mux->next); i++) {
>> + next = gpio_display_mux->next[i];
>> + if (next)
>> + next->encoder = bridge->encoder;
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static bool gpio_display_mux_mode_fixup(struct drm_bridge *bridge,
>> + const struct drm_display_mode *mode,
>> + struct drm_display_mode *adjusted_mode)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> +
>> + next = gpio_display_mux->next[gpio_display_mux->active];
>> +
>> + if (next && next->funcs->mode_fixup)
>> + return next->funcs->mode_fixup(next, mode, adjusted_mode);
>> + else
>> + return true;
>> +}
>> +
>> +static void gpio_display_mux_mode_set(struct drm_bridge *bridge,
>> + struct drm_display_mode *mode,
>> + struct drm_display_mode *adjusted_mode)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> +
>> + next = gpio_display_mux->next[gpio_display_mux->active];
>> +
>> + if (next && next->funcs->mode_set)
>> + next->funcs->mode_set(next, mode, adjusted_mode);
>> +}
>> +
>> +static void gpio_display_mux_enable(struct drm_bridge *bridge)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> +
>> + mutex_lock(&gpio_display_mux->lock);
>> +
>> + next = gpio_display_mux->next[gpio_display_mux->active];
>> + if (next && next->funcs->enable)
>> + next->funcs->enable(next);
>> +
>> + gpio_display_mux->enabled = true;
>> +
>> + mutex_unlock(&gpio_display_mux->lock);
>> +}
>> +
>> +static void gpio_display_mux_disable(struct drm_bridge *bridge)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> +
>> + mutex_lock(&gpio_display_mux->lock);
>> +
>> + next = gpio_display_mux->next[gpio_display_mux->active];
>> + if (next && next->funcs->disable)
>> + next->funcs->disable(next);
>> +
>> + gpio_display_mux->enabled = false;
>> +
>> + mutex_unlock(&gpio_display_mux->lock);
>> +}
>> +
>> +/**
>> + * Since this driver _reacts_ to mux changes, we need to make sure all
>> + * downstream bridges are pre-enabled.
>> + */
>> +static void gpio_display_mux_pre_enable(struct drm_bridge *bridge)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(gpio_display_mux->next); i++) {
>> + next = gpio_display_mux->next[i];
>> + if (next && next->funcs->pre_enable)
>> + next->funcs->pre_enable(next);
>> + }
>> +}
>> +
>> +static void gpio_display_mux_post_disable(struct drm_bridge *bridge)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> + bridge_to_gpio_display_mux(bridge);
>> + struct drm_bridge *next;
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(gpio_display_mux->next); i++) {
>> + next = gpio_display_mux->next[i];
>> + if (next && next->funcs->post_disable)
>> + next->funcs->post_disable(next);
>> + }
>> +}
>> +
>> +static const struct drm_bridge_funcs gpio_display_mux_bridge_funcs = {
>> + .attach = gpio_display_mux_attach,
>> + .mode_fixup = gpio_display_mux_mode_fixup,
>> + .disable = gpio_display_mux_disable,
>> + .post_disable = gpio_display_mux_post_disable,
>> + .mode_set = gpio_display_mux_mode_set,
>> + .pre_enable = gpio_display_mux_pre_enable,
>> + .enable = gpio_display_mux_enable,
>> +};
>> +
>> +static int gpio_display_mux_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct gpio_display_mux *gpio_display_mux;
>> + struct device_node *port, *ep, *remote;
>> + int ret;
>> + u32 reg;
>> +
>> + gpio_display_mux = devm_kzalloc(dev, sizeof(*gpio_display_mux),
>> + GFP_KERNEL);
>> + if (!gpio_display_mux)
>> + return -ENOMEM;
>> +
>> + mutex_init(&gpio_display_mux->lock);
>> +
>> + platform_set_drvdata(pdev, gpio_display_mux);
>> + gpio_display_mux->dev = &pdev->dev;
>> +
>> + gpio_display_mux->bridge.of_node = dev->of_node;
>> +
>> + gpio_display_mux->gpiod_detect =
>> + devm_gpiod_get(dev, "detect", GPIOD_IN);
>> + if (IS_ERR(gpio_display_mux->gpiod_detect))
>> + return PTR_ERR(gpio_display_mux->gpiod_detect);
>> +
>> + gpio_display_mux->detect_irq =
>> + gpiod_to_irq(gpio_display_mux->gpiod_detect);
>> + if (gpio_display_mux->detect_irq < 0) {
>> + dev_err(dev, "Failed to get output irq %d\n",
>> + gpio_display_mux->detect_irq);
>> + return -ENODEV;
>> + }
>> +
>> + port = of_graph_get_port_by_id(dev->of_node, 1);
>> + if (!port) {
>> + dev_err(dev, "Missing output port node\n");
>> + return -EINVAL;
>> + }
>> +
>> + for_each_child_of_node(port, ep) {
>> + if (!ep->name || (of_node_cmp(ep->name, "endpoint") != 0))
>> {
>> + of_node_put(ep);
>> + continue;
>> + }
>> +
>> + if (of_property_read_u32(ep, "reg", ®) < 0 ||
>> + reg >= ARRAY_SIZE(gpio_display_mux->next))
>> {
>> + dev_err(dev,
>> + "Missing/invalid reg property for endpoint
>> %s\n",
>> + ep->full_name);
>> + of_node_put(ep);
>> + of_node_put(port);
>> + return -EINVAL;
>> + }
>> +
>> + remote = of_graph_get_remote_port_parent(ep);
>> + if (!remote) {
>> + dev_err(dev,
>> + "Missing connector/bridge node for endpoint
>> %s\n",
>> + ep->full_name);
>> + of_node_put(ep);
>> + of_node_put(port);
>> + return -EINVAL;
>> + }
>> + of_node_put(ep);
>> +
>> + if (of_device_is_compatible(remote, "hdmi-connector")) {
>> + of_node_put(remote);
>> + continue;
>> + }
>> +
>> + gpio_display_mux->next[reg] = of_drm_find_bridge(remote);
>> + if (!gpio_display_mux->next[reg]) {
>> + dev_err(dev, "Waiting for external bridge %s\n",
>> + remote->name);
>> + of_node_put(remote);
>> + of_node_put(port);
>> + return -EPROBE_DEFER;
>> + }
>> +
>> + of_node_put(remote);
>> + }
>> + of_node_put(port);
>> +
>> + gpio_display_mux->bridge.funcs = &gpio_display_mux_bridge_funcs;
>> + ret = drm_bridge_add(&gpio_display_mux->bridge);
>> + if (ret < 0) {
>> + dev_err(dev, "Failed to add drm bridge\n");
>> + return ret;
>> + }
>> +
>> + ret = devm_request_threaded_irq(dev, gpio_display_mux->detect_irq,
>> + NULL,
>> + gpio_display_mux_det_threaded_handler,
>> + IRQF_TRIGGER_RISING | IRQF_TRIGGER_FALLING
>> |
>> + IRQF_ONESHOT,
>> + "gpio-display-mux-det", gpio_display_mux);
>> + if (ret) {
>> + dev_err(dev, "Failed to request MUX_DET threaded irq\n");
>> + goto err_bridge_remove;
>> + }
>> +
>> + gpio_display_mux->active =
>> + gpiod_get_value(gpio_display_mux->gpiod_detect);
>> +
>> + return 0;
>> +
>> +err_bridge_remove:
>> + drm_bridge_remove(&gpio_display_mux->bridge);
>> +
>> + return ret;
>> +}
>> +
>> +static int gpio_display_mux_remove(struct platform_device *pdev)
>> +{
>> + struct gpio_display_mux *gpio_display_mux =
>> platform_get_drvdata(pdev);
>> +
>> + drm_bridge_remove(&gpio_display_mux->bridge);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id gpio_display_mux_match[] = {
>> + { .compatible = "gpio-display-mux", },
>> + {},
>> +};
>> +
>> +struct platform_driver gpio_display_mux_driver = {
>> + .probe = gpio_display_mux_probe,
>> + .remove = gpio_display_mux_remove,
>> + .driver = {
>> + .name = "gpio-display-mux",
>> + .of_match_table = gpio_display_mux_match,
>> + },
>> +};
>> +
>> +module_platform_driver(gpio_display_mux_driver);
>> +
>> +MODULE_DESCRIPTION("GPIO-controlled display mux");
>> +MODULE_AUTHOR("Nicolas Boichat <drinkcat@...omium.org>");
>> +MODULE_LICENSE("GPL v2");
>>
>
> --
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
> a Linux Foundation Collaborative Project
Powered by blists - more mailing lists