* [PATCH v3 0/3] drm/tilcdc: Add bridge support and sync-lost flood recovery @ 2016-11-17 13:28 Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 1/3] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC Jyri Sarha ` (2 more replies) 0 siblings, 3 replies; 10+ messages in thread From: Jyri Sarha @ 2016-11-17 13:28 UTC (permalink / raw) To: dri-devel, devicetree Cc: bcousson, khilman, Jyri Sarha, bgolaszewski, tomi.valkeinen, laurent.pinchart 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 (3): drm/tilcdc: Recover from sync lost error flood by resetting the LCDC drm/bridge: Add ti-tfp410 DVI transmitter driver drm/tilcdc: Add drm bridge support for attaching drm bridge drivers .../bindings/display/bridge/ti,tfp410.txt | 40 +++ drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++ drivers/gpu/drm/tilcdc/tilcdc_crtc.c | 26 +- drivers/gpu/drm/tilcdc/tilcdc_drv.c | 7 +- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 140 ++++++++-- drivers/gpu/drm/tilcdc/tilcdc_external.h | 1 + 9 files changed, 491 insertions(+), 20 deletions(-) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt 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 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 1/3] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC 2016-11-17 13:28 [PATCH v3 0/3] drm/tilcdc: Add bridge support and sync-lost flood recovery Jyri Sarha @ 2016-11-17 13:28 ` Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 3/3] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers Jyri Sarha 2 siblings, 0 replies; 10+ messages in thread From: Jyri Sarha @ 2016-11-17 13:28 UTC (permalink / raw) To: dri-devel, devicetree Cc: bcousson, khilman, Jyri Sarha, bgolaszewski, tomi.valkeinen, laurent.pinchart 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver 2016-11-17 13:28 [PATCH v3 0/3] drm/tilcdc: Add bridge support and sync-lost flood recovery Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 1/3] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC Jyri Sarha @ 2016-11-17 13:28 ` Jyri Sarha 2016-11-17 13:39 ` Jyri Sarha [not found] ` <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org> 2016-11-17 13:28 ` [PATCH v3 3/3] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers Jyri Sarha 2 siblings, 2 replies; 10+ messages in thread From: Jyri Sarha @ 2016-11-17 13:28 UTC (permalink / raw) To: dri-devel, devicetree Cc: bcousson, khilman, Jyri Sarha, bgolaszewski, tomi.valkeinen, laurent.pinchart Add very basic ti-ftp410 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 added. Signed-off-by: Jyri Sarha <jsarha@ti.com> --- .../bindings/display/bridge/ti,tfp410.txt | 40 +++ drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++ 4 files changed, 335 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt 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 new file mode 100644 index 0000000..6174b18 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt @@ -0,0 +1,40 @@ +TFP410 DVI bridge bindings + +Required properties: + - compatible: "ti,tfp410" + +Optional properties + - 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 subnodes: + - port@0: Video input port node to connect the bridge to a + display controller output [1]. + - port@1: Video output port node to connect the bridge to a + connector input [1]. + +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt + +Example: + hdmi-bridge { + compatible = "ti,tfp410"; + ports { + #address-cells = <1>; + #size-cells = <0>; + + port@0 { + reg = <0>; + bridge_in: endpoint { + remote-endpoint = <&dc_out>; + }; + }; + + port@1 { + reg = <1>; + bridge_out: endpoint { + remote-endpoint = <&hdmi_in>; + }; + }; + }; + }; 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..64f54e4 --- /dev/null +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -0,0 +1,287 @@ +/* + * 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); +} + +/* 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", ®)) { + 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 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, + }, +}; + +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, +}; + +static int __init tfp410_module_init(void) +{ + i2c_add_driver(&tfp410_i2c_driver); + platform_driver_register(&tfp410_platform_driver); + + return 0; +} +module_init(tfp410_module_init); + +static void __exit tfp410_module_exit(void) +{ + i2c_del_driver(&tfp410_i2c_driver); + 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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver 2016-11-17 13:28 ` [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver Jyri Sarha @ 2016-11-17 13:39 ` Jyri Sarha 2016-11-17 17:20 ` Laurent Pinchart [not found] ` <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org> 1 sibling, 1 reply; 10+ messages in thread From: Jyri Sarha @ 2016-11-17 13:39 UTC (permalink / raw) To: dri-devel, devicetree Cc: bcousson, khilman, bgolaszewski, tomi.valkeinen, laurent.pinchart On 11/17/16 15:28, Jyri Sarha wrote: > Add very basic ti-ftp410 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 added. > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > --- > .../bindings/display/bridge/ti,tfp410.txt | 40 +++ > drivers/gpu/drm/bridge/Kconfig | 7 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++ > 4 files changed, 335 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > 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 > new file mode 100644 > index 0000000..6174b18 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > @@ -0,0 +1,40 @@ > +TFP410 DVI bridge bindings > + > +Required properties: > + - compatible: "ti,tfp410" > + > +Optional properties > + - 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 subnodes: > + - port@0: Video input port node to connect the bridge to a > + display controller output [1]. > + - port@1: Video output port node to connect the bridge to a > + connector input [1]. > + > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + hdmi-bridge { > + compatible = "ti,tfp410"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + bridge_in: endpoint { > + remote-endpoint = <&dc_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + bridge_out: endpoint { > + remote-endpoint = <&hdmi_in>; > + }; > + }; > + }; > + }; > 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..64f54e4 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -0,0 +1,287 @@ > +/* > + * 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); > +} > + > +/* 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", ®)) { > + 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 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, > + }, > +}; > + > +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, > +}; > + > +static int __init tfp410_module_init(void) > +{ > + i2c_add_driver(&tfp410_i2c_driver); > + platform_driver_register(&tfp410_platform_driver); Oops, forgot to handle the return values. This is how I fixed it: static int __init tfp410_module_init(void) { int ret; ret = i2c_add_driver(&tfp410_i2c_driver); if (ret) return ret; ret = platform_driver_register(&tfp410_platform_driver); if (ret) i2c_del_driver(&tfp410_i2c_driver); return ret; } > + > + return 0; > +} > +module_init(tfp410_module_init); > + > +static void __exit tfp410_module_exit(void) > +{ > + i2c_del_driver(&tfp410_i2c_driver); > + 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"); > _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver 2016-11-17 13:39 ` Jyri Sarha @ 2016-11-17 17:20 ` Laurent Pinchart 2016-11-17 20:16 ` Jyri Sarha 0 siblings, 1 reply; 10+ messages in thread From: Laurent Pinchart @ 2016-11-17 17:20 UTC (permalink / raw) To: Jyri Sarha Cc: devicetree, bcousson, khilman, dri-devel, bgolaszewski, tomi.valkeinen Hi Jyri, On Thursday 17 Nov 2016 15:39:26 Jyri Sarha wrote: > On 11/17/16 15:28, Jyri Sarha wrote: > > Add very basic ti-ftp410 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 added. > > > > Signed-off-by: Jyri Sarha <jsarha@ti.com> > > --- > > > > .../bindings/display/bridge/ti,tfp410.txt | 40 +++ > > drivers/gpu/drm/bridge/Kconfig | 7 + > > drivers/gpu/drm/bridge/Makefile | 1 + > > drivers/gpu/drm/bridge/ti-tfp410.c | 287 ++++++++++++++++ > > 4 files changed, 335 insertions(+) > > create mode 100644 > > Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt 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 new file > > mode 100644 > > index 0000000..6174b18 > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > > @@ -0,0 +1,40 @@ > > +TFP410 DVI bridge bindings > > + > > +Required properties: > > + - compatible: "ti,tfp410" > > + > > +Optional properties > > + - 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 subnodes: > > + - port@0: Video input port node to connect the bridge to a > > + display controller output [1]. > > + - port@1: Video output port node to connect the bridge to a > > + connector input [1]. > > + > > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > > + > > +Example: > > + hdmi-bridge { > > + compatible = "ti,tfp410"; > > + ports { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > + port@0 { > > + reg = <0>; > > + bridge_in: endpoint { > > + remote-endpoint = <&dc_out>; > > + }; > > + }; > > + > > + port@1 { > > + reg = <1>; > > + bridge_out: endpoint { > > + remote-endpoint = <&hdmi_in>; > > + }; > > + }; > > + }; > > + }; > > 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..64f54e4 > > --- /dev/null > > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > > @@ -0,0 +1,287 @@ > > +/* > > + * 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); > > +} > > + > > +/* 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", ®)) { > > + 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 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, > > + }, > > +}; > > + > > +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, > > +}; > > + > > +static int __init tfp410_module_init(void) > > +{ > > + i2c_add_driver(&tfp410_i2c_driver); > > + platform_driver_register(&tfp410_platform_driver); > > Oops, forgot to handle the return values. This is how I fixed it: > static int __init tfp410_module_init(void) > { > int ret; > > ret = i2c_add_driver(&tfp410_i2c_driver); > if (ret) > return ret; > > ret = platform_driver_register(&tfp410_platform_driver); > if (ret) > i2c_del_driver(&tfp410_i2c_driver); > > return ret; > } If registration of one of the two drivers fail, wouldn't it make sense to still continue (probably with an error message) to avoid breaking the other one ? > > + > > + return 0; > > +} > > +module_init(tfp410_module_init); > > + > > +static void __exit tfp410_module_exit(void) > > +{ > > + i2c_del_driver(&tfp410_i2c_driver); > > + 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"); -- Regards, Laurent Pinchart _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver 2016-11-17 17:20 ` Laurent Pinchart @ 2016-11-17 20:16 ` Jyri Sarha 0 siblings, 0 replies; 10+ messages in thread From: Jyri Sarha @ 2016-11-17 20:16 UTC (permalink / raw) To: Laurent Pinchart Cc: devicetree, bcousson, khilman, dri-devel, bgolaszewski, tomi.valkeinen On 11/17/16 19:20, Laurent Pinchart wrote: >> > Oops, forgot to handle the return values. This is how I fixed it: >> > static int __init tfp410_module_init(void) >> > { >> > int ret; >> > >> > ret = i2c_add_driver(&tfp410_i2c_driver); >> > if (ret) >> > return ret; >> > >> > ret = platform_driver_register(&tfp410_platform_driver); >> > if (ret) >> > i2c_del_driver(&tfp410_i2c_driver); >> > >> > return ret; >> > } > If registration of one of the two drivers fail, wouldn't it make sense to > still continue (probably with an error message) to avoid breaking the other > one ? > Oh, good point. I fix that too. Anything else about this series? I'd like to send a pull request for 4.10 soon. >>> > > + >>> > > + return 0; >>> > > +} _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
[parent not found: <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org>]
* Re: [v3,2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver [not found] ` <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org> @ 2016-11-18 5:00 ` Christopher Spinrath 2016-11-18 21:33 ` Jyri Sarha 2016-11-21 14:55 ` [PATCH v3 2/3] " Rob Herring 1 sibling, 1 reply; 10+ messages in thread From: Christopher Spinrath @ 2016-11-18 5:00 UTC (permalink / raw) To: Jyri Sarha, dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, devicetree-u79uwXL29TY76Z2rM5mHXA Cc: bcousson-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, bgolaszewski-rdvid1DuHRBWk0Htik3J/w, tomi.valkeinen-l0cyMroinI0, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw, Spinrath, Christopher Hi Jyri, On 11/17/2016 02:28 PM, Jyri Sarha wrote: > Add very basic ti-ftp410 DVI transmitter driver. The only feature s/ftp/tfp ? > 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 added. > > Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org> Thanks for working on this. I have tested the driver on my Utilite Pro which is based on the imx6q SoC and has a tfp410 chip hooked up to its parallel rgb24 interface. So you can add my Tested-By: Christopher Spinrath <christopher.spinrath-vA1bhqPz9FBZXbeN9DUtxg@public.gmane.org> However, I have two more comments below. > --- > .../bindings/display/bridge/ti,tfp410.txt | 40 +++ > drivers/gpu/drm/bridge/Kconfig | 7 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++ > 4 files changed, 335 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > 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 > new file mode 100644 > index 0000000..6174b18 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt There already is a binding documentation for the tfp410 in Documentation/devicetree/bindings/display/ti/ti,tfp410.txt . It is used by the omap drm driver. IMHO you should extend, move or deprecate that one. Note that it describes an optional powerdown-gpios property. > @@ -0,0 +1,40 @@ > +TFP410 DVI bridge bindings > + > +Required properties: > + - compatible: "ti,tfp410" > + > +Optional properties > + - 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 subnodes: > + - port@0: Video input port node to connect the bridge to a > + display controller output [1]. > + - port@1: Video output port node to connect the bridge to a > + connector input [1]. > + > +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt > + > +Example: > + hdmi-bridge { > + compatible = "ti,tfp410"; > + ports { > + #address-cells = <1>; > + #size-cells = <0>; > + > + port@0 { > + reg = <0>; > + bridge_in: endpoint { > + remote-endpoint = <&dc_out>; > + }; > + }; > + > + port@1 { > + reg = <1>; > + bridge_out: endpoint { > + remote-endpoint = <&hdmi_in>; > + }; > + }; > + }; > + }; > 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..64f54e4 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -0,0 +1,287 @@ > +/* > + * Copyright (C) 2016 Texas Instruments > + * Author: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org> > + * > + * 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; > + } I wonder what happens if a display with none or defect ddc/edid is attached? The dumb-vga-dac bridge driver reports connector_status_unknown in the case drm_probe_ddc fails. Cheers, Christopher > + > + 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); > +} > + > +/* 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", ®)) { > + 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 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, > + }, > +}; > + > +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, > +}; > + > +static int __init tfp410_module_init(void) > +{ > + i2c_add_driver(&tfp410_i2c_driver); > + platform_driver_register(&tfp410_platform_driver); > + > + return 0; > +} > +module_init(tfp410_module_init); > + > +static void __exit tfp410_module_exit(void) > +{ > + i2c_del_driver(&tfp410_i2c_driver); > + platform_driver_unregister(&tfp410_platform_driver); > +} > +module_exit(tfp410_module_exit); > + > +MODULE_AUTHOR("Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org>"); > +MODULE_DESCRIPTION("TI TFP410 DVI bridge driver"); > +MODULE_LICENSE("GPL"); > > -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [v3,2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver 2016-11-18 5:00 ` [v3,2/3] " Christopher Spinrath @ 2016-11-18 21:33 ` Jyri Sarha 0 siblings, 0 replies; 10+ messages in thread From: Jyri Sarha @ 2016-11-18 21:33 UTC (permalink / raw) To: Christopher Spinrath, dri-devel, devicetree Cc: khilman, tomi.valkeinen, laurent.pinchart, bcousson, bgolaszewski On 11/18/16 07:00, Christopher Spinrath wrote: > Hi Jyri, > > On 11/17/2016 02:28 PM, Jyri Sarha wrote: >> Add very basic ti-ftp410 DVI transmitter driver. The only feature > > s/ftp/tfp ? > My fingers just want type these three letter is that order, wonder why :). >> 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 added. >> >> Signed-off-by: Jyri Sarha <jsarha@ti.com> > > Thanks for working on this. I have tested the driver on my Utilite Pro > which is based on the imx6q SoC and has a tfp410 chip hooked up to its > parallel rgb24 interface. So you can add my > > Tested-By: Christopher Spinrath <christopher.spinrath@rwth-aachen.de> > Thanks, I'll address these comments and resend the corrected patches before sending a pull request. > However, I have two more comments below. > >> --- >> .../bindings/display/bridge/ti,tfp410.txt | 40 +++ >> drivers/gpu/drm/bridge/Kconfig | 7 + >> drivers/gpu/drm/bridge/Makefile | 1 + >> drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++ >> 4 files changed, 335 insertions(+) >> create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt >> 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 >> new file mode 100644 >> index 0000000..6174b18 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > > There already is a binding documentation for the tfp410 in > > Documentation/devicetree/bindings/display/ti/ti,tfp410.txt . > > It is used by the omap drm driver. IMHO you should extend, move or > deprecate that one. Note that it describes an optional powerdown-gpios > property. > Good that you noticed that. Guess we need to unify those. Probably to the new location under bridge directory. >> @@ -0,0 +1,40 @@ >> +TFP410 DVI bridge bindings >> + >> +Required properties: >> + - compatible: "ti,tfp410" >> + >> +Optional properties >> + - 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 subnodes: >> + - port@0: Video input port node to connect the bridge to a >> + display controller output [1]. >> + - port@1: Video output port node to connect the bridge to a >> + connector input [1]. >> + >> +[1]: Documentation/devicetree/bindings/media/video-interfaces.txt >> + >> +Example: >> + hdmi-bridge { >> + compatible = "ti,tfp410"; >> + ports { >> + #address-cells = <1>; >> + #size-cells = <0>; >> + >> + port@0 { >> + reg = <0>; >> + bridge_in: endpoint { >> + remote-endpoint = <&dc_out>; >> + }; >> + }; >> + >> + port@1 { >> + reg = <1>; >> + bridge_out: endpoint { >> + remote-endpoint = <&hdmi_in>; >> + }; >> + }; >> + }; >> + }; >> 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..64f54e4 >> --- /dev/null >> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c >> @@ -0,0 +1,287 @@ >> +/* >> + * 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; >> + } > > I wonder what happens if a display with none or defect ddc/edid is > attached? The dumb-vga-dac bridge driver reports > connector_status_unknown in the case drm_probe_ddc fails. > If there is such a problem with DVI/HDMI device, then the device is clearly broken. In such a case one can hack a custom dtd file without ddc-i2c-bus. The situation with VGA is slightly different. There may still be some perfectly legitimate legacy VGA devices that do not support ddc. I do not have a strong opinion about this, but would like to wait if anybody is bothered about this behaviour and fix it only if there is a problem. > Cheers, > Christopher > >> + >> + 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); >> +} >> + >> +/* 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", ®)) { >> + 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 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, >> + }, >> +}; >> + >> +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, >> +}; >> + >> +static int __init tfp410_module_init(void) >> +{ >> + i2c_add_driver(&tfp410_i2c_driver); >> + platform_driver_register(&tfp410_platform_driver); >> + >> + return 0; >> +} >> +module_init(tfp410_module_init); >> + >> +static void __exit tfp410_module_exit(void) >> +{ >> + i2c_del_driver(&tfp410_i2c_driver); >> + 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"); >> >> _______________________________________________ dri-devel mailing list dri-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/dri-devel ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver [not found] ` <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org> 2016-11-18 5:00 ` [v3,2/3] " Christopher Spinrath @ 2016-11-21 14:55 ` Rob Herring 1 sibling, 0 replies; 10+ messages in thread From: Rob Herring @ 2016-11-21 14:55 UTC (permalink / raw) To: Jyri Sarha Cc: dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, devicetree-u79uwXL29TY76Z2rM5mHXA, bcousson-rdvid1DuHRBWk0Htik3J/w, khilman-rdvid1DuHRBWk0Htik3J/w, bgolaszewski-rdvid1DuHRBWk0Htik3J/w, tomi.valkeinen-l0cyMroinI0, laurent.pinchart-ryLnwIuWjnjg/C1BVhZhaw On Thu, Nov 17, 2016 at 03:28:47PM +0200, Jyri Sarha wrote: > Add very basic ti-ftp410 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 added. > > Signed-off-by: Jyri Sarha <jsarha-l0cyMroinI0@public.gmane.org> > --- > .../bindings/display/bridge/ti,tfp410.txt | 40 +++ Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> > drivers/gpu/drm/bridge/Kconfig | 7 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/ti-tfp410.c | 287 +++++++++++++++++++++ > 4 files changed, 335 insertions(+) > create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c -- To unsubscribe from this list: send the line "unsubscribe devicetree" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH v3 3/3] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers 2016-11-17 13:28 [PATCH v3 0/3] drm/tilcdc: Add bridge support and sync-lost flood recovery Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 1/3] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver Jyri Sarha @ 2016-11-17 13:28 ` Jyri Sarha 2 siblings, 0 replies; 10+ messages in thread From: Jyri Sarha @ 2016-11-17 13:28 UTC (permalink / raw) To: dri-devel, devicetree Cc: bcousson, khilman, Jyri Sarha, bgolaszewski, tomi.valkeinen, laurent.pinchart 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 | 7 +- drivers/gpu/drm/tilcdc/tilcdc_drv.h | 2 + drivers/gpu/drm/tilcdc/tilcdc_external.c | 140 +++++++++++++++++++++++++++---- drivers/gpu/drm/tilcdc/tilcdc_external.h | 1 + 4 files changed, 131 insertions(+), 19 deletions(-) diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c index 3d2cea0..af959df 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c @@ -384,9 +384,14 @@ static int tilcdc_init(struct drm_driver *ddrv, struct device *dev) ret = tilcdc_add_external_encoders(ddev); if (ret < 0) goto init_failed; + } else { + ret = tilcdc_attach_remote_device(ddev); + if (ret) + goto init_failed; } - if ((priv->num_encoders == 0) || (priv->num_connectors == 0)) { + if (!priv->remote_encoder && + ((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..283ff28 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h @@ -90,6 +90,8 @@ struct tilcdc_drm_private { struct drm_connector *connectors[8]; const struct drm_connector_helper_funcs *connector_funcs[8]; + struct drm_encoder *remote_encoder; + 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..e1576ba 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.c +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.c @@ -28,6 +28,18 @@ .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) { @@ -130,6 +142,101 @@ void tilcdc_remove_external_encoders(struct drm_device *dev) priv->connector_funcs[i]); } +static const struct drm_encoder_funcs tilcdc_remote_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; + int ret; + + priv->remote_encoder->possible_crtcs = BIT(0); + priv->remote_encoder->bridge = bridge; + bridge->encoder = priv->remote_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); + + return 0; +} + +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_remote_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->remote_encoder = devm_kzalloc(ddev->dev, + sizeof(*priv->remote_encoder), + GFP_KERNEL); + if (!priv->remote_encoder) + return -ENOMEM; + + ret = drm_encoder_init(ddev, priv->remote_encoder, + &tilcdc_remote_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->remote_encoder); + + return ret; +} + static int dev_match_of(struct device *dev, void *data) { return dev->of_node == data; @@ -141,16 +248,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 +261,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..a27c365 100644 --- a/drivers/gpu/drm/tilcdc/tilcdc_external.h +++ b/drivers/gpu/drm/tilcdc/tilcdc_external.h @@ -22,4 +22,5 @@ void tilcdc_remove_external_encoders(struct drm_device *dev); int tilcdc_get_external_components(struct device *dev, struct component_match **match); +int tilcdc_attach_remote_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 ^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2016-11-21 14:55 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2016-11-17 13:28 [PATCH v3 0/3] drm/tilcdc: Add bridge support and sync-lost flood recovery Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 1/3] drm/tilcdc: Recover from sync lost error flood by resetting the LCDC Jyri Sarha 2016-11-17 13:28 ` [PATCH v3 2/3] drm/bridge: Add ti-tfp410 DVI transmitter driver Jyri Sarha 2016-11-17 13:39 ` Jyri Sarha 2016-11-17 17:20 ` Laurent Pinchart 2016-11-17 20:16 ` Jyri Sarha [not found] ` <6eaa1b5d467cd354f7d5dc5ff8da1b9bf732ff11.1479388871.git.jsarha-l0cyMroinI0@public.gmane.org> 2016-11-18 5:00 ` [v3,2/3] " Christopher Spinrath 2016-11-18 21:33 ` Jyri Sarha 2016-11-21 14:55 ` [PATCH v3 2/3] " Rob Herring 2016-11-17 13:28 ` [PATCH v3 3/3] drm/tilcdc: Add drm bridge support for attaching drm bridge drivers Jyri Sarha
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.