[PATCH 11/60] drm/bridge: Add driver for the TI TPD12S015 HDMI level shifter

[Tomi Valkeinen]

On 07/07/2019 21:18, Laurent Pinchart wrote:
> The TI TPD12S015 is an HDMI level shifter and ESD protector controlled
> through GPIOs. Add a DRM bridge driver for the device.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> ---
>   drivers/gpu/drm/bridge/Kconfig        |   8 +
>   drivers/gpu/drm/bridge/Makefile       |   1 +
>   drivers/gpu/drm/bridge/ti-tpd12s015.c | 204 ++++++++++++++++++++++++++
>   3 files changed, 213 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/ti-tpd12s015.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 295a62f65ef9..3928651a0819 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -159,6 +159,14 @@ config DRM_TI_SN65DSI86
>   	help
>   	  Texas Instruments SN65DSI86 DSI to eDP Bridge driver
>   
> +config DRM_TI_TPD12S015
> +	tristate "TI TPD12S015 HDMI level shifter and ESD protection"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	help
> +	  Texas Instruments TPD12S015 HDMI level shifter and ESD protection
> +	  driver.
> +
>   source "drivers/gpu/drm/bridge/analogix/Kconfig"
>   
>   source "drivers/gpu/drm/bridge/adv7511/Kconfig"
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index e5987b3aaf62..ce635651e31b 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -17,4 +17,5 @@ obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
>   obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
>   obj-$(CONFIG_DRM_TI_SN65DSI86) += ti-sn65dsi86.o
>   obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
> +obj-$(CONFIG_DRM_TI_TPD12S015) += ti-tpd12s015.o
>   obj-y += synopsys/
> diff --git a/drivers/gpu/drm/bridge/ti-tpd12s015.c b/drivers/gpu/drm/bridge/ti-tpd12s015.c
> new file mode 100644
> index 000000000000..d01f0c4133a2
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ti-tpd12s015.c
> @@ -0,0 +1,204 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/*
> + * TPD12S015 HDMI ESD protection & level shifter chip driver
> + *
> + * Copyright (C) 2013 Texas Instruments Incorporated
> + * Author: Tomi Valkeinen <tomi.valkeinen at ti.com>
> + */

You may want to mention that the driver is based on an existing omapdrm 
driver. Otherwise the above lines don't quite make sense when adding a 
new driver =).

> +
> +#include <linux/delay.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/interrupt.h>
> +#include <linux/module.h>
> +#include <linux/mutex.h>
> +#include <linux/of.h>
> +#include <linux/of_graph.h>
> +#include <linux/platform_device.h>
> +
> +#include <drm/drm_bridge.h>
> +
> +struct tpd12s015_device {
> +	struct drm_bridge bridge;
> +
> +	struct gpio_desc *ct_cp_hpd_gpio;
> +	struct gpio_desc *ls_oe_gpio;
> +	struct gpio_desc *hpd_gpio;
> +	int hpd_irq;
> +
> +	struct drm_bridge *next_bridge;
> +};
> +
> +static inline struct tpd12s015_device *to_tpd12s015(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct tpd12s015_device, bridge);
> +}
> +
> +static int tpd12s015_attach(struct drm_bridge *bridge, bool create_connector)
> +{
> +	struct tpd12s015_device *tpd = to_tpd12s015(bridge);
> +	int ret;
> +
> +	if (create_connector)
> +		return -EINVAL;
> +
> +	ret = drm_bridge_attach(bridge->encoder, tpd->next_bridge,
> +				bridge, false);
> +	if (ret < 0)
> +		return ret;
> +
> +	gpiod_set_value_cansleep(tpd->ct_cp_hpd_gpio, 1);
> +	gpiod_set_value_cansleep(tpd->ls_oe_gpio, 1);

These are fine (and as they were in the old driver), but just talking 
out loud: LS_OE is needed when talking over i2c, and CT_CP_HPD is needed 
when interested in HPD.

Not sure what kind of power-savings are possible, but at least in theory 
we could turn those off at times. At least when going to system suspend.

> +	/* DC-DC converter needs at max 300us to get to 90% of 5V. */
> +	usleep_range(300, 1000);
> +
> +	return 0;
> +}
> +
> +static void tpd12s015_detach(struct drm_bridge *bridge)
> +{
> +	struct tpd12s015_device *tpd = to_tpd12s015(bridge);
> +
> +	gpiod_set_value_cansleep(tpd->ct_cp_hpd_gpio, 0);
> +	gpiod_set_value_cansleep(tpd->ls_oe_gpio, 0);
> +}
> +
> +static enum drm_connector_status tpd12s015_detect(struct drm_bridge *bridge)
> +{
> +	struct tpd12s015_device *tpd = to_tpd12s015(bridge);
> +
> +	if (gpiod_get_value_cansleep(tpd->hpd_gpio))
> +		return connector_status_connected;
> +	else
> +		return connector_status_disconnected;
> +}
> +
> +static void tpd12s015_hpd_enable(struct drm_bridge *bridge)
> +{
> +}
> +
> +static void tpd12s015_hpd_disable(struct drm_bridge *bridge)
> +{
> +}

Shouldn't we manage the CT_HPD_GPIO here, and request the irq here? I'm 
fine with leaving that for later, though, as it may be better to keep 
this new driver as similar to the old one as possible to prevent 
regressions during this big patch series.

I guess it's fine for a bridge to do hpd_notify even if hpd_enable has 
not been called?

> +static const struct drm_bridge_funcs tpd12s015_bridge_funcs = {
> +	.attach			= tpd12s015_attach,
> +	.detach			= tpd12s015_detach,
> +	.detect			= tpd12s015_detect,
> +	.hpd_enable		= tpd12s015_hpd_enable,
> +	.hpd_disable		= tpd12s015_hpd_disable,
> +};
> +
> +static irqreturn_t tpd12s015_hpd_isr(int irq, void *data)
> +{
> +	struct tpd12s015_device *tpd = data;
> +	struct drm_bridge *bridge = &tpd->bridge;
> +
> +	drm_bridge_hpd_notify(bridge, tpd12s015_detect(bridge));
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int tpd12s015_probe(struct platform_device *pdev)
> +{
> +	struct tpd12s015_device *tpd;
> +	struct device_node *node;
> +	struct gpio_desc *gpio;
> +	int ret;
> +
> +	tpd = devm_kzalloc(&pdev->dev, sizeof(*tpd), GFP_KERNEL);
> +	if (!tpd)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, tpd);
> +
> +	tpd->bridge.funcs = &tpd12s015_bridge_funcs;
> +	tpd->bridge.of_node = pdev->dev.of_node;
> +	tpd->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> +	tpd->bridge.ops = DRM_BRIDGE_OP_DETECT;
> +
> +	/* Get the next bridge, connected to port at 1. */
> +	node = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
> +	if (!node)
> +		return -ENODEV;
> +
> +	tpd->next_bridge = of_drm_find_bridge(node);
> +	of_node_put(node);
> +
> +	if (!tpd->next_bridge)
> +		return -EPROBE_DEFER;
> +
> +	/* Get the control and HPD GPIOs. */
> +	gpio = devm_gpiod_get_index_optional(&pdev->dev, NULL, 0,
> +					     GPIOD_OUT_LOW);
> +	if (IS_ERR(gpio))
> +		return PTR_ERR(gpio);
> +
> +	tpd->ct_cp_hpd_gpio = gpio;
> +
> +	gpio = devm_gpiod_get_index_optional(&pdev->dev, NULL, 1,
> +					     GPIOD_OUT_LOW);
> +	if (IS_ERR(gpio))
> +		return PTR_ERR(gpio);
> +
> +	tpd->ls_oe_gpio = gpio;
> +
> +	gpio = devm_gpiod_get_index(&pdev->dev, NULL, 2, GPIOD_IN);
> +	if (IS_ERR(gpio))
> +		return PTR_ERR(gpio);
> +
> +	tpd->hpd_gpio = gpio;
> +
> +	/* Register the IRQ if the HPD GPIO is IRQ-capable. */
> +	if (tpd->hpd_gpio)
> +		tpd->hpd_irq = gpiod_to_irq(tpd->hpd_gpio);

We always have tpd->hpd_gpio here, don't we?

  Tomi

-- 
Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki.
Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki