Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1409281 > unrolled thread
| Started by | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| First post | 2016-05-30 18:50 +0200 |
| Last post | 2016-06-09 18:30 +0200 |
| Articles | 20 on this page of 23 — 8 participants |
Back to article view | Back to linux.kernel
[PATCH 0/5] Add driver for GE B850v3 LVDS/DP++ Bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-05-30 18:50 +0200
[PATCH 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-05-30 18:50 +0200
Re: [PATCH 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge Enric Balletbo Serra <eballetbo@gmail.com> - 2016-05-31 09:50 +0200
Re: [PATCH 2/5] arm/dts/imx6q-b850v3: Configure IPU assignment order Fabio Estevam <festevam@gmail.com> - 2016-05-30 18:50 +0200
[PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp Peter Senna Tschudin <peter.senna@collabora.com> - 2016-05-30 18:50 +0200
Re: [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp Philipp Zabel <p.zabel@pengutronix.de> - 2016-06-02 14:50 +0200
Re: [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp "Peter Senna Tschudin" <peter.senna@collabora.co.uk> - 2016-06-03 01:20 +0200
Re: [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp Rob Herring <robh@kernel.org> - 2016-06-03 01:00 +0200
[PATCH 2/5] arm/dts/imx6q-b850v3: Configure IPU assignment order Peter Senna Tschudin <peter.senna@collabora.com> - 2016-05-30 18:50 +0200
Re: [PATCH 2/5] arm/dts/imx6q-b850v3: Configure IPU assignment order Philipp Zabel <p.zabel@pengutronix.de> - 2016-06-02 15:00 +0200
[PATCH 5/5] arm/dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-05-30 18:50 +0200
Re: [PATCH 5/5] arm/dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge Fabio Estevam <festevam@gmail.com> - 2016-05-30 19:00 +0200
[PATCH V2 0/5] Add driver for GE B850v3 LVDS/DP++ Bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-06-09 18:30 +0200
[PATCH V2 2/5] dts/imx6q-b850v3: Configure IPU assignment order Peter Senna Tschudin <peter.senna@collabora.com> - 2016-06-09 18:30 +0200
[PATCH V2 3/5] Documentation/devicetree/bindings: b850v3_lvds_dp Peter Senna Tschudin <peter.senna@collabora.com> - 2016-06-09 18:30 +0200
Re: [PATCH V2 3/5] Documentation/devicetree/bindings: b850v3_lvds_dp Rob Herring <robh@kernel.org> - 2016-06-10 19:50 +0200
Re: [PATCH V2 3/5] Documentation/devicetree/bindings: b850v3_lvds_dp Javier Martinez Canillas <javier@dowhile0.org> - 2016-06-10 21:00 +0200
[PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-06-09 18:30 +0200
Re: [PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge Enric Balletbo Serra <eballetbo@gmail.com> - 2016-06-10 09:40 +0200
Re: [PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge "Peter Senna Tschudin" <peter.senna@collabora.co.uk> - 2016-06-10 11:50 +0200
Re: [PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge Daniel Vetter <daniel@ffwll.ch> - 2016-06-10 16:20 +0200
[PATCH V2 5/5] dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-06-09 18:30 +0200
[PATCH V2 1/5] drm/imx-ldb: Add support to drm-bridge Peter Senna Tschudin <peter.senna@collabora.com> - 2016-06-09 18:30 +0200
Page 1 of 2 [1] 2 Next page →
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-05-30 18:50 +0200 |
| Subject | [PATCH 0/5] Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rEyy5-6IE-3@gated-at.bofh.it> |
The series adds a driver that creates a drm_bridge and a drm_connector for the
LVDS to DP++ display bridge of the GE B850v3. There are two physical bridges on
the video signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++).
However the physical bridges are automatically configured by the input video
signal, and the driver has no access to the video processing pipeline. The
driver is only needed to read EDID from the STDP2690 and to handle HPD events
from the STDP4028. The driver communicates with both bridges over i2c. The
video signal pipeline is as follows:
Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
The patches from the series:
[1/5] Change the imx-ldb driver to allow attaching a bridge and not only a LVDS
panel.
[2/5] Configure the mapping between IPUs and external displays on the dts file
of the B850v3. Needed so the GPU can drive two Full-HD monitors.
[3/5] Devicetree documentation for the GE B850v3 LVDS/DP++ Bridge
[4/5] Add the driver, make changes to MAINTAINERS, Kconfig and Makefile
[5/5] Make the changes to the B850v3 dts file to enalbe the GE B850v3
LVDS/DP++ Bridge.
Peter Senna Tschudin (5):
drm/imx-ldb: Add support to drm-bridge
dts/imx6q-b850v3: Configure IPU assignment order
Documentation/devicetree/bindings: b850v3_lvds_dp
drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge
dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge
.../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++
MAINTAINERS | 8 +
arch/arm/boot/dts/imx6q-b850v3.dts | 36 ++
drivers/gpu/drm/bridge/Kconfig | 8 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 397 +++++++++++++++++++++
drivers/gpu/drm/imx/imx-ldb.c | 75 ++--
7 files changed, 542 insertions(+), 21 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
--
2.5.5
[toc] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-05-30 18:50 +0200 |
| Subject | [PATCH 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rEyy6-6IE-27@gated-at.bofh.it> |
| In reply to | #1409281 |
This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
display bridge of the GE B850v3(imx6q-b850v3.dts). There are two physical
bridges on the video signal pipeline: a STDP4028(LVDS to DP) and a
STDP2690(DP to DP++). However the physical bridges are automatically
configured by the input video signal, and the driver has no access to
the video processing pipeline. The driver is only needed to read EDID
from the STDP2690 and to handle HPD events from the STDP4028. The driver
communicates with both bridges over i2c. The video signal pipeline is as
follows:
Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
MAINTAINERS | 8 +
drivers/gpu/drm/bridge/Kconfig | 8 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 397 +++++++++++++++++++++++++++++
4 files changed, 414 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 3273ffa..7bb5e89 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5009,6 +5009,14 @@ W: https://linuxtv.org
S: Maintained
F: drivers/media/radio/radio-gemtek*
+GENERAL ELECTRIC B850V3 LVDS/DP++ BRIDGE
+M: Martin Donnelly <martin.donnelly@ge.com>
+M: Peter Senna Tschudin <peter.senna@collabora.com>
+M: Martyn Welch <martyn.welch@collabora.co.uk>
+S: Maintained
+F: drivers/gpu/drm/bridge/ge_b850v3_dp2.c
+F: Documentation/devicetree/bindings/ge/b850v3_dp2_bridge.txt
+
GENERIC GPIO I2C DRIVER
M: Haavard Skinnemoen <hskinnemoen@gmail.com>
S: Supported
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 8f7423f..e9c32fc 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -52,4 +52,12 @@ config DRM_PARADE_PS8622
source "drivers/gpu/drm/bridge/analogix/Kconfig"
+config DRM_GE_B850V3_LVDS_DP
+ tristate "LVDS/DP bridge"
+ depends on OF
+ select DRM_KMS_HELPER
+ select DRM_PANEL
+ ---help---
+ Driver for GE B850v3 DP2 LVDS to DP+ Bridge
+
endmenu
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 96b13b3..ba2e355 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -6,3 +6,4 @@ 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_ANALOGIX_DP) += analogix/
+obj-$(CONFIG_DRM_GE_B850V3_LVDS_DP) += ge_b850v3_lvds_dp.o
diff --git a/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
new file mode 100644
index 0000000..37a4e7a
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
@@ -0,0 +1,397 @@
+/*
+ * Driver for GE B850v3 DP display bridge
+
+ * Copyright (c) 2016, Collabora Ltd.
+ * Copyright (c) 2016, General Electric Company
+
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+
+ * This program is distributed in the hope 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.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+ * This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
+ * display bridge of the GE B850v3. There are two physical bridges on the video
+ * signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++). However
+ * the physical bridges are automatically configured by the input video signal,
+ * and the driver has no access to the video processing pipeline. The driver is
+ * only needed to read EDID from the STDP2690 and to handle HPD events from the
+ * STDP4028. The driver communicates with both bridges over i2c. The video
+ * signal pipeline is as follows:
+ *
+ * Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
+
+ *
+ */
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include "drm_crtc.h"
+#include "drm_crtc_helper.h"
+#include "drm_edid.h"
+#include "drmP.h"
+
+#define EDID_EXT_BLOCK_CNT 0x7E
+
+#define STDP4028_IRQ_OUT_CONF_REG 0x02
+#define STDP4028_DPTX_IRQ_EN_REG 0x3C
+#define STDP4028_DPTX_IRQ_STS_REG 0x3D
+#define STDP4028_DPTX_STS_REG 0x3E
+
+#define STDP4028_DPTX_DP_IRQ_EN 0x1000
+
+#define STDP4028_DPTX_HOTPLUG_IRQ_EN 0x0400
+#define STDP4028_DPTX_LINK_CH_IRQ_EN 0x2000
+#define STDP4028_DPTX_IRQ_CONFIG \
+ (STDP4028_DPTX_LINK_CH_IRQ_EN | STDP4028_DPTX_HOTPLUG_IRQ_EN)
+
+#define STDP4028_DPTX_HOTPLUG_STS 0x0200
+#define STDP4028_DPTX_LINK_STS 0x1000
+#define STDP4028_CON_STATE_CONNECTED \
+ (STDP4028_DPTX_HOTPLUG_STS | STDP4028_DPTX_LINK_STS)
+
+#define STDP4028_DPTX_HOTPLUG_CH_STS 0x0400
+#define STDP4028_DPTX_LINK_CH_STS 0x2000
+#define STDP4028_DPTX_IRQ_CLEAR \
+ (STDP4028_DPTX_LINK_CH_STS | STDP4028_DPTX_HOTPLUG_CH_STS)
+
+struct ge_b850v3_lvds_dp {
+ struct drm_connector connector;
+ struct drm_bridge bridge;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c;
+ struct i2c_client *edid_i2c;
+ struct edid *edid;
+};
+
+static inline struct ge_b850v3_lvds_dp *
+ bridge_to_ge_b850v3_lvds_dp(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct ge_b850v3_lvds_dp, bridge);
+}
+
+static inline struct ge_b850v3_lvds_dp *
+ connector_to_ge_b850v3_lvds_dp(struct drm_connector *connector)
+{
+ return container_of(connector, struct ge_b850v3_lvds_dp, connector);
+}
+
+static void ge_b850v3_lvds_dp_pre_enable(struct drm_bridge *bridge)
+{
+}
+
+static void ge_b850v3_lvds_dp_enable(struct drm_bridge *bridge)
+{
+}
+
+static void ge_b850v3_lvds_dp_disable(struct drm_bridge *bridge)
+{
+}
+
+static void ge_b850v3_lvds_dp_post_disable(struct drm_bridge *bridge)
+{
+}
+
+u8 *stdp2690_get_edid(struct i2c_client *client)
+{
+ struct i2c_adapter *adapter = client->adapter;
+ unsigned char start = 0x00;
+ unsigned int total_size;
+ u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = &start,
+ }, {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = EDID_LENGTH,
+ .buf = block,
+ }
+ };
+
+ if (!block)
+ return NULL;
+
+ if (i2c_transfer(adapter, msgs, 2) != 2) {
+ DRM_ERROR("Unable to read EDID.\n");
+ goto err;
+ }
+
+ if (!drm_edid_block_valid(block, 0, false, NULL)) {
+ DRM_ERROR("Invalid EDID block\n");
+ goto err;
+ }
+
+ total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
+ if (total_size > EDID_LENGTH) {
+ kfree(block);
+ block = kmalloc(total_size, GFP_KERNEL);
+ if (!block)
+ return NULL;
+
+ /* Yes, read the entire buffer, and do not skip the first
+ * EDID_LENGTH bytes.
+ */
+ start = 0x00;
+ msgs[1].len = total_size;
+ msgs[1].buf = block;
+
+ if (i2c_transfer(adapter, msgs, 2) != 2) {
+ DRM_ERROR("Unable to read EDID extension blocks.\n");
+ goto err;
+ }
+ }
+
+ return block;
+
+err:
+ kfree(block);
+ return NULL;
+}
+
+static int ge_b850v3_lvds_dp_get_modes(struct drm_connector *connector)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge;
+ struct i2c_client *client;
+ u8 *block;
+ int num_modes;
+
+ ptn_bridge = connector_to_ge_b850v3_lvds_dp(connector);
+ client = ptn_bridge->edid_i2c;
+
+ block = stdp2690_get_edid(client);
+ ptn_bridge->edid = (struct edid *) block;
+
+ drm_mode_connector_update_edid_property(connector, ptn_bridge->edid);
+ num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
+
+ return num_modes;
+}
+
+static struct drm_encoder
+*ge_b850v3_lvds_dp_best_encoder(struct drm_connector *connector)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ connector_to_ge_b850v3_lvds_dp(connector);
+
+ return ptn_bridge->bridge.encoder;
+}
+
+static const struct
+drm_connector_helper_funcs ge_b850v3_lvds_dp_connector_helper_funcs = {
+ .get_modes = ge_b850v3_lvds_dp_get_modes,
+ .best_encoder = ge_b850v3_lvds_dp_best_encoder,
+};
+
+static enum drm_connector_status ge_b850v3_lvds_dp_detect(
+ struct drm_connector *connector, bool force)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ connector_to_ge_b850v3_lvds_dp(connector);
+ struct i2c_client *ge_b850v3_lvds_dp_i2c =
+ ptn_bridge->ge_b850v3_lvds_dp_i2c;
+ s32 link_state;
+
+ link_state = i2c_smbus_read_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_STS_REG);
+
+ if (link_state == STDP4028_CON_STATE_CONNECTED)
+ return connector_status_connected;
+
+ if (link_state == 0)
+ return connector_status_disconnected;
+
+ return connector_status_unknown;
+}
+
+static void ge_b850v3_lvds_dp_connector_force(struct drm_connector *connector)
+{
+}
+
+static void ge_b850v3_lvds_dp_connector_destroy(struct drm_connector *connector)
+{
+ drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs ge_b850v3_lvds_dp_connector_funcs = {
+ .dpms = drm_helper_connector_dpms,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .detect = ge_b850v3_lvds_dp_detect,
+ .destroy = ge_b850v3_lvds_dp_connector_destroy,
+ .force = ge_b850v3_lvds_dp_connector_force,
+};
+
+static irqreturn_t ge_b850v3_lvds_dp_irq_handler(int irq, void *dev_id)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge = dev_id;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c
+ = ptn_bridge->ge_b850v3_lvds_dp_i2c;
+
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
+
+ if (ptn_bridge->connector.dev)
+ drm_kms_helper_hotplug_event(ptn_bridge->connector.dev);
+
+ return IRQ_HANDLED;
+
+}
+
+static int ge_b850v3_lvds_dp_attach(struct drm_bridge *bridge)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge
+ = bridge_to_ge_b850v3_lvds_dp(bridge);
+ struct drm_connector *connector = &ptn_bridge->connector;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c
+ = ptn_bridge->ge_b850v3_lvds_dp_i2c;
+ int ret;
+
+ if (!bridge->encoder) {
+ DRM_ERROR("Parent encoder object not found");
+ return -ENODEV;
+ }
+
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+ drm_connector_helper_add(connector,
+ &ge_b850v3_lvds_dp_connector_helper_funcs);
+
+ ret = drm_connector_init(bridge->dev, connector,
+ &ge_b850v3_lvds_dp_connector_funcs,
+ DRM_MODE_CONNECTOR_DisplayPort);
+ if (ret) {
+ DRM_ERROR("Failed to initialize connector with drm\n");
+ return ret;
+ }
+
+ ret = drm_mode_connector_attach_encoder(connector, bridge->encoder);
+ if (ret)
+ return ret;
+
+ drm_bridge_enable(bridge);
+ if (ge_b850v3_lvds_dp_i2c->irq) {
+ drm_helper_hpd_irq_event(connector->dev);
+
+ ret = devm_request_threaded_irq(&ge_b850v3_lvds_dp_i2c->dev,
+ ge_b850v3_lvds_dp_i2c->irq, NULL,
+ ge_b850v3_lvds_dp_irq_handler,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "ge_b850v3_lvds_dp", ptn_bridge);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct drm_bridge_funcs ge_b850v3_lvds_dp_funcs = {
+ .pre_enable = ge_b850v3_lvds_dp_pre_enable,
+ .enable = ge_b850v3_lvds_dp_enable,
+ .disable = ge_b850v3_lvds_dp_disable,
+ .post_disable = ge_b850v3_lvds_dp_post_disable,
+ .attach = ge_b850v3_lvds_dp_attach,
+};
+
+static int ge_b850v3_lvds_dp_probe(struct i2c_client *ge_b850v3_lvds_dp_i2c,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &ge_b850v3_lvds_dp_i2c->dev;
+ struct ge_b850v3_lvds_dp *ptn_bridge;
+
+ int ret;
+ u32 edid_i2c_reg;
+
+ ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
+ if (!ptn_bridge)
+ return -ENOMEM;
+
+ ptn_bridge->ge_b850v3_lvds_dp_i2c = ge_b850v3_lvds_dp_i2c;
+ ptn_bridge->bridge.driver_private = ptn_bridge;
+ i2c_set_clientdata(ge_b850v3_lvds_dp_i2c, ptn_bridge);
+
+ ret = of_property_read_u32(dev->of_node, "edid-reg", &edid_i2c_reg);
+ if (ret) {
+ dev_err(dev, "edid-reg not specified, aborting...\n");
+ return -ENODEV;
+ }
+
+ ptn_bridge->edid_i2c = devm_kzalloc(dev,
+ sizeof(struct i2c_client), GFP_KERNEL);
+
+ if (!ptn_bridge->edid_i2c)
+ return -ENOMEM;
+
+ memcpy(ptn_bridge->edid_i2c, ge_b850v3_lvds_dp_i2c,
+ sizeof(struct i2c_client));
+
+ ptn_bridge->edid_i2c->addr = (unsigned short) edid_i2c_reg;
+
+
+ /* Configures the bridge to re-enable interrupts after each ack */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_IRQ_OUT_CONF_REG, STDP4028_DPTX_DP_IRQ_EN);
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_EN_REG, STDP4028_DPTX_IRQ_CONFIG);
+
+ /* Clear pending interrupts since power up. */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
+
+ ptn_bridge->bridge.funcs = &ge_b850v3_lvds_dp_funcs;
+ ptn_bridge->bridge.of_node = dev->of_node;
+ ret = drm_bridge_add(&ptn_bridge->bridge);
+ if (ret) {
+ DRM_ERROR("Failed to add bridge\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ge_b850v3_lvds_dp_remove(struct i2c_client *ge_b850v3_lvds_dp_i2c)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ i2c_get_clientdata(ge_b850v3_lvds_dp_i2c);
+
+ drm_bridge_remove(&ptn_bridge->bridge);
+
+ return 0;
+}
+
+static const struct i2c_device_id ge_b850v3_lvds_dp_i2c_table[] = {
+ {"b850v3_lvds_dp", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, ge_b850v3_lvds_dp_i2c_table);
+
+static const struct of_device_id ge_b850v3_lvds_dp_match[] = {
+ { .compatible = "ge,b850v3_lvds_dp" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ge_b850v3_lvds_dp_match);
+
+static struct i2c_driver ge_b850v3_lvds_dp_driver = {
+ .id_table = ge_b850v3_lvds_dp_i2c_table,
+ .probe = ge_b850v3_lvds_dp_probe,
+ .remove = ge_b850v3_lvds_dp_remove,
+ .driver = {
+ .name = "ge,b850v3_lvds_dp",
+ .of_match_table = ge_b850v3_lvds_dp_match,
+ },
+};
+module_i2c_driver(ge_b850v3_lvds_dp_driver);
+
+MODULE_AUTHOR("Peter Senna Tschudin <peter.senna@collabora.com>");
+MODULE_AUTHOR("Martyn Welch <martyn.welch@collabora.co.uk>");
+MODULE_DESCRIPTION("GE LVDS to DP++ bridge)");
+MODULE_LICENSE("GPL v2");
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Enric Balletbo Serra <eballetbo@gmail.com> |
|---|---|
| Date | 2016-05-31 09:50 +0200 |
| Subject | Re: [PATCH 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rEMB4-89y-15@gated-at.bofh.it> |
| In reply to | #1409285 |
Hi Peter,
Just some comments that I received when I submitted my bridge patches
and that I think can help you ...
2016-05-30 18:39 GMT+02:00 Peter Senna Tschudin <peter.senna@collabora.com>:
> This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
> display bridge of the GE B850v3(imx6q-b850v3.dts). There are two physical
> bridges on the video signal pipeline: a STDP4028(LVDS to DP) and a
> STDP2690(DP to DP++). However the physical bridges are automatically
> configured by the input video signal, and the driver has no access to
> the video processing pipeline. The driver is only needed to read EDID
> from the STDP2690 and to handle HPD events from the STDP4028. The driver
> communicates with both bridges over i2c. The video signal pipeline is as
> follows:
>
> Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> ---
> MAINTAINERS | 8 +
> drivers/gpu/drm/bridge/Kconfig | 8 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 397 +++++++++++++++++++++++++++++
> 4 files changed, 414 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 3273ffa..7bb5e89 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5009,6 +5009,14 @@ W: https://linuxtv.org
> S: Maintained
> F: drivers/media/radio/radio-gemtek*
>
> +GENERAL ELECTRIC B850V3 LVDS/DP++ BRIDGE
> +M: Martin Donnelly <martin.donnelly@ge.com>
> +M: Peter Senna Tschudin <peter.senna@collabora.com>
> +M: Martyn Welch <martyn.welch@collabora.co.uk>
> +S: Maintained
> +F: drivers/gpu/drm/bridge/ge_b850v3_dp2.c
> +F: Documentation/devicetree/bindings/ge/b850v3_dp2_bridge.txt
> +
> GENERIC GPIO I2C DRIVER
> M: Haavard Skinnemoen <hskinnemoen@gmail.com>
> S: Supported
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 8f7423f..e9c32fc 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -52,4 +52,12 @@ config DRM_PARADE_PS8622
>
> source "drivers/gpu/drm/bridge/analogix/Kconfig"
>
> +config DRM_GE_B850V3_LVDS_DP
> + tristate "LVDS/DP bridge"
> + depends on OF
> + select DRM_KMS_HELPER
> + select DRM_PANEL
> + ---help---
> + Driver for GE B850v3 DP2 LVDS to DP+ Bridge
> +
I think the maintainer prefers the entries ordered alphabetically (by
vendor, then name), so your config should go before "config
DRM_NXP_PTN3460"
> endmenu
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 96b13b3..ba2e355 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -6,3 +6,4 @@ 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_ANALOGIX_DP) += analogix/
> +obj-$(CONFIG_DRM_GE_B850V3_LVDS_DP) += ge_b850v3_lvds_dp.o
Same here, this needs to be sorted by vendor, then name as well. Also
I think is preferred use hyphens like the others drivers.
Hmm, I see now that CONFIG_DRM_ANALOGIX_DP is not sorted, but I guess
the preferred is be sorted.
> diff --git a/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> new file mode 100644
> index 0000000..37a4e7a
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> @@ -0,0 +1,397 @@
> +/*
> + * Driver for GE B850v3 DP display bridge
> +
> + * Copyright (c) 2016, Collabora Ltd.
> + * Copyright (c) 2016, General Electric Company
> +
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> +
> + * This program is distributed in the hope 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.
> +
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> + * This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
> + * display bridge of the GE B850v3. There are two physical bridges on the video
> + * signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++). However
> + * the physical bridges are automatically configured by the input video signal,
> + * and the driver has no access to the video processing pipeline. The driver is
> + * only needed to read EDID from the STDP2690 and to handle HPD events from the
> + * STDP4028. The driver communicates with both bridges over i2c. The video
> + * signal pipeline is as follows:
> + *
> + * Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
> +
> + *
> + */
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include "drm_crtc.h"
> +#include "drm_crtc_helper.h"
> +#include "drm_edid.h"
> +#include "drmP.h"
> +
> +#define EDID_EXT_BLOCK_CNT 0x7E
> +
> +#define STDP4028_IRQ_OUT_CONF_REG 0x02
> +#define STDP4028_DPTX_IRQ_EN_REG 0x3C
> +#define STDP4028_DPTX_IRQ_STS_REG 0x3D
> +#define STDP4028_DPTX_STS_REG 0x3E
> +
> +#define STDP4028_DPTX_DP_IRQ_EN 0x1000
> +
> +#define STDP4028_DPTX_HOTPLUG_IRQ_EN 0x0400
> +#define STDP4028_DPTX_LINK_CH_IRQ_EN 0x2000
> +#define STDP4028_DPTX_IRQ_CONFIG \
> + (STDP4028_DPTX_LINK_CH_IRQ_EN | STDP4028_DPTX_HOTPLUG_IRQ_EN)
> +
> +#define STDP4028_DPTX_HOTPLUG_STS 0x0200
> +#define STDP4028_DPTX_LINK_STS 0x1000
> +#define STDP4028_CON_STATE_CONNECTED \
> + (STDP4028_DPTX_HOTPLUG_STS | STDP4028_DPTX_LINK_STS)
> +
> +#define STDP4028_DPTX_HOTPLUG_CH_STS 0x0400
> +#define STDP4028_DPTX_LINK_CH_STS 0x2000
> +#define STDP4028_DPTX_IRQ_CLEAR \
> + (STDP4028_DPTX_LINK_CH_STS | STDP4028_DPTX_HOTPLUG_CH_STS)
> +
> +struct ge_b850v3_lvds_dp {
> + struct drm_connector connector;
> + struct drm_bridge bridge;
> + struct i2c_client *ge_b850v3_lvds_dp_i2c;
> + struct i2c_client *edid_i2c;
> + struct edid *edid;
> +};
> +
> +static inline struct ge_b850v3_lvds_dp *
> + bridge_to_ge_b850v3_lvds_dp(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct ge_b850v3_lvds_dp, bridge);
> +}
> +
> +static inline struct ge_b850v3_lvds_dp *
> + connector_to_ge_b850v3_lvds_dp(struct drm_connector *connector)
> +{
> + return container_of(connector, struct ge_b850v3_lvds_dp, connector);
> +}
> +
> +static void ge_b850v3_lvds_dp_pre_enable(struct drm_bridge *bridge)
> +{
> +}
> +
You don't need to keep empty callbacks for the (pre/post)
enable/disable drm_bridge ops anymore so you can remove this function
and the below.
> +static void ge_b850v3_lvds_dp_enable(struct drm_bridge *bridge)
> +{
> +}
> +
> +static void ge_b850v3_lvds_dp_disable(struct drm_bridge *bridge)
> +{
> +}
> +
> +static void ge_b850v3_lvds_dp_post_disable(struct drm_bridge *bridge)
> +{
> +}
> +
> +u8 *stdp2690_get_edid(struct i2c_client *client)
> +{
> + struct i2c_adapter *adapter = client->adapter;
> + unsigned char start = 0x00;
> + unsigned int total_size;
> + u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> +
> + struct i2c_msg msgs[] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 1,
> + .buf = &start,
> + }, {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .len = EDID_LENGTH,
> + .buf = block,
> + }
> + };
> +
> + if (!block)
> + return NULL;
> +
> + if (i2c_transfer(adapter, msgs, 2) != 2) {
> + DRM_ERROR("Unable to read EDID.\n");
> + goto err;
> + }
> +
> + if (!drm_edid_block_valid(block, 0, false, NULL)) {
> + DRM_ERROR("Invalid EDID block\n");
> + goto err;
> + }
> +
> + total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
> + if (total_size > EDID_LENGTH) {
> + kfree(block);
> + block = kmalloc(total_size, GFP_KERNEL);
> + if (!block)
> + return NULL;
> +
> + /* Yes, read the entire buffer, and do not skip the first
> + * EDID_LENGTH bytes.
> + */
> + start = 0x00;
> + msgs[1].len = total_size;
> + msgs[1].buf = block;
> +
> + if (i2c_transfer(adapter, msgs, 2) != 2) {
> + DRM_ERROR("Unable to read EDID extension blocks.\n");
> + goto err;
> + }
> + }
> +
> + return block;
> +
> +err:
> + kfree(block);
> + return NULL;
> +}
> +
> +static int ge_b850v3_lvds_dp_get_modes(struct drm_connector *connector)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge;
> + struct i2c_client *client;
> + u8 *block;
> + int num_modes;
> +
> + ptn_bridge = connector_to_ge_b850v3_lvds_dp(connector);
Move this assignation to ptn_bridge declaration
> + client = ptn_bridge->edid_i2c;
> +
> + block = stdp2690_get_edid(client);
I think you can remove the client variable and pass
ptn_bridge->edid_i2c directly to the stdp2690_get_edid function.
Note that get_modes is called several times and can be called from
userspace, you are allocating memory on every call to get_modes that I
think is not released or I'm missing something?
> + ptn_bridge->edid = (struct edid *) block;
> +
What if block is NULL here ?
> + drm_mode_connector_update_edid_property(connector, ptn_bridge->edid);
> + num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
> +
> + return num_modes;
nit: return drm_add_edid_modes(connector, ptn_bridge->edid); ? And
remove num_modes ?
> +}
> +
> +static struct drm_encoder
> +*ge_b850v3_lvds_dp_best_encoder(struct drm_connector *connector)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge =
> + connector_to_ge_b850v3_lvds_dp(connector);
> +
> + return ptn_bridge->bridge.encoder;
> +}
> +
> +static const struct
> +drm_connector_helper_funcs ge_b850v3_lvds_dp_connector_helper_funcs = {
> + .get_modes = ge_b850v3_lvds_dp_get_modes,
> + .best_encoder = ge_b850v3_lvds_dp_best_encoder,
> +};
> +
> +static enum drm_connector_status ge_b850v3_lvds_dp_detect(
> + struct drm_connector *connector, bool force)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge =
> + connector_to_ge_b850v3_lvds_dp(connector);
> + struct i2c_client *ge_b850v3_lvds_dp_i2c =
> + ptn_bridge->ge_b850v3_lvds_dp_i2c;
> + s32 link_state;
> +
> + link_state = i2c_smbus_read_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_STS_REG);
> +
> + if (link_state == STDP4028_CON_STATE_CONNECTED)
> + return connector_status_connected;
> +
> + if (link_state == 0)
> + return connector_status_disconnected;
> +
> + return connector_status_unknown;
> +}
> +
> +static void ge_b850v3_lvds_dp_connector_force(struct drm_connector *connector)
> +{
> +}
> +
Guess you can remove this empty callback.
> +static void ge_b850v3_lvds_dp_connector_destroy(struct drm_connector *connector)
> +{
> + drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs ge_b850v3_lvds_dp_connector_funcs = {
> + .dpms = drm_helper_connector_dpms,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .detect = ge_b850v3_lvds_dp_detect,
> + .destroy = ge_b850v3_lvds_dp_connector_destroy,
> + .force = ge_b850v3_lvds_dp_connector_force,
Remove .force
> +};
> +
> +static irqreturn_t ge_b850v3_lvds_dp_irq_handler(int irq, void *dev_id)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge = dev_id;
> + struct i2c_client *ge_b850v3_lvds_dp_i2c
> + = ptn_bridge->ge_b850v3_lvds_dp_i2c;
> +
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
> +
> + if (ptn_bridge->connector.dev)
> + drm_kms_helper_hotplug_event(ptn_bridge->connector.dev);
> +
> + return IRQ_HANDLED;
> +
Remove the empty line.
> +}
> +
> +static int ge_b850v3_lvds_dp_attach(struct drm_bridge *bridge)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge
> + = bridge_to_ge_b850v3_lvds_dp(bridge);
> + struct drm_connector *connector = &ptn_bridge->connector;
> + struct i2c_client *ge_b850v3_lvds_dp_i2c
> + = ptn_bridge->ge_b850v3_lvds_dp_i2c;
> + int ret;
> +
> + if (!bridge->encoder) {
> + DRM_ERROR("Parent encoder object not found");
> + return -ENODEV;
> + }
> +
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> +
> + drm_connector_helper_add(connector,
> + &ge_b850v3_lvds_dp_connector_helper_funcs);
> +
> + ret = drm_connector_init(bridge->dev, connector,
> + &ge_b850v3_lvds_dp_connector_funcs,
> + DRM_MODE_CONNECTOR_DisplayPort);
> + if (ret) {
> + DRM_ERROR("Failed to initialize connector with drm\n");
> + return ret;
> + }
> +
> + ret = drm_mode_connector_attach_encoder(connector, bridge->encoder);
> + if (ret)
> + return ret;
> +
> + drm_bridge_enable(bridge);
> + if (ge_b850v3_lvds_dp_i2c->irq) {
> + drm_helper_hpd_irq_event(connector->dev);
> +
> + ret = devm_request_threaded_irq(&ge_b850v3_lvds_dp_i2c->dev,
> + ge_b850v3_lvds_dp_i2c->irq, NULL,
> + ge_b850v3_lvds_dp_irq_handler,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "ge_b850v3_lvds_dp", ptn_bridge);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct drm_bridge_funcs ge_b850v3_lvds_dp_funcs = {
> + .pre_enable = ge_b850v3_lvds_dp_pre_enable,
> + .enable = ge_b850v3_lvds_dp_enable,
> + .disable = ge_b850v3_lvds_dp_disable,
> + .post_disable = ge_b850v3_lvds_dp_post_disable,
Remove the above empty callbacks.
> + .attach = ge_b850v3_lvds_dp_attach,
> +};
> +
> +static int ge_b850v3_lvds_dp_probe(struct i2c_client *ge_b850v3_lvds_dp_i2c,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &ge_b850v3_lvds_dp_i2c->dev;
> + struct ge_b850v3_lvds_dp *ptn_bridge;
> +
Remove this empty line between these two blocks.
> + int ret;
> + u32 edid_i2c_reg;
> +
> + ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
> + if (!ptn_bridge)
> + return -ENOMEM;
> +
> + ptn_bridge->ge_b850v3_lvds_dp_i2c = ge_b850v3_lvds_dp_i2c;
> + ptn_bridge->bridge.driver_private = ptn_bridge;
> + i2c_set_clientdata(ge_b850v3_lvds_dp_i2c, ptn_bridge);
> +
> + ret = of_property_read_u32(dev->of_node, "edid-reg", &edid_i2c_reg);
> + if (ret) {
> + dev_err(dev, "edid-reg not specified, aborting...\n");
Use DRM_ERROR?
> + return -ENODEV;
> + }
> +
> + ptn_bridge->edid_i2c = devm_kzalloc(dev,
> + sizeof(struct i2c_client), GFP_KERNEL);
> +
> + if (!ptn_bridge->edid_i2c)
> + return -ENOMEM;
> +
> + memcpy(ptn_bridge->edid_i2c, ge_b850v3_lvds_dp_i2c,
> + sizeof(struct i2c_client));
> +
> + ptn_bridge->edid_i2c->addr = (unsigned short) edid_i2c_reg;
> +
Remove empty line.
> +
> + /* Configures the bridge to re-enable interrupts after each ack */
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_IRQ_OUT_CONF_REG, STDP4028_DPTX_DP_IRQ_EN);
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_IRQ_EN_REG, STDP4028_DPTX_IRQ_CONFIG);
> +
> + /* Clear pending interrupts since power up. */
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
> +
> + ptn_bridge->bridge.funcs = &ge_b850v3_lvds_dp_funcs;
> + ptn_bridge->bridge.of_node = dev->of_node;
> + ret = drm_bridge_add(&ptn_bridge->bridge);
> + if (ret) {
> + DRM_ERROR("Failed to add bridge\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ge_b850v3_lvds_dp_remove(struct i2c_client *ge_b850v3_lvds_dp_i2c)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge =
> + i2c_get_clientdata(ge_b850v3_lvds_dp_i2c);
> +
> + drm_bridge_remove(&ptn_bridge->bridge);
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ge_b850v3_lvds_dp_i2c_table[] = {
> + {"b850v3_lvds_dp", 0},
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, ge_b850v3_lvds_dp_i2c_table);
> +
> +static const struct of_device_id ge_b850v3_lvds_dp_match[] = {
> + { .compatible = "ge,b850v3_lvds_dp" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ge_b850v3_lvds_dp_match);
> +
> +static struct i2c_driver ge_b850v3_lvds_dp_driver = {
> + .id_table = ge_b850v3_lvds_dp_i2c_table,
> + .probe = ge_b850v3_lvds_dp_probe,
> + .remove = ge_b850v3_lvds_dp_remove,
> + .driver = {
> + .name = "ge,b850v3_lvds_dp",
> + .of_match_table = ge_b850v3_lvds_dp_match,
> + },
> +};
> +module_i2c_driver(ge_b850v3_lvds_dp_driver);
> +
> +MODULE_AUTHOR("Peter Senna Tschudin <peter.senna@collabora.com>");
> +MODULE_AUTHOR("Martyn Welch <martyn.welch@collabora.co.uk>");
> +MODULE_DESCRIPTION("GE LVDS to DP++ bridge)");
> +MODULE_LICENSE("GPL v2");
> --
> 2.5.5
>
Best regards,
Enric
[toc] | [prev] | [next] | [standalone]
| From | Fabio Estevam <festevam@gmail.com> |
|---|---|
| Date | 2016-05-30 18:50 +0200 |
| Subject | Re: [PATCH 2/5] arm/dts/imx6q-b850v3: Configure IPU assignment order |
| Message-ID | <rEyy6-6IE-31@gated-at.bofh.it> |
| In reply to | #1409281 |
On Mon, May 30, 2016 at 1:39 PM, Peter Senna Tschudin <peter.senna@collabora.com> wrote: > Configure the IPU assignment order to assign one IPU per external > display. A single IPU can drive multiple external displays but there are > resolution restrictions. After this patch the GPU is capalbe of driving two I think you meant "the IPU is capable"
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-05-30 18:50 +0200 |
| Subject | [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp |
| Message-ID | <rEyy6-6IE-45@gated-at.bofh.it> |
| In reply to | #1409281 |
Devicetree bindings documentation for the GE B850v3 LVDS/DP++
display bridge.
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
.../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
diff --git a/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
new file mode 100644
index 0000000..32e123a
--- /dev/null
+++ b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
@@ -0,0 +1,38 @@
+Driver for GE B850v3 LVDS/DP++ display bridge
+
+Required properties:
+ - compatible : should be "ge,b850v3_lvds_dp".
+ - reg : should contain the address used to ack the interrupts.
+ - interrupt-parent : should link to the gpio used as interrupt
+ source on the host.
+ - interrupts : one interrupt should be described here, as in
+ <0 IRQ_TYPE_LEVEL_HIGH>.
+ - edid-reg : should contain the address used to read edid information
+ - port : should describe the vide signal connection between the host
+ and the bridge.
+
+Example:
+
+&mux2_i2c2 {
+ status = "okay";
+ clock-frequency = <100000>;
+
+ b850v3_dp_bridge {
+ compatible = "ge,b850v3_lvds_dp";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0x73>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+ edid-reg = <0x72>;
+
+ port@0 {
+ reg = <0>;
+ b850v3_dp_bridge_in: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+ };
+};
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Philipp Zabel <p.zabel@pengutronix.de> |
|---|---|
| Date | 2016-06-02 14:50 +0200 |
| Subject | Re: [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp |
| Message-ID | <rFAet-5Zh-11@gated-at.bofh.it> |
| In reply to | #1409289 |
Hi Peter, Am Montag, den 30.05.2016, 18:39 +0200 schrieb Peter Senna Tschudin: > Devicetree bindings documentation for the GE B850v3 LVDS/DP++ > display bridge. > > Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com> > --- > .../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt > > diff --git a/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt > new file mode 100644 > index 0000000..32e123a > --- /dev/null > +++ b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt > @@ -0,0 +1,38 @@ > +Driver for GE B850v3 LVDS/DP++ display bridge > + > +Required properties: > + - compatible : should be "ge,b850v3_lvds_dp". In the cover mail you write that this is a combination of the STDP4028 DP transmitter and STDP2690 DP/DP++ converter. So shouldn't this binding comprise two device tree nodes, one with compatible "st,stdp4028", one with "st,stdp2690", then? Is the stdp2690 connected to the stdp4028's i2c master? regards Philipp
[toc] | [prev] | [next] | [standalone]
| From | "Peter Senna Tschudin" <peter.senna@collabora.co.uk> |
|---|---|
| Date | 2016-06-03 01:20 +0200 |
| Subject | Re: [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp |
| Message-ID | <rFK49-3Uu-11@gated-at.bofh.it> |
| In reply to | #1412200 |
Hi Philipp, Thank you very much for the review! I'll send V2 soon, doing my best to avoid collisions with other patches that are under review. On Thursday, June 2, 2016 14:49 CEST, Philipp Zabel <p.zabel@pengutronix.de> wrote: > Hi Peter, > > Am Montag, den 30.05.2016, 18:39 +0200 schrieb Peter Senna Tschudin: > > Devicetree bindings documentation for the GE B850v3 LVDS/DP++ > > display bridge. > > > > Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com> > > --- > > .../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++ > > 1 file changed, 38 insertions(+) > > create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt > > > > diff --git a/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt > > new file mode 100644 > > index 0000000..32e123a > > --- /dev/null > > +++ b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt > > @@ -0,0 +1,38 @@ > > +Driver for GE B850v3 LVDS/DP++ display bridge > > + > > +Required properties: > > + - compatible : should be "ge,b850v3_lvds_dp". > > In the cover mail you write that this is a combination of the STDP4028 > DP transmitter and STDP2690 DP/DP++ converter. So shouldn't this binding > comprise two device tree nodes, one with compatible "st,stdp4028", one > with "st,stdp2690", then? > Is the stdp2690 connected to the stdp4028's i2c master? The hardware and firmware made it complicated for this binding to comprise two device tree nodes. The hardware and firmware are designed to configure both bridges based on the LVDS signal. This part works very well, but it leave the driver powerless to control the video processing pipeline. The bridges sort of behave as a single bridge, the sort of part is the need of interacting with both bridges: EDID from the STDP2690 and HPD events from the STDP4028. So the driver is only needed for telling the host about EDID / HPD, and for giving the host powers to ack interrupts. I would find it nicer to be author of two drivers instead of one, but as the behavior handled by the driver is specific of the B850v3, and do not reflect the standard behavior of each bridge, I find it unlikely that the driver could be useful to handle STDP4028 and STPD2690 with stock firmware, or in other designs. On the other hand, I made sure to use the chip name on the defines to maximize chances of reuse. Peter
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-06-03 01:00 +0200 |
| Subject | Re: [PATCH 3/5] Documentation/devicetree/bindings: Add b850v3_lvds_dp |
| Message-ID | <rFJKN-3z9-7@gated-at.bofh.it> |
| In reply to | #1409289 |
On Mon, May 30, 2016 at 06:39:43PM +0200, Peter Senna Tschudin wrote:
> Devicetree bindings documentation for the GE B850v3 LVDS/DP++
> display bridge.
>
> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> ---
> .../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
>
> diff --git a/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
> new file mode 100644
> index 0000000..32e123a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
> @@ -0,0 +1,38 @@
> +Driver for GE B850v3 LVDS/DP++ display bridge
> +
> +Required properties:
> + - compatible : should be "ge,b850v3_lvds_dp".
> + - reg : should contain the address used to ack the interrupts.
> + - interrupt-parent : should link to the gpio used as interrupt
> + source on the host.
> + - interrupts : one interrupt should be described here, as in
> + <0 IRQ_TYPE_LEVEL_HIGH>.
> + - edid-reg : should contain the address used to read edid information
This should be known based on the bridge chip you are using.
> + - port : should describe the vide signal connection between the host
> + and the bridge.
> +
> +Example:
> +
> +&mux2_i2c2 {
> + status = "okay";
> + clock-frequency = <100000>;
> +
> + b850v3_dp_bridge {
Don't use '_' in node or property names or compatible strings.
> + compatible = "ge,b850v3_lvds_dp";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0x73>;
> + interrupt-parent = <&gpio2>;
> + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> + edid-reg = <0x72>;
> +
> + port@0 {
> + reg = <0>;
> + b850v3_dp_bridge_in: endpoint {
> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> + };
> +};
> --
> 2.5.5
>
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-05-30 18:50 +0200 |
| Subject | [PATCH 2/5] arm/dts/imx6q-b850v3: Configure IPU assignment order |
| Message-ID | <rEyy6-6IE-33@gated-at.bofh.it> |
| In reply to | #1409281 |
Configure the IPU assignment order to assign one IPU per external
display. A single IPU can drive multiple external displays but there are
resolution restrictions. After this patch the GPU is capalbe of driving two
Full-HD monitors.
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
arch/arm/boot/dts/imx6q-b850v3.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b850v3.dts b/arch/arm/boot/dts/imx6q-b850v3.dts
index 167f744..88a70de 100644
--- a/arch/arm/boot/dts/imx6q-b850v3.dts
+++ b/arch/arm/boot/dts/imx6q-b850v3.dts
@@ -51,6 +51,11 @@
chosen {
stdout-path = &uart3;
};
+
+ display-subsystem {
+ compatible = "fsl,imx-display-subsystem";
+ ports = <&ipu1_di0>, <&ipu2_di0>, <&ipu1_di1>, <&ipu2_di1>;
+ };
};
&clks {
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Philipp Zabel <p.zabel@pengutronix.de> |
|---|---|
| Date | 2016-06-02 15:00 +0200 |
| Subject | Re: [PATCH 2/5] arm/dts/imx6q-b850v3: Configure IPU assignment order |
| Message-ID | <rFAoa-62y-9@gated-at.bofh.it> |
| In reply to | #1409292 |
Am Montag, den 30.05.2016, 18:39 +0200 schrieb Peter Senna Tschudin: > Configure the IPU assignment order to assign one IPU per external > display. A single IPU can drive multiple external displays but there are > resolution restrictions. After this patch the GPU is capalbe of driving two > Full-HD monitors. It's also capable to do it before this patch, if you use the first and third crtc. You are just reordering the crtcs. Unfortunately the IPU has combined limitations across multiple crtcs in one IPU, which currently can't be communicated to userspace. regards Philipp
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-05-30 18:50 +0200 |
| Subject | [PATCH 5/5] arm/dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rEyy7-6IE-51@gated-at.bofh.it> |
| In reply to | #1409281 |
Configure the GE B850v3 to use the LVDS/DP++ bridge.
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
arch/arm/boot/dts/imx6q-b850v3.dts | 31 +++++++++++++++++++++++++++++++
1 file changed, 31 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b850v3.dts b/arch/arm/boot/dts/imx6q-b850v3.dts
index 88a70de..d366384 100644
--- a/arch/arm/boot/dts/imx6q-b850v3.dts
+++ b/arch/arm/boot/dts/imx6q-b850v3.dts
@@ -77,6 +77,13 @@
fsl,data-mapping = "spwg";
fsl,data-width = <24>;
status = "okay";
+
+ port@4 {
+ reg = <4>;
+ lvds0_out: endpoint {
+ remote-endpoint = <&b850v3_lvds_dp_bridge_in>;
+ };
+ };
};
};
@@ -147,3 +154,27 @@
reg = <0x4a>;
};
};
+
+&mux2_i2c2 {
+ status = "okay";
+ clock-frequency = <100000>;
+
+ b850v3_lvds_dp_bridge {
+ compatible = "ge,b850v3_lvds_dp";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0x73>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+ edid-reg = <0x72>;
+
+ port@0 {
+ reg = <0>;
+ b850v3_lvds_dp_bridge_in: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+ };
+};
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Fabio Estevam <festevam@gmail.com> |
|---|---|
| Date | 2016-05-30 19:00 +0200 |
| Subject | Re: [PATCH 5/5] arm/dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rEyHL-6Mn-11@gated-at.bofh.it> |
| In reply to | #1409293 |
On Mon, May 30, 2016 at 1:39 PM, Peter Senna Tschudin
<peter.senna@collabora.com> wrote:
> +&mux2_i2c2 {
> + status = "okay";
> + clock-frequency = <100000>;
> +
> + b850v3_lvds_dp_bridge {
> + compatible = "ge,b850v3_lvds_dp";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0x73>;
You use 'reg = <0x73>' here, but no @73.
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-06-09 18:30 +0200 |
| Subject | [PATCH V2 0/5] Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rIb0e-lr-9@gated-at.bofh.it> |
| In reply to | #1409281 |
The series adds a driver that creates a drm_bridge and a drm_connector for the
LVDS to DP++ display bridge of the GE B850v3.
There are two physical bridges on the video signal pipeline: a STDP4028(LVDS to
DP) and a STDP2690(DP to DP++). The hardware and firmware made it complicated
for this binding to comprise two device tree nodes, as the design goal is to
configure both bridges based on the LVDS signal, which leave the driver
powerless to control the video processing pipeline. The two bridges behaves as
a single bridge, and the driver is only needed for telling the host about EDID /
HPD, and for giving the host powers to ack interrupts. The video signal
pipeline is as follows:
Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
The patches from the series:
[1/5] Change the imx-ldb driver to allow attaching a bridge and not only a LVDS
panel.
[2/5] Configure the mapping between IPUs and external displays on the dts file
of the B850v3. Needed if connect two Full-HD monitors.
[3/5] Devicetree documentation for the GE B850v3 LVDS/DP++ Bridge
[4/5] Add the driver, make changes to MAINTAINERS, Kconfig and Makefile
[5/5] Make the changes to the B850v3 dts file to enable the GE B850v3
LVDS/DP++ Bridge.
Peter Senna Tschudin (5):
drm/imx-ldb: Add support to drm-bridge
dts/imx6q-b850v3: Configure IPU assignment order
Documentation/devicetree/bindings: b850v3_lvds_dp
drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge
dts/imx6q-b850v3: Use GE B850v3 LVDS/DP++ Bridge
.../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++
MAINTAINERS | 8 +
arch/arm/boot/dts/imx6q-b850v3.dts | 36 ++
drivers/gpu/drm/bridge/Kconfig | 11 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 392 +++++++++++++++++++++
drivers/gpu/drm/imx/imx-ldb.c | 121 ++++---
7 files changed, 565 insertions(+), 42 deletions(-)
create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-06-09 18:30 +0200 |
| Subject | [PATCH V2 2/5] dts/imx6q-b850v3: Configure IPU assignment order |
| Message-ID | <rIb0e-lr-7@gated-at.bofh.it> |
| In reply to | #1418468 |
As the IPU has combined limitations across multiple crtcs, and as that
can't be communicated to userspace at the moment, reorder the crtcs to
allow support to two Full-HD monitors by avoiding assigning two
monitors to a single IPU.
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
Changes from V1:
- New commit message
arch/arm/boot/dts/imx6q-b850v3.dts | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-b850v3.dts b/arch/arm/boot/dts/imx6q-b850v3.dts
index 167f744..88a70de 100644
--- a/arch/arm/boot/dts/imx6q-b850v3.dts
+++ b/arch/arm/boot/dts/imx6q-b850v3.dts
@@ -51,6 +51,11 @@
chosen {
stdout-path = &uart3;
};
+
+ display-subsystem {
+ compatible = "fsl,imx-display-subsystem";
+ ports = <&ipu1_di0>, <&ipu2_di0>, <&ipu1_di1>, <&ipu2_di1>;
+ };
};
&clks {
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-06-09 18:30 +0200 |
| Subject | [PATCH V2 3/5] Documentation/devicetree/bindings: b850v3_lvds_dp |
| Message-ID | <rIb0e-lr-25@gated-at.bofh.it> |
| In reply to | #1418468 |
Devicetree bindings documentation for the GE B850v3 LVDS/DP++
display bridge.
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
Changes from V1:
- Replaced '_' by '-' in node names or compatible strings
- Added missing @73 to the example
.../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++
1 file changed, 38 insertions(+)
create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
diff --git a/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
new file mode 100644
index 0000000..46bbea9
--- /dev/null
+++ b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
@@ -0,0 +1,38 @@
+Driver for GE B850v3 LVDS/DP++ display bridge
+
+Required properties:
+ - compatible : should be "ge,b850v3_lvds_dp".
+ - reg : should contain the address used to ack the interrupts.
+ - interrupt-parent : should link to the gpio used as interrupt
+ source on the host.
+ - interrupts : one interrupt should be described here, as in
+ <0 IRQ_TYPE_LEVEL_HIGH>.
+ - edid-reg : should contain the address used to read edid information
+ - port : should describe the vide signal connection between the host
+ and the bridge.
+
+Example:
+
+&mux2_i2c2 {
+ status = "okay";
+ clock-frequency = <100000>;
+
+ b850v3-lvds-dp-bridge@73 {
+ compatible = "ge,b850v3-lvds-dp";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ reg = <0x73>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
+
+ edid-reg = <0x72>;
+
+ port@0 {
+ reg = <0>;
+ b850v3_dp_bridge_in: endpoint {
+ remote-endpoint = <&lvds0_out>;
+ };
+ };
+ };
+};
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Rob Herring <robh@kernel.org> |
|---|---|
| Date | 2016-06-10 19:50 +0200 |
| Subject | Re: [PATCH V2 3/5] Documentation/devicetree/bindings: b850v3_lvds_dp |
| Message-ID | <rIyJd-7Ck-39@gated-at.bofh.it> |
| In reply to | #1418471 |
On Thu, Jun 09, 2016 at 06:25:03PM +0200, Peter Senna Tschudin wrote: > Devicetree bindings documentation for the GE B850v3 LVDS/DP++ > display bridge. > > Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com> > Cc: Philipp Zabel <p.zabel@pengutronix.de> > Cc: Rob Herring <robh@kernel.org> > Cc: Fabio Estevam <fabio.estevam@nxp.com> > Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com> > --- > Changes from V1: > - Replaced '_' by '-' in node names or compatible strings > - Added missing @73 to the example > > .../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++ > 1 file changed, 38 insertions(+) > create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt Acked-by: Rob Herring <robh@kernel.org>
[toc] | [prev] | [next] | [standalone]
| From | Javier Martinez Canillas <javier@dowhile0.org> |
|---|---|
| Date | 2016-06-10 21:00 +0200 |
| Subject | Re: [PATCH V2 3/5] Documentation/devicetree/bindings: b850v3_lvds_dp |
| Message-ID | <rIzOW-8sC-13@gated-at.bofh.it> |
| In reply to | #1418471 |
Hello Peter,
On Thu, Jun 9, 2016 at 12:25 PM, Peter Senna Tschudin
<peter.senna@collabora.com> wrote:
> Devicetree bindings documentation for the GE B850v3 LVDS/DP++
> display bridge.
>
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> ---
> Changes from V1:
> - Replaced '_' by '-' in node names or compatible strings
> - Added missing @73 to the example
>
> .../devicetree/bindings/ge/b850v3_lvds_dp.txt | 38 ++++++++++++++++++++++
> 1 file changed, 38 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
>
> diff --git a/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
> new file mode 100644
> index 0000000..46bbea9
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/ge/b850v3_lvds_dp.txt
> @@ -0,0 +1,38 @@
> +Driver for GE B850v3 LVDS/DP++ display bridge
> +
> +Required properties:
> + - compatible : should be "ge,b850v3_lvds_dp".
It seems you forgot to replace '_' by '-' (you did in the example though)
> + - reg : should contain the address used to ack the interrupts.
> + - interrupt-parent : should link to the gpio used as interrupt
> + source on the host.
Is the interrupt parent always a GPIO controller since that is what
this description says.
Shouldn't be instead something like instead?
interrupt-parent: phandle of the interrupt controller that services
interrupts to the device
> + - interrupts : one interrupt should be described here, as in
> + <0 IRQ_TYPE_LEVEL_HIGH>.
> + - edid-reg : should contain the address used to read edid information
> + - port : should describe the vide signal connection between the host
s/vide/video
> + and the bridge.
> +
> +Example:
> +
> +&mux2_i2c2 {
> + status = "okay";
> + clock-frequency = <100000>;
> +
> + b850v3-lvds-dp-bridge@73 {
> + compatible = "ge,b850v3-lvds-dp";
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + reg = <0x73>;
> + interrupt-parent = <&gpio2>;
> + interrupts = <0 IRQ_TYPE_LEVEL_HIGH>;
> +
> + edid-reg = <0x72>;
> +
> + port@0 {
> + reg = <0>;
AFAIU a unit-address and reg property for ports are only needed if you
have more than one port according to
Documentation/devicetree/bindings/graph.txt and
Documentation/devicetree/bindings/media/video-interfaces.txt.
> + b850v3_dp_bridge_in: endpoint {
> + remote-endpoint = <&lvds0_out>;
> + };
> + };
> + };
> +};
> --
Best regards,
Javier
[toc] | [prev] | [next] | [standalone]
| From | Peter Senna Tschudin <peter.senna@collabora.com> |
|---|---|
| Date | 2016-06-09 18:30 +0200 |
| Subject | [PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rIb0e-lr-13@gated-at.bofh.it> |
| In reply to | #1418468 |
Add a driver that create a drm_bridge and a drm_connector for the LVDS
to DP++ display bridge of the GE B850v3.
There are two physical bridges on the video signal pipeline: a
STDP4028(LVDS to DP) and a STDP2690(DP to DP++). The hardware and
firmware made it complicated for this binding to comprise two device
tree nodes, as the design goal is to configure both bridges based on
the LVDS signal, which leave the driver powerless to control the video
processing pipeline. The two bridges behaves as a single bridge, and
the driver is only needed for telling the host about EDID / HPD, and
for giving the host powers to ack interrupts. The video signal pipeline
is as follows:
Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
Cc: Philipp Zabel <p.zabel@pengutronix.de>
Cc: Rob Herring <robh@kernel.org>
Cc: Fabio Estevam <fabio.estevam@nxp.com>
CC: David Airlie <airlied@linux.ie>
CC: Thierry Reding <treding@nvidia.com>
CC: Thierry Reding <thierry.reding@gmail.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
---
Changes from V1:
- New commit message
- Removed 3 empty entry points
- Removed memory leak from ge_b850v3_lvds_dp_get_modes()
- Added a lock for mode setting
- Removed a few blank lines
- Changed the order at Makefile and Kconfig
MAINTAINERS | 8 +
drivers/gpu/drm/bridge/Kconfig | 11 +
drivers/gpu/drm/bridge/Makefile | 1 +
drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 392 +++++++++++++++++++++++++++++
4 files changed, 412 insertions(+)
create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 2ce5e91..2dd3d7f 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5010,6 +5010,14 @@ W: https://linuxtv.org
S: Maintained
F: drivers/media/radio/radio-gemtek*
+GENERAL ELECTRIC B850V3 LVDS/DP++ BRIDGE
+M: Martin Donnelly <martin.donnelly@ge.com>
+M: Peter Senna Tschudin <peter.senna@collabora.com>
+M: Martyn Welch <martyn.welch@collabora.co.uk>
+S: Maintained
+F: drivers/gpu/drm/bridge/ge_b850v3_dp2.c
+F: Documentation/devicetree/bindings/ge/b850v3_dp2_bridge.txt
+
GENERIC GPIO I2C DRIVER
M: Haavard Skinnemoen <hskinnemoen@gmail.com>
S: Supported
diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
index 8f7423f..93dae5bd 100644
--- a/drivers/gpu/drm/bridge/Kconfig
+++ b/drivers/gpu/drm/bridge/Kconfig
@@ -32,6 +32,17 @@ config DRM_DW_HDMI_AHB_AUDIO
Designware HDMI block. This is used in conjunction with
the i.MX6 HDMI driver.
+config DRM_GE_B850V3_LVDS_DP
+ tristate "GE B850v3 LVDS to DP++ display bridge"
+ depends on OF
+ select DRM_KMS_HELPER
+ select DRM_PANEL
+ ---help---
+ This is a driver for the display bridge of
+ GE B850v3 that convert dual channel LVDS
+ to DP++. This is used with the i.MX6 imx-ldb
+ driver.
+
config DRM_NXP_PTN3460
tristate "NXP PTN3460 DP/LVDS bridge"
depends on OF
diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
index 96b13b3..47ea6c1 100644
--- a/drivers/gpu/drm/bridge/Makefile
+++ b/drivers/gpu/drm/bridge/Makefile
@@ -3,6 +3,7 @@ ccflags-y := -Iinclude/drm
obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
+obj-$(CONFIG_DRM_GE_B850V3_LVDS_DP) += ge_b850v3_lvds_dp.o
obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
diff --git a/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
new file mode 100644
index 0000000..c73cd77
--- /dev/null
+++ b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
@@ -0,0 +1,392 @@
+/*
+ * Driver for GE B850v3 DP display bridge
+
+ * Copyright (c) 2016, Collabora Ltd.
+ * Copyright (c) 2016, General Electric Company
+
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+
+ * This program is distributed in the hope 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.
+
+ * You should have received a copy of the GNU General Public License
+ * along with this program. If not, see <http://www.gnu.org/licenses/>.
+
+ * This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
+ * display bridge of the GE B850v3. There are two physical bridges on the video
+ * signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++). However
+ * the physical bridges are automatically configured by the input video signal,
+ * and the driver has no access to the video processing pipeline. The driver is
+ * only needed to read EDID from the STDP2690 and to handle HPD events from the
+ * STDP4028. The driver communicates with both bridges over i2c. The video
+ * signal pipeline is as follows:
+ *
+ * Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
+
+ *
+ */
+#include <linux/gpio.h>
+#include <linux/i2c.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include "drm_crtc.h"
+#include "drm_crtc_helper.h"
+#include "drm_edid.h"
+#include "drmP.h"
+
+#define EDID_EXT_BLOCK_CNT 0x7E
+
+#define STDP4028_IRQ_OUT_CONF_REG 0x02
+#define STDP4028_DPTX_IRQ_EN_REG 0x3C
+#define STDP4028_DPTX_IRQ_STS_REG 0x3D
+#define STDP4028_DPTX_STS_REG 0x3E
+
+#define STDP4028_DPTX_DP_IRQ_EN 0x1000
+
+#define STDP4028_DPTX_HOTPLUG_IRQ_EN 0x0400
+#define STDP4028_DPTX_LINK_CH_IRQ_EN 0x2000
+#define STDP4028_DPTX_IRQ_CONFIG \
+ (STDP4028_DPTX_LINK_CH_IRQ_EN | STDP4028_DPTX_HOTPLUG_IRQ_EN)
+
+#define STDP4028_DPTX_HOTPLUG_STS 0x0200
+#define STDP4028_DPTX_LINK_STS 0x1000
+#define STDP4028_CON_STATE_CONNECTED \
+ (STDP4028_DPTX_HOTPLUG_STS | STDP4028_DPTX_LINK_STS)
+
+#define STDP4028_DPTX_HOTPLUG_CH_STS 0x0400
+#define STDP4028_DPTX_LINK_CH_STS 0x2000
+#define STDP4028_DPTX_IRQ_CLEAR \
+ (STDP4028_DPTX_LINK_CH_STS | STDP4028_DPTX_HOTPLUG_CH_STS)
+
+struct ge_b850v3_lvds_dp {
+ struct drm_connector connector;
+ struct drm_bridge bridge;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c;
+ struct i2c_client *edid_i2c;
+ struct edid *edid;
+ struct mutex lock;
+};
+
+static inline struct ge_b850v3_lvds_dp *
+ bridge_to_ge_b850v3_lvds_dp(struct drm_bridge *bridge)
+{
+ return container_of(bridge, struct ge_b850v3_lvds_dp, bridge);
+}
+
+static inline struct ge_b850v3_lvds_dp *
+ connector_to_ge_b850v3_lvds_dp(struct drm_connector *connector)
+{
+ return container_of(connector, struct ge_b850v3_lvds_dp, connector);
+}
+
+static void ge_b850v3_lvds_dp_enable(struct drm_bridge *bridge)
+{
+}
+
+static void ge_b850v3_lvds_dp_disable(struct drm_bridge *bridge)
+{
+}
+
+u8 *stdp2690_get_edid(struct i2c_client *client)
+{
+ struct i2c_adapter *adapter = client->adapter;
+ unsigned char start = 0x00;
+ unsigned int total_size;
+ u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
+
+ struct i2c_msg msgs[] = {
+ {
+ .addr = client->addr,
+ .flags = 0,
+ .len = 1,
+ .buf = &start,
+ }, {
+ .addr = client->addr,
+ .flags = I2C_M_RD,
+ .len = EDID_LENGTH,
+ .buf = block,
+ }
+ };
+
+ if (!block)
+ return NULL;
+
+ if (i2c_transfer(adapter, msgs, 2) != 2) {
+ DRM_ERROR("Unable to read EDID.\n");
+ goto err;
+ }
+
+ if (!drm_edid_block_valid(block, 0, false, NULL)) {
+ DRM_ERROR("Invalid EDID block\n");
+ goto err;
+ }
+
+ total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
+ if (total_size > EDID_LENGTH) {
+ kfree(block);
+ block = kmalloc(total_size, GFP_KERNEL);
+ if (!block)
+ return NULL;
+
+ /* Yes, read the entire buffer, and do not skip the first
+ * EDID_LENGTH bytes.
+ */
+ start = 0x00;
+ msgs[1].len = total_size;
+ msgs[1].buf = block;
+
+ if (i2c_transfer(adapter, msgs, 2) != 2) {
+ DRM_ERROR("Unable to read EDID extension blocks.\n");
+ goto err;
+ }
+ }
+
+ return block;
+
+err:
+ kfree(block);
+ return NULL;
+}
+
+static int ge_b850v3_lvds_dp_get_modes(struct drm_connector *connector)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge;
+ struct i2c_client *client;
+ int num_modes = 0;
+
+ ptn_bridge = connector_to_ge_b850v3_lvds_dp(connector);
+ client = ptn_bridge->edid_i2c;
+
+ mutex_lock(&ptn_bridge->lock);
+
+ kfree(ptn_bridge->edid);
+ ptn_bridge->edid = (struct edid *) stdp2690_get_edid(client);
+
+ if (ptn_bridge->edid) {
+ drm_mode_connector_update_edid_property(connector,
+ ptn_bridge->edid);
+ num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
+ }
+
+ mutex_unlock(&ptn_bridge->lock);
+
+ return num_modes;
+}
+
+static struct drm_encoder
+*ge_b850v3_lvds_dp_best_encoder(struct drm_connector *connector)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ connector_to_ge_b850v3_lvds_dp(connector);
+
+ return ptn_bridge->bridge.encoder;
+}
+
+static const struct
+drm_connector_helper_funcs ge_b850v3_lvds_dp_connector_helper_funcs = {
+ .get_modes = ge_b850v3_lvds_dp_get_modes,
+ .best_encoder = ge_b850v3_lvds_dp_best_encoder,
+};
+
+static enum drm_connector_status ge_b850v3_lvds_dp_detect(
+ struct drm_connector *connector, bool force)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ connector_to_ge_b850v3_lvds_dp(connector);
+ struct i2c_client *ge_b850v3_lvds_dp_i2c =
+ ptn_bridge->ge_b850v3_lvds_dp_i2c;
+ s32 link_state;
+
+ link_state = i2c_smbus_read_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_STS_REG);
+
+ if (link_state == STDP4028_CON_STATE_CONNECTED)
+ return connector_status_connected;
+
+ if (link_state == 0)
+ return connector_status_disconnected;
+
+ return connector_status_unknown;
+}
+
+static void ge_b850v3_lvds_dp_connector_destroy(struct drm_connector *connector)
+{
+ drm_connector_cleanup(connector);
+}
+
+static const struct drm_connector_funcs ge_b850v3_lvds_dp_connector_funcs = {
+ .dpms = drm_helper_connector_dpms,
+ .fill_modes = drm_helper_probe_single_connector_modes,
+ .detect = ge_b850v3_lvds_dp_detect,
+ .destroy = ge_b850v3_lvds_dp_connector_destroy,
+};
+
+static irqreturn_t ge_b850v3_lvds_dp_irq_handler(int irq, void *dev_id)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge = dev_id;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c
+ = ptn_bridge->ge_b850v3_lvds_dp_i2c;
+
+ mutex_lock(&ptn_bridge->lock);
+
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
+
+ mutex_unlock(&ptn_bridge->lock);
+
+ if (ptn_bridge->connector.dev)
+ drm_kms_helper_hotplug_event(ptn_bridge->connector.dev);
+
+ return IRQ_HANDLED;
+}
+
+static int ge_b850v3_lvds_dp_attach(struct drm_bridge *bridge)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge
+ = bridge_to_ge_b850v3_lvds_dp(bridge);
+ struct drm_connector *connector = &ptn_bridge->connector;
+ struct i2c_client *ge_b850v3_lvds_dp_i2c
+ = ptn_bridge->ge_b850v3_lvds_dp_i2c;
+ int ret;
+
+ if (!bridge->encoder) {
+ DRM_ERROR("Parent encoder object not found");
+ return -ENODEV;
+ }
+
+ connector->polled = DRM_CONNECTOR_POLL_HPD;
+
+ drm_connector_helper_add(connector,
+ &ge_b850v3_lvds_dp_connector_helper_funcs);
+
+ ret = drm_connector_init(bridge->dev, connector,
+ &ge_b850v3_lvds_dp_connector_funcs,
+ DRM_MODE_CONNECTOR_DisplayPort);
+ if (ret) {
+ DRM_ERROR("Failed to initialize connector with drm\n");
+ return ret;
+ }
+
+ ret = drm_mode_connector_attach_encoder(connector, bridge->encoder);
+ if (ret)
+ return ret;
+
+ drm_bridge_enable(bridge);
+ if (ge_b850v3_lvds_dp_i2c->irq) {
+ drm_helper_hpd_irq_event(connector->dev);
+
+ ret = devm_request_threaded_irq(&ge_b850v3_lvds_dp_i2c->dev,
+ ge_b850v3_lvds_dp_i2c->irq, NULL,
+ ge_b850v3_lvds_dp_irq_handler,
+ IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
+ "ge-b850v3-lvds-dp", ptn_bridge);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
+
+static const struct drm_bridge_funcs ge_b850v3_lvds_dp_funcs = {
+ .enable = ge_b850v3_lvds_dp_enable,
+ .disable = ge_b850v3_lvds_dp_disable,
+ .attach = ge_b850v3_lvds_dp_attach,
+};
+
+static int ge_b850v3_lvds_dp_probe(struct i2c_client *ge_b850v3_lvds_dp_i2c,
+ const struct i2c_device_id *id)
+{
+ struct device *dev = &ge_b850v3_lvds_dp_i2c->dev;
+ struct ge_b850v3_lvds_dp *ptn_bridge;
+ int ret;
+ u32 edid_i2c_reg;
+
+ ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
+ if (!ptn_bridge)
+ return -ENOMEM;
+
+ mutex_init(&ptn_bridge->lock);
+
+ ptn_bridge->ge_b850v3_lvds_dp_i2c = ge_b850v3_lvds_dp_i2c;
+ ptn_bridge->bridge.driver_private = ptn_bridge;
+ i2c_set_clientdata(ge_b850v3_lvds_dp_i2c, ptn_bridge);
+
+ ret = of_property_read_u32(dev->of_node, "edid-reg", &edid_i2c_reg);
+ if (ret) {
+ dev_err(dev, "edid-reg not specified, aborting...\n");
+ return -ENODEV;
+ }
+
+ ptn_bridge->edid_i2c = devm_kzalloc(dev,
+ sizeof(struct i2c_client), GFP_KERNEL);
+
+ if (!ptn_bridge->edid_i2c)
+ return -ENOMEM;
+
+ memcpy(ptn_bridge->edid_i2c, ge_b850v3_lvds_dp_i2c,
+ sizeof(struct i2c_client));
+
+ ptn_bridge->edid_i2c->addr = (unsigned short) edid_i2c_reg;
+
+ /* Configures the bridge to re-enable interrupts after each ack */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_IRQ_OUT_CONF_REG, STDP4028_DPTX_DP_IRQ_EN);
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_EN_REG, STDP4028_DPTX_IRQ_CONFIG);
+
+ /* Clear pending interrupts since power up. */
+ i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
+ STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
+
+ ptn_bridge->bridge.funcs = &ge_b850v3_lvds_dp_funcs;
+ ptn_bridge->bridge.of_node = dev->of_node;
+ ret = drm_bridge_add(&ptn_bridge->bridge);
+ if (ret) {
+ DRM_ERROR("Failed to add bridge\n");
+ return ret;
+ }
+
+ return 0;
+}
+
+static int ge_b850v3_lvds_dp_remove(struct i2c_client *ge_b850v3_lvds_dp_i2c)
+{
+ struct ge_b850v3_lvds_dp *ptn_bridge =
+ i2c_get_clientdata(ge_b850v3_lvds_dp_i2c);
+
+ drm_bridge_remove(&ptn_bridge->bridge);
+
+ return 0;
+}
+
+static const struct i2c_device_id ge_b850v3_lvds_dp_i2c_table[] = {
+ {"b850v3-lvds-dp", 0},
+ {},
+};
+MODULE_DEVICE_TABLE(i2c, ge_b850v3_lvds_dp_i2c_table);
+
+static const struct of_device_id ge_b850v3_lvds_dp_match[] = {
+ { .compatible = "ge,b850v3-lvds-dp" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ge_b850v3_lvds_dp_match);
+
+static struct i2c_driver ge_b850v3_lvds_dp_driver = {
+ .id_table = ge_b850v3_lvds_dp_i2c_table,
+ .probe = ge_b850v3_lvds_dp_probe,
+ .remove = ge_b850v3_lvds_dp_remove,
+ .driver = {
+ .name = "ge,b850v3-lvds-dp",
+ .of_match_table = ge_b850v3_lvds_dp_match,
+ },
+};
+module_i2c_driver(ge_b850v3_lvds_dp_driver);
+
+MODULE_AUTHOR("Peter Senna Tschudin <peter.senna@collabora.com>");
+MODULE_AUTHOR("Martyn Welch <martyn.welch@collabora.co.uk>");
+MODULE_DESCRIPTION("GE LVDS to DP++ bridge)");
+MODULE_LICENSE("GPL v2");
--
2.5.5
[toc] | [prev] | [next] | [standalone]
| From | Enric Balletbo Serra <eballetbo@gmail.com> |
|---|---|
| Date | 2016-06-10 09:40 +0200 |
| Subject | Re: [PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rIpcT-1FF-39@gated-at.bofh.it> |
| In reply to | #1418473 |
Hi Peter,
Only a few comments ;)
2016-06-09 18:25 GMT+02:00 Peter Senna Tschudin <peter.senna@collabora.com>:
> Add a driver that create a drm_bridge and a drm_connector for the LVDS
> to DP++ display bridge of the GE B850v3.
>
> There are two physical bridges on the video signal pipeline: a
> STDP4028(LVDS to DP) and a STDP2690(DP to DP++). The hardware and
> firmware made it complicated for this binding to comprise two device
> tree nodes, as the design goal is to configure both bridges based on
> the LVDS signal, which leave the driver powerless to control the video
> processing pipeline. The two bridges behaves as a single bridge, and
> the driver is only needed for telling the host about EDID / HPD, and
> for giving the host powers to ack interrupts. The video signal pipeline
> is as follows:
>
> Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
>
> Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> Cc: Philipp Zabel <p.zabel@pengutronix.de>
> Cc: Rob Herring <robh@kernel.org>
> Cc: Fabio Estevam <fabio.estevam@nxp.com>
> CC: David Airlie <airlied@linux.ie>
> CC: Thierry Reding <treding@nvidia.com>
> CC: Thierry Reding <thierry.reding@gmail.com>
> Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> ---
> Changes from V1:
> - New commit message
> - Removed 3 empty entry points
> - Removed memory leak from ge_b850v3_lvds_dp_get_modes()
> - Added a lock for mode setting
> - Removed a few blank lines
> - Changed the order at Makefile and Kconfig
>
> MAINTAINERS | 8 +
> drivers/gpu/drm/bridge/Kconfig | 11 +
> drivers/gpu/drm/bridge/Makefile | 1 +
> drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 392 +++++++++++++++++++++++++++++
> 4 files changed, 412 insertions(+)
> create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2ce5e91..2dd3d7f 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -5010,6 +5010,14 @@ W: https://linuxtv.org
> S: Maintained
> F: drivers/media/radio/radio-gemtek*
>
> +GENERAL ELECTRIC B850V3 LVDS/DP++ BRIDGE
> +M: Martin Donnelly <martin.donnelly@ge.com>
> +M: Peter Senna Tschudin <peter.senna@collabora.com>
> +M: Martyn Welch <martyn.welch@collabora.co.uk>
> +S: Maintained
> +F: drivers/gpu/drm/bridge/ge_b850v3_dp2.c
> +F: Documentation/devicetree/bindings/ge/b850v3_dp2_bridge.txt
> +
> GENERIC GPIO I2C DRIVER
> M: Haavard Skinnemoen <hskinnemoen@gmail.com>
> S: Supported
> diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> index 8f7423f..93dae5bd 100644
> --- a/drivers/gpu/drm/bridge/Kconfig
> +++ b/drivers/gpu/drm/bridge/Kconfig
> @@ -32,6 +32,17 @@ config DRM_DW_HDMI_AHB_AUDIO
> Designware HDMI block. This is used in conjunction with
> the i.MX6 HDMI driver.
>
> +config DRM_GE_B850V3_LVDS_DP
> + tristate "GE B850v3 LVDS to DP++ display bridge"
> + depends on OF
> + select DRM_KMS_HELPER
> + select DRM_PANEL
> + ---help---
> + This is a driver for the display bridge of
> + GE B850v3 that convert dual channel LVDS
> + to DP++. This is used with the i.MX6 imx-ldb
> + driver.
> +
> config DRM_NXP_PTN3460
> tristate "NXP PTN3460 DP/LVDS bridge"
> depends on OF
> diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> index 96b13b3..47ea6c1 100644
> --- a/drivers/gpu/drm/bridge/Makefile
> +++ b/drivers/gpu/drm/bridge/Makefile
> @@ -3,6 +3,7 @@ ccflags-y := -Iinclude/drm
> obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
> obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> +obj-$(CONFIG_DRM_GE_B850V3_LVDS_DP) += ge_b850v3_lvds_dp.o
> obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> diff --git a/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> new file mode 100644
> index 0000000..c73cd77
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> @@ -0,0 +1,392 @@
> +/*
> + * Driver for GE B850v3 DP display bridge
> +
> + * Copyright (c) 2016, Collabora Ltd.
> + * Copyright (c) 2016, General Electric Company
> +
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> +
> + * This program is distributed in the hope 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.
> +
> + * You should have received a copy of the GNU General Public License
> + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> +
> + * This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
> + * display bridge of the GE B850v3. There are two physical bridges on the video
> + * signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++). However
> + * the physical bridges are automatically configured by the input video signal,
> + * and the driver has no access to the video processing pipeline. The driver is
> + * only needed to read EDID from the STDP2690 and to handle HPD events from the
> + * STDP4028. The driver communicates with both bridges over i2c. The video
> + * signal pipeline is as follows:
> + *
> + * Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
> +
> + *
> + */
> +#include <linux/gpio.h>
> +#include <linux/i2c.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include "drm_crtc.h"
> +#include "drm_crtc_helper.h"
> +#include "drm_edid.h"
> +#include "drmP.h"
> +
> +#define EDID_EXT_BLOCK_CNT 0x7E
> +
> +#define STDP4028_IRQ_OUT_CONF_REG 0x02
> +#define STDP4028_DPTX_IRQ_EN_REG 0x3C
> +#define STDP4028_DPTX_IRQ_STS_REG 0x3D
> +#define STDP4028_DPTX_STS_REG 0x3E
> +
> +#define STDP4028_DPTX_DP_IRQ_EN 0x1000
> +
> +#define STDP4028_DPTX_HOTPLUG_IRQ_EN 0x0400
> +#define STDP4028_DPTX_LINK_CH_IRQ_EN 0x2000
> +#define STDP4028_DPTX_IRQ_CONFIG \
> + (STDP4028_DPTX_LINK_CH_IRQ_EN | STDP4028_DPTX_HOTPLUG_IRQ_EN)
> +
> +#define STDP4028_DPTX_HOTPLUG_STS 0x0200
> +#define STDP4028_DPTX_LINK_STS 0x1000
> +#define STDP4028_CON_STATE_CONNECTED \
> + (STDP4028_DPTX_HOTPLUG_STS | STDP4028_DPTX_LINK_STS)
> +
> +#define STDP4028_DPTX_HOTPLUG_CH_STS 0x0400
> +#define STDP4028_DPTX_LINK_CH_STS 0x2000
> +#define STDP4028_DPTX_IRQ_CLEAR \
> + (STDP4028_DPTX_LINK_CH_STS | STDP4028_DPTX_HOTPLUG_CH_STS)
> +
> +struct ge_b850v3_lvds_dp {
> + struct drm_connector connector;
> + struct drm_bridge bridge;
> + struct i2c_client *ge_b850v3_lvds_dp_i2c;
> + struct i2c_client *edid_i2c;
> + struct edid *edid;
> + struct mutex lock;
> +};
> +
> +static inline struct ge_b850v3_lvds_dp *
> + bridge_to_ge_b850v3_lvds_dp(struct drm_bridge *bridge)
> +{
> + return container_of(bridge, struct ge_b850v3_lvds_dp, bridge);
> +}
> +
> +static inline struct ge_b850v3_lvds_dp *
> + connector_to_ge_b850v3_lvds_dp(struct drm_connector *connector)
> +{
> + return container_of(connector, struct ge_b850v3_lvds_dp, connector);
> +}
> +
> +static void ge_b850v3_lvds_dp_enable(struct drm_bridge *bridge)
> +{
> +}
> +
You can remove this function, see
http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_bridge.c#L277
> +static void ge_b850v3_lvds_dp_disable(struct drm_bridge *bridge)
> +{
> +}
> +
And this one, see
http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_bridge.c#L182
> +u8 *stdp2690_get_edid(struct i2c_client *client)
> +{
> + struct i2c_adapter *adapter = client->adapter;
> + unsigned char start = 0x00;
> + unsigned int total_size;
> + u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> +
> + struct i2c_msg msgs[] = {
> + {
> + .addr = client->addr,
> + .flags = 0,
> + .len = 1,
> + .buf = &start,
> + }, {
> + .addr = client->addr,
> + .flags = I2C_M_RD,
> + .len = EDID_LENGTH,
> + .buf = block,
> + }
> + };
> +
> + if (!block)
> + return NULL;
> +
> + if (i2c_transfer(adapter, msgs, 2) != 2) {
> + DRM_ERROR("Unable to read EDID.\n");
> + goto err;
> + }
> +
> + if (!drm_edid_block_valid(block, 0, false, NULL)) {
> + DRM_ERROR("Invalid EDID block\n");
> + goto err;
> + }
> +
> + total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
> + if (total_size > EDID_LENGTH) {
> + kfree(block);
> + block = kmalloc(total_size, GFP_KERNEL);
> + if (!block)
> + return NULL;
> +
> + /* Yes, read the entire buffer, and do not skip the first
> + * EDID_LENGTH bytes.
> + */
> + start = 0x00;
> + msgs[1].len = total_size;
> + msgs[1].buf = block;
> +
> + if (i2c_transfer(adapter, msgs, 2) != 2) {
> + DRM_ERROR("Unable to read EDID extension blocks.\n");
> + goto err;
> + }
> + }
> +
> + return block;
> +
> +err:
> + kfree(block);
> + return NULL;
> +}
> +
> +static int ge_b850v3_lvds_dp_get_modes(struct drm_connector *connector)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge;
> + struct i2c_client *client;
> + int num_modes = 0;
> +
> + ptn_bridge = connector_to_ge_b850v3_lvds_dp(connector);
> + client = ptn_bridge->edid_i2c;
> +
> + mutex_lock(&ptn_bridge->lock);
> +
> + kfree(ptn_bridge->edid);
> + ptn_bridge->edid = (struct edid *) stdp2690_get_edid(client);
> +
> + if (ptn_bridge->edid) {
> + drm_mode_connector_update_edid_property(connector,
> + ptn_bridge->edid);
> + num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
> + }
> +
> + mutex_unlock(&ptn_bridge->lock);
> +
> + return num_modes;
> +}
> +
> +static struct drm_encoder
> +*ge_b850v3_lvds_dp_best_encoder(struct drm_connector *connector)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge =
> + connector_to_ge_b850v3_lvds_dp(connector);
> +
> + return ptn_bridge->bridge.encoder;
> +}
> +
> +static const struct
> +drm_connector_helper_funcs ge_b850v3_lvds_dp_connector_helper_funcs = {
> + .get_modes = ge_b850v3_lvds_dp_get_modes,
> + .best_encoder = ge_b850v3_lvds_dp_best_encoder,
> +};
> +
> +static enum drm_connector_status ge_b850v3_lvds_dp_detect(
> + struct drm_connector *connector, bool force)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge =
> + connector_to_ge_b850v3_lvds_dp(connector);
> + struct i2c_client *ge_b850v3_lvds_dp_i2c =
> + ptn_bridge->ge_b850v3_lvds_dp_i2c;
> + s32 link_state;
> +
> + link_state = i2c_smbus_read_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_STS_REG);
> +
> + if (link_state == STDP4028_CON_STATE_CONNECTED)
> + return connector_status_connected;
> +
> + if (link_state == 0)
> + return connector_status_disconnected;
> +
> + return connector_status_unknown;
> +}
> +
> +static void ge_b850v3_lvds_dp_connector_destroy(struct drm_connector *connector)
> +{
> + drm_connector_cleanup(connector);
> +}
> +
> +static const struct drm_connector_funcs ge_b850v3_lvds_dp_connector_funcs = {
> + .dpms = drm_helper_connector_dpms,
> + .fill_modes = drm_helper_probe_single_connector_modes,
> + .detect = ge_b850v3_lvds_dp_detect,
> + .destroy = ge_b850v3_lvds_dp_connector_destroy,
> +};
> +
> +static irqreturn_t ge_b850v3_lvds_dp_irq_handler(int irq, void *dev_id)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge = dev_id;
> + struct i2c_client *ge_b850v3_lvds_dp_i2c
> + = ptn_bridge->ge_b850v3_lvds_dp_i2c;
> +
> + mutex_lock(&ptn_bridge->lock);
> +
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
> +
> + mutex_unlock(&ptn_bridge->lock);
> +
> + if (ptn_bridge->connector.dev)
> + drm_kms_helper_hotplug_event(ptn_bridge->connector.dev);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static int ge_b850v3_lvds_dp_attach(struct drm_bridge *bridge)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge
> + = bridge_to_ge_b850v3_lvds_dp(bridge);
> + struct drm_connector *connector = &ptn_bridge->connector;
> + struct i2c_client *ge_b850v3_lvds_dp_i2c
> + = ptn_bridge->ge_b850v3_lvds_dp_i2c;
> + int ret;
> +
> + if (!bridge->encoder) {
> + DRM_ERROR("Parent encoder object not found");
> + return -ENODEV;
> + }
> +
> + connector->polled = DRM_CONNECTOR_POLL_HPD;
> +
> + drm_connector_helper_add(connector,
> + &ge_b850v3_lvds_dp_connector_helper_funcs);
> +
> + ret = drm_connector_init(bridge->dev, connector,
> + &ge_b850v3_lvds_dp_connector_funcs,
> + DRM_MODE_CONNECTOR_DisplayPort);
> + if (ret) {
> + DRM_ERROR("Failed to initialize connector with drm\n");
> + return ret;
> + }
> +
> + ret = drm_mode_connector_attach_encoder(connector, bridge->encoder);
> + if (ret)
> + return ret;
> +
> + drm_bridge_enable(bridge);
> + if (ge_b850v3_lvds_dp_i2c->irq) {
> + drm_helper_hpd_irq_event(connector->dev);
> +
> + ret = devm_request_threaded_irq(&ge_b850v3_lvds_dp_i2c->dev,
> + ge_b850v3_lvds_dp_i2c->irq, NULL,
> + ge_b850v3_lvds_dp_irq_handler,
> + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> + "ge-b850v3-lvds-dp", ptn_bridge);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static const struct drm_bridge_funcs ge_b850v3_lvds_dp_funcs = {
> + .enable = ge_b850v3_lvds_dp_enable,
> + .disable = ge_b850v3_lvds_dp_disable,
Remove the above empty callbacks.
> + .attach = ge_b850v3_lvds_dp_attach,
> +};
> +
> +static int ge_b850v3_lvds_dp_probe(struct i2c_client *ge_b850v3_lvds_dp_i2c,
> + const struct i2c_device_id *id)
> +{
> + struct device *dev = &ge_b850v3_lvds_dp_i2c->dev;
> + struct ge_b850v3_lvds_dp *ptn_bridge;
> + int ret;
> + u32 edid_i2c_reg;
> +
> + ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
> + if (!ptn_bridge)
> + return -ENOMEM;
> +
> + mutex_init(&ptn_bridge->lock);
> +
> + ptn_bridge->ge_b850v3_lvds_dp_i2c = ge_b850v3_lvds_dp_i2c;
> + ptn_bridge->bridge.driver_private = ptn_bridge;
> + i2c_set_clientdata(ge_b850v3_lvds_dp_i2c, ptn_bridge);
> +
> + ret = of_property_read_u32(dev->of_node, "edid-reg", &edid_i2c_reg);
> + if (ret) {
> + dev_err(dev, "edid-reg not specified, aborting...\n");
> + return -ENODEV;
> + }
> +
> + ptn_bridge->edid_i2c = devm_kzalloc(dev,
> + sizeof(struct i2c_client), GFP_KERNEL);
> +
> + if (!ptn_bridge->edid_i2c)
> + return -ENOMEM;
> +
> + memcpy(ptn_bridge->edid_i2c, ge_b850v3_lvds_dp_i2c,
> + sizeof(struct i2c_client));
> +
> + ptn_bridge->edid_i2c->addr = (unsigned short) edid_i2c_reg;
> +
> + /* Configures the bridge to re-enable interrupts after each ack */
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_IRQ_OUT_CONF_REG, STDP4028_DPTX_DP_IRQ_EN);
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_IRQ_EN_REG, STDP4028_DPTX_IRQ_CONFIG);
> +
> + /* Clear pending interrupts since power up. */
> + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> + STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
> +
> + ptn_bridge->bridge.funcs = &ge_b850v3_lvds_dp_funcs;
> + ptn_bridge->bridge.of_node = dev->of_node;
> + ret = drm_bridge_add(&ptn_bridge->bridge);
> + if (ret) {
> + DRM_ERROR("Failed to add bridge\n");
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +static int ge_b850v3_lvds_dp_remove(struct i2c_client *ge_b850v3_lvds_dp_i2c)
> +{
> + struct ge_b850v3_lvds_dp *ptn_bridge =
> + i2c_get_clientdata(ge_b850v3_lvds_dp_i2c);
> +
> + drm_bridge_remove(&ptn_bridge->bridge);
Guess you need to free ptn_bridge->edid here.
> +
> + return 0;
> +}
> +
> +static const struct i2c_device_id ge_b850v3_lvds_dp_i2c_table[] = {
> + {"b850v3-lvds-dp", 0},
> + {},
> +};
> +MODULE_DEVICE_TABLE(i2c, ge_b850v3_lvds_dp_i2c_table);
> +
> +static const struct of_device_id ge_b850v3_lvds_dp_match[] = {
> + { .compatible = "ge,b850v3-lvds-dp" },
> + {},
> +};
> +MODULE_DEVICE_TABLE(of, ge_b850v3_lvds_dp_match);
> +
> +static struct i2c_driver ge_b850v3_lvds_dp_driver = {
> + .id_table = ge_b850v3_lvds_dp_i2c_table,
> + .probe = ge_b850v3_lvds_dp_probe,
> + .remove = ge_b850v3_lvds_dp_remove,
> + .driver = {
> + .name = "ge,b850v3-lvds-dp",
> + .of_match_table = ge_b850v3_lvds_dp_match,
> + },
> +};
> +module_i2c_driver(ge_b850v3_lvds_dp_driver);
> +
> +MODULE_AUTHOR("Peter Senna Tschudin <peter.senna@collabora.com>");
> +MODULE_AUTHOR("Martyn Welch <martyn.welch@collabora.co.uk>");
> +MODULE_DESCRIPTION("GE LVDS to DP++ bridge)");
> +MODULE_LICENSE("GPL v2");
> --
> 2.5.5
>
[toc] | [prev] | [next] | [standalone]
| From | "Peter Senna Tschudin" <peter.senna@collabora.co.uk> |
|---|---|
| Date | 2016-06-10 11:50 +0200 |
| Subject | Re: [PATCH V2 4/5] drm/bridge: Add driver for GE B850v3 LVDS/DP++ Bridge |
| Message-ID | <rIreF-2SG-11@gated-at.bofh.it> |
| In reply to | #1419100 |
Hi Enric,
On Friday, June 10, 2016 09:39 CEST, Enric Balletbo Serra <eballetbo@gmail.com> wrote:
> Hi Peter,
>
> Only a few comments ;)
Thanks a lot for the review!
>
> 2016-06-09 18:25 GMT+02:00 Peter Senna Tschudin <peter.senna@collabora.com>:
> > Add a driver that create a drm_bridge and a drm_connector for the LVDS
> > to DP++ display bridge of the GE B850v3.
> >
> > There are two physical bridges on the video signal pipeline: a
> > STDP4028(LVDS to DP) and a STDP2690(DP to DP++). The hardware and
> > firmware made it complicated for this binding to comprise two device
> > tree nodes, as the design goal is to configure both bridges based on
> > the LVDS signal, which leave the driver powerless to control the video
> > processing pipeline. The two bridges behaves as a single bridge, and
> > the driver is only needed for telling the host about EDID / HPD, and
> > for giving the host powers to ack interrupts. The video signal pipeline
> > is as follows:
> >
> > Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
> >
> > Cc: Enric Balletbo i Serra <enric.balletbo@collabora.com>
> > Cc: Philipp Zabel <p.zabel@pengutronix.de>
> > Cc: Rob Herring <robh@kernel.org>
> > Cc: Fabio Estevam <fabio.estevam@nxp.com>
> > CC: David Airlie <airlied@linux.ie>
> > CC: Thierry Reding <treding@nvidia.com>
> > CC: Thierry Reding <thierry.reding@gmail.com>
> > Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
> > ---
> > Changes from V1:
> > - New commit message
> > - Removed 3 empty entry points
> > - Removed memory leak from ge_b850v3_lvds_dp_get_modes()
> > - Added a lock for mode setting
> > - Removed a few blank lines
> > - Changed the order at Makefile and Kconfig
> >
> > MAINTAINERS | 8 +
> > drivers/gpu/drm/bridge/Kconfig | 11 +
> > drivers/gpu/drm/bridge/Makefile | 1 +
> > drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c | 392 +++++++++++++++++++++++++++++
> > 4 files changed, 412 insertions(+)
> > create mode 100644 drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> >
> > diff --git a/MAINTAINERS b/MAINTAINERS
> > index 2ce5e91..2dd3d7f 100644
> > --- a/MAINTAINERS
> > +++ b/MAINTAINERS
> > @@ -5010,6 +5010,14 @@ W: https://linuxtv.org
> > S: Maintained
> > F: drivers/media/radio/radio-gemtek*
> >
> > +GENERAL ELECTRIC B850V3 LVDS/DP++ BRIDGE
> > +M: Martin Donnelly <martin.donnelly@ge.com>
> > +M: Peter Senna Tschudin <peter.senna@collabora.com>
> > +M: Martyn Welch <martyn.welch@collabora.co.uk>
> > +S: Maintained
> > +F: drivers/gpu/drm/bridge/ge_b850v3_dp2.c
> > +F: Documentation/devicetree/bindings/ge/b850v3_dp2_bridge.txt
> > +
> > GENERIC GPIO I2C DRIVER
> > M: Haavard Skinnemoen <hskinnemoen@gmail.com>
> > S: Supported
> > diff --git a/drivers/gpu/drm/bridge/Kconfig b/drivers/gpu/drm/bridge/Kconfig
> > index 8f7423f..93dae5bd 100644
> > --- a/drivers/gpu/drm/bridge/Kconfig
> > +++ b/drivers/gpu/drm/bridge/Kconfig
> > @@ -32,6 +32,17 @@ config DRM_DW_HDMI_AHB_AUDIO
> > Designware HDMI block. This is used in conjunction with
> > the i.MX6 HDMI driver.
> >
> > +config DRM_GE_B850V3_LVDS_DP
> > + tristate "GE B850v3 LVDS to DP++ display bridge"
> > + depends on OF
> > + select DRM_KMS_HELPER
> > + select DRM_PANEL
> > + ---help---
> > + This is a driver for the display bridge of
> > + GE B850v3 that convert dual channel LVDS
> > + to DP++. This is used with the i.MX6 imx-ldb
> > + driver.
> > +
> > config DRM_NXP_PTN3460
> > tristate "NXP PTN3460 DP/LVDS bridge"
> > depends on OF
> > diff --git a/drivers/gpu/drm/bridge/Makefile b/drivers/gpu/drm/bridge/Makefile
> > index 96b13b3..47ea6c1 100644
> > --- a/drivers/gpu/drm/bridge/Makefile
> > +++ b/drivers/gpu/drm/bridge/Makefile
> > @@ -3,6 +3,7 @@ ccflags-y := -Iinclude/drm
> > obj-$(CONFIG_DRM_ANALOGIX_ANX78XX) += analogix-anx78xx.o
> > obj-$(CONFIG_DRM_DW_HDMI) += dw-hdmi.o
> > obj-$(CONFIG_DRM_DW_HDMI_AHB_AUDIO) += dw-hdmi-ahb-audio.o
> > +obj-$(CONFIG_DRM_GE_B850V3_LVDS_DP) += ge_b850v3_lvds_dp.o
> > obj-$(CONFIG_DRM_NXP_PTN3460) += nxp-ptn3460.o
> > obj-$(CONFIG_DRM_PARADE_PS8622) += parade-ps8622.o
> > obj-$(CONFIG_DRM_ANALOGIX_DP) += analogix/
> > diff --git a/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> > new file mode 100644
> > index 0000000..c73cd77
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/ge_b850v3_lvds_dp.c
> > @@ -0,0 +1,392 @@
> > +/*
> > + * Driver for GE B850v3 DP display bridge
> > +
> > + * Copyright (c) 2016, Collabora Ltd.
> > + * Copyright (c) 2016, General Electric Company
> > +
> > + * This program is free software; you can redistribute it and/or modify it
> > + * under the terms and conditions of the GNU General Public License,
> > + * version 2, as published by the Free Software Foundation.
> > +
> > + * This program is distributed in the hope 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.
> > +
> > + * You should have received a copy of the GNU General Public License
> > + * along with this program. If not, see <http://www.gnu.org/licenses/>.
> > +
> > + * This driver creates a drm_bridge and a drm_connector for the LVDS to DP++
> > + * display bridge of the GE B850v3. There are two physical bridges on the video
> > + * signal pipeline: a STDP4028(LVDS to DP) and a STDP2690(DP to DP++). However
> > + * the physical bridges are automatically configured by the input video signal,
> > + * and the driver has no access to the video processing pipeline. The driver is
> > + * only needed to read EDID from the STDP2690 and to handle HPD events from the
> > + * STDP4028. The driver communicates with both bridges over i2c. The video
> > + * signal pipeline is as follows:
> > + *
> > + * Host -> LVDS|--(STDP4028)--|DP -> DP|--(STDP2690)--|DP++ -> Video output
> > +
> > + *
> > + */
> > +#include <linux/gpio.h>
> > +#include <linux/i2c.h>
> > +#include <linux/module.h>
> > +#include <linux/of.h>
> > +#include "drm_crtc.h"
> > +#include "drm_crtc_helper.h"
> > +#include "drm_edid.h"
> > +#include "drmP.h"
> > +
> > +#define EDID_EXT_BLOCK_CNT 0x7E
> > +
> > +#define STDP4028_IRQ_OUT_CONF_REG 0x02
> > +#define STDP4028_DPTX_IRQ_EN_REG 0x3C
> > +#define STDP4028_DPTX_IRQ_STS_REG 0x3D
> > +#define STDP4028_DPTX_STS_REG 0x3E
> > +
> > +#define STDP4028_DPTX_DP_IRQ_EN 0x1000
> > +
> > +#define STDP4028_DPTX_HOTPLUG_IRQ_EN 0x0400
> > +#define STDP4028_DPTX_LINK_CH_IRQ_EN 0x2000
> > +#define STDP4028_DPTX_IRQ_CONFIG \
> > + (STDP4028_DPTX_LINK_CH_IRQ_EN | STDP4028_DPTX_HOTPLUG_IRQ_EN)
> > +
> > +#define STDP4028_DPTX_HOTPLUG_STS 0x0200
> > +#define STDP4028_DPTX_LINK_STS 0x1000
> > +#define STDP4028_CON_STATE_CONNECTED \
> > + (STDP4028_DPTX_HOTPLUG_STS | STDP4028_DPTX_LINK_STS)
> > +
> > +#define STDP4028_DPTX_HOTPLUG_CH_STS 0x0400
> > +#define STDP4028_DPTX_LINK_CH_STS 0x2000
> > +#define STDP4028_DPTX_IRQ_CLEAR \
> > + (STDP4028_DPTX_LINK_CH_STS | STDP4028_DPTX_HOTPLUG_CH_STS)
> > +
> > +struct ge_b850v3_lvds_dp {
> > + struct drm_connector connector;
> > + struct drm_bridge bridge;
> > + struct i2c_client *ge_b850v3_lvds_dp_i2c;
> > + struct i2c_client *edid_i2c;
> > + struct edid *edid;
> > + struct mutex lock;
> > +};
> > +
> > +static inline struct ge_b850v3_lvds_dp *
> > + bridge_to_ge_b850v3_lvds_dp(struct drm_bridge *bridge)
> > +{
> > + return container_of(bridge, struct ge_b850v3_lvds_dp, bridge);
> > +}
> > +
> > +static inline struct ge_b850v3_lvds_dp *
> > + connector_to_ge_b850v3_lvds_dp(struct drm_connector *connector)
> > +{
> > + return container_of(connector, struct ge_b850v3_lvds_dp, connector);
> > +}
> > +
> > +static void ge_b850v3_lvds_dp_enable(struct drm_bridge *bridge)
> > +{
> > +}
> > +
>
> You can remove this function, see
> http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_bridge.c#L277
You are right, I removed the empty callbacks, but they returned due me doing something wrong when rebasing. V3 will be out soon.
>
> > +static void ge_b850v3_lvds_dp_disable(struct drm_bridge *bridge)
> > +{
> > +}
> > +
>
> And this one, see
> http://lxr.free-electrons.com/source/drivers/gpu/drm/drm_bridge.c#L182
Same. This should not be here.
>
> > +u8 *stdp2690_get_edid(struct i2c_client *client)
> > +{
> > + struct i2c_adapter *adapter = client->adapter;
> > + unsigned char start = 0x00;
> > + unsigned int total_size;
> > + u8 *block = kmalloc(EDID_LENGTH, GFP_KERNEL);
> > +
> > + struct i2c_msg msgs[] = {
> > + {
> > + .addr = client->addr,
> > + .flags = 0,
> > + .len = 1,
> > + .buf = &start,
> > + }, {
> > + .addr = client->addr,
> > + .flags = I2C_M_RD,
> > + .len = EDID_LENGTH,
> > + .buf = block,
> > + }
> > + };
> > +
> > + if (!block)
> > + return NULL;
> > +
> > + if (i2c_transfer(adapter, msgs, 2) != 2) {
> > + DRM_ERROR("Unable to read EDID.\n");
> > + goto err;
> > + }
> > +
> > + if (!drm_edid_block_valid(block, 0, false, NULL)) {
> > + DRM_ERROR("Invalid EDID block\n");
> > + goto err;
> > + }
> > +
> > + total_size = (block[EDID_EXT_BLOCK_CNT] + 1) * EDID_LENGTH;
> > + if (total_size > EDID_LENGTH) {
> > + kfree(block);
> > + block = kmalloc(total_size, GFP_KERNEL);
> > + if (!block)
> > + return NULL;
> > +
> > + /* Yes, read the entire buffer, and do not skip the first
> > + * EDID_LENGTH bytes.
> > + */
> > + start = 0x00;
> > + msgs[1].len = total_size;
> > + msgs[1].buf = block;
> > +
> > + if (i2c_transfer(adapter, msgs, 2) != 2) {
> > + DRM_ERROR("Unable to read EDID extension blocks.\n");
> > + goto err;
> > + }
> > + }
> > +
> > + return block;
> > +
> > +err:
> > + kfree(block);
> > + return NULL;
> > +}
> > +
> > +static int ge_b850v3_lvds_dp_get_modes(struct drm_connector *connector)
> > +{
> > + struct ge_b850v3_lvds_dp *ptn_bridge;
> > + struct i2c_client *client;
> > + int num_modes = 0;
> > +
> > + ptn_bridge = connector_to_ge_b850v3_lvds_dp(connector);
> > + client = ptn_bridge->edid_i2c;
> > +
> > + mutex_lock(&ptn_bridge->lock);
> > +
> > + kfree(ptn_bridge->edid);
> > + ptn_bridge->edid = (struct edid *) stdp2690_get_edid(client);
> > +
> > + if (ptn_bridge->edid) {
> > + drm_mode_connector_update_edid_property(connector,
> > + ptn_bridge->edid);
> > + num_modes = drm_add_edid_modes(connector, ptn_bridge->edid);
> > + }
> > +
> > + mutex_unlock(&ptn_bridge->lock);
> > +
> > + return num_modes;
> > +}
> > +
> > +static struct drm_encoder
> > +*ge_b850v3_lvds_dp_best_encoder(struct drm_connector *connector)
> > +{
> > + struct ge_b850v3_lvds_dp *ptn_bridge =
> > + connector_to_ge_b850v3_lvds_dp(connector);
> > +
> > + return ptn_bridge->bridge.encoder;
> > +}
> > +
> > +static const struct
> > +drm_connector_helper_funcs ge_b850v3_lvds_dp_connector_helper_funcs = {
> > + .get_modes = ge_b850v3_lvds_dp_get_modes,
> > + .best_encoder = ge_b850v3_lvds_dp_best_encoder,
> > +};
> > +
> > +static enum drm_connector_status ge_b850v3_lvds_dp_detect(
> > + struct drm_connector *connector, bool force)
> > +{
> > + struct ge_b850v3_lvds_dp *ptn_bridge =
> > + connector_to_ge_b850v3_lvds_dp(connector);
> > + struct i2c_client *ge_b850v3_lvds_dp_i2c =
> > + ptn_bridge->ge_b850v3_lvds_dp_i2c;
> > + s32 link_state;
> > +
> > + link_state = i2c_smbus_read_word_data(ge_b850v3_lvds_dp_i2c,
> > + STDP4028_DPTX_STS_REG);
> > +
> > + if (link_state == STDP4028_CON_STATE_CONNECTED)
> > + return connector_status_connected;
> > +
> > + if (link_state == 0)
> > + return connector_status_disconnected;
> > +
> > + return connector_status_unknown;
> > +}
> > +
> > +static void ge_b850v3_lvds_dp_connector_destroy(struct drm_connector *connector)
> > +{
> > + drm_connector_cleanup(connector);
> > +}
> > +
> > +static const struct drm_connector_funcs ge_b850v3_lvds_dp_connector_funcs = {
> > + .dpms = drm_helper_connector_dpms,
> > + .fill_modes = drm_helper_probe_single_connector_modes,
> > + .detect = ge_b850v3_lvds_dp_detect,
> > + .destroy = ge_b850v3_lvds_dp_connector_destroy,
> > +};
> > +
> > +static irqreturn_t ge_b850v3_lvds_dp_irq_handler(int irq, void *dev_id)
> > +{
> > + struct ge_b850v3_lvds_dp *ptn_bridge = dev_id;
> > + struct i2c_client *ge_b850v3_lvds_dp_i2c
> > + = ptn_bridge->ge_b850v3_lvds_dp_i2c;
> > +
> > + mutex_lock(&ptn_bridge->lock);
> > +
> > + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> > + STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
> > +
> > + mutex_unlock(&ptn_bridge->lock);
> > +
> > + if (ptn_bridge->connector.dev)
> > + drm_kms_helper_hotplug_event(ptn_bridge->connector.dev);
> > +
> > + return IRQ_HANDLED;
> > +}
> > +
> > +static int ge_b850v3_lvds_dp_attach(struct drm_bridge *bridge)
> > +{
> > + struct ge_b850v3_lvds_dp *ptn_bridge
> > + = bridge_to_ge_b850v3_lvds_dp(bridge);
> > + struct drm_connector *connector = &ptn_bridge->connector;
> > + struct i2c_client *ge_b850v3_lvds_dp_i2c
> > + = ptn_bridge->ge_b850v3_lvds_dp_i2c;
> > + int ret;
> > +
> > + if (!bridge->encoder) {
> > + DRM_ERROR("Parent encoder object not found");
> > + return -ENODEV;
> > + }
> > +
> > + connector->polled = DRM_CONNECTOR_POLL_HPD;
> > +
> > + drm_connector_helper_add(connector,
> > + &ge_b850v3_lvds_dp_connector_helper_funcs);
> > +
> > + ret = drm_connector_init(bridge->dev, connector,
> > + &ge_b850v3_lvds_dp_connector_funcs,
> > + DRM_MODE_CONNECTOR_DisplayPort);
> > + if (ret) {
> > + DRM_ERROR("Failed to initialize connector with drm\n");
> > + return ret;
> > + }
> > +
> > + ret = drm_mode_connector_attach_encoder(connector, bridge->encoder);
> > + if (ret)
> > + return ret;
> > +
> > + drm_bridge_enable(bridge);
> > + if (ge_b850v3_lvds_dp_i2c->irq) {
> > + drm_helper_hpd_irq_event(connector->dev);
> > +
> > + ret = devm_request_threaded_irq(&ge_b850v3_lvds_dp_i2c->dev,
> > + ge_b850v3_lvds_dp_i2c->irq, NULL,
> > + ge_b850v3_lvds_dp_irq_handler,
> > + IRQF_TRIGGER_HIGH | IRQF_ONESHOT,
> > + "ge-b850v3-lvds-dp", ptn_bridge);
> > + if (ret)
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static const struct drm_bridge_funcs ge_b850v3_lvds_dp_funcs = {
> > + .enable = ge_b850v3_lvds_dp_enable,
> > + .disable = ge_b850v3_lvds_dp_disable,
>
> Remove the above empty callbacks.
Here too.
>
> > + .attach = ge_b850v3_lvds_dp_attach,
> > +};
> > +
> > +static int ge_b850v3_lvds_dp_probe(struct i2c_client *ge_b850v3_lvds_dp_i2c,
> > + const struct i2c_device_id *id)
> > +{
> > + struct device *dev = &ge_b850v3_lvds_dp_i2c->dev;
> > + struct ge_b850v3_lvds_dp *ptn_bridge;
> > + int ret;
> > + u32 edid_i2c_reg;
> > +
> > + ptn_bridge = devm_kzalloc(dev, sizeof(*ptn_bridge), GFP_KERNEL);
> > + if (!ptn_bridge)
> > + return -ENOMEM;
> > +
> > + mutex_init(&ptn_bridge->lock);
> > +
> > + ptn_bridge->ge_b850v3_lvds_dp_i2c = ge_b850v3_lvds_dp_i2c;
> > + ptn_bridge->bridge.driver_private = ptn_bridge;
> > + i2c_set_clientdata(ge_b850v3_lvds_dp_i2c, ptn_bridge);
> > +
> > + ret = of_property_read_u32(dev->of_node, "edid-reg", &edid_i2c_reg);
> > + if (ret) {
> > + dev_err(dev, "edid-reg not specified, aborting...\n");
> > + return -ENODEV;
> > + }
> > +
> > + ptn_bridge->edid_i2c = devm_kzalloc(dev,
> > + sizeof(struct i2c_client), GFP_KERNEL);
> > +
> > + if (!ptn_bridge->edid_i2c)
> > + return -ENOMEM;
> > +
> > + memcpy(ptn_bridge->edid_i2c, ge_b850v3_lvds_dp_i2c,
> > + sizeof(struct i2c_client));
> > +
> > + ptn_bridge->edid_i2c->addr = (unsigned short) edid_i2c_reg;
> > +
> > + /* Configures the bridge to re-enable interrupts after each ack */
> > + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> > + STDP4028_IRQ_OUT_CONF_REG, STDP4028_DPTX_DP_IRQ_EN);
> > + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> > + STDP4028_DPTX_IRQ_EN_REG, STDP4028_DPTX_IRQ_CONFIG);
> > +
> > + /* Clear pending interrupts since power up. */
> > + i2c_smbus_write_word_data(ge_b850v3_lvds_dp_i2c,
> > + STDP4028_DPTX_IRQ_STS_REG, STDP4028_DPTX_IRQ_CLEAR);
> > +
> > + ptn_bridge->bridge.funcs = &ge_b850v3_lvds_dp_funcs;
> > + ptn_bridge->bridge.of_node = dev->of_node;
> > + ret = drm_bridge_add(&ptn_bridge->bridge);
> > + if (ret) {
> > + DRM_ERROR("Failed to add bridge\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int ge_b850v3_lvds_dp_remove(struct i2c_client *ge_b850v3_lvds_dp_i2c)
> > +{
> > + struct ge_b850v3_lvds_dp *ptn_bridge =
> > + i2c_get_clientdata(ge_b850v3_lvds_dp_i2c);
> > +
> > + drm_bridge_remove(&ptn_bridge->bridge);
>
> Guess you need to free ptn_bridge->edid here.
Thanks a lot! I'll fix it when sending V3.
Thank you for the review!
[toc] | [prev] | [next] | [standalone]
Page 1 of 2 [1] 2 Next page →
Back to top | Article view | linux.kernel
csiph-web