Re: [PATCH 2/3] drm/bridge: Add ti-ftp410 HDMI transmitter driver

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




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@xxxxxx>
>> ---
>>  .../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@xxxxxx>
>> + *
>> + * 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@xxxxxx>");
>> +MODULE_DESCRIPTION("TI TFP410 HDMI bridge driver");
>> +MODULE_LICENSE("GPL");
> 

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]
  Powered by Linux