| Message ID | 20190707181937.6250-7-laurent.pinchart@ideasonboard.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/omap: Replace custom display drivers with drm_bridge and drm_panel | expand |
Hi Laurent. Two small details, see below. Sam On Sun, Jul 07, 2019 at 09:18:47PM +0300, Laurent Pinchart wrote: > Display connectors are modelled in DT as a device node, but have so far > been handled manually in several bridge drivers. This resulted in > duplicate code in several bridge drivers, with slightly different (and > thus confusing) logics. > > In order to fix this, implement a bridge driver for display connectors. > The driver centralises logic for the DVI, HDMI, VGAn composite and > S-video connectors and exposes corresponding bridge operations. > > This driver in itself doesn't solve the issue completely, changes in > bridge and display controller drivers are needed to make use of the new > connector driver. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/bridge/Kconfig | 11 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/display-connector.c | 327 +++++++++++++++++++++ > 3 files changed, 339 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/display-connector.c > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > index a78392e2dbb9..295a62f65ef9 100644 > --- a/drivers/gpu/drm/bridge/Kconfig > +++ b/drivers/gpu/drm/bridge/Kconfig > @@ -37,6 +37,17 @@ config DRM_CDNS_DSI > Support Cadence DPI to DSI bridge. This is an internal > bridge and is meant to be directly embedded in a SoC. > > +config DRM_DISPLAY_CONNECTOR > + tristate "Display connector support" > + depends on OF > + help > + Driver for display connectors with support for DDC and hot-plug > + detection. Most display controller handle display connectors > + internally and don't need this driver, but the DRM subsystem is > + moving towards separating connector handling from display controllers > + on ARM-based platforms. Saying Y here when this driver is not needed > + will not cause any issue. > + > config DRM_LVDS_ENCODER > tristate "Transparent parallel to LVDS encoder support" > depends on OF > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > index 6ff7f2adbb0e..e5987b3aaf62 100644 > --- a/drivers/gpu/drm/bridge/Makefile > +++ b/drivers/gpu/drm/bridge/Makefile > @@ -1,6 +1,7 @@ > # SPDX-License-Identifier: GPL-2.0 > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > +obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o > obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c > new file mode 100644 > index 000000000000..2e1e7ee89275 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/display-connector.c > @@ -0,0 +1,327 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (C) 2019 Laurent Pinchart <laurent.pinchart@ideasonboard.com> > + */ > + ... > + > +static void display_connector_hpd_enable(struct drm_bridge *bridge) > +{ > +} > + > +static void display_connector_hpd_disable(struct drm_bridge *bridge) > +{ > +} It seems wrong that a new driver needs empty implementation of hpd_enable() and hpd_disable(). I noticed the same in a later patch too. Can we do without these empty functions? > + > +static const struct drm_bridge_funcs display_connector_bridge_funcs = { > + .attach = display_connector_attach, > + .detect = display_connector_detect, > + .get_edid = display_connector_get_edid, > + .hpd_enable = display_connector_hpd_enable, > + .hpd_disable = display_connector_hpd_disable, > +}; > + > + struct display_connector *conn = arg; > + struct drm_bridge *bridge = &conn->bridge; > + > + drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); > + > + return IRQ_HANDLED; > +} > + > +static const char *display_connector_type_name(struct display_connector *conn) > +{ > + switch (conn->bridge.type) { > + case DRM_MODE_CONNECTOR_Composite: > + return "Composite"; > + case DRM_MODE_CONNECTOR_DVIA: > + return "DVI-A"; > + case DRM_MODE_CONNECTOR_DVID: > + return "DVI-D"; > + case DRM_MODE_CONNECTOR_DVII: > + return "DVI-I"; > + case DRM_MODE_CONNECTOR_HDMIA: > + return "HDMI-A"; > + case DRM_MODE_CONNECTOR_HDMIB: > + return "HDMI-B"; > + case DRM_MODE_CONNECTOR_SVIDEO: > + return "S-Video"; > + case DRM_MODE_CONNECTOR_VGA: > + return "VGA"; > + default: > + return "unknown"; > + } > +} We already have the relation DRM_MODE_CONNECTOR <=> name in drm_connector - see drm_connector_enum_list. Add a small function in drm_connector.c get the name, so we do not hardcode the name twice? Sam
Hi Sam, On Tue, Jul 16, 2019 at 11:28:57AM +0200, Sam Ravnborg wrote: > On Sun, Jul 07, 2019 at 09:18:47PM +0300, Laurent Pinchart wrote: > > Display connectors are modelled in DT as a device node, but have so far > > been handled manually in several bridge drivers. This resulted in > > duplicate code in several bridge drivers, with slightly different (and > > thus confusing) logics. > > > > In order to fix this, implement a bridge driver for display connectors. > > The driver centralises logic for the DVI, HDMI, VGAn composite and > > S-video connectors and exposes corresponding bridge operations. > > > > This driver in itself doesn't solve the issue completely, changes in > > bridge and display controller drivers are needed to make use of the new > > connector driver. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/gpu/drm/bridge/Kconfig | 11 + > > drivers/gpu/drm/bridge/Makefile | 1 + > > drivers/gpu/drm/bridge/display-connector.c | 327 +++++++++++++++++++++ > > 3 files changed, 339 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/display-connector.c > > > > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig > > index a78392e2dbb9..295a62f65ef9 100644 > > --- a/drivers/gpu/drm/bridge/Kconfig > > +++ b/drivers/gpu/drm/bridge/Kconfig > > @@ -37,6 +37,17 @@ config DRM_CDNS_DSI > > Support Cadence DPI to DSI bridge. This is an internal > > bridge and is meant to be directly embedded in a SoC. > > > > +config DRM_DISPLAY_CONNECTOR > > + tristate "Display connector support" > > + depends on OF > > + help > > + Driver for display connectors with support for DDC and hot-plug > > + detection. Most display controller handle display connectors > > + internally and don't need this driver, but the DRM subsystem is > > + moving towards separating connector handling from display controllers > > + on ARM-based platforms. Saying Y here when this driver is not needed > > + will not cause any issue. > > + > > config DRM_LVDS_ENCODER > > tristate "Transparent parallel to LVDS encoder support" > > depends on OF > > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile > > index 6ff7f2adbb0e..e5987b3aaf62 100644 > > --- a/drivers/gpu/drm/bridge/Makefile > > +++ b/drivers/gpu/drm/bridge/Makefile > > @@ -1,6 +1,7 @@ > > # SPDX-License-Identifier: GPL-2.0 > > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o > > obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o > > +obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o > > obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o > > obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o > > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o > > diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c > > new file mode 100644 > > index 000000000000..2e1e7ee89275 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/display-connector.c > > @@ -0,0 +1,327 @@ > > +// SPDX-License-Identifier: GPL-2.0 > > +/* > > + * Copyright (C) 2019 Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > + */ > > + > ... > > + > > +static void display_connector_hpd_enable(struct drm_bridge *bridge) > > +{ > > +} > > + > > +static void display_connector_hpd_disable(struct drm_bridge *bridge) > > +{ > > +} > > It seems wrong that a new driver needs empty implementation of > hpd_enable() and hpd_disable(). > I noticed the same in a later patch too. > > Can we do without these empty functions? Absolutely, I'll fix that. > > + > > +static const struct drm_bridge_funcs display_connector_bridge_funcs = { > > + .attach = display_connector_attach, > > + .detect = display_connector_detect, > > + .get_edid = display_connector_get_edid, > > + .hpd_enable = display_connector_hpd_enable, > > + .hpd_disable = display_connector_hpd_disable, > > +}; > > + > > + struct display_connector *conn = arg; > > + struct drm_bridge *bridge = &conn->bridge; > > + > > + drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); > > + > > + return IRQ_HANDLED; > > +} > > + > > +static const char *display_connector_type_name(struct display_connector *conn) > > +{ > > + switch (conn->bridge.type) { > > + case DRM_MODE_CONNECTOR_Composite: > > + return "Composite"; > > + case DRM_MODE_CONNECTOR_DVIA: > > + return "DVI-A"; > > + case DRM_MODE_CONNECTOR_DVID: > > + return "DVI-D"; > > + case DRM_MODE_CONNECTOR_DVII: > > + return "DVI-I"; > > + case DRM_MODE_CONNECTOR_HDMIA: > > + return "HDMI-A"; > > + case DRM_MODE_CONNECTOR_HDMIB: > > + return "HDMI-B"; > > + case DRM_MODE_CONNECTOR_SVIDEO: > > + return "S-Video"; > > + case DRM_MODE_CONNECTOR_VGA: > > + return "VGA"; > > + default: > > + return "unknown"; > > + } > > +} > We already have the relation DRM_MODE_CONNECTOR <=> name in drm_connector - > see drm_connector_enum_list. > > Add a small function in drm_connector.c get the name, so we do not hardcode the > name twice? That will be in v2.
Hi Laurent, On 07/07/2019 21:18, Laurent Pinchart wrote: > Display connectors are modelled in DT as a device node, but have so far > been handled manually in several bridge drivers. This resulted in > duplicate code in several bridge drivers, with slightly different (and > thus confusing) logics. > > In order to fix this, implement a bridge driver for display connectors. > The driver centralises logic for the DVI, HDMI, VGAn composite and > S-video connectors and exposes corresponding bridge operations. > > This driver in itself doesn't solve the issue completely, changes in > bridge and display controller drivers are needed to make use of the new > connector driver. > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > --- > drivers/gpu/drm/bridge/Kconfig | 11 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/display-connector.c | 327 +++++++++++++++++++++ > 3 files changed, 339 insertions(+) > create mode 100644 drivers/gpu/drm/bridge/display-connector.c > + dev_info(&pdev->dev, > + "Found %s display connector '%s' %s DDC bus and %s HPD GPIO (ops 0x%x)\n", > + display_connector_type_name(conn), > + conn->label ? conn->label : "<unlabelled>", > + conn->ddc ? "with" : "without", > + conn->hpd_gpio ? "with" : "without", > + conn->bridge.ops); dev_dbg()? Tomi
Hi Tomi, On Mon, Sep 30, 2019 at 02:15:20PM +0300, Tomi Valkeinen wrote: > On 07/07/2019 21:18, Laurent Pinchart wrote: > > Display connectors are modelled in DT as a device node, but have so far > > been handled manually in several bridge drivers. This resulted in > > duplicate code in several bridge drivers, with slightly different (and > > thus confusing) logics. > > > > In order to fix this, implement a bridge driver for display connectors. > > The driver centralises logic for the DVI, HDMI, VGAn composite and > > S-video connectors and exposes corresponding bridge operations. > > > > This driver in itself doesn't solve the issue completely, changes in > > bridge and display controller drivers are needed to make use of the new > > connector driver. > > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> > > --- > > drivers/gpu/drm/bridge/Kconfig | 11 + > > drivers/gpu/drm/bridge/Makefile | 1 + > > drivers/gpu/drm/bridge/display-connector.c | 327 +++++++++++++++++++++ > > 3 files changed, 339 insertions(+) > > create mode 100644 drivers/gpu/drm/bridge/display-connector.c > > > > + dev_info(&pdev->dev, > > + "Found %s display connector '%s' %s DDC bus and %s HPD GPIO (ops 0x%x)\n", > > + display_connector_type_name(conn), > > + conn->label ? conn->label : "<unlabelled>", > > + conn->ddc ? "with" : "without", > > + conn->hpd_gpio ? "with" : "without", > > + conn->bridge.ops); > > dev_dbg()? Many drivers print an info message at probe time when everything goes fine, to inform about the device that has been succesfully probed. Do you think this is overkill and a dev_dbg() would be better ?
On 01/10/2019 23:08, Laurent Pinchart wrote: > Many drivers print an info message at probe time when everything goes > fine, to inform about the device that has been succesfully probed. Do > you think this is overkill and a dev_dbg() would be better ? Ah, I didn't realize this is a "probed" message. Yep, those prints sometimes help. I don't have strong feelings here, but usually I like the drivers to be quiet if everything is ok. Tomi
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig index a78392e2dbb9..295a62f65ef9 100644 --- a/drivers/gpu/drm/bridge/Kconfig +++ b/drivers/gpu/drm/bridge/Kconfig @@ -37,6 +37,17 @@ config DRM_CDNS_DSI Support Cadence DPI to DSI bridge. This is an internal bridge and is meant to be directly embedded in a SoC. +config DRM_DISPLAY_CONNECTOR + tristate "Display connector support" + depends on OF + help + Driver for display connectors with support for DDC and hot-plug + detection. Most display controller handle display connectors + internally and don't need this driver, but the DRM subsystem is + moving towards separating connector handling from display controllers + on ARM-based platforms. Saying Y here when this driver is not needed + will not cause any issue. + config DRM_LVDS_ENCODER tristate "Transparent parallel to LVDS encoder support" depends on OF diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile index 6ff7f2adbb0e..e5987b3aaf62 100644 --- a/drivers/gpu/drm/bridge/Makefile +++ b/drivers/gpu/drm/bridge/Makefile @@ -1,6 +1,7 @@ # SPDX-License-Identifier: GPL-2.0 obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o obj-$(CONFIG_DRM_CDNS_DSI) += cdns-dsi.o +obj-$(CONFIG_DRM_DISPLAY_CONNECTOR) += display-connector.o obj-$(CONFIG_DRM_LVDS_ENCODER) += lvds-encoder.o obj-$(CONFIG_DRM_MEGACHIPS_STDPXXXX_GE_B850V3_FW) += megachips-stdpxxxx-ge-b850v3-fw.o obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c new file mode 100644 index 000000000000..2e1e7ee89275 --- /dev/null +++ b/drivers/gpu/drm/bridge/display-connector.c @@ -0,0 +1,327 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * Copyright (C) 2019 Laurent Pinchart <laurent.pinchart@ideasonboard.com> + */ + +#include <linux/gpio/consumer.h> +#include <linux/i2c.h> +#include <linux/interrupt.h> +#include <linux/module.h> +#include <linux/mutex.h> +#include <linux/of.h> +#include <linux/of_device.h> +#include <linux/platform_device.h> + +#include <drm/drm_bridge.h> +#include <drm/drm_edid.h> + +struct display_connector { + struct drm_bridge bridge; + + const char *label; + struct i2c_adapter *ddc; + struct gpio_desc *hpd_gpio; + int hpd_irq; +}; + +static inline struct display_connector * +to_display_connector(struct drm_bridge *bridge) +{ + return container_of(bridge, struct display_connector, bridge); +} + +static int display_connector_attach(struct drm_bridge *bridge, + bool create_connector) +{ + return create_connector ? -EINVAL : 0; +} + +static enum drm_connector_status +display_connector_detect(struct drm_bridge *bridge) +{ + struct display_connector *conn = to_display_connector(bridge); + + if (conn->hpd_gpio) { + if (gpiod_get_value_cansleep(conn->hpd_gpio)) + return connector_status_connected; + else + return connector_status_disconnected; + } + + if (conn->ddc && drm_probe_ddc(conn->ddc)) + return connector_status_connected; + + switch (conn->bridge.type) { + case DRM_MODE_CONNECTOR_DVIA: + case DRM_MODE_CONNECTOR_DVID: + case DRM_MODE_CONNECTOR_DVII: + case DRM_MODE_CONNECTOR_HDMIA: + case DRM_MODE_CONNECTOR_HDMIB: + /* + * For DVI and HDMI connectors a DDC probe failure indicates + * that no cable is connected. + */ + return connector_status_disconnected; + + case DRM_MODE_CONNECTOR_Composite: + case DRM_MODE_CONNECTOR_SVIDEO: + case DRM_MODE_CONNECTOR_VGA: + default: + /* + * Composite and S-Video connectors have no other detection + * mean than the HPD GPIO. For VGA connectors, even if we have + * an I2C bus, we can't assume that the cable is disconnected + * if drm_probe_ddc fails, as some cables don't wire the DDC + * pins. + */ + return connector_status_unknown; + } +} + +static struct edid *display_connector_get_edid(struct drm_bridge *bridge, + struct drm_connector *connector) +{ + struct display_connector *conn = to_display_connector(bridge); + + return drm_get_edid(connector, conn->ddc); +} + +static void display_connector_hpd_enable(struct drm_bridge *bridge) +{ +} + +static void display_connector_hpd_disable(struct drm_bridge *bridge) +{ +} + +static const struct drm_bridge_funcs display_connector_bridge_funcs = { + .attach = display_connector_attach, + .detect = display_connector_detect, + .get_edid = display_connector_get_edid, + .hpd_enable = display_connector_hpd_enable, + .hpd_disable = display_connector_hpd_disable, +}; + +static irqreturn_t display_connector_hpd_irq(int irq, void *arg) +{ + struct display_connector *conn = arg; + struct drm_bridge *bridge = &conn->bridge; + + drm_bridge_hpd_notify(bridge, display_connector_detect(bridge)); + + return IRQ_HANDLED; +} + +static const char *display_connector_type_name(struct display_connector *conn) +{ + switch (conn->bridge.type) { + case DRM_MODE_CONNECTOR_Composite: + return "Composite"; + case DRM_MODE_CONNECTOR_DVIA: + return "DVI-A"; + case DRM_MODE_CONNECTOR_DVID: + return "DVI-D"; + case DRM_MODE_CONNECTOR_DVII: + return "DVI-I"; + case DRM_MODE_CONNECTOR_HDMIA: + return "HDMI-A"; + case DRM_MODE_CONNECTOR_HDMIB: + return "HDMI-B"; + case DRM_MODE_CONNECTOR_SVIDEO: + return "S-Video"; + case DRM_MODE_CONNECTOR_VGA: + return "VGA"; + default: + return "unknown"; + } +} + +static int display_connector_probe(struct platform_device *pdev) +{ + struct display_connector *conn; + unsigned int type; + int ret; + + conn = devm_kzalloc(&pdev->dev, sizeof(*conn), GFP_KERNEL); + if (!conn) + return -ENOMEM; + + platform_set_drvdata(pdev, conn); + + type = (uintptr_t)of_device_get_match_data(&pdev->dev); + + /* Get the exact connector type. */ + switch (type) { + case DRM_MODE_CONNECTOR_DVII: { + bool analog, digital; + + analog = of_property_read_bool(pdev->dev.of_node, "analog"); + digital = of_property_read_bool(pdev->dev.of_node, "digital"); + if (analog && !digital) { + conn->bridge.type = DRM_MODE_CONNECTOR_DVIA; + } else if (!analog && digital) { + conn->bridge.type = DRM_MODE_CONNECTOR_DVID; + } else if (analog && digital) { + conn->bridge.type = DRM_MODE_CONNECTOR_DVII; + } else { + dev_err(&pdev->dev, "DVI connector with no type\n"); + return -EINVAL; + } + break; + } + + case DRM_MODE_CONNECTOR_HDMIA: { + const char *hdmi_type; + + ret = of_property_read_string(pdev->dev.of_node, "type", + &hdmi_type); + if (ret < 0) { + dev_err(&pdev->dev, "HDMI connector with no type\n"); + return -EINVAL; + } + + if (!strcmp(hdmi_type, "a") || !strcmp(hdmi_type, "c") || + !strcmp(hdmi_type, "d") || !strcmp(hdmi_type, "e")) { + conn->bridge.type = DRM_MODE_CONNECTOR_HDMIA; + } else if (!strcmp(hdmi_type, "b")) { + conn->bridge.type = DRM_MODE_CONNECTOR_HDMIB; + } else { + dev_err(&pdev->dev, + "Unsupported HDMI connector type '%s'\n", + hdmi_type); + return -EINVAL; + } + + break; + } + + default: + conn->bridge.type = type; + break; + } + + /* Get the optional connector label. */ + of_property_read_string(pdev->dev.of_node, "label", &conn->label); + + /* + * Get the HPD GPIO for DVI and HDMI connectors. If the GPIO can provide + * interrupts, register an interrupt handler. + */ + if (type == DRM_MODE_CONNECTOR_DVII || + type == DRM_MODE_CONNECTOR_HDMIA) { + conn->hpd_gpio = devm_gpiod_get_optional(&pdev->dev, "hpd", + GPIOD_IN); + if (IS_ERR(conn->hpd_gpio)) { + if (PTR_ERR(conn->hpd_gpio) != -EPROBE_DEFER) + dev_err(&pdev->dev, + "Unable to retrieve HPD GPIO\n"); + return PTR_ERR(conn->hpd_gpio); + } + + conn->hpd_irq = gpiod_to_irq(conn->hpd_gpio); + } else { + conn->hpd_irq = -EINVAL; + } + + if (conn->hpd_irq >= 0) { + ret = devm_request_threaded_irq(&pdev->dev, conn->hpd_irq, + NULL, display_connector_hpd_irq, + IRQF_TRIGGER_RISING | + IRQF_TRIGGER_FALLING | + IRQF_ONESHOT, + "HPD", conn); + if (ret) { + dev_err(&pdev->dev, + "Failed to request HPD interrupt\n"); + return ret; + } + } + + /* Retrieve the DDC I2C adapter for DVI, HDMI and VGA connectors. */ + if (type == DRM_MODE_CONNECTOR_DVII || + type == DRM_MODE_CONNECTOR_HDMIA || + type == DRM_MODE_CONNECTOR_VGA) { + struct device_node *phandle; + + phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c-bus", 0); + if (phandle) { + conn->ddc = of_get_i2c_adapter_by_node(phandle); + of_node_put(phandle); + if (!conn->ddc) + return -EPROBE_DEFER; + } else { + dev_dbg(&pdev->dev, + "No I2C bus specified, disabling EDID readout\n"); + } + } + + conn->bridge.funcs = &display_connector_bridge_funcs; + conn->bridge.of_node = pdev->dev.of_node; + + if (conn->ddc) + conn->bridge.ops |= DRM_BRIDGE_OP_EDID + | DRM_BRIDGE_OP_DETECT; + if (conn->hpd_gpio) + conn->bridge.ops |= DRM_BRIDGE_OP_DETECT; + if (conn->hpd_irq >= 0) + conn->bridge.ops |= DRM_BRIDGE_OP_HPD; + + dev_info(&pdev->dev, + "Found %s display connector '%s' %s DDC bus and %s HPD GPIO (ops 0x%x)\n", + display_connector_type_name(conn), + conn->label ? conn->label : "<unlabelled>", + conn->ddc ? "with" : "without", + conn->hpd_gpio ? "with" : "without", + conn->bridge.ops); + + drm_bridge_add(&conn->bridge); + + return 0; +} + +static int display_connector_remove(struct platform_device *pdev) +{ + struct display_connector *conn = platform_get_drvdata(pdev); + + drm_bridge_remove(&conn->bridge); + + if (!IS_ERR(conn->ddc)) + i2c_put_adapter(conn->ddc); + + return 0; +} + +static const struct of_device_id display_connector_match[] = { + { + .compatible = "composite-video-connector", + .data = (void *)DRM_MODE_CONNECTOR_Composite, + }, { + .compatible = "dvi-connector", + .data = (void *)DRM_MODE_CONNECTOR_DVII, + }, { + .compatible = "hdmi-connector", + .data = (void *)DRM_MODE_CONNECTOR_HDMIA, + }, { + .compatible = "svideo-connector", + .data = (void *)DRM_MODE_CONNECTOR_SVIDEO, + }, { + .compatible = "vga-connector", + .data = (void *)DRM_MODE_CONNECTOR_VGA, + }, + {}, +}; +MODULE_DEVICE_TABLE(of, display_connector_match); + +static struct platform_driver display_connector_driver = { + .probe = display_connector_probe, + .remove = display_connector_remove, + .driver = { + .name = "display-connector", + .of_match_table = display_connector_match, + }, +}; +module_platform_driver(display_connector_driver); + +MODULE_AUTHOR("Laurent Pinchart <laurent.pinchart@ideasonboard.com>"); +MODULE_DESCRIPTION("Display connector driver"); +MODULE_LICENSE("GPL");
Display connectors are modelled in DT as a device node, but have so far been handled manually in several bridge drivers. This resulted in duplicate code in several bridge drivers, with slightly different (and thus confusing) logics. In order to fix this, implement a bridge driver for display connectors. The driver centralises logic for the DVI, HDMI, VGAn composite and S-video connectors and exposes corresponding bridge operations. This driver in itself doesn't solve the issue completely, changes in bridge and display controller drivers are needed to make use of the new connector driver. Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com> --- drivers/gpu/drm/bridge/Kconfig | 11 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/display-connector.c | 327 +++++++++++++++++++++ 3 files changed, 339 insertions(+) create mode 100644 drivers/gpu/drm/bridge/display-connector.c