[PATCH v5 0/4] drm/tilcdc: Add bridge support and sync-lost flood recovery

[PATCH v5 0/4] drm/tilcdc: Add bridge support and sync-lost flood recovery

[Jyri Sarha]

Changes since v4:
- "drm/bridge: Add ti-tfp410 DVI transmitter driver"
  - Put i2c behind #if IS_ENABLED(CONFIG_I2C)
- "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers"
  - Use exsisting infrastructure to hookup crtc mode validation code
    to newly connected connector, whether that came from componentized
    driver or trough an attached bridge

Changes since v3:
- "drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC"
  - Fix broken irq enable/disble code for LCDC rev 1
- Add: "dt-bindings: Move "ti,tfp410.txt" from display/ti to display/bridge"
- "drm/bridge: Add ti-tfp410 DVI transmitter driver"
  - Don't fail if either i2c or platform driver register succeeds
  - ftp410 -> tfp410
  - Merge the old display/ti,tfp410.txt document with my addition

Changes since v2:
- "drm/tilcdc: Recover from sync lost error flood by resetting the LCDC"
  - no change
- "drm/bridge: Add ti-tfp410 DVI transmitter driver"
  - Fix deveice-tree document
    - "driver node" -> "device node"
    - remove "(the current implementation does not yet support this)"
  - Add dummy i2c support. The driver probe works also if placed under
    i2c controller node, but there is no actual i2c probing.
- "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers"
  - no change

Changes since first version of the series:
- "drm/tilcdc: Recover from sync lost error flood by resetting the LCDC"
  - no change
- "drm/bridge: Add ti-tfp410 DVI transmitter driver"
  - HDMI -> DVI
  - DT Binding document
    - Prepare for tfp410 connected trough i2c by optional reg property
    - Require two port nodes
  - Implementation
    - Implement connector node functionality with in tfp410 bridge
      drive, but follow generic connector binding by pulling the
      ddc-i2c-bus property from the connector node.
- "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers"
  - Remove earlier change in TD binding document. There is no need to
    mention DRM implementation details, like bridge support, in DT
    binding.

The first patch is an independent on and I've been testing it for
quite a while now.

The tfp410 bridge driver and the tilcdc bridge support are tested with
BeagleBone DVI-D Cape Rev A3. The tfp410 bridge driver is missing a
lot of features, because the DVI-D cape does not have too many wires
connected. The missing features can be added later when they are
needed.

Jyri Sarha (4):
  drm/tilcdc: Recover from sync lost error flood by resetting the LCDC
  dt-bindings: Move "ti,tfp410.txt" from display/ti to display/bridge
  drm/bridge: Add ti-tfp410 DVI transmitter driver
  drm/tilcdc: Add drm bridge support for attaching drm bridge drivers

 .../bindings/display/{ti => bridge}/ti,tfp410.txt  |   9 +-
 drivers/gpu/drm/bridge/Kconfig                     |   7 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/ti-tfp410.c                 | 317 +++++++++++++++++++++
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  26 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.c                |  11 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h                |   5 +-
 drivers/gpu/drm/tilcdc/tilcdc_external.c           | 260 ++++++++++++-----
 drivers/gpu/drm/tilcdc/tilcdc_external.h           |   5 +-
 9 files changed, 564 insertions(+), 77 deletions(-)
 rename Documentation/devicetree/bindings/display/{ti => bridge}/ti,tfp410.txt (65%)
 create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 1/4] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC

[Jyri Sarha]

Recover from sync lost error flood by resetting the LCDC instead of
turning off the SYNC_LOST error IRQ. When LCDC starves on limited
memory bandwidth it may sometimes result an error situation when the
picture may have shifted couple of pixels to right and SYNC_LOST
interrupt is generated on every frame. LCDC main reset recovers from
this situation and causes a brief blanking on the screen.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 26 +++++++++++++++++++++++++-
 1 file changed, 25 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
index 0d09acc..c787349 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_crtc.c
@@ -55,6 +55,7 @@ struct tilcdc_crtc {
 
 	int sync_lost_count;
 	bool frame_intact;
+	struct work_struct recover_work;
 };
 #define to_tilcdc_crtc(x) container_of(x, struct tilcdc_crtc, base)
 
@@ -252,6 +253,25 @@ static bool tilcdc_crtc_is_on(struct drm_crtc *crtc)
 	return crtc->state && crtc->state->enable && crtc->state->active;
 }
 
+static void tilcdc_crtc_recover_work(struct work_struct *work)
+{
+	struct tilcdc_crtc *tilcdc_crtc =
+		container_of(work, struct tilcdc_crtc, recover_work);
+	struct drm_crtc *crtc = &tilcdc_crtc->base;
+
+	dev_info(crtc->dev->dev, "%s: Reset CRTC", __func__);
+
+	drm_modeset_lock_crtc(crtc, NULL);
+
+	if (!tilcdc_crtc_is_on(crtc))
+		goto out;
+
+	tilcdc_crtc_disable(crtc);
+	tilcdc_crtc_enable(crtc);
+out:
+	drm_modeset_unlock_crtc(crtc);
+}
+
 static void tilcdc_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct tilcdc_crtc *tilcdc_crtc = to_tilcdc_crtc(crtc);
@@ -838,9 +858,12 @@ irqreturn_t tilcdc_crtc_irq(struct drm_crtc *crtc)
 			tilcdc_crtc->frame_intact = false;
 			if (tilcdc_crtc->sync_lost_count++ >
 			    SYNC_LOST_COUNT_LIMIT) {
-				dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, disabling the interrupt", __func__, stat);
+				dev_err(dev->dev, "%s(0x%08x): Sync lost flood detected, recovering", __func__, stat);
+				queue_work(system_wq,
+					   &tilcdc_crtc->recover_work);
 				tilcdc_write(dev, LCDC_INT_ENABLE_CLR_REG,
 					     LCDC_SYNC_LOST);
+				tilcdc_crtc->sync_lost_count = 0;
 			}
 		}
 
@@ -880,6 +903,7 @@ struct drm_crtc *tilcdc_crtc_create(struct drm_device *dev)
 			"unref", unref_worker);
 
 	spin_lock_init(&tilcdc_crtc->irq_lock);
+	INIT_WORK(&tilcdc_crtc->recover_work, tilcdc_crtc_recover_work);
 
 	ret = drm_crtc_init_with_planes(dev, crtc,
 					&tilcdc_crtc->primary,
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 2/4] dt-bindings: Move "ti, tfp410.txt" from display/ti to display/bridge

[Jyri Sarha]

Move "ti,tfp410.txt" from display/ti to display/bridge before adding
generic (non omapdrm/dss specific) implementation and new features.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 Documentation/devicetree/bindings/display/{ti => bridge}/ti,tfp410.txt | 0
 1 file changed, 0 insertions(+), 0 deletions(-)
 rename Documentation/devicetree/bindings/display/{ti => bridge}/ti,tfp410.txt (100%)

diff --git a/Documentation/devicetree/bindings/display/ti/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
similarity index 100%
rename from Documentation/devicetree/bindings/display/ti/ti,tfp410.txt
rename to Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Jyri Sarha]

Add very basic ti-tfp410 DVI transmitter driver. The only feature
separating this from a completely dummy bridge is the EDID read
support trough DDC I2C. Even that functionality should be in a
separate generic connector driver. However, because of missing DRM
infrastructure support the connector is implemented within the bridge
driver. Some tfp410 HW specific features may be added later if needed,
because there is a set of registers behind i2c if it is connected.

This implementation is tested against my new tilcdc bridge support
and it works with BeagleBone DVI-D Cape Rev A3. A DT binding document
is also updated.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 .../bindings/display/bridge/ti,tfp410.txt          |   9 +-
 drivers/gpu/drm/bridge/Kconfig                     |   7 +
 drivers/gpu/drm/bridge/Makefile                    |   1 +
 drivers/gpu/drm/bridge/ti-tfp410.c                 | 317 +++++++++++++++++++++
 4 files changed, 332 insertions(+), 2 deletions(-)
 create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c

diff --git a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
index 2cbe32a..54d7e31 100644
--- a/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
+++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt
@@ -6,10 +6,15 @@ Required properties:
 
 Optional properties:
 - powerdown-gpios: power-down gpio
+- reg: I2C address. If and only if present the device node
+       should be placed into the i2c controller node where the
+       tfp410 i2c is connected to.
 
 Required nodes:
-- Video port 0 for DPI input
-- Video port 1 for DVI output
+- Video port 0 for DPI input [1].
+- Video port 1 for DVI output [1].
+
+[1]: Documentation/devicetree/bindings/media/video-interfaces.txt
 
 Example
 -------
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index bd6acc8..a424e03 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -81,6 +81,13 @@ config DRM_TOSHIBA_TC358767
 	---help---
 	  Toshiba TC358767 eDP bridge chip driver.
 
+config DRM_TI_TFP410
+	tristate "TI TFP410 DVI/HDMI bridge"
+	depends on OF
+	select DRM_KMS_HELPER
+	---help---
+	  Texas Instruments TFP410 DVI/HDMI Transmitter 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 97ed1a5..8b065d9 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -11,3 +11,4 @@ obj-$(CONFIG_DRM_SII902X) += sii902x.o
 obj-$(CONFIG_DRM_TOSHIBA_TC358767) += tc358767.o
 obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
 obj-$(CONFIG_DRM_I2C_ADV7511) += adv7511/
+obj-$(CONFIG_DRM_TI_TFP410) += ti-tfp410.o
diff --git a/drivers/gpu/drm/bridge/ti-tfp410.c b/drivers/gpu/drm/bridge/ti-tfp410.c
new file mode 100644
index 0000000..b054ea3
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ti-tfp410.c
@@ -0,0 +1,317 @@
+/*
+ * Copyright (C) 2016 Texas Instruments
+ * Author: Jyri Sarha <jsarha@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/of_graph.h>
+#include <linux/platform_device.h>
+#include <linux/i2c.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
+
+struct tfp410 {
+	struct drm_bridge	bridge;
+	struct drm_connector	connector;
+
+	struct i2c_adapter	*ddc;
+
+	struct device *dev;
+};
+
+static inline struct tfp410 *
+drm_bridge_to_tfp410(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct tfp410, bridge);
+}
+
+static inline struct tfp410 *
+drm_connector_to_tfp410(struct drm_connector *connector)
+{
+	return container_of(connector, struct tfp410, connector);
+}
+
+static int tfp410_get_modes(struct drm_connector *connector)
+{
+	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
+	struct edid *edid;
+	int ret;
+
+	if (!dvi->ddc)
+		goto fallback;
+
+	edid = drm_get_edid(connector, dvi->ddc);
+	if (!edid) {
+		DRM_INFO("EDID read failed. Fallback to standard modes\n");
+		goto fallback;
+	}
+
+	drm_mode_connector_update_edid_property(connector, edid);
+
+	return drm_add_edid_modes(connector, edid);
+fallback:
+	/* No EDID, fallback on the XGA standard modes */
+	ret = drm_add_modes_noedid(connector, 1920, 1200);
+
+	/* And prefer a mode pretty much anything can handle */
+	drm_set_preferred_mode(connector, 1024, 768);
+
+	return ret;
+}
+
+static const struct drm_connector_helper_funcs tfp410_con_helper_funcs = {
+	.get_modes	= tfp410_get_modes,
+};
+
+static enum drm_connector_status
+tfp410_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct tfp410 *dvi = drm_connector_to_tfp410(connector);
+
+	if (dvi->ddc) {
+		if (drm_probe_ddc(dvi->ddc))
+			return connector_status_connected;
+		else
+			return connector_status_disconnected;
+	}
+
+	return connector_status_unknown;
+}
+
+static const struct drm_connector_funcs tfp410_con_funcs = {
+	.dpms			= drm_atomic_helper_connector_dpms,
+	.detect			= tfp410_connector_detect,
+	.fill_modes		= drm_helper_probe_single_connector_modes,
+	.destroy		= drm_connector_cleanup,
+	.reset			= drm_atomic_helper_connector_reset,
+	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
+};
+
+static int tfp410_attach(struct drm_bridge *bridge)
+{
+	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
+	int ret;
+
+	if (!bridge->encoder) {
+		dev_err(dvi->dev, "Missing encoder\n");
+		return -ENODEV;
+	}
+
+	drm_connector_helper_add(&dvi->connector,
+				 &tfp410_con_helper_funcs);
+	ret = drm_connector_init(bridge->dev, &dvi->connector,
+				 &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
+	if (ret) {
+		dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
+		return ret;
+	}
+
+	drm_mode_connector_attach_encoder(&dvi->connector,
+					  bridge->encoder);
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs tfp410_bridge_funcs = {
+	.attach		= tfp410_attach,
+};
+
+static int tfp410_get_connector_ddc(struct tfp410 *dvi)
+{
+	struct device_node *ep = NULL, *connector_node = NULL;
+	struct device_node *ddc_phandle = NULL;
+	int ret = 0;
+
+	/* port@1 is the connector node */
+	ep = of_graph_get_endpoint_by_regs(dvi->dev->of_node, 1, -1);
+	if (!ep)
+		goto fail;
+
+	connector_node = of_graph_get_remote_port_parent(ep);
+	if (!connector_node)
+		goto fail;
+
+	ddc_phandle = of_parse_phandle(connector_node, "ddc-i2c-bus", 0);
+	if (!ddc_phandle)
+		goto fail;
+
+	dvi->ddc = of_get_i2c_adapter_by_node(ddc_phandle);
+	if (dvi->ddc)
+		dev_info(dvi->dev, "Connector's ddc i2c bus found\n");
+	else
+		ret = -EPROBE_DEFER;
+
+fail:
+	of_node_put(ep);
+	of_node_put(connector_node);
+	of_node_put(ddc_phandle);
+	return ret;
+}
+
+static int tfp410_init(struct device *dev)
+{
+	struct tfp410 *dvi;
+	int ret;
+
+	if (!dev->of_node) {
+		dev_err(dev, "device-tree data is missing\n");
+		return -ENXIO;
+	}
+
+	dvi = devm_kzalloc(dev, sizeof(*dvi), GFP_KERNEL);
+	if (!dvi)
+		return -ENOMEM;
+	dev_set_drvdata(dev, dvi);
+
+	dvi->bridge.funcs = &tfp410_bridge_funcs;
+	dvi->bridge.of_node = dev->of_node;
+	dvi->dev = dev;
+
+	ret = tfp410_get_connector_ddc(dvi);
+	if (ret)
+		goto fail;
+
+	ret = drm_bridge_add(&dvi->bridge);
+	if (ret) {
+		dev_err(dev, "drm_bridge_add() failed: %d\n", ret);
+		goto fail;
+	}
+
+	return 0;
+fail:
+	i2c_put_adapter(dvi->ddc);
+	return ret;
+}
+
+static int tfp410_fini(struct device *dev)
+{
+	struct tfp410 *dvi = dev_get_drvdata(dev);
+
+	drm_bridge_remove(&dvi->bridge);
+
+	if (dvi->ddc)
+		i2c_put_adapter(dvi->ddc);
+
+	return 0;
+}
+
+static int tfp410_probe(struct platform_device *pdev)
+{
+	return tfp410_init(&pdev->dev);
+}
+
+static int tfp410_remove(struct platform_device *pdev)
+{
+	return tfp410_fini(&pdev->dev);
+}
+
+static const struct of_device_id tfp410_match[] = {
+	{ .compatible = "ti,tfp410" },
+	{},
+};
+MODULE_DEVICE_TABLE(of, tfp410_match);
+
+struct platform_driver tfp410_platform_driver = {
+	.probe	= tfp410_probe,
+	.remove	= tfp410_remove,
+	.driver	= {
+		.name		= "tfp410-bridge",
+		.of_match_table	= tfp410_match,
+	},
+};
+
+#if IS_ENABLED(CONFIG_I2C)
+/* There is currently no i2c functionality. */
+static int tfp410_i2c_probe(struct i2c_client *client,
+			    const struct i2c_device_id *id)
+{
+	int reg;
+
+	if (!client->dev.of_node ||
+	    of_property_read_u32(client->dev.of_node, "reg", &reg)) {
+		dev_err(&client->dev,
+			"Can't get i2c reg property from device-tree\n");
+		return -ENXIO;
+	}
+
+	return tfp410_init(&client->dev);
+}
+
+static int tfp410_i2c_remove(struct i2c_client *client)
+{
+	return tfp410_fini(&client->dev);
+}
+
+static const struct i2c_device_id tfp410_i2c_ids[] = {
+	{ "tfp410", 0 },
+	{ }
+};
+MODULE_DEVICE_TABLE(i2c, tfp410_i2c_ids);
+
+static struct i2c_driver tfp410_i2c_driver = {
+	.driver = {
+		.name	= "tfp410",
+		.of_match_table = of_match_ptr(tfp410_match),
+	},
+	.id_table	= tfp410_i2c_ids,
+	.probe		= tfp410_i2c_probe,
+	.remove		= tfp410_i2c_remove,
+};
+#endif /* IS_ENABLED(CONFIG_I2C) */
+
+static struct {
+	uint i2c:1;
+	uint platform:1;
+}  tfp410_registered_driver;
+
+static int __init tfp410_module_init(void)
+{
+	int ret;
+
+#if IS_ENABLED(CONFIG_I2C)
+	ret = i2c_add_driver(&tfp410_i2c_driver);
+	if (ret)
+		pr_err("%s: registering i2c driver failed: %d",
+		       __func__, ret);
+	else
+		tfp410_registered_driver.i2c = 1;
+#endif
+
+	ret = platform_driver_register(&tfp410_platform_driver);
+	if (ret)
+		pr_err("%s: registering platform driver failed: %d",
+		       __func__, ret);
+	else
+		tfp410_registered_driver.platform = 1;
+
+	if (tfp410_registered_driver.i2c ||
+	    tfp410_registered_driver.platform)
+		return 0;
+
+	return ret;
+}
+module_init(tfp410_module_init);
+
+static void __exit tfp410_module_exit(void)
+{
+#if IS_ENABLED(CONFIG_I2C)
+	if (tfp410_registered_driver.i2c)
+		i2c_del_driver(&tfp410_i2c_driver);
+#endif
+	if (tfp410_registered_driver.platform)
+		platform_driver_unregister(&tfp410_platform_driver);
+}
+module_exit(tfp410_module_exit);
+
+MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>");
+MODULE_DESCRIPTION("TI TFP410 DVI bridge driver");
+MODULE_LICENSE("GPL");
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 4/4] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers

[Jyri Sarha]

Adds drm bride support for attaching drm bridge drivers to tilcdc. The
decision whether a video port leads to an external encoder or bridge
is made simply based on remote device's compatible string. The code
has been tested with BeagleBone-Black with and without BeagleBone
DVI-D Cape Rev A3 using ti-tfp410 driver.

Signed-off-by: Jyri Sarha <jsarha@ti.com>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c      |  11 +-
 drivers/gpu/drm/tilcdc/tilcdc_drv.h      |   5 +-
 drivers/gpu/drm/tilcdc/tilcdc_external.c | 260 +++++++++++++++++++++++--------
 drivers/gpu/drm/tilcdc/tilcdc_external.h |   5 +-
 4 files changed, 207 insertions(+), 74 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index 3d2cea0..7f4d3bc 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -209,7 +209,7 @@ static void tilcdc_fini(struct drm_device *dev)
 
 	drm_irq_uninstall(dev);
 	drm_mode_config_cleanup(dev);
-	tilcdc_remove_external_encoders(dev);
+	tilcdc_remove_external_device(dev);
 
 #ifdef CONFIG_CPU_FREQ
 	if (priv->freq_transition.notifier_call)
@@ -381,12 +381,17 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev)
 		if (ret < 0)
 			goto init_failed;
 
-		ret = tilcdc_add_external_encoders(ddev);
+		ret = tilcdc_add_component_encoder(ddev);
 		if (ret < 0)
 			goto init_failed;
+	} else {
+		ret = tilcdc_attach_external_device(ddev);
+		if (ret)
+			goto init_failed;
 	}
 
-	if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) {
+	if (!priv->external_connector &&
+	    ((priv->num_encoders == 0) || (priv->num_connectors == 0))) {
 		dev_err(dev, "no encoders/connectors found\n");
 		ret = -ENXIO;
 		goto init_failed;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index d31fe5d..411f8a8 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -88,7 +88,10 @@ struct tilcdc_drm_private {
 
 	unsigned int num_connectors;
 	struct drm_connector *connectors[8];
-	const struct drm_connector_helper_funcs *connector_funcs[8];
+
+	struct drm_encoder *external_encoder;
+	struct drm_connector *external_connector;
+	const struct drm_connector_helper_funcs *connector_funcs;
 
 	bool is_registered;
 	bool is_componentized;
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.c b/drivers/gpu/drm/tilcdc/tilcdc_external.c
index 06a4c58..c67d7cd 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c
@@ -28,44 +28,50 @@
 		.raster_order           = 0,
 };
 
+static const struct tilcdc_panel_info panel_info_default = {
+		.ac_bias                = 255,
+		.ac_bias_intrpt         = 0,
+		.dma_burst_sz           = 16,
+		.bpp                    = 16,
+		.fdd                    = 0x80,
+		.tft_alt_mode           = 0,
+		.sync_edge              = 0,
+		.sync_ctrl              = 1,
+		.raster_order           = 0,
+};
+
 static int tilcdc_external_mode_valid(struct drm_connector *connector,
 				      struct drm_display_mode *mode)
 {
 	struct tilcdc_drm_private *priv = connector->dev->dev_private;
-	int ret, i;
+	int ret;
 
 	ret = tilcdc_crtc_mode_valid(priv->crtc, mode);
 	if (ret != MODE_OK)
 		return ret;
 
-	for (i = 0; i < priv->num_connectors &&
-		     priv->connectors[i] != connector; i++)
-		;
-
-	BUG_ON(priv->connectors[i] != connector);
-	BUG_ON(!priv->connector_funcs[i]);
+	BUG_ON(priv->external_connector != connector);
+	BUG_ON(!priv->connector_funcs);
 
 	/* If the connector has its own mode_valid call it. */
-	if (!IS_ERR(priv->connector_funcs[i]) &&
-	    priv->connector_funcs[i]->mode_valid)
-		return priv->connector_funcs[i]->mode_valid(connector, mode);
+	if (!IS_ERR(priv->connector_funcs) &&
+	    priv->connector_funcs->mode_valid)
+		return priv->connector_funcs->mode_valid(connector, mode);
 
 	return MODE_OK;
 }
 
-static int tilcdc_add_external_encoder(struct drm_device *dev,
-				       struct drm_connector *connector)
+static int tilcdc_add_external_connector(struct drm_device *dev,
+					 struct drm_connector *connector)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct drm_connector_helper_funcs *connector_funcs;
 
-	priv->connectors[priv->num_connectors] = connector;
-	priv->encoders[priv->num_encoders++] = connector->encoder;
-
-	/* Only tda998x is supported at the moment. */
-	tilcdc_crtc_set_simulate_vesa_sync(priv->crtc, true);
-	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_tda998x);
+	/* There should never be more than one connector */
+	if (WARN_ON(priv->external_connector))
+		return -EINVAL;
 
+	priv->external_connector = connector;
 	connector_funcs = devm_kzalloc(dev->dev, sizeof(*connector_funcs),
 				       GFP_KERNEL);
 	if (!connector_funcs)
@@ -78,56 +84,177 @@ static int tilcdc_add_external_encoder(struct drm_device *dev,
 	 * everything else but use our own mode_valid() (above).
 	 */
 	if (connector->helper_private) {
-		priv->connector_funcs[priv->num_connectors] =
-			connector->helper_private;
-		*connector_funcs = *priv->connector_funcs[priv->num_connectors];
+		priv->connector_funcs =	connector->helper_private;
+		*connector_funcs = *priv->connector_funcs;
 	} else {
-		priv->connector_funcs[priv->num_connectors] = ERR_PTR(-ENOENT);
+		priv->connector_funcs = ERR_PTR(-ENOENT);
 	}
 	connector_funcs->mode_valid = tilcdc_external_mode_valid;
 	drm_connector_helper_add(connector, connector_funcs);
-	priv->num_connectors++;
 
-	dev_dbg(dev->dev, "External encoder '%s' connected\n",
-		connector->encoder->name);
+	dev_dbg(dev->dev, "External connector '%s' connected\n",
+		connector->name);
 
 	return 0;
 }
 
-int tilcdc_add_external_encoders(struct drm_device *dev)
+static
+struct drm_connector *tilcdc_encoder_find_connector(struct drm_device *ddev,
+						    struct drm_encoder *encoder)
 {
-	struct tilcdc_drm_private *priv = dev->dev_private;
 	struct drm_connector *connector;
-	int num_internal_connectors = priv->num_connectors;
-
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		bool found = false;
-		int i, ret;
-
-		for (i = 0; i < num_internal_connectors; i++)
-			if (connector == priv->connectors[i])
-				found = true;
-		if (!found) {
-			ret = tilcdc_add_external_encoder(dev, connector);
-			if (ret)
-				return ret;
-		}
+	int i;
+
+	list_for_each_entry(connector, &ddev->mode_config.connector_list, head)
+		for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++)
+			if (connector->encoder_ids[i] == encoder->base.id)
+				return connector;
+
+	dev_err(ddev->dev, "No connector found for %s encoder (id %d)\n",
+		encoder->name, encoder->base.id);
+
+	return NULL;
+}
+
+int tilcdc_add_component_encoder(struct drm_device *ddev)
+{
+	struct tilcdc_drm_private *priv = ddev->dev_private;
+	struct drm_connector *connector;
+	struct drm_encoder *encoder;
+
+	list_for_each_entry(encoder, &ddev->mode_config.encoder_list, head)
+		if (encoder->possible_crtcs & (1 << priv->crtc->index))
+			break;
+
+	if (!encoder) {
+		dev_err(ddev->dev, "%s: No suitable encoder found\n", __func__);
+		return -ENODEV;
 	}
-	return 0;
+
+	connector = tilcdc_encoder_find_connector(ddev, encoder);
+
+	if (!connector)
+		return -ENODEV;
+
+	/* Only tda998x is supported at the moment. */
+	tilcdc_crtc_set_simulate_vesa_sync(priv->crtc, true);
+	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_tda998x);
+
+	return tilcdc_add_external_connector(ddev, connector);
 }
 
-void tilcdc_remove_external_encoders(struct drm_device *dev)
+void tilcdc_remove_external_device(struct drm_device *dev)
 {
 	struct tilcdc_drm_private *priv = dev->dev_private;
-	int i;
 
 	/* Restore the original helper functions, if any. */
-	for (i = 0; i < priv->num_connectors; i++)
-		if (IS_ERR(priv->connector_funcs[i]))
-			drm_connector_helper_add(priv->connectors[i], NULL);
-		else if (priv->connector_funcs[i])
-			drm_connector_helper_add(priv->connectors[i],
-						 priv->connector_funcs[i]);
+	if (IS_ERR(priv->connector_funcs))
+		drm_connector_helper_add(priv->external_connector, NULL);
+	else if (priv->connector_funcs)
+		drm_connector_helper_add(priv->external_connector,
+					 priv->connector_funcs);
+}
+
+static const struct drm_encoder_funcs tilcdc_external_encoder_funcs = {
+	.destroy	= drm_encoder_cleanup,
+};
+
+static
+int tilcdc_attach_bridge(struct drm_device *ddev, struct drm_bridge *bridge)
+{
+	struct tilcdc_drm_private *priv = ddev->dev_private;
+	struct drm_connector *connector;
+	int ret;
+
+	priv->external_encoder->possible_crtcs = BIT(0);
+	priv->external_encoder->bridge = bridge;
+	bridge->encoder = priv->external_encoder;
+
+	ret = drm_bridge_attach(ddev, bridge);
+	if (ret) {
+		dev_err(ddev->dev, "drm_bridge_attach() failed %d\n", ret);
+		return ret;
+	}
+
+	tilcdc_crtc_set_panel_info(priv->crtc, &panel_info_default);
+
+	connector = tilcdc_encoder_find_connector(ddev, priv->external_encoder);
+	if (!connector)
+		return -ENODEV;
+
+	ret = tilcdc_add_external_connector(ddev, connector);
+
+	return ret;
+}
+
+static int tilcdc_node_has_port(struct device_node *dev_node)
+{
+	struct device_node *node;
+
+	node = of_get_child_by_name(dev_node, "ports");
+	if (!node)
+		node = of_get_child_by_name(dev_node, "port");
+	if (!node)
+		return 0;
+	of_node_put(node);
+
+	return 1;
+}
+
+static
+struct device_node *tilcdc_get_remote_node(struct device_node *node)
+{
+	struct device_node *ep;
+	struct device_node *parent;
+
+	if (!tilcdc_node_has_port(node))
+		return NULL;
+
+	ep = of_graph_get_next_endpoint(node, NULL);
+	if (!ep)
+		return NULL;
+
+	parent = of_graph_get_remote_port_parent(ep);
+	of_node_put(ep);
+
+	return parent;
+}
+
+int tilcdc_attach_external_device(struct drm_device *ddev)
+{
+	struct tilcdc_drm_private *priv = ddev->dev_private;
+	struct device_node *remote_node;
+	struct drm_bridge *bridge;
+	int ret;
+
+	remote_node = tilcdc_get_remote_node(ddev->dev->of_node);
+	if (!remote_node)
+		return 0;
+
+	bridge = of_drm_find_bridge(remote_node);
+	of_node_put(remote_node);
+	if (!bridge)
+		return -EPROBE_DEFER;
+
+	priv->external_encoder = devm_kzalloc(ddev->dev,
+					      sizeof(*priv->external_encoder),
+					      GFP_KERNEL);
+	if (!priv->external_encoder)
+		return -ENOMEM;
+
+	ret = drm_encoder_init(ddev, priv->external_encoder,
+			       &tilcdc_external_encoder_funcs,
+			       DRM_MODE_ENCODER_NONE, NULL);
+	if (ret) {
+		dev_err(ddev->dev, "drm_encoder_init() failed %d\n", ret);
+		return ret;
+	}
+
+	ret = tilcdc_attach_bridge(ddev, bridge);
+	if (ret)
+		drm_encoder_cleanup(priv->external_encoder);
+
+	return ret;
 }
 
 static int dev_match_of(struct device *dev, void *data)
@@ -141,16 +268,10 @@ int tilcdc_get_external_components(struct device *dev,
 	struct device_node *node;
 	struct device_node *ep = NULL;
 	int count = 0;
+	int ret = 0;
 
-	/* Avoid error print by of_graph_get_next_endpoint() if there
-	 * is no ports present.
-	 */
-	node = of_get_child_by_name(dev->of_node, "ports");
-	if (!node)
-		node = of_get_child_by_name(dev->of_node, "port");
-	if (!node)
+	if (!tilcdc_node_has_port(dev->of_node))
 		return 0;
-	of_node_put(node);
 
 	while ((ep = of_graph_get_next_endpoint(dev->of_node, ep))) {
 		node = of_graph_get_remote_port_parent(ep);
@@ -160,17 +281,20 @@ int tilcdc_get_external_components(struct device *dev,
 		}
 
 		dev_dbg(dev, "Subdevice node '%s' found\n", node->name);
-		if (match)
-			drm_of_component_match_add(dev, match, dev_match_of,
-						   node);
-		of_node_put(node);
-		count++;
-	}
 
-	if (count > 1) {
-		dev_err(dev, "Only one external encoder is supported\n");
-		return -EINVAL;
+		if (of_device_is_compatible(node, "nxp,tda998x")) {
+			if (match)
+				drm_of_component_match_add(dev, match,
+							   dev_match_of, node);
+			ret = 1;
+		}
+
+		of_node_put(node);
+		if (count++ > 1) {
+			dev_err(dev, "Only one port is supported\n");
+			return -EINVAL;
+		}
 	}
 
-	return count;
+	return ret;
 }
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_external.h b/drivers/gpu/drm/tilcdc/tilcdc_external.h
index c700e0c..763d18f 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_external.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_external.h
@@ -18,8 +18,9 @@
 #ifndef __TILCDC_EXTERNAL_H__
 #define __TILCDC_EXTERNAL_H__
 
-int tilcdc_add_external_encoders(struct drm_device *dev);
-void tilcdc_remove_external_encoders(struct drm_device *dev);
+int tilcdc_add_component_encoder(struct drm_device *dev);
+void tilcdc_remove_external_device(struct drm_device *dev);
 int tilcdc_get_external_components(struct device *dev,
 				   struct component_match **match);
+int tilcdc_attach_external_device(struct drm_device *ddev);
 #endif /* __TILCDC_SLAVE_H__ */
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 0/4] drm/tilcdc: Add bridge support and sync-lost flood recovery

[Bartosz Golaszewski]

2016-11-25 10:02 GMT+01:00 Jyri Sarha <jsarha@ti.com>:
> Changes since v4:
> - "drm/bridge: Add ti-tfp410 DVI transmitter driver"
>   - Put i2c behind #if IS_ENABLED(CONFIG_I2C)
> - "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers"
>   - Use exsisting infrastructure to hookup crtc mode validation code
>     to newly connected connector, whether that came from componentized
>     driver or trough an attached bridge
>
> Changes since v3:
> - "drm/tilcdc: Enable sync lost error and recovery handling for rev 1 LCDC"
>   - Fix broken irq enable/disble code for LCDC rev 1
> - Add: "dt-bindings: Move "ti,tfp410.txt" from display/ti to display/bridge"
> - "drm/bridge: Add ti-tfp410 DVI transmitter driver"
>   - Don't fail if either i2c or platform driver register succeeds
>   - ftp410 -> tfp410
>   - Merge the old display/ti,tfp410.txt document with my addition
>
> Changes since v2:
> - "drm/tilcdc: Recover from sync lost error flood by resetting the LCDC"
>   - no change
> - "drm/bridge: Add ti-tfp410 DVI transmitter driver"
>   - Fix deveice-tree document
>     - "driver node" -> "device node"
>     - remove "(the current implementation does not yet support this)"
>   - Add dummy i2c support. The driver probe works also if placed under
>     i2c controller node, but there is no actual i2c probing.
> - "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers"
>   - no change
>
> Changes since first version of the series:
> - "drm/tilcdc: Recover from sync lost error flood by resetting the LCDC"
>   - no change
> - "drm/bridge: Add ti-tfp410 DVI transmitter driver"
>   - HDMI -> DVI
>   - DT Binding document
>     - Prepare for tfp410 connected trough i2c by optional reg property
>     - Require two port nodes
>   - Implementation
>     - Implement connector node functionality with in tfp410 bridge
>       drive, but follow generic connector binding by pulling the
>       ddc-i2c-bus property from the connector node.
> - "drm/tilcdc: Add drm bridge support for attaching drm bridge drivers"
>   - Remove earlier change in TD binding document. There is no need to
>     mention DRM implementation details, like bridge support, in DT
>     binding.
>
> The first patch is an independent on and I've been testing it for
> quite a while now.
>
> The tfp410 bridge driver and the tilcdc bridge support are tested with
> BeagleBone DVI-D Cape Rev A3. The tfp410 bridge driver is missing a
> lot of features, because the DVI-D cape does not have too many wires
> connected. The missing features can be added later when they are
> needed.
>
> Jyri Sarha (4):
>   drm/tilcdc: Recover from sync lost error flood by resetting the LCDC
>   dt-bindings: Move "ti,tfp410.txt" from display/ti to display/bridge
>   drm/bridge: Add ti-tfp410 DVI transmitter driver
>   drm/tilcdc: Add drm bridge support for attaching drm bridge drivers
>
>  .../bindings/display/{ti => bridge}/ti,tfp410.txt  |   9 +-
>  drivers/gpu/drm/bridge/Kconfig                     |   7 +
>  drivers/gpu/drm/bridge/Makefile                    |   1 +
>  drivers/gpu/drm/bridge/ti-tfp410.c                 | 317 +++++++++++++++++++++
>  drivers/gpu/drm/tilcdc/tilcdc_crtc.c               |  26 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c                |  11 +-
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h                |   5 +-
>  drivers/gpu/drm/tilcdc/tilcdc_external.c           | 260 ++++++++++++-----
>  drivers/gpu/drm/tilcdc/tilcdc_external.h           |   5 +-
>  9 files changed, 564 insertions(+), 77 deletions(-)
>  rename Documentation/devicetree/bindings/display/{ti => bridge}/ti,tfp410.txt (65%)
>  create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c
>
> --
> 1.9.1
>

For 1/4 and 4/4:

Tested-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Jyri Sarha]

The module unload should not be allowed while the bridge is attached. So
still need to add these:

On 11/25/16 11:02, Jyri Sarha wrote:
> +
> +static int tfp410_attach(struct drm_bridge *bridge)
> +{
> +	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> +	int ret;
> +


+       if (!try_module_get(THIS_MODULE)) {
+               dev_err(dvi->dev, "Module unloading\n");
+               return -ENODEV;
+       }
+


> +	if (!bridge->encoder) {
> +		dev_err(dvi->dev, "Missing encoder\n");
> +		return -ENODEV;
> +	}
> +
> +	drm_connector_helper_add(&dvi->connector,
> +				 &tfp410_con_helper_funcs);
> +	ret = drm_connector_init(bridge->dev, &dvi->connector,
> +				 &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
> +	if (ret) {
> +		dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
> +		return ret;
> +	}
> +
> +	drm_mode_connector_attach_encoder(&dvi->connector,
> +					  bridge->encoder);
> +
> +	return 0;
> +}

+static void tfp410_detach(struct drm_bridge *bridge)
+{
+       module_put(THIS_MODULE);
+}
+

> +
> +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> +	.attach		= tfp410_attach,

+       .detach         = tfp410_detach,

> +};
> +

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 4/4] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers

[Jyri Sarha]

On 11/25/16 11:02, Jyri Sarha wrote:
> -void tilcdc_remove_external_encoders(struct drm_device *dev)
> +void tilcdc_remove_external_device(struct drm_device *dev)
>  {
>  	struct tilcdc_drm_private *priv = dev->dev_private;
> -	int i;
>  
>  	/* Restore the original helper functions, if any. */
> -	for (i = 0; i < priv->num_connectors; i++)
> -		if (IS_ERR(priv->connector_funcs[i]))
> -			drm_connector_helper_add(priv->connectors[i], NULL);
> -		else if (priv->connector_funcs[i])
> -			drm_connector_helper_add(priv->connectors[i],
> -						 priv->connector_funcs[i]);
> +	if (IS_ERR(priv->connector_funcs))
> +		drm_connector_helper_add(priv->external_connector, NULL);
> +	else if (priv->connector_funcs)
> +		drm_connector_helper_add(priv->external_connector,
> +					 priv->connector_funcs);


I still need to add:
+
+       if (priv->external_encoder && priv->external_encoder->bridge)
+               drm_bridge_detach(priv->external_encoder->bridge);

> +}

... and reorder the tilcdc_fini() a bit. I need to put
tilcdc_remove_external_device() before drm_mode_config_cleanup()
for drm_bridge_detach() to be called before whole mode config is teared
apart.

There is also a bug in tearing down drm debugfs that had to be fixed
before I got module unload and reload to work properly. I'll send a
patch shortly.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Laurent Pinchart]

Hi Jyri,

On Tuesday 29 Nov 2016 22:18:25 Jyri Sarha wrote:
> The module unload should not be allowed while the bridge is attached. So
> still need to add these:
> 
> On 11/25/16 11:02, Jyri Sarha wrote:
> > +
> > +static int tfp410_attach(struct drm_bridge *bridge)
> > +{
> > +	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
> > +	int ret;
> > +
> 
> +       if (!try_module_get(THIS_MODULE)) {
> +               dev_err(dvi->dev, "Module unloading\n");
> +               return -ENODEV;
> +       }
> +

Shouldn't this be done in core code ?

> > +	if (!bridge->encoder) {
> > +		dev_err(dvi->dev, "Missing encoder\n");
> > +		return -ENODEV;
> > +	}
> > +
> > +	drm_connector_helper_add(&dvi->connector,
> > +				 &tfp410_con_helper_funcs);
> > +	ret = drm_connector_init(bridge->dev, &dvi->connector,
> > +				 &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA);
> > +	if (ret) {
> > +		dev_err(dvi->dev, "drm_connector_init() failed: %d\n", ret);
> > +		return ret;
> > +	}
> > +
> > +	drm_mode_connector_attach_encoder(&dvi->connector,
> > +					  bridge->encoder);
> > +
> > +	return 0;
> > +}
> 
> +static void tfp410_detach(struct drm_bridge *bridge)
> +{
> +       module_put(THIS_MODULE);
> +}
> +
> 
> > +
> > +static const struct drm_bridge_funcs tfp410_bridge_funcs = {
> > +	.attach		= tfp410_attach,
> 
> +       .detach         = tfp410_detach,
> 
> > +};
> > +

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 4/4] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers

[Laurent Pinchart]

Hi Jyri,

On Tuesday 29 Nov 2016 22:18:56 Jyri Sarha wrote:
> On 11/25/16 11:02, Jyri Sarha wrote:
> > -void tilcdc_remove_external_encoders(struct drm_device *dev)
> > +void tilcdc_remove_external_device(struct drm_device *dev)
> >  {
> >  	struct tilcdc_drm_private *priv = dev->dev_private;
> > -	int i;
> > 
> >  	/* Restore the original helper functions, if any. */
> > -	for (i = 0; i < priv->num_connectors; i++)
> > -		if (IS_ERR(priv->connector_funcs[i]))
> > -			drm_connector_helper_add(priv->connectors[i], NULL);
> > -		else if (priv->connector_funcs[i])
> > -			drm_connector_helper_add(priv->connectors[i],
> > -						 priv->connector_funcs[i]);
> > +	if (IS_ERR(priv->connector_funcs))
> > +		drm_connector_helper_add(priv->external_connector, NULL);
> > +	else if (priv->connector_funcs)
> > +		drm_connector_helper_add(priv->external_connector,
> > +					 priv->connector_funcs);
> 
> I still need to add:
> +
> +       if (priv->external_encoder && priv->external_encoder->bridge)
> +               drm_bridge_detach(priv->external_encoder->bridge);

"drm: bridge: Detach bridge from encoder at encoder cleanup time" :-)

> > +}
> 
> ... and reorder the tilcdc_fini() a bit. I need to put
> tilcdc_remove_external_device() before drm_mode_config_cleanup()
> for drm_bridge_detach() to be called before whole mode config is teared
> apart.
> 
> There is also a bug in tearing down drm debugfs that had to be fixed
> before I got module unload and reload to work properly. I'll send a
> patch shortly.

-- 
Regards,

Laurent Pinchart

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v5 3/4] drm/bridge: Add ti-tfp410 DVI transmitter driver

[Jyri Sarha]

On 11/29/16 22:26, Laurent Pinchart wrote:
> Hi Jyri,
> 
> On Tuesday 29 Nov 2016 22:18:25 Jyri Sarha wrote:
>> > The module unload should not be allowed while the bridge is attached. So
>> > still need to add these:
>> > 
>> > On 11/25/16 11:02, Jyri Sarha wrote:
>>> > > +
>>> > > +static int tfp410_attach(struct drm_bridge *bridge)
>>> > > +{
>>> > > +	struct tfp410 *dvi = drm_bridge_to_tfp410(bridge);
>>> > > +	int ret;
>>> > > +
>> > 
>> > +       if (!try_module_get(THIS_MODULE)) {
>> > +               dev_err(dvi->dev, "Module unloading\n");
>> > +               return -ENODEV;
>> > +       }
>> > +
> Shouldn't this be done in core code ?
> 

Hmmm, probably. I'll send a patch shortly.

Cheers,
Jyri
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel