[PATCH 1/2] drm: bridge: sil902x

[PATCH 1/2] drm: bridge: sil902x

[Boris Brezillon]

Add basic support for the sil902x RGB -> HDMI bridge.
This driver does not support audio output yet.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Hello,

This patch is only adding basic support for the sil9022 chip.
As stated in the commit log, there's no audio support, but the
driver also hardcodes a lot of things (like the RGB input format to
use).
There are two reasons for this:
1/ the DRM framework does not allow for advanced link description
   between an encoder and a bridge (that's for the RGB format
   limitation). Any idea how this should be described?

2/ I don't have the datasheet of this HDMI encoder, and all logic
   has been extracted from those two drivers [1][2], which means
   I may miss some important things in my encoder setup.

Another thing I find weird in the drm_bridge interface is the fact
that we have a ->attach() method, but no ->detach(), which can be
a problem if we allow drm bridges and KMS drivers to be compiled as
modules. Any reason for that?

That's all for the questions part :-).

Best Regards,

Boris
---
 drivers/gpu/drm/bridge/Kconfig   |   8 +
 drivers/gpu/drm/bridge/Makefile  |   1 +
 drivers/gpu/drm/bridge/sil902x.c | 491 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 500 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/sil902x.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 27e2022..9701fd2 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -40,4 +40,12 @@ config DRM_PARADE_PS8622
 	---help---
 	  Parade eDP-LVDS bridge chip driver.
 
+config DRM_SIL902X
+	tristate "Silicon Image sil902x RGB/HDMI bridge"
+	depends on OF
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	---help---
+	  Silicon Image sil902x bridge chip driver.
+
 endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index f13c33d..a689aad 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_SIL902X) += sil902x.o
diff --git a/drivers/gpu/drm/bridge/sil902x.c b/drivers/gpu/drm/bridge/sil902x.c
new file mode 100644
index 0000000..230732d
--- /dev/null
+++ b/drivers/gpu/drm/bridge/sil902x.c
@@ -0,0 +1,491 @@
+/*
+ * Copyright (C) 2014 Atmel
+ *		      Bo Shen <voice.shen@atmel.com>
+ *
+ * Authors:	      Bo Shen <voice.shen@atmel.com>
+ *		      Boris Brezillon <boris.brezillon@free-electrons.com>
+ *		      Wu, Songjun <Songjun.Wu@atmel.com>
+ *
+ *
+ * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/component.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder_slave.h>
+
+#define SIL902X_TPI_VIDEO_DATA			0x0
+
+#define SIL902X_TPI_PIXEL_REPETITION		0x8
+#define SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT     BIT(5)
+#define SIL902X_TPI_AVI_PIXEL_REP_RISING_EDGE   BIT(4)
+#define SIL902X_TPI_AVI_PIXEL_REP_4X		3
+#define SIL902X_TPI_AVI_PIXEL_REP_2X		1
+#define SIL902X_TPI_AVI_PIXEL_REP_NONE		0
+#define SIL902X_TPI_CLK_RATIO_HALF		(0 << 6)
+#define SIL902X_TPI_CLK_RATIO_1X		(1 << 6)
+#define SIL902X_TPI_CLK_RATIO_2X		(2 << 6)
+#define SIL902X_TPI_CLK_RATIO_4X		(3 << 6)
+
+#define SIL902X_TPI_AVI_IN_FORMAT		0x9
+#define SIL902X_TPI_AVI_INPUT_BITMODE_12BIT	BIT(7)
+#define SIL902X_TPI_AVI_INPUT_DITHER		BIT(6)
+#define SIL902X_TPI_AVI_INPUT_RANGE_LIMITED	(2 << 2)
+#define SIL902X_TPI_AVI_INPUT_RANGE_FULL	(1 << 2)
+#define SIL902X_TPI_AVI_INPUT_RANGE_AUTO	(0 << 2)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_BLACK	(3 << 0)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV422	(2 << 0)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV444	(1 << 0)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB	(0 << 0)
+
+#define SIL902X_TPI_AVI_INFOFRAME		0x0c
+
+#define SIL902X_SYS_CTRL_DATA			0x1a
+#define SIL902X_SYS_CTRL_PWR_DWN		BIT(4)
+#define SIL902X_SYS_CTRL_AV_MUTE		BIT(3)
+#define SIL902X_SYS_CTRL_DDC_BUS_REQ		BIT(2)
+#define SIL902X_SYS_CTRL_DDC_BUS_GRTD		BIT(1)
+#define SIL902X_SYS_CTRL_OUTPUT_MODE		BIT(0)
+#define SIL902X_SYS_CTRL_OUTPUT_HDMI		1
+#define SIL902X_SYS_CTRL_OUTPUT_DVI		0
+
+#define SIL902X_REG_CHIPID(n)			(0x1b + (n))
+
+#define SIL902X_PWR_STATE_CTRL			0x1e
+#define SIL902X_AVI_POWER_STATE_MSK		GENMASK(1, 0)
+#define SIL902X_AVI_POWER_STATE_D(l)		((l) & SIL902X_AVI_POWER_STATE_MSK)
+
+#define SI902X_INT_ENABLE			0x3c
+#define SI902X_INT_STATUS			0x3d
+#define SI902X_HOTPLUG_EVENT			BIT(0)
+#define SI902X_PLUGGED_STATUS			BIT(2)
+
+#define SIL902X_REG_TPI_RQB			0xc7
+
+struct sil902x {
+	struct i2c_client *i2c;
+	struct regmap *regmap;
+	struct drm_bridge bridge;
+	struct drm_connector connector;
+	struct gpio_desc *reset_gpio;
+	struct work_struct hotplug_work;
+};
+
+static inline struct sil902x *bridge_to_sil902x(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct sil902x, bridge);
+}
+
+static inline struct sil902x *connector_to_sil902x(struct drm_connector *con)
+{
+	return container_of(con, struct sil902x, connector);
+}
+
+static void sil902x_reset(struct sil902x *sil902x)
+{
+	gpiod_set_value(sil902x->reset_gpio, 1);
+
+	msleep(100);
+
+	gpiod_set_value(sil902x->reset_gpio, 0);
+}
+
+static void sil902x_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static enum drm_connector_status
+sil902x_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct sil902x *sil902x = connector_to_sil902x(connector);
+	unsigned int status;
+
+	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
+
+	return (status & SI902X_PLUGGED_STATUS) ?
+	       connector_status_connected : connector_status_disconnected;
+}
+
+static const struct drm_connector_funcs sil902x_atomic_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.detect = sil902x_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = sil902x_connector_destroy,
+	.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 const struct drm_connector_funcs sil902x_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.detect = sil902x_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = sil902x_connector_destroy,
+};
+
+static int sil902x_get_modes(struct drm_connector *connector)
+{
+	struct sil902x *sil902x = connector_to_sil902x(connector);
+	struct regmap *regmap = sil902x->regmap;
+	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	unsigned int status;
+	struct edid *edid;
+	int num = 0;
+	int ret;
+	int i;
+
+	ret = regmap_update_bits(regmap, SIL902X_PWR_STATE_CTRL,
+				 SIL902X_AVI_POWER_STATE_MSK,
+				 SIL902X_AVI_POWER_STATE_D(2));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA,
+			   SIL902X_SYS_CTRL_OUTPUT_HDMI |
+			   SIL902X_SYS_CTRL_PWR_DWN);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
+				 SIL902X_SYS_CTRL_DDC_BUS_REQ,
+				 SIL902X_SYS_CTRL_DDC_BUS_REQ);
+	if (ret)
+		return ret;
+
+	i = 0;
+	do {
+		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
+		if (ret)
+			return ret;
+		i++;
+	} while (!(status & SIL902X_SYS_CTRL_DDC_BUS_GRTD));
+
+	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA, status);
+	if (ret)
+		return ret;
+
+	edid = drm_get_edid(connector, sil902x->i2c->adapter);
+	drm_mode_connector_update_edid_property(connector, edid);
+	if (edid) {
+		num += drm_add_edid_modes(connector, edid);
+		kfree(edid);
+	}
+
+	ret = drm_display_info_set_bus_formats(&connector->display_info,
+					       &bus_format, 1);
+	if (ret)
+		return ret;
+
+	regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
+				 SIL902X_SYS_CTRL_DDC_BUS_REQ |
+				 SIL902X_SYS_CTRL_DDC_BUS_GRTD, 0);
+	if (ret)
+		return ret;
+
+	i = 0;
+	do {
+		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
+		if (ret)
+			return ret;
+		i++;
+	} while (status & (SIL902X_SYS_CTRL_DDC_BUS_REQ |
+			   SIL902X_SYS_CTRL_DDC_BUS_GRTD));
+
+	return num;
+}
+
+static enum drm_mode_status sil902x_mode_valid(struct drm_connector *connector,
+					       struct drm_display_mode *mode)
+{
+	/* TODO: check mode */
+
+	return MODE_OK;
+}
+
+static struct drm_encoder *sil902x_best_encoder(struct drm_connector *connector)
+{
+	struct sil902x *sil902x = connector_to_sil902x(connector);
+
+	return sil902x->bridge.encoder;
+}
+
+static const struct drm_connector_helper_funcs sil902x_connector_helper_funcs = {
+	.get_modes = sil902x_get_modes,
+	.mode_valid = sil902x_mode_valid,
+	.best_encoder = sil902x_best_encoder,
+};
+
+static void sil902x_bridge_disable(struct drm_bridge *bridge)
+{
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+
+	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
+			   SIL902X_SYS_CTRL_PWR_DWN,
+			   SIL902X_SYS_CTRL_PWR_DWN);
+}
+
+static void sil902x_bridge_enable(struct drm_bridge *bridge)
+{
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+
+	regmap_update_bits(sil902x->regmap, SIL902X_PWR_STATE_CTRL,
+			   SIL902X_AVI_POWER_STATE_MSK,
+			   SIL902X_AVI_POWER_STATE_D(0));
+	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
+			   SIL902X_SYS_CTRL_PWR_DWN, 0);
+}
+
+static void sil902x_bridge_mode_set(struct drm_bridge *bridge,
+				    struct drm_display_mode *mode,
+				    struct drm_display_mode *adj)
+{
+	u8 buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+	struct regmap *regmap = sil902x->regmap;
+	struct hdmi_avi_infoframe frame;
+	int ret;
+
+	buf[0] = adj->clock;
+	buf[1] = adj->clock >> 8;
+	buf[2] = adj->vrefresh;
+	buf[3] = 0x00;
+	buf[4] = adj->hdisplay;
+	buf[5] = adj->hdisplay >> 8;
+	buf[6] = adj->vdisplay;
+	buf[7] = adj->vdisplay >> 8;
+	buf[8] = SIL902X_TPI_CLK_RATIO_1X | SIL902X_TPI_AVI_PIXEL_REP_NONE |
+		 SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT;
+	buf[9] = SIL902X_TPI_AVI_INPUT_RANGE_AUTO |
+		 SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB;
+
+	ret = regmap_bulk_write(regmap, SIL902X_TPI_VIDEO_DATA, buf, 10);
+	if (ret)
+		return;
+
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
+	if (ret < 0) {
+		DRM_ERROR("couldn't fill AVI infoframe\n");
+		return;
+	}
+
+	ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
+	if (ret < 0) {
+		DRM_ERROR("failed to pack AVI infoframe: %zd\n", ret);
+		return;
+	}
+
+	/* Do not send the infoframe header, but keep the CRC field. */
+	regmap_bulk_write(regmap, SIL902X_TPI_AVI_INFOFRAME,
+			  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
+			  HDMI_AVI_INFOFRAME_SIZE + 1);
+}
+
+static int sil902x_bridge_attach(struct drm_bridge *bridge)
+{
+	const struct drm_connector_funcs *funcs = &sil902x_connector_funcs;
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+	struct drm_device *drm = bridge->dev;
+	int ret;
+
+	drm_connector_helper_add(&sil902x->connector,
+				 &sil902x_connector_helper_funcs);
+
+	if (drm_core_check_feature(drm, DRIVER_ATOMIC))
+		funcs = &sil902x_atomic_connector_funcs;
+
+	ret = drm_connector_init(drm, &sil902x->connector, funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret)
+		return ret;
+
+	if (sil902x->i2c->irq > 0)
+		sil902x->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		sil902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
+
+	drm_mode_connector_attach_encoder(&sil902x->connector, bridge->encoder);
+
+	return 0;
+}
+
+static void sil902x_bridge_nop(struct drm_bridge *bridge)
+{
+}
+
+static const struct drm_bridge_funcs sil902x_bridge_funcs = {
+	.attach = sil902x_bridge_attach,
+	.mode_set = sil902x_bridge_mode_set,
+	.disable = sil902x_bridge_disable,
+	.post_disable = sil902x_bridge_nop,
+	.pre_enable = sil902x_bridge_nop,
+	.enable = sil902x_bridge_enable,
+};
+
+static const struct regmap_range sil902x_volatile_ranges[] = {
+	{ .range_min = 0, .range_max = 0xff },
+};
+
+static const struct regmap_access_table sil902x_volatile_table = {
+	.yes_ranges = sil902x_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(sil902x_volatile_ranges),
+};
+
+static const struct regmap_config sil902x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &sil902x_volatile_table,
+	.cache_type = REGCACHE_NONE,
+};
+
+static irqreturn_t sil902x_interrupt(int irq, void *data)
+{
+	struct sil902x *sil902x = data;
+	unsigned int status = 0;
+
+	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
+	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
+
+	if ((status & SI902X_HOTPLUG_EVENT) && sil902x->bridge.dev)
+		drm_helper_hpd_irq_event(sil902x->bridge.dev);
+
+	return IRQ_HANDLED;
+}
+
+static int sil902x_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	unsigned int status = 0;
+	struct sil902x *sil902x;
+	u8 chipid[4];
+	int ret;
+
+	sil902x = devm_kzalloc(dev, sizeof(*sil902x), GFP_KERNEL);
+	if (!sil902x)
+		return -ENOMEM;
+
+	sil902x->i2c = client;
+	sil902x->regmap = devm_regmap_init_i2c(client, &sil902x_regmap_config);
+	if (IS_ERR(sil902x->regmap))
+		return PTR_ERR(sil902x->regmap);
+
+	sil902x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(sil902x->reset_gpio)) {
+		dev_err(dev, "Failed to retrieve/request reset gpio: %d\n",
+			PTR_ERR(sil902x->reset_gpio));
+		return PTR_ERR(sil902x->reset_gpio);
+	}
+
+	sil902x_reset(sil902x);
+
+	ret = regmap_write(sil902x->regmap, SIL902X_REG_TPI_RQB, 0x0);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(sil902x->regmap, SIL902X_REG_CHIPID(0),
+			       &chipid, 4);
+	if (ret) {
+		dev_err(dev, "regmap_read failed %d\n", ret);
+		return ret;
+	}
+
+	if (chipid[0] != 0xb0) {
+		dev_err(dev, "Invalid chipid: %02x (expecting 0xb0)\n",
+			chipid[0]);
+		return -EINVAL;
+	}
+
+	/* Clear all pending interrupts */
+	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
+	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
+
+	if (client->irq > 0) {
+		regmap_write(sil902x->regmap, SI902X_INT_ENABLE,
+			     SI902X_HOTPLUG_EVENT);
+
+		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+						sil902x_interrupt,
+						IRQF_ONESHOT, dev_name(dev),
+						sil902x);
+		if (ret)
+			return ret;
+	}
+
+	sil902x->bridge.funcs = &sil902x_bridge_funcs;
+	sil902x->bridge.of_node = dev->of_node;
+	ret = drm_bridge_add(&sil902x->bridge);
+	if (ret) {
+		dev_err(dev, "Failed to add drm_bridge\n");
+		return ret;
+	}
+
+	i2c_set_clientdata(client, sil902x);
+
+	return 0;
+}
+
+static int sil902x_remove(struct i2c_client *client)
+
+{
+	struct sil902x *sil902x = i2c_get_clientdata(client);
+
+	drm_bridge_remove(&sil902x->bridge);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sil902x_dt_ids[] = {
+	{ .compatible = "sil,sil9022", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sil902x_dt_ids);
+#endif
+
+static const struct i2c_device_id sil902x_i2c_ids[] = {
+	{ "sil9022", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, sil9022_id);
+
+static struct i2c_driver sil902x_driver = {
+	.probe = sil902x_probe,
+	.remove = sil902x_remove,
+	.driver = {
+		.name = "sil902x",
+		.of_match_table = of_match_ptr(sil902x_dt_ids),
+	},
+	.id_table = sil902x_i2c_ids,
+};
+module_i2c_driver(sil902x_driver);
+
+MODULE_AUTHOR("Boris Brezillon <boris.brezillon@free-electrons.com>");
+MODULE_DESCRIPTION("SIL902x RGB -> HDMI bridges");
+MODULE_LICENSE("GPL");
-- 
2.1.4

[PATCH 2/2] drm: bridge: add sil902x DT bindings doc

[Boris Brezillon]

Add Sil9022 DT bindings description.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
 .../devicetree/bindings/display/bridge/sil902x.txt | 31 ++++++++++++++++++++++
 1 file changed, 31 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/bridge/sil902x.txt

diff --git a/Documentation/devicetree/bindings/display/bridge/sil902x.txt b/Documentation/devicetree/bindings/display/bridge/sil902x.txt
new file mode 100644
index 0000000..7f1339f
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/bridge/sil902x.txt
@@ -0,0 +1,31 @@
+sil902x HDMI bridge bindings
+
+Required properties:
+	- compatible: "sil,sil9022"
+	- reg: i2c address of the bridge
+	- reset-gpios: OF device-tree gpio specification for RST_N pin.
+
+Optional properties:
+	- interrupts-extended or interrupt-parent + interrupts: describe
+	  the interrupt line used to inform the host about hotplug events.
+
+Optional subnodes:
+	- video input: Device node can contain 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@39 {
+		compatible = "sil,sil9022";
+		reg = <0x39>;
+		reset-gpios = <&gpx1 5 1 0 0>;
+		ports {
+			port@0 {
+				bridge_in: endpoint {
+					remote-endpoint = <&dc_out>;
+				};
+			};
+		};
+	};
-- 
2.1.4

Re: [PATCH 1/2] drm: bridge: sil902x

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 4486 bytes --]

Hi Boris,

[auto build test ERROR on drm/drm-next]
[also build test ERROR on v4.4-rc8 next-20160106]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Boris-Brezillon/drm-bridge-sil902x/20160106-192921
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: i386-allmodconfig (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All error/warnings (new ones prefixed by >>):

   drivers/gpu/drm/bridge/sil902x.c: In function 'sil902x_probe':
>> drivers/gpu/drm/bridge/sil902x.c:401:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
      dev_err(dev, "Failed to retrieve/request reset gpio: %d\n",
                   ^
   In file included from drivers/gpu/drm/bridge/sil902x.c:26:0:
   drivers/gpu/drm/bridge/sil902x.c: At top level:
>> drivers/gpu/drm/bridge/sil902x.c:476:26: error: 'sil9022_id' undeclared here (not in a function)
    MODULE_DEVICE_TABLE(i2c, sil9022_id);
                             ^
   include/linux/module.h:223:21: note: in definition of macro 'MODULE_DEVICE_TABLE'
    extern const typeof(name) __mod_##type##__##name##_device_table  \
                        ^
>> include/linux/module.h:223:27: error: '__mod_i2c__sil9022_id_device_table' aliased to undefined symbol 'sil9022_id'
    extern const typeof(name) __mod_##type##__##name##_device_table  \
                              ^
>> drivers/gpu/drm/bridge/sil902x.c:476:1: note: in expansion of macro 'MODULE_DEVICE_TABLE'
    MODULE_DEVICE_TABLE(i2c, sil9022_id);
    ^

vim +/sil9022_id +476 drivers/gpu/drm/bridge/sil902x.c

   395		sil902x->regmap = devm_regmap_init_i2c(client, &sil902x_regmap_config);
   396		if (IS_ERR(sil902x->regmap))
   397			return PTR_ERR(sil902x->regmap);
   398	
   399		sil902x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
   400		if (IS_ERR(sil902x->reset_gpio)) {
 > 401			dev_err(dev, "Failed to retrieve/request reset gpio: %d\n",
   402				PTR_ERR(sil902x->reset_gpio));
   403			return PTR_ERR(sil902x->reset_gpio);
   404		}
   405	
   406		sil902x_reset(sil902x);
   407	
   408		ret = regmap_write(sil902x->regmap, SIL902X_REG_TPI_RQB, 0x0);
   409		if (ret)
   410			return ret;
   411	
   412		ret = regmap_bulk_read(sil902x->regmap, SIL902X_REG_CHIPID(0),
   413				       &chipid, 4);
   414		if (ret) {
   415			dev_err(dev, "regmap_read failed %d\n", ret);
   416			return ret;
   417		}
   418	
   419		if (chipid[0] != 0xb0) {
   420			dev_err(dev, "Invalid chipid: %02x (expecting 0xb0)\n",
   421				chipid[0]);
   422			return -EINVAL;
   423		}
   424	
   425		/* Clear all pending interrupts */
   426		regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
   427		regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
   428	
   429		if (client->irq > 0) {
   430			regmap_write(sil902x->regmap, SI902X_INT_ENABLE,
   431				     SI902X_HOTPLUG_EVENT);
   432	
   433			ret = devm_request_threaded_irq(dev, client->irq, NULL,
   434							sil902x_interrupt,
   435							IRQF_ONESHOT, dev_name(dev),
   436							sil902x);
   437			if (ret)
   438				return ret;
   439		}
   440	
   441		sil902x->bridge.funcs = &sil902x_bridge_funcs;
   442		sil902x->bridge.of_node = dev->of_node;
   443		ret = drm_bridge_add(&sil902x->bridge);
   444		if (ret) {
   445			dev_err(dev, "Failed to add drm_bridge\n");
   446			return ret;
   447		}
   448	
   449		i2c_set_clientdata(client, sil902x);
   450	
   451		return 0;
   452	}
   453	
   454	static int sil902x_remove(struct i2c_client *client)
   455	
   456	{
   457		struct sil902x *sil902x = i2c_get_clientdata(client);
   458	
   459		drm_bridge_remove(&sil902x->bridge);
   460	
   461		return 0;
   462	}
   463	
   464	#ifdef CONFIG_OF
   465	static const struct of_device_id sil902x_dt_ids[] = {
   466		{ .compatible = "sil,sil9022", },
   467		{ }
   468	};
   469	MODULE_DEVICE_TABLE(of, sil902x_dt_ids);
   470	#endif
   471	
   472	static const struct i2c_device_id sil902x_i2c_ids[] = {
   473		{ "sil9022", 0 },
   474		{ },
   475	};
 > 476	MODULE_DEVICE_TABLE(i2c, sil9022_id);
   477	
   478	static struct i2c_driver sil902x_driver = {
   479		.probe = sil902x_probe,

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 52578 bytes --]

[PATCH v2 1/2] drm: bridge: sil902x

[Boris Brezillon]

Add basic support for the sil902x RGB -> HDMI bridge.
This driver does not support audio output yet.

Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
---
Hello,

This patch is only adding basic support for the sil9022 chip.
As stated in the commit log, there's no audio support, but the
driver also hardcodes a lot of things (like the RGB input format to
use).
There are two reasons for this:
1/ the DRM framework does not allow for advanced link description
   between an encoder and a bridge (that's for the RGB format
   limitation). Any idea how this should be described?

2/ I don't have the datasheet of this HDMI encoder, and all logic
   has been extracted from those two drivers [1][2], which means
   I may miss some important things in my encoder setup.

Another thing I find weird in the drm_bridge interface is the fact
that we have a ->attach() method, but no ->detach(), which can be
a problem if we allow drm bridges and KMS drivers to be compiled as
modules. Any reason for that?

That's all for the questions part :-).

Best Regards,

Boris

Changes in v2:
- fix errors reported by kbuild test robot

---
 drivers/gpu/drm/bridge/Kconfig   |   8 +
 drivers/gpu/drm/bridge/Makefile  |   1 +
 drivers/gpu/drm/bridge/sil902x.c | 491 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 500 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/sil902x.c

diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 27e2022..9701fd2 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -40,4 +40,12 @@ config DRM_PARADE_PS8622
 	---help---
 	  Parade eDP-LVDS bridge chip driver.
 
+config DRM_SIL902X
+	tristate "Silicon Image sil902x RGB/HDMI bridge"
+	depends on OF
+	select DRM_KMS_HELPER
+	select REGMAP_I2C
+	---help---
+	  Silicon Image sil902x bridge chip driver.
+
 endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index f13c33d..a689aad 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
 obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
 obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
 obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
+obj-$(CONFIG_DRM_SIL902X) += sil902x.o
diff --git a/drivers/gpu/drm/bridge/sil902x.c b/drivers/gpu/drm/bridge/sil902x.c
new file mode 100644
index 0000000..2657031
--- /dev/null
+++ b/drivers/gpu/drm/bridge/sil902x.c
@@ -0,0 +1,491 @@
+/*
+ * Copyright (C) 2014 Atmel
+ *		      Bo Shen <voice.shen@atmel.com>
+ *
+ * Authors:	      Bo Shen <voice.shen@atmel.com>
+ *		      Boris Brezillon <boris.brezillon@free-electrons.com>
+ *		      Wu, Songjun <Songjun.Wu@atmel.com>
+ *
+ *
+ * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/component.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_gpio.h>
+#include <linux/regmap.h>
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_encoder_slave.h>
+
+#define SIL902X_TPI_VIDEO_DATA			0x0
+
+#define SIL902X_TPI_PIXEL_REPETITION		0x8
+#define SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT     BIT(5)
+#define SIL902X_TPI_AVI_PIXEL_REP_RISING_EDGE   BIT(4)
+#define SIL902X_TPI_AVI_PIXEL_REP_4X		3
+#define SIL902X_TPI_AVI_PIXEL_REP_2X		1
+#define SIL902X_TPI_AVI_PIXEL_REP_NONE		0
+#define SIL902X_TPI_CLK_RATIO_HALF		(0 << 6)
+#define SIL902X_TPI_CLK_RATIO_1X		(1 << 6)
+#define SIL902X_TPI_CLK_RATIO_2X		(2 << 6)
+#define SIL902X_TPI_CLK_RATIO_4X		(3 << 6)
+
+#define SIL902X_TPI_AVI_IN_FORMAT		0x9
+#define SIL902X_TPI_AVI_INPUT_BITMODE_12BIT	BIT(7)
+#define SIL902X_TPI_AVI_INPUT_DITHER		BIT(6)
+#define SIL902X_TPI_AVI_INPUT_RANGE_LIMITED	(2 << 2)
+#define SIL902X_TPI_AVI_INPUT_RANGE_FULL	(1 << 2)
+#define SIL902X_TPI_AVI_INPUT_RANGE_AUTO	(0 << 2)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_BLACK	(3 << 0)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV422	(2 << 0)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV444	(1 << 0)
+#define SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB	(0 << 0)
+
+#define SIL902X_TPI_AVI_INFOFRAME		0x0c
+
+#define SIL902X_SYS_CTRL_DATA			0x1a
+#define SIL902X_SYS_CTRL_PWR_DWN		BIT(4)
+#define SIL902X_SYS_CTRL_AV_MUTE		BIT(3)
+#define SIL902X_SYS_CTRL_DDC_BUS_REQ		BIT(2)
+#define SIL902X_SYS_CTRL_DDC_BUS_GRTD		BIT(1)
+#define SIL902X_SYS_CTRL_OUTPUT_MODE		BIT(0)
+#define SIL902X_SYS_CTRL_OUTPUT_HDMI		1
+#define SIL902X_SYS_CTRL_OUTPUT_DVI		0
+
+#define SIL902X_REG_CHIPID(n)			(0x1b + (n))
+
+#define SIL902X_PWR_STATE_CTRL			0x1e
+#define SIL902X_AVI_POWER_STATE_MSK		GENMASK(1, 0)
+#define SIL902X_AVI_POWER_STATE_D(l)		((l) & SIL902X_AVI_POWER_STATE_MSK)
+
+#define SI902X_INT_ENABLE			0x3c
+#define SI902X_INT_STATUS			0x3d
+#define SI902X_HOTPLUG_EVENT			BIT(0)
+#define SI902X_PLUGGED_STATUS			BIT(2)
+
+#define SIL902X_REG_TPI_RQB			0xc7
+
+struct sil902x {
+	struct i2c_client *i2c;
+	struct regmap *regmap;
+	struct drm_bridge bridge;
+	struct drm_connector connector;
+	struct gpio_desc *reset_gpio;
+	struct work_struct hotplug_work;
+};
+
+static inline struct sil902x *bridge_to_sil902x(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct sil902x, bridge);
+}
+
+static inline struct sil902x *connector_to_sil902x(struct drm_connector *con)
+{
+	return container_of(con, struct sil902x, connector);
+}
+
+static void sil902x_reset(struct sil902x *sil902x)
+{
+	gpiod_set_value(sil902x->reset_gpio, 1);
+
+	msleep(100);
+
+	gpiod_set_value(sil902x->reset_gpio, 0);
+}
+
+static void sil902x_connector_destroy(struct drm_connector *connector)
+{
+	drm_connector_unregister(connector);
+	drm_connector_cleanup(connector);
+}
+
+static enum drm_connector_status
+sil902x_connector_detect(struct drm_connector *connector, bool force)
+{
+	struct sil902x *sil902x = connector_to_sil902x(connector);
+	unsigned int status;
+
+	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
+
+	return (status & SI902X_PLUGGED_STATUS) ?
+	       connector_status_connected : connector_status_disconnected;
+}
+
+static const struct drm_connector_funcs sil902x_atomic_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.detect = sil902x_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = sil902x_connector_destroy,
+	.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 const struct drm_connector_funcs sil902x_connector_funcs = {
+	.dpms = drm_atomic_helper_connector_dpms,
+	.detect = sil902x_connector_detect,
+	.fill_modes = drm_helper_probe_single_connector_modes,
+	.destroy = sil902x_connector_destroy,
+};
+
+static int sil902x_get_modes(struct drm_connector *connector)
+{
+	struct sil902x *sil902x = connector_to_sil902x(connector);
+	struct regmap *regmap = sil902x->regmap;
+	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
+	unsigned int status;
+	struct edid *edid;
+	int num = 0;
+	int ret;
+	int i;
+
+	ret = regmap_update_bits(regmap, SIL902X_PWR_STATE_CTRL,
+				 SIL902X_AVI_POWER_STATE_MSK,
+				 SIL902X_AVI_POWER_STATE_D(2));
+	if (ret)
+		return ret;
+
+	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA,
+			   SIL902X_SYS_CTRL_OUTPUT_HDMI |
+			   SIL902X_SYS_CTRL_PWR_DWN);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
+				 SIL902X_SYS_CTRL_DDC_BUS_REQ,
+				 SIL902X_SYS_CTRL_DDC_BUS_REQ);
+	if (ret)
+		return ret;
+
+	i = 0;
+	do {
+		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
+		if (ret)
+			return ret;
+		i++;
+	} while (!(status & SIL902X_SYS_CTRL_DDC_BUS_GRTD));
+
+	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA, status);
+	if (ret)
+		return ret;
+
+	edid = drm_get_edid(connector, sil902x->i2c->adapter);
+	drm_mode_connector_update_edid_property(connector, edid);
+	if (edid) {
+		num += drm_add_edid_modes(connector, edid);
+		kfree(edid);
+	}
+
+	ret = drm_display_info_set_bus_formats(&connector->display_info,
+					       &bus_format, 1);
+	if (ret)
+		return ret;
+
+	regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
+	if (ret)
+		return ret;
+
+	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
+				 SIL902X_SYS_CTRL_DDC_BUS_REQ |
+				 SIL902X_SYS_CTRL_DDC_BUS_GRTD, 0);
+	if (ret)
+		return ret;
+
+	i = 0;
+	do {
+		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
+		if (ret)
+			return ret;
+		i++;
+	} while (status & (SIL902X_SYS_CTRL_DDC_BUS_REQ |
+			   SIL902X_SYS_CTRL_DDC_BUS_GRTD));
+
+	return num;
+}
+
+static enum drm_mode_status sil902x_mode_valid(struct drm_connector *connector,
+					       struct drm_display_mode *mode)
+{
+	/* TODO: check mode */
+
+	return MODE_OK;
+}
+
+static struct drm_encoder *sil902x_best_encoder(struct drm_connector *connector)
+{
+	struct sil902x *sil902x = connector_to_sil902x(connector);
+
+	return sil902x->bridge.encoder;
+}
+
+static const struct drm_connector_helper_funcs sil902x_connector_helper_funcs = {
+	.get_modes = sil902x_get_modes,
+	.mode_valid = sil902x_mode_valid,
+	.best_encoder = sil902x_best_encoder,
+};
+
+static void sil902x_bridge_disable(struct drm_bridge *bridge)
+{
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+
+	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
+			   SIL902X_SYS_CTRL_PWR_DWN,
+			   SIL902X_SYS_CTRL_PWR_DWN);
+}
+
+static void sil902x_bridge_enable(struct drm_bridge *bridge)
+{
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+
+	regmap_update_bits(sil902x->regmap, SIL902X_PWR_STATE_CTRL,
+			   SIL902X_AVI_POWER_STATE_MSK,
+			   SIL902X_AVI_POWER_STATE_D(0));
+	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
+			   SIL902X_SYS_CTRL_PWR_DWN, 0);
+}
+
+static void sil902x_bridge_mode_set(struct drm_bridge *bridge,
+				    struct drm_display_mode *mode,
+				    struct drm_display_mode *adj)
+{
+	u8 buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+	struct regmap *regmap = sil902x->regmap;
+	struct hdmi_avi_infoframe frame;
+	int ret;
+
+	buf[0] = adj->clock;
+	buf[1] = adj->clock >> 8;
+	buf[2] = adj->vrefresh;
+	buf[3] = 0x00;
+	buf[4] = adj->hdisplay;
+	buf[5] = adj->hdisplay >> 8;
+	buf[6] = adj->vdisplay;
+	buf[7] = adj->vdisplay >> 8;
+	buf[8] = SIL902X_TPI_CLK_RATIO_1X | SIL902X_TPI_AVI_PIXEL_REP_NONE |
+		 SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT;
+	buf[9] = SIL902X_TPI_AVI_INPUT_RANGE_AUTO |
+		 SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB;
+
+	ret = regmap_bulk_write(regmap, SIL902X_TPI_VIDEO_DATA, buf, 10);
+	if (ret)
+		return;
+
+	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
+	if (ret < 0) {
+		DRM_ERROR("couldn't fill AVI infoframe\n");
+		return;
+	}
+
+	ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
+	if (ret < 0) {
+		DRM_ERROR("failed to pack AVI infoframe: %zd\n", ret);
+		return;
+	}
+
+	/* Do not send the infoframe header, but keep the CRC field. */
+	regmap_bulk_write(regmap, SIL902X_TPI_AVI_INFOFRAME,
+			  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
+			  HDMI_AVI_INFOFRAME_SIZE + 1);
+}
+
+static int sil902x_bridge_attach(struct drm_bridge *bridge)
+{
+	const struct drm_connector_funcs *funcs = &sil902x_connector_funcs;
+	struct sil902x *sil902x = bridge_to_sil902x(bridge);
+	struct drm_device *drm = bridge->dev;
+	int ret;
+
+	drm_connector_helper_add(&sil902x->connector,
+				 &sil902x_connector_helper_funcs);
+
+	if (drm_core_check_feature(drm, DRIVER_ATOMIC))
+		funcs = &sil902x_atomic_connector_funcs;
+
+	ret = drm_connector_init(drm, &sil902x->connector, funcs,
+				 DRM_MODE_CONNECTOR_HDMIA);
+	if (ret)
+		return ret;
+
+	if (sil902x->i2c->irq > 0)
+		sil902x->connector.polled = DRM_CONNECTOR_POLL_HPD;
+	else
+		sil902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
+
+	drm_mode_connector_attach_encoder(&sil902x->connector, bridge->encoder);
+
+	return 0;
+}
+
+static void sil902x_bridge_nop(struct drm_bridge *bridge)
+{
+}
+
+static const struct drm_bridge_funcs sil902x_bridge_funcs = {
+	.attach = sil902x_bridge_attach,
+	.mode_set = sil902x_bridge_mode_set,
+	.disable = sil902x_bridge_disable,
+	.post_disable = sil902x_bridge_nop,
+	.pre_enable = sil902x_bridge_nop,
+	.enable = sil902x_bridge_enable,
+};
+
+static const struct regmap_range sil902x_volatile_ranges[] = {
+	{ .range_min = 0, .range_max = 0xff },
+};
+
+static const struct regmap_access_table sil902x_volatile_table = {
+	.yes_ranges = sil902x_volatile_ranges,
+	.n_yes_ranges = ARRAY_SIZE(sil902x_volatile_ranges),
+};
+
+static const struct regmap_config sil902x_regmap_config = {
+	.reg_bits = 8,
+	.val_bits = 8,
+	.volatile_table = &sil902x_volatile_table,
+	.cache_type = REGCACHE_NONE,
+};
+
+static irqreturn_t sil902x_interrupt(int irq, void *data)
+{
+	struct sil902x *sil902x = data;
+	unsigned int status = 0;
+
+	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
+	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
+
+	if ((status & SI902X_HOTPLUG_EVENT) && sil902x->bridge.dev)
+		drm_helper_hpd_irq_event(sil902x->bridge.dev);
+
+	return IRQ_HANDLED;
+}
+
+static int sil902x_probe(struct i2c_client *client,
+			 const struct i2c_device_id *id)
+{
+	struct device *dev = &client->dev;
+	unsigned int status = 0;
+	struct sil902x *sil902x;
+	u8 chipid[4];
+	int ret;
+
+	sil902x = devm_kzalloc(dev, sizeof(*sil902x), GFP_KERNEL);
+	if (!sil902x)
+		return -ENOMEM;
+
+	sil902x->i2c = client;
+	sil902x->regmap = devm_regmap_init_i2c(client, &sil902x_regmap_config);
+	if (IS_ERR(sil902x->regmap))
+		return PTR_ERR(sil902x->regmap);
+
+	sil902x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
+	if (IS_ERR(sil902x->reset_gpio)) {
+		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
+			PTR_ERR(sil902x->reset_gpio));
+		return PTR_ERR(sil902x->reset_gpio);
+	}
+
+	sil902x_reset(sil902x);
+
+	ret = regmap_write(sil902x->regmap, SIL902X_REG_TPI_RQB, 0x0);
+	if (ret)
+		return ret;
+
+	ret = regmap_bulk_read(sil902x->regmap, SIL902X_REG_CHIPID(0),
+			       &chipid, 4);
+	if (ret) {
+		dev_err(dev, "regmap_read failed %d\n", ret);
+		return ret;
+	}
+
+	if (chipid[0] != 0xb0) {
+		dev_err(dev, "Invalid chipid: %02x (expecting 0xb0)\n",
+			chipid[0]);
+		return -EINVAL;
+	}
+
+	/* Clear all pending interrupts */
+	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
+	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
+
+	if (client->irq > 0) {
+		regmap_write(sil902x->regmap, SI902X_INT_ENABLE,
+			     SI902X_HOTPLUG_EVENT);
+
+		ret = devm_request_threaded_irq(dev, client->irq, NULL,
+						sil902x_interrupt,
+						IRQF_ONESHOT, dev_name(dev),
+						sil902x);
+		if (ret)
+			return ret;
+	}
+
+	sil902x->bridge.funcs = &sil902x_bridge_funcs;
+	sil902x->bridge.of_node = dev->of_node;
+	ret = drm_bridge_add(&sil902x->bridge);
+	if (ret) {
+		dev_err(dev, "Failed to add drm_bridge\n");
+		return ret;
+	}
+
+	i2c_set_clientdata(client, sil902x);
+
+	return 0;
+}
+
+static int sil902x_remove(struct i2c_client *client)
+
+{
+	struct sil902x *sil902x = i2c_get_clientdata(client);
+
+	drm_bridge_remove(&sil902x->bridge);
+
+	return 0;
+}
+
+#ifdef CONFIG_OF
+static const struct of_device_id sil902x_dt_ids[] = {
+	{ .compatible = "sil,sil9022", },
+	{ }
+};
+MODULE_DEVICE_TABLE(of, sil902x_dt_ids);
+#endif
+
+static const struct i2c_device_id sil902x_i2c_ids[] = {
+	{ "sil9022", 0 },
+	{ },
+};
+MODULE_DEVICE_TABLE(i2c, sil902x_i2c_ids);
+
+static struct i2c_driver sil902x_driver = {
+	.probe = sil902x_probe,
+	.remove = sil902x_remove,
+	.driver = {
+		.name = "sil902x",
+		.of_match_table = of_match_ptr(sil902x_dt_ids),
+	},
+	.id_table = sil902x_i2c_ids,
+};
+module_i2c_driver(sil902x_driver);
+
+MODULE_AUTHOR("Boris Brezillon <boris.brezillon@free-electrons.com>");
+MODULE_DESCRIPTION("SIL902x RGB -> HDMI bridges");
+MODULE_LICENSE("GPL");
-- 
2.1.4

Re: [PATCH 1/2] drm: bridge: sil902x

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 2579 bytes --]

Hi Boris,

[auto build test WARNING on drm/drm-next]
[also build test WARNING on v4.4-rc8 next-20160106]
[if your patch is applied to the wrong git tree, please drop us a note to help improving the system]

url:    https://github.com/0day-ci/linux/commits/Boris-Brezillon/drm-bridge-sil902x/20160106-192921
base:   git://people.freedesktop.org/~airlied/linux.git drm-next
config: powerpc-allyesconfig (attached as .config)
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=powerpc 

All warnings (new ones prefixed by >>):

   In file included from drivers/gpu/drm/bridge/sil902x.c:31:0:
   drivers/gpu/drm/bridge/sil902x.c: In function 'sil902x_bridge_mode_set':
>> drivers/gpu/drm/bridge/sil902x.c:300:13: warning: format '%zd' expects argument of type 'signed size_t', but argument 2 has type 'int' [-Wformat=]
      DRM_ERROR("failed to pack AVI infoframe: %zd\n", ret);
                ^
   include/drm/drmP.h:168:10: note: in definition of macro 'DRM_ERROR'
     drm_err(fmt, ##__VA_ARGS__)
             ^
   drivers/gpu/drm/bridge/sil902x.c: In function 'sil902x_probe':
   drivers/gpu/drm/bridge/sil902x.c:401:16: warning: format '%d' expects argument of type 'int', but argument 3 has type 'long int' [-Wformat=]
      dev_err(dev, "Failed to retrieve/request reset gpio: %d\n",
                   ^

vim +300 drivers/gpu/drm/bridge/sil902x.c

   284			 SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT;
   285		buf[9] = SIL902X_TPI_AVI_INPUT_RANGE_AUTO |
   286			 SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB;
   287	
   288		ret = regmap_bulk_write(regmap, SIL902X_TPI_VIDEO_DATA, buf, 10);
   289		if (ret)
   290			return;
   291	
   292		ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
   293		if (ret < 0) {
   294			DRM_ERROR("couldn't fill AVI infoframe\n");
   295			return;
   296		}
   297	
   298		ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
   299		if (ret < 0) {
 > 300			DRM_ERROR("failed to pack AVI infoframe: %zd\n", ret);
   301			return;
   302		}
   303	
   304		/* Do not send the infoframe header, but keep the CRC field. */
   305		regmap_bulk_write(regmap, SIL902X_TPI_AVI_INFOFRAME,
   306				  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
   307				  HDMI_AVI_INFOFRAME_SIZE + 1);
   308	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 47204 bytes --]

Re: [PATCH 2/2] drm: bridge: add sil902x DT bindings doc

[Rob Herring]

On Wed, Jan 06, 2016 at 12:25:51PM +0100, Boris Brezillon wrote:
> Add Sil9022 DT bindings description.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
>  .../devicetree/bindings/display/bridge/sil902x.txt | 31 ++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/bridge/sil902x.txt
> 
> diff --git a/Documentation/devicetree/bindings/display/bridge/sil902x.txt b/Documentation/devicetree/bindings/display/bridge/sil902x.txt
> new file mode 100644
> index 0000000..7f1339f
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/bridge/sil902x.txt
> @@ -0,0 +1,31 @@
> +sil902x HDMI bridge bindings
> +
> +Required properties:
> +	- compatible: "sil,sil9022"
> +	- reg: i2c address of the bridge
> +	- reset-gpios: OF device-tree gpio specification for RST_N pin.
> +
> +Optional properties:
> +	- interrupts-extended or interrupt-parent + interrupts: describe
> +	  the interrupt line used to inform the host about hotplug events.
> +
> +Optional subnodes:
> +	- video input: Device node can contain 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@39 {
> +		compatible = "sil,sil9022";
> +		reg = <0x39>;
> +		reset-gpios = <&gpx1 5 1 0 0>;

4 GPIO cells? Valid, but unusual.

> +		ports {
> +			port@0 {

Either need a reg property here or drop the unit address. You could 
remove ports as well.

> +				bridge_in: endpoint {
> +					remote-endpoint = <&dc_out>;
> +				};
> +			};
> +		};
> +	};
> -- 
> 2.1.4
> 

Re: [PATCH 2/2] drm: bridge: add sil902x DT bindings doc

[Boris Brezillon]

Hi Rob,

On Wed, 6 Jan 2016 07:19:59 -0600
Rob Herring <robh@kernel.org> wrote:

> On Wed, Jan 06, 2016 at 12:25:51PM +0100, Boris Brezillon wrote:
> > Add Sil9022 DT bindings description.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> >  .../devicetree/bindings/display/bridge/sil902x.txt | 31 ++++++++++++++++++++++
> >  1 file changed, 31 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/bridge/sil902x.txt
> > 
> > diff --git a/Documentation/devicetree/bindings/display/bridge/sil902x.txt b/Documentation/devicetree/bindings/display/bridge/sil902x.txt
> > new file mode 100644
> > index 0000000..7f1339f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/bridge/sil902x.txt
> > @@ -0,0 +1,31 @@
> > +sil902x HDMI bridge bindings
> > +
> > +Required properties:
> > +	- compatible: "sil,sil9022"
> > +	- reg: i2c address of the bridge
> > +	- reset-gpios: OF device-tree gpio specification for RST_N pin.
> > +
> > +Optional properties:
> > +	- interrupts-extended or interrupt-parent + interrupts: describe
> > +	  the interrupt line used to inform the host about hotplug events.
> > +
> > +Optional subnodes:
> > +	- video input: Device node can contain 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@39 {
> > +		compatible = "sil,sil9022";
> > +		reg = <0x39>;
> > +		reset-gpios = <&gpx1 5 1 0 0>;
> 
> 4 GPIO cells? Valid, but unusual.

Hehe, I blindly copied nxp,ptn3460 doc, which is defining such GPIO
descriptors :-). I can change that if you want.

> 
> > +		ports {
> > +			port@0 {
> 
> Either need a reg property here or drop the unit address. You could 
> remove ports as well.

Actually we'll likely define a 2nd port for the audio input, so I'd
prefer keeping the ports and @0 suffix and adding a reg property rather
than just defining port { ... };

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] drm: bridge: sil902x

[Sascha Hauer]

Hi Boris,

On Wed, Jan 06, 2016 at 12:25:50PM +0100, Boris Brezillon wrote:
> Add basic support for the sil902x RGB -> HDMI bridge.
> This driver does not support audio output yet.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Hello,
> 
> This patch is only adding basic support for the sil9022 chip.

This thing is a SiI9022 for camel case "Silicon Image" with a capital 'I',
not a small 'l'.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] drm: bridge: sil902x

[Boris Brezillon]

Hi Sascha,

On Wed, 6 Jan 2016 14:47:36 +0100
Sascha Hauer <s.hauer@pengutronix.de> wrote:

> Hi Boris,
> 
> On Wed, Jan 06, 2016 at 12:25:50PM +0100, Boris Brezillon wrote:
> > Add basic support for the sil902x RGB -> HDMI bridge.
> > This driver does not support audio output yet.
> > 
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > Hello,
> > 
> > This patch is only adding basic support for the sil9022 chip.
> 
> This thing is a SiI9022 for camel case "Silicon Image" with a capital 'I',
> not a small 'l'.

Oh, my bad, I'll fix that, but the vendor prefix defined in
Documentation/devicetree/bindings/vendor-prefixes.txt is not helping in
getting this right.

Should I also change the driver name?

Best Regards,

Boris

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH 1/2] drm: bridge: sil902x

[Sascha Hauer]

On Wed, Jan 06, 2016 at 02:53:30PM +0100, Boris Brezillon wrote:
> Hi Sascha,
> 
> On Wed, 6 Jan 2016 14:47:36 +0100
> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> 
> > Hi Boris,
> > 
> > On Wed, Jan 06, 2016 at 12:25:50PM +0100, Boris Brezillon wrote:
> > > Add basic support for the sil902x RGB -> HDMI bridge.
> > > This driver does not support audio output yet.
> > > 
> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > > ---
> > > Hello,
> > > 
> > > This patch is only adding basic support for the sil9022 chip.
> > 
> > This thing is a SiI9022 for camel case "Silicon Image" with a capital 'I',
> > not a small 'l'.
> 
> Oh, my bad, I'll fix that, but the vendor prefix defined in
> Documentation/devicetree/bindings/vendor-prefixes.txt is not helping in
> getting this right.

No, indeed not. Unfortunately sii is already taken by Seiko.

> 
> Should I also change the driver name?

I would suggest so, yes.

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] drm: bridge: sil902x

[Ilia Mirkin]

On Wed, Jan 6, 2016 at 10:26 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> On Wed, Jan 06, 2016 at 02:53:30PM +0100, Boris Brezillon wrote:
>> Hi Sascha,
>>
>> On Wed, 6 Jan 2016 14:47:36 +0100
>> Sascha Hauer <s.hauer@pengutronix.de> wrote:
>>
>> > Hi Boris,
>> >
>> > On Wed, Jan 06, 2016 at 12:25:50PM +0100, Boris Brezillon wrote:
>> > > Add basic support for the sil902x RGB -> HDMI bridge.
>> > > This driver does not support audio output yet.
>> > >
>> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
>> > > ---
>> > > Hello,
>> > >
>> > > This patch is only adding basic support for the sil9022 chip.
>> >
>> > This thing is a SiI9022 for camel case "Silicon Image" with a capital 'I',
>> > not a small 'l'.
>>
>> Oh, my bad, I'll fix that, but the vendor prefix defined in
>> Documentation/devicetree/bindings/vendor-prefixes.txt is not helping in
>> getting this right.
>
> No, indeed not. Unfortunately sii is already taken by Seiko.
>
>>
>> Should I also change the driver name?
>
> I would suggest so, yes.

For opposing opinions:

drivers/gpu/drm/i2c/sil164_drv.c
drivers/media/platform/s5p-tv/sii9234_drv.c

One of each :)

  -ilia

Re: [PATCH v2 1/2] drm: bridge: sil902x

[Archit Taneja]

On 01/06/2016 05:55 PM, Boris Brezillon wrote:
> Add basic support for the sil902x RGB -> HDMI bridge.
> This driver does not support audio output yet.
>
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> ---
> Hello,
>
> This patch is only adding basic support for the sil9022 chip.
> As stated in the commit log, there's no audio support, but the
> driver also hardcodes a lot of things (like the RGB input format to
> use).
> There are two reasons for this:
> 1/ the DRM framework does not allow for advanced link description
>     between an encoder and a bridge (that's for the RGB format
>     limitation). Any idea how this should be described?

The adv7511 driver uses a DT param "input_colorspace" to chose between
RGB or YUV. I don't think DT is the best idea, but I don't know of a
better way either.

The connector's display_info.color_formats field gives us some info
about the color formats supported by the monitor, but I guess that
isn't sufficient data to know what format the encoder is pushing out.

We get around this problem in case the case of DSI encoders by using
the mipi_dsi_device api, where a DSI based bridge or panel can pass
color format/lane info to the DSI host (i.e, encoder) by using
mipi_dsi_attach().


>
> 2/ I don't have the datasheet of this HDMI encoder, and all logic
>     has been extracted from those two drivers [1][2], which means
>     I may miss some important things in my encoder setup.
>
> Another thing I find weird in the drm_bridge interface is the fact
> that we have a ->attach() method, but no ->detach(), which can be
> a problem if we allow drm bridges and KMS drivers to be compiled as
> modules. Any reason for that?

I guess the drm_bridge_add/remove ops make can ensure that the bridge
driver itself can be compiled as a module. However, you're right that
we would need a bridge detach op that the kms driver should call
before it is removed (to unregister the connector we created).

Someone would still need to make sure about the order in which the
modules are removed. If the bridge driver is removed first, then
it would really mess up the kms driver using the bridge.


Would the kms driver using this chip really have an encoder?
Since the chip takes in RGB input, I'm assuming that the encoder in
the kms driver would more or less be nop funcs, giving their way to
bridge ops.

In such cases, I think the bridge still doesn't fit in so well. The
best fit here is how the current tda998x driver is modeled. It adds
itself as a component that creates both an encoder and connector,
which the kms driver can use directly. This approach, of course,
prevents us using tda998x in kms drivers that don't accept any
encoders that it didn't create itself.

Thanks,
Archit

>
> That's all for the questions part :-).
>
> Best Regards,
>
> Boris
>
> Changes in v2:
> - fix errors reported by kbuild test robot
>
> ---
>   drivers/gpu/drm/bridge/Kconfig   |   8 +
>   drivers/gpu/drm/bridge/Makefile  |   1 +
>   drivers/gpu/drm/bridge/sil902x.c | 491 +++++++++++++++++++++++++++++++++++++++
>   3 files changed, 500 insertions(+)
>   create mode 100644 drivers/gpu/drm/bridge/sil902x.c
>
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 27e2022..9701fd2 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -40,4 +40,12 @@ config DRM_PARADE_PS8622
>   	---help---
>   	  Parade eDP-LVDS bridge chip driver.
>
> +config DRM_SIL902X
> +	tristate "Silicon Image sil902x RGB/HDMI bridge"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	select REGMAP_I2C
> +	---help---
> +	  Silicon Image sil902x bridge chip driver.
> +
>   endmenu
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index f13c33d..a689aad 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>   obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>   obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>   obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> +obj-$(CONFIG_DRM_SIL902X) += sil902x.o
> diff --git a/drivers/gpu/drm/bridge/sil902x.c b/drivers/gpu/drm/bridge/sil902x.c
> new file mode 100644
> index 0000000..2657031
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/sil902x.c
> @@ -0,0 +1,491 @@
> +/*
> + * Copyright (C) 2014 Atmel
> + *		      Bo Shen <voice.shen@atmel.com>
> + *
> + * Authors:	      Bo Shen <voice.shen@atmel.com>
> + *		      Boris Brezillon <boris.brezillon@free-electrons.com>
> + *		      Wu, Songjun <Songjun.Wu@atmel.com>
> + *
> + *
> + * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/component.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder_slave.h>
> +
> +#define SIL902X_TPI_VIDEO_DATA			0x0
> +
> +#define SIL902X_TPI_PIXEL_REPETITION		0x8
> +#define SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT     BIT(5)
> +#define SIL902X_TPI_AVI_PIXEL_REP_RISING_EDGE   BIT(4)
> +#define SIL902X_TPI_AVI_PIXEL_REP_4X		3
> +#define SIL902X_TPI_AVI_PIXEL_REP_2X		1
> +#define SIL902X_TPI_AVI_PIXEL_REP_NONE		0
> +#define SIL902X_TPI_CLK_RATIO_HALF		(0 << 6)
> +#define SIL902X_TPI_CLK_RATIO_1X		(1 << 6)
> +#define SIL902X_TPI_CLK_RATIO_2X		(2 << 6)
> +#define SIL902X_TPI_CLK_RATIO_4X		(3 << 6)
> +
> +#define SIL902X_TPI_AVI_IN_FORMAT		0x9
> +#define SIL902X_TPI_AVI_INPUT_BITMODE_12BIT	BIT(7)
> +#define SIL902X_TPI_AVI_INPUT_DITHER		BIT(6)
> +#define SIL902X_TPI_AVI_INPUT_RANGE_LIMITED	(2 << 2)
> +#define SIL902X_TPI_AVI_INPUT_RANGE_FULL	(1 << 2)
> +#define SIL902X_TPI_AVI_INPUT_RANGE_AUTO	(0 << 2)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_BLACK	(3 << 0)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV422	(2 << 0)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV444	(1 << 0)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB	(0 << 0)
> +
> +#define SIL902X_TPI_AVI_INFOFRAME		0x0c
> +
> +#define SIL902X_SYS_CTRL_DATA			0x1a
> +#define SIL902X_SYS_CTRL_PWR_DWN		BIT(4)
> +#define SIL902X_SYS_CTRL_AV_MUTE		BIT(3)
> +#define SIL902X_SYS_CTRL_DDC_BUS_REQ		BIT(2)
> +#define SIL902X_SYS_CTRL_DDC_BUS_GRTD		BIT(1)
> +#define SIL902X_SYS_CTRL_OUTPUT_MODE		BIT(0)
> +#define SIL902X_SYS_CTRL_OUTPUT_HDMI		1
> +#define SIL902X_SYS_CTRL_OUTPUT_DVI		0
> +
> +#define SIL902X_REG_CHIPID(n)			(0x1b + (n))
> +
> +#define SIL902X_PWR_STATE_CTRL			0x1e
> +#define SIL902X_AVI_POWER_STATE_MSK		GENMASK(1, 0)
> +#define SIL902X_AVI_POWER_STATE_D(l)		((l) & SIL902X_AVI_POWER_STATE_MSK)
> +
> +#define SI902X_INT_ENABLE			0x3c
> +#define SI902X_INT_STATUS			0x3d
> +#define SI902X_HOTPLUG_EVENT			BIT(0)
> +#define SI902X_PLUGGED_STATUS			BIT(2)
> +
> +#define SIL902X_REG_TPI_RQB			0xc7
> +
> +struct sil902x {
> +	struct i2c_client *i2c;
> +	struct regmap *regmap;
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +	struct gpio_desc *reset_gpio;
> +	struct work_struct hotplug_work;
> +};
> +
> +static inline struct sil902x *bridge_to_sil902x(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct sil902x, bridge);
> +}
> +
> +static inline struct sil902x *connector_to_sil902x(struct drm_connector *con)
> +{
> +	return container_of(con, struct sil902x, connector);
> +}
> +
> +static void sil902x_reset(struct sil902x *sil902x)
> +{
> +	gpiod_set_value(sil902x->reset_gpio, 1);
> +
> +	msleep(100);
> +
> +	gpiod_set_value(sil902x->reset_gpio, 0);
> +}
> +
> +static void sil902x_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_unregister(connector);
> +	drm_connector_cleanup(connector);
> +}
> +
> +static enum drm_connector_status
> +sil902x_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct sil902x *sil902x = connector_to_sil902x(connector);
> +	unsigned int status;
> +
> +	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
> +
> +	return (status & SI902X_PLUGGED_STATUS) ?
> +	       connector_status_connected : connector_status_disconnected;
> +}
> +
> +static const struct drm_connector_funcs sil902x_atomic_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.detect = sil902x_connector_detect,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = sil902x_connector_destroy,
> +	.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 const struct drm_connector_funcs sil902x_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.detect = sil902x_connector_detect,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = sil902x_connector_destroy,
> +};
> +
> +static int sil902x_get_modes(struct drm_connector *connector)
> +{
> +	struct sil902x *sil902x = connector_to_sil902x(connector);
> +	struct regmap *regmap = sil902x->regmap;
> +	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	unsigned int status;
> +	struct edid *edid;
> +	int num = 0;
> +	int ret;
> +	int i;
> +
> +	ret = regmap_update_bits(regmap, SIL902X_PWR_STATE_CTRL,
> +				 SIL902X_AVI_POWER_STATE_MSK,
> +				 SIL902X_AVI_POWER_STATE_D(2));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA,
> +			   SIL902X_SYS_CTRL_OUTPUT_HDMI |
> +			   SIL902X_SYS_CTRL_PWR_DWN);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
> +				 SIL902X_SYS_CTRL_DDC_BUS_REQ,
> +				 SIL902X_SYS_CTRL_DDC_BUS_REQ);
> +	if (ret)
> +		return ret;
> +
> +	i = 0;
> +	do {
> +		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
> +		if (ret)
> +			return ret;
> +		i++;
> +	} while (!(status & SIL902X_SYS_CTRL_DDC_BUS_GRTD));
> +
> +	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA, status);
> +	if (ret)
> +		return ret;
> +
> +	edid = drm_get_edid(connector, sil902x->i2c->adapter);
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	if (edid) {
> +		num += drm_add_edid_modes(connector, edid);
> +		kfree(edid);
> +	}
> +
> +	ret = drm_display_info_set_bus_formats(&connector->display_info,
> +					       &bus_format, 1);
> +	if (ret)
> +		return ret;
> +
> +	regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
> +				 SIL902X_SYS_CTRL_DDC_BUS_REQ |
> +				 SIL902X_SYS_CTRL_DDC_BUS_GRTD, 0);
> +	if (ret)
> +		return ret;
> +
> +	i = 0;
> +	do {
> +		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
> +		if (ret)
> +			return ret;
> +		i++;
> +	} while (status & (SIL902X_SYS_CTRL_DDC_BUS_REQ |
> +			   SIL902X_SYS_CTRL_DDC_BUS_GRTD));
> +
> +	return num;
> +}
> +
> +static enum drm_mode_status sil902x_mode_valid(struct drm_connector *connector,
> +					       struct drm_display_mode *mode)
> +{
> +	/* TODO: check mode */
> +
> +	return MODE_OK;
> +}
> +
> +static struct drm_encoder *sil902x_best_encoder(struct drm_connector *connector)
> +{
> +	struct sil902x *sil902x = connector_to_sil902x(connector);
> +
> +	return sil902x->bridge.encoder;
> +}
> +
> +static const struct drm_connector_helper_funcs sil902x_connector_helper_funcs = {
> +	.get_modes = sil902x_get_modes,
> +	.mode_valid = sil902x_mode_valid,
> +	.best_encoder = sil902x_best_encoder,
> +};
> +
> +static void sil902x_bridge_disable(struct drm_bridge *bridge)
> +{
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +
> +	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
> +			   SIL902X_SYS_CTRL_PWR_DWN,
> +			   SIL902X_SYS_CTRL_PWR_DWN);
> +}
> +
> +static void sil902x_bridge_enable(struct drm_bridge *bridge)
> +{
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +
> +	regmap_update_bits(sil902x->regmap, SIL902X_PWR_STATE_CTRL,
> +			   SIL902X_AVI_POWER_STATE_MSK,
> +			   SIL902X_AVI_POWER_STATE_D(0));
> +	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
> +			   SIL902X_SYS_CTRL_PWR_DWN, 0);
> +}
> +
> +static void sil902x_bridge_mode_set(struct drm_bridge *bridge,
> +				    struct drm_display_mode *mode,
> +				    struct drm_display_mode *adj)
> +{
> +	u8 buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +	struct regmap *regmap = sil902x->regmap;
> +	struct hdmi_avi_infoframe frame;
> +	int ret;
> +
> +	buf[0] = adj->clock;
> +	buf[1] = adj->clock >> 8;
> +	buf[2] = adj->vrefresh;
> +	buf[3] = 0x00;
> +	buf[4] = adj->hdisplay;
> +	buf[5] = adj->hdisplay >> 8;
> +	buf[6] = adj->vdisplay;
> +	buf[7] = adj->vdisplay >> 8;
> +	buf[8] = SIL902X_TPI_CLK_RATIO_1X | SIL902X_TPI_AVI_PIXEL_REP_NONE |
> +		 SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT;
> +	buf[9] = SIL902X_TPI_AVI_INPUT_RANGE_AUTO |
> +		 SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB;
> +
> +	ret = regmap_bulk_write(regmap, SIL902X_TPI_VIDEO_DATA, buf, 10);
> +	if (ret)
> +		return;
> +
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
> +	if (ret < 0) {
> +		DRM_ERROR("couldn't fill AVI infoframe\n");
> +		return;
> +	}
> +
> +	ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
> +	if (ret < 0) {
> +		DRM_ERROR("failed to pack AVI infoframe: %zd\n", ret);
> +		return;
> +	}
> +
> +	/* Do not send the infoframe header, but keep the CRC field. */
> +	regmap_bulk_write(regmap, SIL902X_TPI_AVI_INFOFRAME,
> +			  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
> +			  HDMI_AVI_INFOFRAME_SIZE + 1);
> +}
> +
> +static int sil902x_bridge_attach(struct drm_bridge *bridge)
> +{
> +	const struct drm_connector_funcs *funcs = &sil902x_connector_funcs;
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +	struct drm_device *drm = bridge->dev;
> +	int ret;
> +
> +	drm_connector_helper_add(&sil902x->connector,
> +				 &sil902x_connector_helper_funcs);
> +
> +	if (drm_core_check_feature(drm, DRIVER_ATOMIC))
> +		funcs = &sil902x_atomic_connector_funcs;
> +
> +	ret = drm_connector_init(drm, &sil902x->connector, funcs,
> +				 DRM_MODE_CONNECTOR_HDMIA);
> +	if (ret)
> +		return ret;
> +
> +	if (sil902x->i2c->irq > 0)
> +		sil902x->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	else
> +		sil902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
> +
> +	drm_mode_connector_attach_encoder(&sil902x->connector, bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static void sil902x_bridge_nop(struct drm_bridge *bridge)
> +{
> +}
> +
> +static const struct drm_bridge_funcs sil902x_bridge_funcs = {
> +	.attach = sil902x_bridge_attach,
> +	.mode_set = sil902x_bridge_mode_set,
> +	.disable = sil902x_bridge_disable,
> +	.post_disable = sil902x_bridge_nop,
> +	.pre_enable = sil902x_bridge_nop,
> +	.enable = sil902x_bridge_enable,
> +};
> +
> +static const struct regmap_range sil902x_volatile_ranges[] = {
> +	{ .range_min = 0, .range_max = 0xff },
> +};
> +
> +static const struct regmap_access_table sil902x_volatile_table = {
> +	.yes_ranges = sil902x_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(sil902x_volatile_ranges),
> +};
> +
> +static const struct regmap_config sil902x_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &sil902x_volatile_table,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
> +static irqreturn_t sil902x_interrupt(int irq, void *data)
> +{
> +	struct sil902x *sil902x = data;
> +	unsigned int status = 0;
> +
> +	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
> +	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
> +
> +	if ((status & SI902X_HOTPLUG_EVENT) && sil902x->bridge.dev)
> +		drm_helper_hpd_irq_event(sil902x->bridge.dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int sil902x_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	unsigned int status = 0;
> +	struct sil902x *sil902x;
> +	u8 chipid[4];
> +	int ret;
> +
> +	sil902x = devm_kzalloc(dev, sizeof(*sil902x), GFP_KERNEL);
> +	if (!sil902x)
> +		return -ENOMEM;
> +
> +	sil902x->i2c = client;
> +	sil902x->regmap = devm_regmap_init_i2c(client, &sil902x_regmap_config);
> +	if (IS_ERR(sil902x->regmap))
> +		return PTR_ERR(sil902x->regmap);
> +
> +	sil902x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(sil902x->reset_gpio)) {
> +		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
> +			PTR_ERR(sil902x->reset_gpio));
> +		return PTR_ERR(sil902x->reset_gpio);
> +	}
> +
> +	sil902x_reset(sil902x);
> +
> +	ret = regmap_write(sil902x->regmap, SIL902X_REG_TPI_RQB, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_bulk_read(sil902x->regmap, SIL902X_REG_CHIPID(0),
> +			       &chipid, 4);
> +	if (ret) {
> +		dev_err(dev, "regmap_read failed %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (chipid[0] != 0xb0) {
> +		dev_err(dev, "Invalid chipid: %02x (expecting 0xb0)\n",
> +			chipid[0]);
> +		return -EINVAL;
> +	}
> +
> +	/* Clear all pending interrupts */
> +	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
> +	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
> +
> +	if (client->irq > 0) {
> +		regmap_write(sil902x->regmap, SI902X_INT_ENABLE,
> +			     SI902X_HOTPLUG_EVENT);
> +
> +		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +						sil902x_interrupt,
> +						IRQF_ONESHOT, dev_name(dev),
> +						sil902x);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	sil902x->bridge.funcs = &sil902x_bridge_funcs;
> +	sil902x->bridge.of_node = dev->of_node;
> +	ret = drm_bridge_add(&sil902x->bridge);
> +	if (ret) {
> +		dev_err(dev, "Failed to add drm_bridge\n");
> +		return ret;
> +	}
> +
> +	i2c_set_clientdata(client, sil902x);
> +
> +	return 0;
> +}
> +
> +static int sil902x_remove(struct i2c_client *client)
> +
> +{
> +	struct sil902x *sil902x = i2c_get_clientdata(client);
> +
> +	drm_bridge_remove(&sil902x->bridge);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id sil902x_dt_ids[] = {
> +	{ .compatible = "sil,sil9022", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sil902x_dt_ids);
> +#endif
> +
> +static const struct i2c_device_id sil902x_i2c_ids[] = {
> +	{ "sil9022", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, sil902x_i2c_ids);
> +
> +static struct i2c_driver sil902x_driver = {
> +	.probe = sil902x_probe,
> +	.remove = sil902x_remove,
> +	.driver = {
> +		.name = "sil902x",
> +		.of_match_table = of_match_ptr(sil902x_dt_ids),
> +	},
> +	.id_table = sil902x_i2c_ids,
> +};
> +module_i2c_driver(sil902x_driver);
> +
> +MODULE_AUTHOR("Boris Brezillon <boris.brezillon@free-electrons.com>");
> +MODULE_DESCRIPTION("SIL902x RGB -> HDMI bridges");
> +MODULE_LICENSE("GPL");
>

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora 
Forum, hosted by The Linux Foundation

Re: [PATCH 1/2] drm: bridge: sil902x

[Sascha Hauer]

On Wed, Jan 06, 2016 at 10:35:37AM -0500, Ilia Mirkin wrote:
> On Wed, Jan 6, 2016 at 10:26 AM, Sascha Hauer <s.hauer@pengutronix.de> wrote:
> > On Wed, Jan 06, 2016 at 02:53:30PM +0100, Boris Brezillon wrote:
> >> Hi Sascha,
> >>
> >> On Wed, 6 Jan 2016 14:47:36 +0100
> >> Sascha Hauer <s.hauer@pengutronix.de> wrote:
> >>
> >> > Hi Boris,
> >> >
> >> > On Wed, Jan 06, 2016 at 12:25:50PM +0100, Boris Brezillon wrote:
> >> > > Add basic support for the sil902x RGB -> HDMI bridge.
> >> > > This driver does not support audio output yet.
> >> > >
> >> > > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> >> > > ---
> >> > > Hello,
> >> > >
> >> > > This patch is only adding basic support for the sil9022 chip.
> >> >
> >> > This thing is a SiI9022 for camel case "Silicon Image" with a capital 'I',
> >> > not a small 'l'.
> >>
> >> Oh, my bad, I'll fix that, but the vendor prefix defined in
> >> Documentation/devicetree/bindings/vendor-prefixes.txt is not helping in
> >> getting this right.
> >
> > No, indeed not. Unfortunately sii is already taken by Seiko.
> >
> >>
> >> Should I also change the driver name?
> >
> > I would suggest so, yes.
> 
> For opposing opinions:
> 
> drivers/gpu/drm/i2c/sil164_drv.c
> drivers/media/platform/s5p-tv/sii9234_drv.c
> 
> One of each :)

So we have examples for both, then we can continue with the correct
name ;)

Sascha

-- 
Pengutronix e.K.                           |                             |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |
Peiner Str. 6-8, 31137 Hildesheim, Germany | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH v2 1/2] drm: bridge: sil902x

[Boris Brezillon]

Hi Archit,

On Thu, 7 Jan 2016 11:12:47 +0530
Archit Taneja <architt@codeaurora.org> wrote:

> 
> 
> On 01/06/2016 05:55 PM, Boris Brezillon wrote:
> > Add basic support for the sil902x RGB -> HDMI bridge.
> > This driver does not support audio output yet.
> >
> > Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>
> > ---
> > Hello,
> >
> > This patch is only adding basic support for the sil9022 chip.
> > As stated in the commit log, there's no audio support, but the
> > driver also hardcodes a lot of things (like the RGB input format to
> > use).
> > There are two reasons for this:
> > 1/ the DRM framework does not allow for advanced link description
> >     between an encoder and a bridge (that's for the RGB format
> >     limitation). Any idea how this should be described?
> 
> The adv7511 driver uses a DT param "input_colorspace" to chose between
> RGB or YUV. I don't think DT is the best idea, but I don't know of a
> better way either.

Yes, maybe not the best place, but I must admit that's a convenient way
to set the link format.

> 
> The connector's display_info.color_formats field gives us some info
> about the color formats supported by the monitor, but I guess that
> isn't sufficient data to know what format the encoder is pushing out.

I created the ->bus_formats field in drm_display_info exactly for this
purpose (it's used to detect the appropriate output format when
connecting panels), but as you said, this is only describing the link
between the connector and the display, not the link between the encoder
and the bridge.
I'm currently abusing this field (setting bus_formats to RGB888 in the
sii902x driver), but that would be better to have this description at
the encoder/bridge level.

> 
> We get around this problem in case the case of DSI encoders by using
> the mipi_dsi_device api, where a DSI based bridge or panel can pass
> color format/lane info to the DSI host (i.e, encoder) by using
> mipi_dsi_attach().

Yep, that's also an approach I considered a while ago: creating a DPI
bus layer (not sure DPI is the correct name here), and implementing a
DPI driver in atmel_hlcdc drivers and using DPI APIs on the slave side
(panels and bridges/encoders). But I never had any feedback.

> 
> 
> >
> > 2/ I don't have the datasheet of this HDMI encoder, and all logic
> >     has been extracted from those two drivers [1][2], which means
> >     I may miss some important things in my encoder setup.
> >
> > Another thing I find weird in the drm_bridge interface is the fact
> > that we have a ->attach() method, but no ->detach(), which can be
> > a problem if we allow drm bridges and KMS drivers to be compiled as
> > modules. Any reason for that?
> 
> I guess the drm_bridge_add/remove ops make can ensure that the bridge
> driver itself can be compiled as a module. However, you're right that
> we would need a bridge detach op that the kms driver should call
> before it is removed (to unregister the connector we created).
> 
> Someone would still need to make sure about the order in which the
> modules are removed. If the bridge driver is removed first, then
> it would really mess up the kms driver using the bridge.

Yes, the remove order is another problem we have to deal with

> 
> 
> Would the kms driver using this chip really have an encoder?

No, I have to create an encoder of type NONE to make everybody happy
(we need an encoder + a connector to attach to a panel), but as I
understand, when the KMS device outputs the pixel stream in raw RGB it's
not considered as an encoder (which IMO is not such a good idea,
because I have no way to represent my raw RGB connector :-/).

> Since the chip takes in RGB input, I'm assuming that the encoder in
> the kms driver would more or less be nop funcs, giving their way to
> bridge ops.

Exactly, that's a nop unless you have a panel connected to it. In the
case it calls the drm_panel_xxx() functions.

> 
> In such cases, I think the bridge still doesn't fit in so well. The
> best fit here is how the current tda998x driver is modeled. It adds
> itself as a component that creates both an encoder and connector,
> which the kms driver can use directly. This approach, of course,
> prevents us using tda998x in kms drivers that don't accept any
> encoders that it didn't create itself.

Actually that's what I did first [1], but I asked for some advice to
other DRM developers, and they suggested to expose it as a drm_bridge.

Now, I don't know what's the best option: I heard that some work was
being done to merge the encoder slave and bridge concepts. I just
thought external encoders were falling in that case too.
And BTW, the different between all those components is still obscure to
me.

Thanks for the suggestions.

Best Regards,

Boris

[1]https://github.com/linux4sam/linux-at91/blob/linux-3.18-at91/drivers/gpu/drm/i2c/sil902x.c


-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com

Re: [PATCH v2 1/2] drm: bridge: sil902x

[Nicolas Ferre]

Le 06/01/2016 13:25, Boris Brezillon a écrit :
> Add basic support for the sil902x RGB -> HDMI bridge.
> This driver does not support audio output yet.
> 
> Signed-off-by: Boris Brezillon <boris.brezillon@free-electrons.com>

You can add my:
Tested-by: Nicolas Ferre <nicolas.ferre@atmel.com>

I tested it on a SAMA5D4 Xplained board with a Dell HDMI monitor.

Thanks, bye.


> ---
> Hello,
> 
> This patch is only adding basic support for the sil9022 chip.
> As stated in the commit log, there's no audio support, but the
> driver also hardcodes a lot of things (like the RGB input format to
> use).
> There are two reasons for this:
> 1/ the DRM framework does not allow for advanced link description
>    between an encoder and a bridge (that's for the RGB format
>    limitation). Any idea how this should be described?
> 
> 2/ I don't have the datasheet of this HDMI encoder, and all logic
>    has been extracted from those two drivers [1][2], which means
>    I may miss some important things in my encoder setup.
> 
> Another thing I find weird in the drm_bridge interface is the fact
> that we have a ->attach() method, but no ->detach(), which can be
> a problem if we allow drm bridges and KMS drivers to be compiled as
> modules. Any reason for that?
> 
> That's all for the questions part :-).
> 
> Best Regards,
> 
> Boris
> 
> Changes in v2:
> - fix errors reported by kbuild test robot
> 
> ---
>  drivers/gpu/drm/bridge/Kconfig   |   8 +
>  drivers/gpu/drm/bridge/Makefile  |   1 +
>  drivers/gpu/drm/bridge/sil902x.c | 491 +++++++++++++++++++++++++++++++++++++++
>  3 files changed, 500 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/sil902x.c
> 
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 27e2022..9701fd2 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -40,4 +40,12 @@ config DRM_PARADE_PS8622
>  	---help---
>  	  Parade eDP-LVDS bridge chip driver.
>  
> +config DRM_SIL902X
> +	tristate "Silicon Image sil902x RGB/HDMI bridge"
> +	depends on OF
> +	select DRM_KMS_HELPER
> +	select REGMAP_I2C
> +	---help---
> +	  Silicon Image sil902x bridge chip driver.
> +
>  endmenu
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index f13c33d..a689aad 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -4,3 +4,4 @@ obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
>  obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
>  obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
>  obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> +obj-$(CONFIG_DRM_SIL902X) += sil902x.o
> diff --git a/drivers/gpu/drm/bridge/sil902x.c b/drivers/gpu/drm/bridge/sil902x.c
> new file mode 100644
> index 0000000..2657031
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/sil902x.c
> @@ -0,0 +1,491 @@
> +/*
> + * Copyright (C) 2014 Atmel
> + *		      Bo Shen <voice.shen@atmel.com>
> + *
> + * Authors:	      Bo Shen <voice.shen@atmel.com>
> + *		      Boris Brezillon <boris.brezillon@free-electrons.com>
> + *		      Wu, Songjun <Songjun.Wu@atmel.com>
> + *
> + *
> + * Copyright (C) 2010-2011 Freescale Semiconductor, Inc. All Rights Reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/component.h>
> +#include <linux/gpio/consumer.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/regmap.h>
> +
> +#include <drm/drmP.h>
> +#include <drm/drm_atomic.h>
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_crtc_helper.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_encoder_slave.h>
> +
> +#define SIL902X_TPI_VIDEO_DATA			0x0
> +
> +#define SIL902X_TPI_PIXEL_REPETITION		0x8
> +#define SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT     BIT(5)
> +#define SIL902X_TPI_AVI_PIXEL_REP_RISING_EDGE   BIT(4)
> +#define SIL902X_TPI_AVI_PIXEL_REP_4X		3
> +#define SIL902X_TPI_AVI_PIXEL_REP_2X		1
> +#define SIL902X_TPI_AVI_PIXEL_REP_NONE		0
> +#define SIL902X_TPI_CLK_RATIO_HALF		(0 << 6)
> +#define SIL902X_TPI_CLK_RATIO_1X		(1 << 6)
> +#define SIL902X_TPI_CLK_RATIO_2X		(2 << 6)
> +#define SIL902X_TPI_CLK_RATIO_4X		(3 << 6)
> +
> +#define SIL902X_TPI_AVI_IN_FORMAT		0x9
> +#define SIL902X_TPI_AVI_INPUT_BITMODE_12BIT	BIT(7)
> +#define SIL902X_TPI_AVI_INPUT_DITHER		BIT(6)
> +#define SIL902X_TPI_AVI_INPUT_RANGE_LIMITED	(2 << 2)
> +#define SIL902X_TPI_AVI_INPUT_RANGE_FULL	(1 << 2)
> +#define SIL902X_TPI_AVI_INPUT_RANGE_AUTO	(0 << 2)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_BLACK	(3 << 0)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV422	(2 << 0)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_YUV444	(1 << 0)
> +#define SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB	(0 << 0)
> +
> +#define SIL902X_TPI_AVI_INFOFRAME		0x0c
> +
> +#define SIL902X_SYS_CTRL_DATA			0x1a
> +#define SIL902X_SYS_CTRL_PWR_DWN		BIT(4)
> +#define SIL902X_SYS_CTRL_AV_MUTE		BIT(3)
> +#define SIL902X_SYS_CTRL_DDC_BUS_REQ		BIT(2)
> +#define SIL902X_SYS_CTRL_DDC_BUS_GRTD		BIT(1)
> +#define SIL902X_SYS_CTRL_OUTPUT_MODE		BIT(0)
> +#define SIL902X_SYS_CTRL_OUTPUT_HDMI		1
> +#define SIL902X_SYS_CTRL_OUTPUT_DVI		0
> +
> +#define SIL902X_REG_CHIPID(n)			(0x1b + (n))
> +
> +#define SIL902X_PWR_STATE_CTRL			0x1e
> +#define SIL902X_AVI_POWER_STATE_MSK		GENMASK(1, 0)
> +#define SIL902X_AVI_POWER_STATE_D(l)		((l) & SIL902X_AVI_POWER_STATE_MSK)
> +
> +#define SI902X_INT_ENABLE			0x3c
> +#define SI902X_INT_STATUS			0x3d
> +#define SI902X_HOTPLUG_EVENT			BIT(0)
> +#define SI902X_PLUGGED_STATUS			BIT(2)
> +
> +#define SIL902X_REG_TPI_RQB			0xc7
> +
> +struct sil902x {
> +	struct i2c_client *i2c;
> +	struct regmap *regmap;
> +	struct drm_bridge bridge;
> +	struct drm_connector connector;
> +	struct gpio_desc *reset_gpio;
> +	struct work_struct hotplug_work;
> +};
> +
> +static inline struct sil902x *bridge_to_sil902x(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct sil902x, bridge);
> +}
> +
> +static inline struct sil902x *connector_to_sil902x(struct drm_connector *con)
> +{
> +	return container_of(con, struct sil902x, connector);
> +}
> +
> +static void sil902x_reset(struct sil902x *sil902x)
> +{
> +	gpiod_set_value(sil902x->reset_gpio, 1);
> +
> +	msleep(100);
> +
> +	gpiod_set_value(sil902x->reset_gpio, 0);
> +}
> +
> +static void sil902x_connector_destroy(struct drm_connector *connector)
> +{
> +	drm_connector_unregister(connector);
> +	drm_connector_cleanup(connector);
> +}
> +
> +static enum drm_connector_status
> +sil902x_connector_detect(struct drm_connector *connector, bool force)
> +{
> +	struct sil902x *sil902x = connector_to_sil902x(connector);
> +	unsigned int status;
> +
> +	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
> +
> +	return (status & SI902X_PLUGGED_STATUS) ?
> +	       connector_status_connected : connector_status_disconnected;
> +}
> +
> +static const struct drm_connector_funcs sil902x_atomic_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.detect = sil902x_connector_detect,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = sil902x_connector_destroy,
> +	.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 const struct drm_connector_funcs sil902x_connector_funcs = {
> +	.dpms = drm_atomic_helper_connector_dpms,
> +	.detect = sil902x_connector_detect,
> +	.fill_modes = drm_helper_probe_single_connector_modes,
> +	.destroy = sil902x_connector_destroy,
> +};
> +
> +static int sil902x_get_modes(struct drm_connector *connector)
> +{
> +	struct sil902x *sil902x = connector_to_sil902x(connector);
> +	struct regmap *regmap = sil902x->regmap;
> +	u32 bus_format = MEDIA_BUS_FMT_RGB888_1X24;
> +	unsigned int status;
> +	struct edid *edid;
> +	int num = 0;
> +	int ret;
> +	int i;
> +
> +	ret = regmap_update_bits(regmap, SIL902X_PWR_STATE_CTRL,
> +				 SIL902X_AVI_POWER_STATE_MSK,
> +				 SIL902X_AVI_POWER_STATE_D(2));
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA,
> +			   SIL902X_SYS_CTRL_OUTPUT_HDMI |
> +			   SIL902X_SYS_CTRL_PWR_DWN);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
> +				 SIL902X_SYS_CTRL_DDC_BUS_REQ,
> +				 SIL902X_SYS_CTRL_DDC_BUS_REQ);
> +	if (ret)
> +		return ret;
> +
> +	i = 0;
> +	do {
> +		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
> +		if (ret)
> +			return ret;
> +		i++;
> +	} while (!(status & SIL902X_SYS_CTRL_DDC_BUS_GRTD));
> +
> +	ret = regmap_write(regmap, SIL902X_SYS_CTRL_DATA, status);
> +	if (ret)
> +		return ret;
> +
> +	edid = drm_get_edid(connector, sil902x->i2c->adapter);
> +	drm_mode_connector_update_edid_property(connector, edid);
> +	if (edid) {
> +		num += drm_add_edid_modes(connector, edid);
> +		kfree(edid);
> +	}
> +
> +	ret = drm_display_info_set_bus_formats(&connector->display_info,
> +					       &bus_format, 1);
> +	if (ret)
> +		return ret;
> +
> +	regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_update_bits(regmap, SIL902X_SYS_CTRL_DATA,
> +				 SIL902X_SYS_CTRL_DDC_BUS_REQ |
> +				 SIL902X_SYS_CTRL_DDC_BUS_GRTD, 0);
> +	if (ret)
> +		return ret;
> +
> +	i = 0;
> +	do {
> +		ret = regmap_read(regmap, SIL902X_SYS_CTRL_DATA, &status);
> +		if (ret)
> +			return ret;
> +		i++;
> +	} while (status & (SIL902X_SYS_CTRL_DDC_BUS_REQ |
> +			   SIL902X_SYS_CTRL_DDC_BUS_GRTD));
> +
> +	return num;
> +}
> +
> +static enum drm_mode_status sil902x_mode_valid(struct drm_connector *connector,
> +					       struct drm_display_mode *mode)
> +{
> +	/* TODO: check mode */
> +
> +	return MODE_OK;
> +}
> +
> +static struct drm_encoder *sil902x_best_encoder(struct drm_connector *connector)
> +{
> +	struct sil902x *sil902x = connector_to_sil902x(connector);
> +
> +	return sil902x->bridge.encoder;
> +}
> +
> +static const struct drm_connector_helper_funcs sil902x_connector_helper_funcs = {
> +	.get_modes = sil902x_get_modes,
> +	.mode_valid = sil902x_mode_valid,
> +	.best_encoder = sil902x_best_encoder,
> +};
> +
> +static void sil902x_bridge_disable(struct drm_bridge *bridge)
> +{
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +
> +	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
> +			   SIL902X_SYS_CTRL_PWR_DWN,
> +			   SIL902X_SYS_CTRL_PWR_DWN);
> +}
> +
> +static void sil902x_bridge_enable(struct drm_bridge *bridge)
> +{
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +
> +	regmap_update_bits(sil902x->regmap, SIL902X_PWR_STATE_CTRL,
> +			   SIL902X_AVI_POWER_STATE_MSK,
> +			   SIL902X_AVI_POWER_STATE_D(0));
> +	regmap_update_bits(sil902x->regmap, SIL902X_SYS_CTRL_DATA,
> +			   SIL902X_SYS_CTRL_PWR_DWN, 0);
> +}
> +
> +static void sil902x_bridge_mode_set(struct drm_bridge *bridge,
> +				    struct drm_display_mode *mode,
> +				    struct drm_display_mode *adj)
> +{
> +	u8 buf[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AVI_INFOFRAME_SIZE];
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +	struct regmap *regmap = sil902x->regmap;
> +	struct hdmi_avi_infoframe frame;
> +	int ret;
> +
> +	buf[0] = adj->clock;
> +	buf[1] = adj->clock >> 8;
> +	buf[2] = adj->vrefresh;
> +	buf[3] = 0x00;
> +	buf[4] = adj->hdisplay;
> +	buf[5] = adj->hdisplay >> 8;
> +	buf[6] = adj->vdisplay;
> +	buf[7] = adj->vdisplay >> 8;
> +	buf[8] = SIL902X_TPI_CLK_RATIO_1X | SIL902X_TPI_AVI_PIXEL_REP_NONE |
> +		 SIL902X_TPI_AVI_PIXEL_REP_BUS_24BIT;
> +	buf[9] = SIL902X_TPI_AVI_INPUT_RANGE_AUTO |
> +		 SIL902X_TPI_AVI_INPUT_COLORSPACE_RGB;
> +
> +	ret = regmap_bulk_write(regmap, SIL902X_TPI_VIDEO_DATA, buf, 10);
> +	if (ret)
> +		return;
> +
> +	ret = drm_hdmi_avi_infoframe_from_display_mode(&frame, adj);
> +	if (ret < 0) {
> +		DRM_ERROR("couldn't fill AVI infoframe\n");
> +		return;
> +	}
> +
> +	ret = hdmi_avi_infoframe_pack(&frame, buf, sizeof(buf));
> +	if (ret < 0) {
> +		DRM_ERROR("failed to pack AVI infoframe: %zd\n", ret);
> +		return;
> +	}
> +
> +	/* Do not send the infoframe header, but keep the CRC field. */
> +	regmap_bulk_write(regmap, SIL902X_TPI_AVI_INFOFRAME,
> +			  buf + HDMI_INFOFRAME_HEADER_SIZE - 1,
> +			  HDMI_AVI_INFOFRAME_SIZE + 1);
> +}
> +
> +static int sil902x_bridge_attach(struct drm_bridge *bridge)
> +{
> +	const struct drm_connector_funcs *funcs = &sil902x_connector_funcs;
> +	struct sil902x *sil902x = bridge_to_sil902x(bridge);
> +	struct drm_device *drm = bridge->dev;
> +	int ret;
> +
> +	drm_connector_helper_add(&sil902x->connector,
> +				 &sil902x_connector_helper_funcs);
> +
> +	if (drm_core_check_feature(drm, DRIVER_ATOMIC))
> +		funcs = &sil902x_atomic_connector_funcs;
> +
> +	ret = drm_connector_init(drm, &sil902x->connector, funcs,
> +				 DRM_MODE_CONNECTOR_HDMIA);
> +	if (ret)
> +		return ret;
> +
> +	if (sil902x->i2c->irq > 0)
> +		sil902x->connector.polled = DRM_CONNECTOR_POLL_HPD;
> +	else
> +		sil902x->connector.polled = DRM_CONNECTOR_POLL_CONNECT;
> +
> +	drm_mode_connector_attach_encoder(&sil902x->connector, bridge->encoder);
> +
> +	return 0;
> +}
> +
> +static void sil902x_bridge_nop(struct drm_bridge *bridge)
> +{
> +}
> +
> +static const struct drm_bridge_funcs sil902x_bridge_funcs = {
> +	.attach = sil902x_bridge_attach,
> +	.mode_set = sil902x_bridge_mode_set,
> +	.disable = sil902x_bridge_disable,
> +	.post_disable = sil902x_bridge_nop,
> +	.pre_enable = sil902x_bridge_nop,
> +	.enable = sil902x_bridge_enable,
> +};
> +
> +static const struct regmap_range sil902x_volatile_ranges[] = {
> +	{ .range_min = 0, .range_max = 0xff },
> +};
> +
> +static const struct regmap_access_table sil902x_volatile_table = {
> +	.yes_ranges = sil902x_volatile_ranges,
> +	.n_yes_ranges = ARRAY_SIZE(sil902x_volatile_ranges),
> +};
> +
> +static const struct regmap_config sil902x_regmap_config = {
> +	.reg_bits = 8,
> +	.val_bits = 8,
> +	.volatile_table = &sil902x_volatile_table,
> +	.cache_type = REGCACHE_NONE,
> +};
> +
> +static irqreturn_t sil902x_interrupt(int irq, void *data)
> +{
> +	struct sil902x *sil902x = data;
> +	unsigned int status = 0;
> +
> +	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
> +	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
> +
> +	if ((status & SI902X_HOTPLUG_EVENT) && sil902x->bridge.dev)
> +		drm_helper_hpd_irq_event(sil902x->bridge.dev);
> +
> +	return IRQ_HANDLED;
> +}
> +
> +static int sil902x_probe(struct i2c_client *client,
> +			 const struct i2c_device_id *id)
> +{
> +	struct device *dev = &client->dev;
> +	unsigned int status = 0;
> +	struct sil902x *sil902x;
> +	u8 chipid[4];
> +	int ret;
> +
> +	sil902x = devm_kzalloc(dev, sizeof(*sil902x), GFP_KERNEL);
> +	if (!sil902x)
> +		return -ENOMEM;
> +
> +	sil902x->i2c = client;
> +	sil902x->regmap = devm_regmap_init_i2c(client, &sil902x_regmap_config);
> +	if (IS_ERR(sil902x->regmap))
> +		return PTR_ERR(sil902x->regmap);
> +
> +	sil902x->reset_gpio = devm_gpiod_get(dev, "reset", GPIOD_OUT_LOW);
> +	if (IS_ERR(sil902x->reset_gpio)) {
> +		dev_err(dev, "Failed to retrieve/request reset gpio: %ld\n",
> +			PTR_ERR(sil902x->reset_gpio));
> +		return PTR_ERR(sil902x->reset_gpio);
> +	}
> +
> +	sil902x_reset(sil902x);
> +
> +	ret = regmap_write(sil902x->regmap, SIL902X_REG_TPI_RQB, 0x0);
> +	if (ret)
> +		return ret;
> +
> +	ret = regmap_bulk_read(sil902x->regmap, SIL902X_REG_CHIPID(0),
> +			       &chipid, 4);
> +	if (ret) {
> +		dev_err(dev, "regmap_read failed %d\n", ret);
> +		return ret;
> +	}
> +
> +	if (chipid[0] != 0xb0) {
> +		dev_err(dev, "Invalid chipid: %02x (expecting 0xb0)\n",
> +			chipid[0]);
> +		return -EINVAL;
> +	}
> +
> +	/* Clear all pending interrupts */
> +	regmap_read(sil902x->regmap, SI902X_INT_STATUS, &status);
> +	regmap_write(sil902x->regmap, SI902X_INT_STATUS, status);
> +
> +	if (client->irq > 0) {
> +		regmap_write(sil902x->regmap, SI902X_INT_ENABLE,
> +			     SI902X_HOTPLUG_EVENT);
> +
> +		ret = devm_request_threaded_irq(dev, client->irq, NULL,
> +						sil902x_interrupt,
> +						IRQF_ONESHOT, dev_name(dev),
> +						sil902x);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	sil902x->bridge.funcs = &sil902x_bridge_funcs;
> +	sil902x->bridge.of_node = dev->of_node;
> +	ret = drm_bridge_add(&sil902x->bridge);
> +	if (ret) {
> +		dev_err(dev, "Failed to add drm_bridge\n");
> +		return ret;
> +	}
> +
> +	i2c_set_clientdata(client, sil902x);
> +
> +	return 0;
> +}
> +
> +static int sil902x_remove(struct i2c_client *client)
> +
> +{
> +	struct sil902x *sil902x = i2c_get_clientdata(client);
> +
> +	drm_bridge_remove(&sil902x->bridge);
> +
> +	return 0;
> +}
> +
> +#ifdef CONFIG_OF
> +static const struct of_device_id sil902x_dt_ids[] = {
> +	{ .compatible = "sil,sil9022", },
> +	{ }
> +};
> +MODULE_DEVICE_TABLE(of, sil902x_dt_ids);
> +#endif
> +
> +static const struct i2c_device_id sil902x_i2c_ids[] = {
> +	{ "sil9022", 0 },
> +	{ },
> +};
> +MODULE_DEVICE_TABLE(i2c, sil902x_i2c_ids);
> +
> +static struct i2c_driver sil902x_driver = {
> +	.probe = sil902x_probe,
> +	.remove = sil902x_remove,
> +	.driver = {
> +		.name = "sil902x",
> +		.of_match_table = of_match_ptr(sil902x_dt_ids),
> +	},
> +	.id_table = sil902x_i2c_ids,
> +};
> +module_i2c_driver(sil902x_driver);
> +
> +MODULE_AUTHOR("Boris Brezillon <boris.brezillon@free-electrons.com>");
> +MODULE_DESCRIPTION("SIL902x RGB -> HDMI bridges");
> +MODULE_LICENSE("GPL");
> 


-- 
Nicolas Ferre