(whoops, apologies for the CHROMIUM tag in the subject line...) On Tue, Jun 28, 2016 at 4:52 PM, Archit Taneja <architt@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxx> >> --- >> 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@xxxxxxxxxxxx>"); >> +MODULE_LICENSE("GPL v2"); >> > > -- > Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, > a Linux Foundation Collaborative Project _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel