Message-ID: <57723AD2.8020806@codeaurora.org>
Date:	Tue, 28 Jun 2016 14:22:34 +0530
From:	Archit Taneja <architt@...eaurora.org>
To:	Nicolas Boichat <drinkcat@...omium.org>,
	dri-devel@...ts.freedesktop.org
Cc:	Thierry Reding <treding@...dia.com>,
	Russell King <rmk+kernel@....linux.org.uk>,
	linux-kernel@...r.kernel.org, p.zabel@...gutronix.de,
	marcheu@...omium.org
Subject: Re: [RFC PATCH 3/4] CHROMIUM: drm: bridge: Generic GPIO mux driver



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.

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.

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", &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