| Message ID | 9b6a6a4405de344cb9fbea735fea37adee1d726e.1478103726.git.jsarha@ti.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 02/11/16 18:32, Jyri Sarha wrote:
> Add very basic ti-ftp410 HDMI transmitter driver. The only feature
It's DVI encoder, not HDMI.
Tomi
Hi Jyri, Thank you for the patch. On Wednesday 02 Nov 2016 18:32:16 Jyri Sarha wrote: > Add very basic ti-ftp410 HDMI transmitter driver. The only feature > separating this from a completely dummy bridge is the DDC i2c > support. However, other HW specific features may be added later when > needed. For instance there is a set of registers behind i2c if it is > connected. The implementations is tested against my new tilcdc bridge > support and 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 | 30 ++++ > drivers/gpu/drm/bridge/Kconfig | 7 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/ti-tfp410.c | 199 ++++++++++++++++++ > 4 files changed, 237 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..dc93713 > --- /dev/null > +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > @@ -0,0 +1,30 @@ > +TFP410 HDMI/DVI bridge bindings I'd name the document "TI TFP410 DVI Transmitter". DVI bridge doesn't tell whether the device is a receiver or transmitter. > +Required properties: > + - compatible: "ti,tfp410" The device is an I2C slave, it should have a reg property. Given that the chip can be used without being controlled through I2C, the reg property should be optional. You should document this clearly, and explain how the DT node can be instantiated as a child of an I2C controller when the I2C interface is used, or in other parts of the device tree otherwise. > +Optional properties: > + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing The TFP410 doesn't handle DDC, this property should be part of the connector node. > +Optional subnodes: > + - video input: this subnode can contain a video input port node > + to connect the bridge to a display controller output (See this > + documentation [1]). You also need an output port for the DVI output. Those two ports should be required, not optional. > +[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>; > + }; > + }; > + }; > + }; > 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..b0753d2 > --- /dev/null > +++ b/drivers/gpu/drm/bridge/ti-tfp410.c > @@ -0,0 +1,199 @@ > +/* > + * 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 <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 *hdmi = drm_connector_to_tfp410(connector); > + struct edid *edid; > + int ret; > + > + if (!hdmi->ddc) > + goto fallback; > + > + edid = drm_get_edid(connector, hdmi->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 *hdmi = drm_connector_to_tfp410(connector); > + > + if (hdmi->ddc) { > + if (drm_probe_ddc(hdmi->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 *hdmi = drm_bridge_to_tfp410(bridge); > + int ret; > + > + if (!bridge->encoder) { > + dev_err(hdmi->dev, "Missing encoder\n"); > + return -ENODEV; > + } > + > + drm_connector_helper_add(&hdmi->connector, > + &tfp410_con_helper_funcs); > + ret = drm_connector_init(bridge->dev, &hdmi->connector, > + &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA); > + if (ret) { > + dev_err(hdmi->dev, "drm_connector_init() failed: %d\n", ret); > + return ret; > + } > + > + drm_mode_connector_attach_encoder(&hdmi->connector, > + bridge->encoder); > + > + return 0; > +} > + > +static const struct drm_bridge_funcs tfp410_bridge_funcs = { > + .attach = tfp410_attach, > +}; > + > +static int tfp410_probe(struct platform_device *pdev) > +{ > + struct device_node *ddc_phandle; > + struct tfp410 *hdmi; > + int ret; > + > + if (!pdev->dev.of_node) { > + dev_err(&pdev->dev, "device-tree data is missing\n"); > + return -ENXIO; > + } > + > + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); > + if (!hdmi) > + return -ENOMEM; > + platform_set_drvdata(pdev, hdmi); > + > + ddc_phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c", 0); > + if (ddc_phandle) { > + hdmi->ddc = of_get_i2c_adapter_by_node(ddc_phandle); > + of_node_put(ddc_phandle); > + if (!hdmi->ddc) > + return -EPROBE_DEFER; > + } else { > + dev_info(&pdev->dev, > + "No ddc i2c bus, disabling EDID readout\n"); > + } > + > + hdmi->bridge.funcs = &tfp410_bridge_funcs; > + hdmi->bridge.of_node = pdev->dev.of_node; > + hdmi->dev = &pdev->dev; > + > + ret = drm_bridge_add(&hdmi->bridge); > + if (ret) { > + dev_err(&pdev->dev, "drm_bridge_add() failed: %d\n", ret); > + goto fail; > + } > + > + return 0; > +fail: > + i2c_put_adapter(hdmi->ddc); > + return ret; > +} > + > +static int tfp410_remove(struct platform_device *pdev) > +{ > + struct tfp410 *hdmi = platform_get_drvdata(pdev); > + > + drm_bridge_remove(&hdmi->bridge); > + > + if (hdmi->ddc) > + i2c_put_adapter(hdmi->ddc); > + > + return 0; > +} > + > +static const struct of_device_id tfp410_match[] = { > + { .compatible = "ti,tfp410" }, > + {}, > +}; > +MODULE_DEVICE_TABLE(of, tfp410_match); > + > +struct platform_driver tfp410_driver = { > + .probe = tfp410_probe, > + .remove = tfp410_remove, > + .driver = { > + .name = "tfp410-bridge", > + .of_match_table = tfp410_match, > + }, > +}; > +module_platform_driver(tfp410_driver); > + > +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>"); > +MODULE_DESCRIPTION("TI TFP410 HDMI bridge driver"); > +MODULE_LICENSE("GPL");
On 11/03/16 19:46, Laurent Pinchart wrote: > Hi Jyri, > > Thank you for the patch. > > On Wednesday 02 Nov 2016 18:32:16 Jyri Sarha wrote: >> Add very basic ti-ftp410 HDMI transmitter driver. The only feature >> separating this from a completely dummy bridge is the DDC i2c >> support. However, other HW specific features may be added later when >> needed. For instance there is a set of registers behind i2c if it is >> connected. The implementations is tested against my new tilcdc bridge >> support and 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 | 30 ++++ >> drivers/gpu/drm/bridge/Kconfig | 7 + >> drivers/gpu/drm/bridge/Makefile | 1 + >> drivers/gpu/drm/bridge/ti-tfp410.c | 199 ++++++++++++++++++ >> 4 files changed, 237 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..dc93713 >> --- /dev/null >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt >> @@ -0,0 +1,30 @@ >> +TFP410 HDMI/DVI bridge bindings > > I'd name the document "TI TFP410 DVI Transmitter". DVI bridge doesn't tell > whether the device is a receiver or transmitter. > >> +Required properties: >> + - compatible: "ti,tfp410" > > The device is an I2C slave, it should have a reg property. Given that the chip > can be used without being controlled through I2C, the reg property should be > optional. You should document this clearly, and explain how the DT node can be > instantiated as a child of an I2C controller when the I2C interface is used, > or in other parts of the device tree otherwise. > >> +Optional properties: >> + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing > > The TFP410 doesn't handle DDC, this property should be part of the connector > node. > >> +Optional subnodes: >> + - video input: this subnode can contain a video input port node >> + to connect the bridge to a display controller output (See this >> + documentation [1]). > > You also need an output port for the DVI output. Those two ports should be > required, not optional. > Ok. So I need another device node. Should I create some specific compatible string for connectors behind tfp410, or a generic DVI/HDMI connector with optional ddc-i2c phandle? The implementation side is not so critical, because it more easily changed, but should I create an independent generic platform-device driver for such DVI/HDMI connector or just implement the connector side within tfp410 driver? >> +[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>; >> + }; >> + }; >> + }; >> + }; > > >> 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..b0753d2 >> --- /dev/null >> +++ b/drivers/gpu/drm/bridge/ti-tfp410.c >> @@ -0,0 +1,199 @@ >> +/* >> + * 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 <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 *hdmi = drm_connector_to_tfp410(connector); >> + struct edid *edid; >> + int ret; >> + >> + if (!hdmi->ddc) >> + goto fallback; >> + >> + edid = drm_get_edid(connector, hdmi->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 *hdmi = drm_connector_to_tfp410(connector); >> + >> + if (hdmi->ddc) { >> + if (drm_probe_ddc(hdmi->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 *hdmi = drm_bridge_to_tfp410(bridge); >> + int ret; >> + >> + if (!bridge->encoder) { >> + dev_err(hdmi->dev, "Missing encoder\n"); >> + return -ENODEV; >> + } >> + >> + drm_connector_helper_add(&hdmi->connector, >> + &tfp410_con_helper_funcs); >> + ret = drm_connector_init(bridge->dev, &hdmi->connector, >> + &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA); >> + if (ret) { >> + dev_err(hdmi->dev, "drm_connector_init() failed: %d\n", ret); >> + return ret; >> + } >> + >> + drm_mode_connector_attach_encoder(&hdmi->connector, >> + bridge->encoder); >> + >> + return 0; >> +} >> + >> +static const struct drm_bridge_funcs tfp410_bridge_funcs = { >> + .attach = tfp410_attach, >> +}; >> + >> +static int tfp410_probe(struct platform_device *pdev) >> +{ >> + struct device_node *ddc_phandle; >> + struct tfp410 *hdmi; >> + int ret; >> + >> + if (!pdev->dev.of_node) { >> + dev_err(&pdev->dev, "device-tree data is missing\n"); >> + return -ENXIO; >> + } >> + >> + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); >> + if (!hdmi) >> + return -ENOMEM; >> + platform_set_drvdata(pdev, hdmi); >> + >> + ddc_phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c", 0); >> + if (ddc_phandle) { >> + hdmi->ddc = of_get_i2c_adapter_by_node(ddc_phandle); >> + of_node_put(ddc_phandle); >> + if (!hdmi->ddc) >> + return -EPROBE_DEFER; >> + } else { >> + dev_info(&pdev->dev, >> + "No ddc i2c bus, disabling EDID readout\n"); >> + } >> + >> + hdmi->bridge.funcs = &tfp410_bridge_funcs; >> + hdmi->bridge.of_node = pdev->dev.of_node; >> + hdmi->dev = &pdev->dev; >> + >> + ret = drm_bridge_add(&hdmi->bridge); >> + if (ret) { >> + dev_err(&pdev->dev, "drm_bridge_add() failed: %d\n", ret); >> + goto fail; >> + } >> + >> + return 0; >> +fail: >> + i2c_put_adapter(hdmi->ddc); >> + return ret; >> +} >> + >> +static int tfp410_remove(struct platform_device *pdev) >> +{ >> + struct tfp410 *hdmi = platform_get_drvdata(pdev); >> + >> + drm_bridge_remove(&hdmi->bridge); >> + >> + if (hdmi->ddc) >> + i2c_put_adapter(hdmi->ddc); >> + >> + return 0; >> +} >> + >> +static const struct of_device_id tfp410_match[] = { >> + { .compatible = "ti,tfp410" }, >> + {}, >> +}; >> +MODULE_DEVICE_TABLE(of, tfp410_match); >> + >> +struct platform_driver tfp410_driver = { >> + .probe = tfp410_probe, >> + .remove = tfp410_remove, >> + .driver = { >> + .name = "tfp410-bridge", >> + .of_match_table = tfp410_match, >> + }, >> +}; >> +module_platform_driver(tfp410_driver); >> + >> +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>"); >> +MODULE_DESCRIPTION("TI TFP410 HDMI bridge driver"); >> +MODULE_LICENSE("GPL"); >
Hi Jyri, On Thursday 10 Nov 2016 11:16:53 Jyri Sarha wrote: > On 11/03/16 19:46, Laurent Pinchart wrote: > > On Wednesday 02 Nov 2016 18:32:16 Jyri Sarha wrote: > >> Add very basic ti-ftp410 HDMI transmitter driver. The only feature > >> separating this from a completely dummy bridge is the DDC i2c > >> support. However, other HW specific features may be added later when > >> needed. For instance there is a set of registers behind i2c if it is > >> connected. The implementations is tested against my new tilcdc bridge > >> support and 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 | 30 ++++ > >> drivers/gpu/drm/bridge/Kconfig | 7 + > >> drivers/gpu/drm/bridge/Makefile | 1 + > >> drivers/gpu/drm/bridge/ti-tfp410.c | 199 +++++++++++++++ > >> 4 files changed, 237 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..dc93713 > >> --- /dev/null > >> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt > >> @@ -0,0 +1,30 @@ > >> +TFP410 HDMI/DVI bridge bindings > > > > I'd name the document "TI TFP410 DVI Transmitter". DVI bridge doesn't tell > > whether the device is a receiver or transmitter. > > > >> +Required properties: > >> + - compatible: "ti,tfp410" > > > > The device is an I2C slave, it should have a reg property. Given that the > > chip can be used without being controlled through I2C, the reg property > > should be optional. You should document this clearly, and explain how the > > DT node can be instantiated as a child of an I2C controller when the I2C > > interface is used, or in other parts of the device tree otherwise. > > > >> +Optional properties: > >> + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing > > > > The TFP410 doesn't handle DDC, this property should be part of the > > connector node. > > > >> +Optional subnodes: > >> + - video input: this subnode can contain a video input port node > >> + to connect the bridge to a display controller output (See this > >> + documentation [1]). > > > > You also need an output port for the DVI output. Those two ports should be > > required, not optional. > > Ok. So I need another device node. Should I create some specific > compatible string for connectors behind tfp410, or a generic DVI/HDMI > connector with optional ddc-i2c phandle? The generic DVI/HDMI connector bindings should work fine. > The implementation side is not so critical, because it more easily > changed, but should I create an independent generic platform-device > driver for such DVI/HDMI connector or just implement the connector side > within tfp410 driver? Longer term I'd like to go for connector drivers, but it might take a bit of infrastructure work. If you can give it a try it would be great ! Otherwise I'm fine with handling that in the tfp410 driver for now. > >> +[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>; > >> + }; > >> + }; > >> + }; > >> + };
On 10/11/16 11:16, Jyri Sarha wrote: > On 11/03/16 19:46, Laurent Pinchart wrote: >> Hi Jyri, >> >> Thank you for the patch. >> >> On Wednesday 02 Nov 2016 18:32:16 Jyri Sarha wrote: >>> Add very basic ti-ftp410 HDMI transmitter driver. The only feature >>> separating this from a completely dummy bridge is the DDC i2c >>> support. However, other HW specific features may be added later when >>> needed. For instance there is a set of registers behind i2c if it is >>> connected. The implementations is tested against my new tilcdc bridge >>> support and 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 | 30 ++++ >>> drivers/gpu/drm/bridge/Kconfig | 7 + >>> drivers/gpu/drm/bridge/Makefile | 1 + >>> drivers/gpu/drm/bridge/ti-tfp410.c | 199 ++++++++++++++++++ >>> 4 files changed, 237 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..dc93713 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt >>> @@ -0,0 +1,30 @@ >>> +TFP410 HDMI/DVI bridge bindings >> >> I'd name the document "TI TFP410 DVI Transmitter". DVI bridge doesn't tell >> whether the device is a receiver or transmitter. >> >>> +Required properties: >>> + - compatible: "ti,tfp410" >> >> The device is an I2C slave, it should have a reg property. Given that the chip >> can be used without being controlled through I2C, the reg property should be >> optional. You should document this clearly, and explain how the DT node can be >> instantiated as a child of an I2C controller when the I2C interface is used, >> or in other parts of the device tree otherwise. >> >>> +Optional properties: >>> + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing >> >> The TFP410 doesn't handle DDC, this property should be part of the connector >> node. >> >>> +Optional subnodes: >>> + - video input: this subnode can contain a video input port node >>> + to connect the bridge to a display controller output (See this >>> + documentation [1]). >> >> You also need an output port for the DVI output. Those two ports should be >> required, not optional. >> > > Ok. So I need another device node. Should I create some specific > compatible string for connectors behind tfp410, or a generic DVI/HDMI > connector with optional ddc-i2c phandle? omapdrm uses connector nodes. See, for example, pandaboard's dts files. It has TFP410 and a connector node. With omap specific drivers, for now, but that's another matter. Tomi
On 11/03/16 19:46, Laurent Pinchart wrote: >> +Required properties: >> > + - compatible: "ti,tfp410" > The device is an I2C slave, it should have a reg property. Given that the chip > can be used without being controlled through I2C, the reg property should be > optional. You should document this clearly, and explain how the DT node can be > instantiated as a child of an I2C controller when the I2C interface is used, > or in other parts of the device tree otherwise. > Shouldn't I have two different compatible strings if want to make both platform driver probe and i2c client probe to work? Or can it be done with single compatible string? Would you know of an example of such a driver? Cheers, Jyri
Hi Jyri, On Monday 14 Nov 2016 10:49:43 Jyri Sarha wrote: > On 11/03/16 19:46, Laurent Pinchart wrote: > >> +Required properties: > >> > + - compatible: "ti,tfp410" > > > > The device is an I2C slave, it should have a reg property. Given that the > > chip can be used without being controlled through I2C, the reg property > > should be optional. You should document this clearly, and explain how the > > DT node can be instantiated as a child of an I2C controller when the I2C > > interface is used, or in other parts of the device tree otherwise. > > Shouldn't I have two different compatible strings if want to make both > platform driver probe and i2c client probe to work? I don't think so, it's still the same chip. > Or can it be done with single compatible string? Would you know of an > example of such a driver? You will need to register both a i2c_driver and a platform_driver in the tfp410 driver. Both will advertise the same compatible string. As you'll have two probe functions, it should be easy to handle the differences between the two situations there, with common code shared in common functions. A quick grep points to at least drivers/power/bq27x00_battery.c as an example (albeit without DT support).
On 14/11/16 13:10, Laurent Pinchart wrote: > Hi Jyri, > > On Monday 14 Nov 2016 10:49:43 Jyri Sarha wrote: >> On 11/03/16 19:46, Laurent Pinchart wrote: >>>> +Required properties: >>>>> + - compatible: "ti,tfp410" >>> >>> The device is an I2C slave, it should have a reg property. Given that the >>> chip can be used without being controlled through I2C, the reg property >>> should be optional. You should document this clearly, and explain how the >>> DT node can be instantiated as a child of an I2C controller when the I2C >>> interface is used, or in other parts of the device tree otherwise. >> >> Shouldn't I have two different compatible strings if want to make both >> platform driver probe and i2c client probe to work? > > I don't think so, it's still the same chip. > >> Or can it be done with single compatible string? Would you know of an >> example of such a driver? > > You will need to register both a i2c_driver and a platform_driver in the > tfp410 driver. Both will advertise the same compatible string. As you'll have > two probe functions, it should be easy to handle the differences between the If you have the same compatible string, won't both probes trigger? If so, how does, e.g., the platform driver know this is actually i2c case, and bail out? And if both probes don't trigger, why not? How does the device probing machinery know that this DT node is actually an i2c node, not a platform device node? Tomi
Hi Tomi, On Monday 14 Nov 2016 13:16:49 Tomi Valkeinen wrote: > On 14/11/16 13:10, Laurent Pinchart wrote: > > On Monday 14 Nov 2016 10:49:43 Jyri Sarha wrote: > >> On 11/03/16 19:46, Laurent Pinchart wrote: > >>>> +Required properties: > >>>>> + - compatible: "ti,tfp410" > >>> > >>> The device is an I2C slave, it should have a reg property. Given that > >>> the chip can be used without being controlled through I2C, the reg > >>> property should be optional. You should document this clearly, and > >>> explain how the DT node can be instantiated as a child of an I2C > >>> controller when the I2C interface is used, or in other parts of the > >>> device tree otherwise. > >> > >> Shouldn't I have two different compatible strings if want to make both > >> platform driver probe and i2c client probe to work? > > > > I don't think so, it's still the same chip. > > > >> Or can it be done with single compatible string? Would you know of an > >> example of such a driver? > > > > You will need to register both a i2c_driver and a platform_driver in the > > tfp410 driver. Both will advertise the same compatible string. As you'll > > have two probe functions, it should be easy to handle the differences > > between the > > If you have the same compatible string, won't both probes trigger? If > so, how does, e.g., the platform driver know this is actually i2c case, > and bail out? And if both probes don't trigger, why not? How does the > device probing machinery know that this DT node is actually an i2c node, > not a platform device node? The driver for the bus on which the device is sitting is responsible for instantiating a struct device corresponding to the bus type, and for binding the corresponding bus drivers. This will be a struct i2c_client for I2C buses, and a struct platform_device for platform buses. Only the corresponding driver type will be probed.
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..dc93713 --- /dev/null +++ b/Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt @@ -0,0 +1,30 @@ +TFP410 HDMI/DVI bridge bindings + +Required properties: + - compatible: "ti,tfp410" + +Optional properties: + - ddc-i2c: phandle of an I2C controller used for DDC EDID probing + +Optional subnodes: + - video input: this subnode can contain a video input port node + to connect the bridge to a display controller output (See this + documentation [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>; + }; + }; + }; + }; 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..b0753d2 --- /dev/null +++ b/drivers/gpu/drm/bridge/ti-tfp410.c @@ -0,0 +1,199 @@ +/* + * 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 <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 *hdmi = drm_connector_to_tfp410(connector); + struct edid *edid; + int ret; + + if (!hdmi->ddc) + goto fallback; + + edid = drm_get_edid(connector, hdmi->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 *hdmi = drm_connector_to_tfp410(connector); + + if (hdmi->ddc) { + if (drm_probe_ddc(hdmi->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 *hdmi = drm_bridge_to_tfp410(bridge); + int ret; + + if (!bridge->encoder) { + dev_err(hdmi->dev, "Missing encoder\n"); + return -ENODEV; + } + + drm_connector_helper_add(&hdmi->connector, + &tfp410_con_helper_funcs); + ret = drm_connector_init(bridge->dev, &hdmi->connector, + &tfp410_con_funcs, DRM_MODE_CONNECTOR_HDMIA); + if (ret) { + dev_err(hdmi->dev, "drm_connector_init() failed: %d\n", ret); + return ret; + } + + drm_mode_connector_attach_encoder(&hdmi->connector, + bridge->encoder); + + return 0; +} + +static const struct drm_bridge_funcs tfp410_bridge_funcs = { + .attach = tfp410_attach, +}; + +static int tfp410_probe(struct platform_device *pdev) +{ + struct device_node *ddc_phandle; + struct tfp410 *hdmi; + int ret; + + if (!pdev->dev.of_node) { + dev_err(&pdev->dev, "device-tree data is missing\n"); + return -ENXIO; + } + + hdmi = devm_kzalloc(&pdev->dev, sizeof(*hdmi), GFP_KERNEL); + if (!hdmi) + return -ENOMEM; + platform_set_drvdata(pdev, hdmi); + + ddc_phandle = of_parse_phandle(pdev->dev.of_node, "ddc-i2c", 0); + if (ddc_phandle) { + hdmi->ddc = of_get_i2c_adapter_by_node(ddc_phandle); + of_node_put(ddc_phandle); + if (!hdmi->ddc) + return -EPROBE_DEFER; + } else { + dev_info(&pdev->dev, + "No ddc i2c bus, disabling EDID readout\n"); + } + + hdmi->bridge.funcs = &tfp410_bridge_funcs; + hdmi->bridge.of_node = pdev->dev.of_node; + hdmi->dev = &pdev->dev; + + ret = drm_bridge_add(&hdmi->bridge); + if (ret) { + dev_err(&pdev->dev, "drm_bridge_add() failed: %d\n", ret); + goto fail; + } + + return 0; +fail: + i2c_put_adapter(hdmi->ddc); + return ret; +} + +static int tfp410_remove(struct platform_device *pdev) +{ + struct tfp410 *hdmi = platform_get_drvdata(pdev); + + drm_bridge_remove(&hdmi->bridge); + + if (hdmi->ddc) + i2c_put_adapter(hdmi->ddc); + + return 0; +} + +static const struct of_device_id tfp410_match[] = { + { .compatible = "ti,tfp410" }, + {}, +}; +MODULE_DEVICE_TABLE(of, tfp410_match); + +struct platform_driver tfp410_driver = { + .probe = tfp410_probe, + .remove = tfp410_remove, + .driver = { + .name = "tfp410-bridge", + .of_match_table = tfp410_match, + }, +}; +module_platform_driver(tfp410_driver); + +MODULE_AUTHOR("Jyri Sarha <jsarha@ti.com>"); +MODULE_DESCRIPTION("TI TFP410 HDMI bridge driver"); +MODULE_LICENSE("GPL");
Add very basic ti-ftp410 HDMI transmitter driver. The only feature separating this from a completely dummy bridge is the DDC i2c support. However, other HW specific features may be added later when needed. For instance there is a set of registers behind i2c if it is connected. The implementations is tested against my new tilcdc bridge support and 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 | 30 ++++ drivers/gpu/drm/bridge/Kconfig | 7 + drivers/gpu/drm/bridge/Makefile | 1 + drivers/gpu/drm/bridge/ti-tfp410.c | 199 +++++++++++++++++++++ 4 files changed, 237 insertions(+) create mode 100644 Documentation/devicetree/bindings/display/bridge/ti,tfp410.txt create mode 100644 drivers/gpu/drm/bridge/ti-tfp410.c