Re: [PATCH v2 11/50] drm/bridge: Add bridge driver for display connectors
On Tue, Aug 20, 2019 at 04:16:42AM +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 Reviewed-by: Maxime Ripard Maxime -- Maxime Ripard, Bootlin Embedded Linux and Kernel engineering https://bootlin.com signature.asc Description: PGP signature ___ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/50] drm/bridge: Add bridge driver for display connectors
Hi Boris,
On Thu, Aug 22, 2019 at 06:05:20PM +0200, Boris Brezillon wrote:
> On Tue, 20 Aug 2019 04:16:42 +0300 Laurent Pinchart wrote:
>
> > + /*
> > +* 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(>dev, "hpd",
> > +GPIOD_IN);
> > + if (IS_ERR(conn->hpd_gpio)) {
> > + if (PTR_ERR(conn->hpd_gpio) != -EPROBE_DEFER)
> > + dev_err(>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(>dev, conn->hpd_irq,
> > + NULL, display_connector_hpd_irq,
> > + IRQF_TRIGGER_RISING |
> > + IRQF_TRIGGER_FALLING |
> > + IRQF_ONESHOT,
> > + "HPD", conn);
> > + if (ret) {
> > + dev_err(>dev,
> > + "Failed to request HPD interrupt\n");
> > + return ret;
>
> Is there anything that mandates support of edge events on GPIO chips?
> I know it's quite common, but maybe we should fallback to polling
> mode when devm_request_threaded_irq() fails.
That's a good point, I'll change this.
> > + }
> > + }
> > +
> > + /* 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->bridge.ddc = of_get_i2c_adapter_by_node(phandle);
> > + of_node_put(phandle);
> > + if (!conn->bridge.ddc)
> > + return -EPROBE_DEFER;
> > + } else {
> > + dev_dbg(>dev,
> > + "No I2C bus specified, disabling EDID
> > readout\n");
> > + }
> > + }
> > +
--
Regards,
Laurent Pinchart
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH v2 11/50] drm/bridge: Add bridge driver for display connectors
On Tue, 20 Aug 2019 04:16:42 +0300
Laurent Pinchart wrote:
> + /*
> + * 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(>dev, "hpd",
> + GPIOD_IN);
> + if (IS_ERR(conn->hpd_gpio)) {
> + if (PTR_ERR(conn->hpd_gpio) != -EPROBE_DEFER)
> + dev_err(>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(>dev, conn->hpd_irq,
> + NULL, display_connector_hpd_irq,
> + IRQF_TRIGGER_RISING |
> + IRQF_TRIGGER_FALLING |
> + IRQF_ONESHOT,
> + "HPD", conn);
> + if (ret) {
> + dev_err(>dev,
> + "Failed to request HPD interrupt\n");
> + return ret;
Is there anything that mandates support of edge events on GPIO chips?
I know it's quite common, but maybe we should fallback to polling
mode when devm_request_threaded_irq() fails.
> + }
> + }
> +
> + /* 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->bridge.ddc = of_get_i2c_adapter_by_node(phandle);
> + of_node_put(phandle);
> + if (!conn->bridge.ddc)
> + return -EPROBE_DEFER;
> + } else {
> + dev_dbg(>dev,
> + "No I2C bus specified, disabling EDID
> readout\n");
> + }
> + }
> +
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH v2 11/50] drm/bridge: Add bridge driver for display connectors
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
---
Changes since v1:
- Use drm_get_connector_type_name() instead of open-coding
display_connector_type_name()
- Remove empty .hpd_enable() and .hpd_disable() operations
- Set bridge.ddc
---
drivers/gpu/drm/bridge/Kconfig | 11 +
drivers/gpu/drm/bridge/Makefile| 1 +
drivers/gpu/drm/bridge/display-connector.c | 292 +
3 files changed, 304 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 d0146438b0f5..f776d522c5aa 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_STDP_GE_B850V3_FW) +=
megachips-stdp-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 ..988626b91036
--- /dev/null
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (C) 2019 Laurent Pinchart
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include
+#include
+
+struct display_connector {
+ struct drm_bridge bridge;
+
+ const char *label;
+ 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,
+ enum drm_bridge_attach_flags flags)
+{
+ return flags & DRM_BRIDGE_ATTACH_NO_CONNECTOR ? 0 : -EINVAL;
+}
+
+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->bridge.ddc && drm_probe_ddc(conn->bridge.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
