linux-media.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
@ 2017-07-11 13:30 Hans Verkuil
  2017-07-11 13:30 ` [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX Hans Verkuil
                   ` (3 more replies)
  0 siblings, 4 replies; 9+ messages in thread
From: Hans Verkuil @ 2017-07-11 13:30 UTC (permalink / raw)
  To: linux-media; +Cc: dri-devel, Daniel Vetter

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
feature. This patch series is based on the latest mainline kernel (as of today)
which has all the needed cec and drm 4.13 patches merged.

This patch series has been tested with my NUC7i5BNK and a Samsung USB-C to 
HDMI adapter.

Please note this comment at the start of drm_dp_cec.c:

----------------------------------------------------------------------
Unfortunately it turns out that we have a chicken-and-egg situation
here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
have a converter chip that supports CEC-Tunneling-over-AUX (usually the
Parade PS176), but they do not wire up the CEC pin, thus making CEC
useless.

Sadly there is no way for this driver to know this. What happens is 
that a /dev/cecX device is created that is isolated and unable to see
any of the other CEC devices. Quite literally the CEC wire is cut
(or in this case, never connected in the first place).

I suspect that the reason so few adapters support this is that this
tunneling protocol was never supported by any OS. So there was no 
easy way of testing it, and no incentive to correctly wire up the
CEC pin.

Hopefully by creating this driver it will be easier for vendors to 
finally fix their adapters and test the CEC functionality.

I keep a list of known working adapters here:

https://hverkuil.home.xs4all.nl/cec-status.txt

Please mail me (hverkuil@xs4all.nl) if you find an adapter that works
and is not yet listed there.
----------------------------------------------------------------------

I really hope that this work will provide an incentive for vendors to
finally connect the CEC pin. It's a shame that there are so few adapters
that work (I found only two USB-C to HDMI adapters that work, and no
(mini-)DP to HDMI adapters at all).

Daniel, I incorporated all your suggestions/comments from the RFC patch
series from about 2 months ago.

Regards,

        Hans

Hans Verkuil (3):
  drm: add support for DisplayPort CEC-Tunneling-over-AUX
  drm-kms-helpers.rst: document the DP CEC helpers
  drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

 Documentation/gpu/drm-kms-helpers.rst |   9 +
 drivers/gpu/drm/Kconfig               |  10 ++
 drivers/gpu/drm/Makefile              |   1 +
 drivers/gpu/drm/drm_dp_cec.c          | 308 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c       |  18 +-
 include/drm/drm_dp_helper.h           |  24 +++
 6 files changed, 366 insertions(+), 4 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_dp_cec.c

-- 
2.11.0

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX
  2017-07-11 13:30 [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
@ 2017-07-11 13:30 ` Hans Verkuil
  2017-08-10  0:34   ` Carlos Santa
  2017-08-10 19:19   ` Sean Paul
  2017-07-11 13:30 ` [PATCH 2/3] drm-kms-helpers.rst: document the DP CEC helpers Hans Verkuil
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 9+ messages in thread
From: Hans Verkuil @ 2017-07-11 13:30 UTC (permalink / raw)
  To: linux-media; +Cc: dri-devel, Daniel Vetter, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

This adds support for the DisplayPort CEC-Tunneling-over-AUX
feature that is part of the DisplayPort 1.3 standard.

Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a
chip that has this capability actually hook up the CEC pin, so
even though a CEC device is created, it may not actually work.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/gpu/drm/Kconfig      |  10 ++
 drivers/gpu/drm/Makefile     |   1 +
 drivers/gpu/drm/drm_dp_cec.c | 308 +++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h  |  24 ++++
 4 files changed, 343 insertions(+)
 create mode 100644 drivers/gpu/drm/drm_dp_cec.c

diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 83cb2a88c204..1f2708df5c4e 100644
--- a/drivers/gpu/drm/Kconfig
+++ b/drivers/gpu/drm/Kconfig
@@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE
 	  default case is N. Details and instructions how to build your own
 	  EDID data are given in Documentation/EDID/HOWTO.txt.
 
+config DRM_DP_CEC
+	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
+	select CEC_CORE
+	help
+	  Choose this option if you want to enable HDMI CEC support for
+	  DisplayPort/USB-C to HDMI adapters.
+
+	  Note: not all adapters support this feature, and even for those
+	  that do support this they often do not hook up the CEC pin.
+
 config DRM_TTM
 	tristate
 	depends on DRM && MMU
diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 24a066e1841c..c6552c62049e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
 drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
+drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
 
 obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
 obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
new file mode 100644
index 000000000000..7f2e298d45c0
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_cec.c
@@ -0,0 +1,308 @@
+/*
+ * DisplayPort CEC-Tunneling-over-AUX support
+ *
+ * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ *
+ * This program is free software; you may redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; version 2 of the License.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
+ * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
+ * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
+ * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
+ * SOFTWARE.
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/slab.h>
+#include <drm/drm_dp_helper.h>
+#include <media/cec.h>
+
+/*
+ * Unfortunately it turns out that we have a chicken-and-egg situation
+ * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
+ * have a converter chip that supports CEC-Tunneling-over-AUX (usually the
+ * Parade PS176), but they do not wire up the CEC pin, thus making CEC
+ * useless.
+ *
+ * Sadly there is no way for this driver to know this. What happens is
+ * that a /dev/cecX device is created that is isolated and unable to see
+ * any of the other CEC devices. Quite literally the CEC wire is cut
+ * (or in this case, never connected in the first place).
+ *
+ * I suspect that the reason so few adapters support this is that this
+ * tunneling protocol was never supported by any OS. So there was no
+ * easy way of testing it, and no incentive to correctly wire up the
+ * CEC pin.
+ *
+ * Hopefully by creating this driver it will be easier for vendors to
+ * finally fix their adapters and test the CEC functionality.
+ *
+ * I keep a list of known working adapters here:
+ *
+ * https://hverkuil.home.xs4all.nl/cec-status.txt
+ *
+ * Please mail me (hverkuil@xs4all.nl) if you find an adapter that works
+ * and is not yet listed there.
+ */
+
+/**
+ * DOC: dp cec helpers
+ *
+ * These functions take care of supporting the CEC-Tunneling-over-AUX
+ * feature of DisplayPort-to-HDMI adapters.
+ */
+
+static int drm_dp_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+	struct drm_dp_aux *aux = cec_get_drvdata(adap);
+	ssize_t err = 0;
+
+	if (enable)
+		err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL,
+					 DP_CEC_TUNNELING_ENABLE);
+	else
+		err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, 0);
+	return (enable && err < 0) ? err : 0;
+}
+
+static int drm_dp_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
+{
+	struct drm_dp_aux *aux = cec_get_drvdata(adap);
+	u8 mask[2] = { 0x00, 0x80 };
+	ssize_t err;
+
+	if (addr != CEC_LOG_ADDR_INVALID) {
+		u16 la_mask = adap->log_addrs.log_addr_mask;
+
+		la_mask |= 0x8000 | (1 << addr);
+		mask[0] = la_mask & 0xff;
+		mask[1] = la_mask >> 8;
+	}
+	err = drm_dp_dpcd_write(aux, DP_CEC_LOGICAL_ADDRESS_MASK, mask, 2);
+	return (addr != CEC_LOG_ADDR_INVALID && err < 0) ? err : 0;
+}
+
+static int drm_dp_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+				    u32 signal_free_time, struct cec_msg *msg)
+{
+	struct drm_dp_aux *aux = cec_get_drvdata(adap);
+	unsigned int retries = attempts - 1;
+	ssize_t err;
+
+	err = drm_dp_dpcd_write(aux, DP_CEC_TX_MESSAGE_BUFFER,
+				msg->msg, msg->len);
+	if (err < 0)
+		return err;
+
+	if (retries > 5)
+		retries = 5;
+	err = drm_dp_dpcd_writeb(aux, DP_CEC_TX_MESSAGE_INFO,
+				 (msg->len - 1) | (retries << 4) |
+				 DP_CEC_TX_MESSAGE_SEND);
+	return err < 0 ? err : 0;
+}
+
+static int drm_dp_cec_adap_monitor_all_enable(struct cec_adapter *adap,
+					      bool enable)
+{
+	struct drm_dp_aux *aux = cec_get_drvdata(adap);
+	ssize_t err;
+	u8 val;
+
+	if (!(adap->capabilities & CEC_CAP_MONITOR_ALL))
+		return 0;
+
+	err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CONTROL, &val);
+	if (err >= 0) {
+		if (enable)
+			val |= DP_CEC_SNOOPING_ENABLE;
+		else
+			val &= ~DP_CEC_SNOOPING_ENABLE;
+		err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, val);
+	}
+	return (enable && err < 0) ? err : 0;
+}
+
+static void drm_dp_cec_adap_status(struct cec_adapter *adap,
+				   struct seq_file *file)
+{
+	struct drm_dp_aux *aux = cec_get_drvdata(adap);
+	u8 buf[DP_AUX_MAX_PAYLOAD_BYTES];
+	u8 hw_rev;
+
+	if (drm_dp_dpcd_read(aux, DP_BRANCH_OUI,
+			     buf, sizeof(buf)) != sizeof(buf))
+		return;
+	hw_rev = buf[9];
+	buf[9] = 0;
+	seq_printf(file, "OUI: %02x-%02x-%02x\n",
+		   buf[0], buf[1], buf[2]);
+	seq_printf(file, "ID: %s\n", buf + 3);
+	seq_printf(file, "HW Rev: %d.%d\n", hw_rev >> 4, hw_rev & 0xf);
+	/*
+	 * Show this both in decimal and hex: at least one vendor
+	 * always reports this in hex.
+	 */
+	seq_printf(file, "FW/SW Rev: %d.%d (0x%02x.0x%02x)\n",
+		   buf[10], buf[11], buf[10], buf[11]);
+}
+
+static const struct cec_adap_ops drm_dp_cec_adap_ops = {
+	.adap_enable = drm_dp_cec_adap_enable,
+	.adap_log_addr = drm_dp_cec_adap_log_addr,
+	.adap_transmit = drm_dp_cec_adap_transmit,
+	.adap_monitor_all_enable = drm_dp_cec_adap_monitor_all_enable,
+	.adap_status = drm_dp_cec_adap_status,
+};
+
+static int drm_dp_cec_received(struct drm_dp_aux *aux)
+{
+	struct cec_adapter *adap = aux->cec_adap;
+	struct cec_msg msg;
+	u8 rx_msg_info;
+	ssize_t err;
+
+	err = drm_dp_dpcd_readb(aux, DP_CEC_RX_MESSAGE_INFO, &rx_msg_info);
+	if (err < 0)
+		return err;
+	if (!(rx_msg_info & DP_CEC_RX_MESSAGE_ENDED))
+		return 0;
+	msg.len = (rx_msg_info & DP_CEC_RX_MESSAGE_LEN_MASK) + 1;
+	err = drm_dp_dpcd_read(aux, DP_CEC_RX_MESSAGE_BUFFER, msg.msg, msg.len);
+	if (err < 0)
+		return err;
+	cec_received_msg(adap, &msg);
+	return 0;
+}
+
+static int drm_dp_cec_handle_irq(struct drm_dp_aux *aux)
+{
+	struct cec_adapter *adap = aux->cec_adap;
+	u8 flags;
+	ssize_t err;
+
+	err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, &flags);
+	if (err < 0)
+		return err;
+
+	if (flags & DP_CEC_RX_MESSAGE_INFO_VALID)
+		drm_dp_cec_received(aux);
+
+	if (flags & DP_CEC_TX_MESSAGE_SENT)
+		cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK);
+	else if (flags & DP_CEC_TX_LINE_ERROR)
+		cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR |
+						CEC_TX_STATUS_MAX_RETRIES);
+	else if (flags &
+		 (DP_CEC_TX_ADDRESS_NACK_ERROR | DP_CEC_TX_DATA_NACK_ERROR))
+		cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK |
+						CEC_TX_STATUS_MAX_RETRIES);
+	drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, flags);
+	return 0;
+}
+
+/**
+ * drm_dp_cec_irq() - handle CEC interrupt, if any
+ * @aux: DisplayPort AUX channel
+ *
+ * Should be called when handling an IRQ_HPD request. If CEC-tunneling-over-AUX
+ * is present, then it will check for a CEC_IRQ and handle it accordingly.
+ *
+ * Returns true if an interrupt was handled successfully or false otherwise.
+ */
+bool drm_dp_cec_irq(struct drm_dp_aux *aux)
+{
+	int attempts = 4;
+	bool handled = false;
+
+	if (!aux->cec_adap)
+		return false;
+
+	while (attempts--) {
+		u8 cec_irq;
+		int ret;
+
+		ret = drm_dp_dpcd_readb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
+					&cec_irq);
+		if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
+			break;
+
+		if (!drm_dp_cec_handle_irq(aux))
+			handled = true;
+
+		ret = drm_dp_dpcd_writeb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
+					 DP_CEC_IRQ);
+		if (ret < 0)
+			break;
+	}
+	return handled;
+}
+EXPORT_SYMBOL(drm_dp_cec_irq);
+
+/**
+ * drm_dp_cec_configure_adapter() - configure the CEC adapter
+ * @aux: DisplayPort AUX channel
+ * @name: name of the CEC device
+ * @parent: parent device
+ *
+ * Checks if this is a DisplayPort-to-HDMI adapter that supports
+ * CEC-tunneling-over-AUX, and if so it creates a CEC device.
+ *
+ * If a CEC device was already created, then check if the capabilities
+ * have changed. If not, then do nothing. Otherwise destroy the old
+ * CEC device and create a new CEC device.
+ *
+ * This can happen when one DP-to-HDMI adapter is disconnected and
+ * replaced by another adapter with different CEC capabilities.
+ *
+ * Returns 0 on success or a negative error code on failure.
+ */
+int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char *name,
+				 struct device *parent)
+{
+	u32 cec_caps = CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
+		       CEC_CAP_PASSTHROUGH | CEC_CAP_RC | CEC_CAP_NEEDS_HPD;
+	unsigned int num_las = 1;
+	int err;
+	u8 cap;
+
+	if (drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CAPABILITY, &cap) != 1 ||
+	    !(cap & DP_CEC_TUNNELING_CAPABLE)) {
+		cec_unregister_adapter(aux->cec_adap);
+		aux->cec_adap = NULL;
+		return -ENODEV;
+	}
+
+	if (cap & DP_CEC_SNOOPING_CAPABLE)
+		cec_caps |= CEC_CAP_MONITOR_ALL;
+	if (cap & DP_CEC_MULTIPLE_LA_CAPABLE)
+		num_las = CEC_MAX_LOG_ADDRS;
+
+	if (!IS_ERR_OR_NULL(aux->cec_adap)) {
+		if (aux->cec_adap->capabilities == cec_caps &&
+		    aux->cec_adap->available_log_addrs == num_las)
+			return 0;
+		cec_unregister_adapter(aux->cec_adap);
+	}
+
+	aux->cec_adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
+			 aux, name, cec_caps, num_las);
+	if (IS_ERR(aux->cec_adap)) {
+		err = PTR_ERR(aux->cec_adap);
+		aux->cec_adap = NULL;
+		return err;
+	}
+	err = cec_register_adapter(aux->cec_adap, parent);
+	if (err) {
+		cec_delete_adapter(aux->cec_adap);
+		aux->cec_adap = NULL;
+	}
+	return err;
+}
+EXPORT_SYMBOL(drm_dp_cec_configure_adapter);
diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
index b17476a6909c..0e236dd40b42 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -952,6 +952,8 @@ struct drm_dp_aux_msg {
 	size_t size;
 };
 
+struct cec_adapter;
+
 /**
  * struct drm_dp_aux - DisplayPort AUX channel
  * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
@@ -1010,6 +1012,10 @@ struct drm_dp_aux {
 	 * @i2c_defer_count: Counts I2C DEFERs, used for DP validation.
 	 */
 	unsigned i2c_defer_count;
+	/**
+	 * @cec_adap: the CEC adapter for CEC-Tunneling-over-AUX support.
+	 */
+	struct cec_adapter *cec_adap;
 };
 
 ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
@@ -1132,4 +1138,22 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
 	return desc->quirks & BIT(quirk);
 }
 
+#ifdef CONFIG_DRM_DP_CEC
+bool drm_dp_cec_irq(struct drm_dp_aux *aux);
+int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char *name,
+				 struct device *parent);
+#else
+static inline bool drm_dp_cec_irq(struct drm_dp_aux *aux)
+{
+	return false;
+}
+
+static inline int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux,
+					       const char *name,
+					       struct device *parent)
+{
+	return -ENODEV;
+}
+#endif
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] drm-kms-helpers.rst: document the DP CEC helpers
  2017-07-11 13:30 [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
  2017-07-11 13:30 ` [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX Hans Verkuil
@ 2017-07-11 13:30 ` Hans Verkuil
  2017-07-11 13:30 ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
  2017-08-09 10:01 ` [PATCH 0/3] " Hans Verkuil
  3 siblings, 0 replies; 9+ messages in thread
From: Hans Verkuil @ 2017-07-11 13:30 UTC (permalink / raw)
  To: linux-media; +Cc: dri-devel, Daniel Vetter, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Document the Display Port CEC helper functions.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/gpu/drm-kms-helpers.rst | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/gpu/drm-kms-helpers.rst b/Documentation/gpu/drm-kms-helpers.rst
index 7c5e2549a58a..0d2fa879edd1 100644
--- a/Documentation/gpu/drm-kms-helpers.rst
+++ b/Documentation/gpu/drm-kms-helpers.rst
@@ -175,6 +175,15 @@ Display Port Helper Functions Reference
 .. kernel-doc:: drivers/gpu/drm/drm_dp_helper.c
    :export:
 
+Display Port CEC Helper Functions Reference
+===========================================
+
+.. kernel-doc:: drivers/gpu/drm/drm_dp_cec.c
+   :doc: dp cec helpers
+
+.. kernel-doc:: drivers/gpu/drm/drm_dp_cec.c
+   :export:
+
 Display Port Dual Mode Adaptor Helper Functions Reference
 =========================================================
 
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
  2017-07-11 13:30 [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
  2017-07-11 13:30 ` [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX Hans Verkuil
  2017-07-11 13:30 ` [PATCH 2/3] drm-kms-helpers.rst: document the DP CEC helpers Hans Verkuil
@ 2017-07-11 13:30 ` Hans Verkuil
  2017-07-12 20:07   ` [PATCH for 4.13] cec: cec_transmit_attempt_done: ignore CEC_TX_STATUS_MAX_RETRIES Hans Verkuil
  2017-08-10  0:39   ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Carlos Santa
  2017-08-09 10:01 ` [PATCH 0/3] " Hans Verkuil
  3 siblings, 2 replies; 9+ messages in thread
From: Hans Verkuil @ 2017-07-11 13:30 UTC (permalink / raw)
  To: linux-media; +Cc: dri-devel, Daniel Vetter, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Implement support for this DisplayPort feature.

The cec device is created whenever it detects an adapter that
has this feature. It is only removed when a new adapter is connected
that does not support this. If a new adapter is connected that has
different properties than the previous one, then the old cec device is
unregistered and a new one is registered to replace the old one.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 64fa774c855b..fdb853d2c458 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -32,6 +32,7 @@
 #include <linux/notifier.h>
 #include <linux/reboot.h>
 #include <asm/byteorder.h>
+#include <media/cec.h>
 #include <drm/drmP.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
@@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp *intel_dp)
 static void
 intel_dp_aux_fini(struct intel_dp *intel_dp)
 {
+	cec_unregister_adapter(intel_dp->aux.cec_adap);
 	kfree(intel_dp->aux.name);
 }
 
@@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
 	intel_connector->detect_edid = edid;
 
 	intel_dp->has_audio = drm_detect_monitor_audio(edid);
+	cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
 }
 
 static void
@@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 
 	kfree(intel_connector->detect_edid);
 	intel_connector->detect_edid = NULL;
+	cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
 
 	intel_dp->has_audio = false;
 }
@@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
 
 	/* Can't disconnect eDP, but you can close the lid... */
-	if (is_edp(intel_dp))
+	if (is_edp(intel_dp)) {
 		status = edp_detect(intel_dp);
-	else if (intel_digital_port_connected(to_i915(dev),
-					      dp_to_dig_port(intel_dp)))
+	} else if (intel_digital_port_connected(to_i915(dev),
+						dp_to_dig_port(intel_dp))) {
 		status = intel_dp_detect_dpcd(intel_dp);
-	else
+		if (status == connector_status_connected)
+			drm_dp_cec_configure_adapter(&intel_dp->aux,
+				     intel_dp->aux.name, dev->dev);
+	} else {
 		status = connector_status_disconnected;
+	}
 
 	if (status == connector_status_disconnected) {
 		memset(&intel_dp->compliance, 0, sizeof(intel_dp->compliance));
@@ -5011,6 +5019,8 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 
 	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
+	drm_dp_cec_irq(&intel_dp->aux);
+
 	if (intel_dp->is_mst) {
 		if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
 			/*
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH for 4.13] cec: cec_transmit_attempt_done: ignore CEC_TX_STATUS_MAX_RETRIES
  2017-07-11 13:30 ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
@ 2017-07-12 20:07   ` Hans Verkuil
  2017-08-10  0:39   ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Carlos Santa
  1 sibling, 0 replies; 9+ messages in thread
From: Hans Verkuil @ 2017-07-12 20:07 UTC (permalink / raw)
  To: linux-media; +Cc: dri-devel, Daniel Vetter, Hans Verkuil

The switch in cec_transmit_attempt_done() should ignore the
CEC_TX_STATUS_MAX_RETRIES status bit.

Calling this function with e.g. CEC_TX_STATUS_NACK | CEC_TX_STATUS_MAX_RETRIES
is perfectly legal and should not trigger the WARN(1).

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
After testing the DisplayPort CEC-Tunneling-over-AUX support with an adapter
that didn't hook up the CEC pin correctly I found a bug in this CEC core function
that is corrected with this patch.
---
 drivers/media/cec/cec-adap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index bf45977b2823..d596b601ff42 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -559,7 +559,7 @@ EXPORT_SYMBOL_GPL(cec_transmit_done);

 void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status)
 {
-	switch (status) {
+	switch (status & ~CEC_TX_STATUS_MAX_RETRIES) {
 	case CEC_TX_STATUS_OK:
 		cec_transmit_done(adap, status, 0, 0, 0, 0);
 		return;
-- 
2.11.0

^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
  2017-07-11 13:30 [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
                   ` (2 preceding siblings ...)
  2017-07-11 13:30 ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
@ 2017-08-09 10:01 ` Hans Verkuil
  3 siblings, 0 replies; 9+ messages in thread
From: Hans Verkuil @ 2017-08-09 10:01 UTC (permalink / raw)
  To: dri-devel; +Cc: linux-media, Daniel Vetter, David Airlie

On 11/07/17 15:30, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
> feature. This patch series is based on the latest mainline kernel (as of today)
> which has all the needed cec and drm 4.13 patches merged.
> 
> This patch series has been tested with my NUC7i5BNK and a Samsung USB-C to 
> HDMI adapter.

Ping?

> Please note this comment at the start of drm_dp_cec.c:
> 
> ----------------------------------------------------------------------
> Unfortunately it turns out that we have a chicken-and-egg situation
> here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
> have a converter chip that supports CEC-Tunneling-over-AUX (usually the
> Parade PS176), but they do not wire up the CEC pin, thus making CEC
> useless.
> 
> Sadly there is no way for this driver to know this. What happens is 
> that a /dev/cecX device is created that is isolated and unable to see
> any of the other CEC devices. Quite literally the CEC wire is cut
> (or in this case, never connected in the first place).
> 
> I suspect that the reason so few adapters support this is that this
> tunneling protocol was never supported by any OS. So there was no 
> easy way of testing it, and no incentive to correctly wire up the
> CEC pin.

I got confirmation of this suspicion. This is indeed the reason why so
few adapters hook this up. Having native linux support for this feature
will definitely help the adoption of CEC over DP.

Regards,

	Hans

> Hopefully by creating this driver it will be easier for vendors to 
> finally fix their adapters and test the CEC functionality.
> 
> I keep a list of known working adapters here:
> 
> https://hverkuil.home.xs4all.nl/cec-status.txt
> 
> Please mail me (hverkuil@xs4all.nl) if you find an adapter that works
> and is not yet listed there.
> ----------------------------------------------------------------------
> 
> I really hope that this work will provide an incentive for vendors to
> finally connect the CEC pin. It's a shame that there are so few adapters
> that work (I found only two USB-C to HDMI adapters that work, and no
> (mini-)DP to HDMI adapters at all).
> 
> Daniel, I incorporated all your suggestions/comments from the RFC patch
> series from about 2 months ago.
> 
> Regards,
> 
>         Hans
> 
> Hans Verkuil (3):
>   drm: add support for DisplayPort CEC-Tunneling-over-AUX
>   drm-kms-helpers.rst: document the DP CEC helpers
>   drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
> 
>  Documentation/gpu/drm-kms-helpers.rst |   9 +
>  drivers/gpu/drm/Kconfig               |  10 ++
>  drivers/gpu/drm/Makefile              |   1 +
>  drivers/gpu/drm/drm_dp_cec.c          | 308 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_dp.c       |  18 +-
>  include/drm/drm_dp_helper.h           |  24 +++
>  6 files changed, 366 insertions(+), 4 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_dp_cec.c
> 

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX
  2017-07-11 13:30 ` [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX Hans Verkuil
@ 2017-08-10  0:34   ` Carlos Santa
  2017-08-10 19:19   ` Sean Paul
  1 sibling, 0 replies; 9+ messages in thread
From: Carlos Santa @ 2017-08-10  0:34 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Daniel Vetter, Hans Verkuil, dri-devel

On Tue, 2017-07-11 at 15:30 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This adds support for the DisplayPort CEC-Tunneling-over-AUX
> feature that is part of the DisplayPort 1.3 standard.
> 
> Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a
> chip that has this capability actually hook up the CEC pin, so
> even though a CEC device is created, it may not actually work.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---

This was tested internally using the EE-PW700 adapter (DisplayPort/USB-
C to HDMI) from Samsung on a Skylake platform.

Several commands were tested including: 

# cec-ctl --playback

# turns the TV off
# cec-ctl -t=0 --standby

# turns the TV on
# cec-ctl -t=0 --image-view-on

The cec-compliance binary was also run and it reported no failures.

Tested-by: Carlos Santa <carlos.santa@intel.com>


>  drivers/gpu/drm/Kconfig      |  10 ++
>  drivers/gpu/drm/Makefile     |   1 +
>  drivers/gpu/drm/drm_dp_cec.c | 308
> +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_helper.h  |  24 ++++
>  4 files changed, 343 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_dp_cec.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 83cb2a88c204..1f2708df5c4e 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE
>  	  default case is N. Details and instructions how to build
> your own
>  	  EDID data are given in Documentation/EDID/HOWTO.txt.
>  
> +config DRM_DP_CEC
> +	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI
> support"
> +	select CEC_CORE
> +	help
> +	  Choose this option if you want to enable HDMI CEC support
> for
> +	  DisplayPort/USB-C to HDMI adapters.
> +
> +	  Note: not all adapters support this feature, and even for
> those
> +	  that do support this they often do not hook up the CEC
> pin.
> +
>  config DRM_TTM
>  	tristate
>  	depends on DRM && MMU
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 24a066e1841c..c6552c62049e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) +=
> drm_edid_load.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> +drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>  obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
> diff --git a/drivers/gpu/drm/drm_dp_cec.c
> b/drivers/gpu/drm/drm_dp_cec.c
> new file mode 100644
> index 000000000000..7f2e298d45c0
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_cec.c
> @@ -0,0 +1,308 @@
> +/*
> + * DisplayPort CEC-Tunneling-over-AUX support
> + *
> + * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All
> rights reserved.
> + *
> + * This program is free software; you may redistribute it and/or
> modify
> + * it under the terms of the GNU General Public License as published
> by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN
> AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <drm/drm_dp_helper.h>
> +#include <media/cec.h>
> +
> +/*
> + * Unfortunately it turns out that we have a chicken-and-egg
> situation
> + * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI
> adapters
> + * have a converter chip that supports CEC-Tunneling-over-AUX
> (usually the
> + * Parade PS176), but they do not wire up the CEC pin, thus making
> CEC
> + * useless.
> + *
> + * Sadly there is no way for this driver to know this. What happens
> is
> + * that a /dev/cecX device is created that is isolated and unable to
> see
> + * any of the other CEC devices. Quite literally the CEC wire is cut
> + * (or in this case, never connected in the first place).
> + *
> + * I suspect that the reason so few adapters support this is that
> this
> + * tunneling protocol was never supported by any OS. So there was no
> + * easy way of testing it, and no incentive to correctly wire up the
> + * CEC pin.
> + *
> + * Hopefully by creating this driver it will be easier for vendors
> to
> + * finally fix their adapters and test the CEC functionality.
> + *
> + * I keep a list of known working adapters here:
> + *
> + * https://hverkuil.home.xs4all.nl/cec-status.txt
> + *
> + * Please mail me (hverkuil@xs4all.nl) if you find an adapter that
> works
> + * and is not yet listed there.
> + */
> +
> +/**
> + * DOC: dp cec helpers
> + *
> + * These functions take care of supporting the CEC-Tunneling-over-
> AUX
> + * feature of DisplayPort-to-HDMI adapters.
> + */
> +
> +static int drm_dp_cec_adap_enable(struct cec_adapter *adap, bool
> enable)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	ssize_t err = 0;
> +
> +	if (enable)
> +		err = drm_dp_dpcd_writeb(aux,
> DP_CEC_TUNNELING_CONTROL,
> +					 DP_CEC_TUNNELING_ENABLE);
> +	else
> +		err = drm_dp_dpcd_writeb(aux,
> DP_CEC_TUNNELING_CONTROL, 0);
> +	return (enable && err < 0) ? err : 0;
> +}
> +
> +static int drm_dp_cec_adap_log_addr(struct cec_adapter *adap, u8
> addr)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	u8 mask[2] = { 0x00, 0x80 };
> +	ssize_t err;
> +
> +	if (addr != CEC_LOG_ADDR_INVALID) {
> +		u16 la_mask = adap->log_addrs.log_addr_mask;
> +
> +		la_mask |= 0x8000 | (1 << addr);
> +		mask[0] = la_mask & 0xff;
> +		mask[1] = la_mask >> 8;
> +	}
> +	err = drm_dp_dpcd_write(aux, DP_CEC_LOGICAL_ADDRESS_MASK,
> mask, 2);
> +	return (addr != CEC_LOG_ADDR_INVALID && err < 0) ? err : 0;
> +}
> +
> +static int drm_dp_cec_adap_transmit(struct cec_adapter *adap, u8
> attempts,
> +				    u32 signal_free_time, struct
> cec_msg *msg)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	unsigned int retries = attempts - 1;
> +	ssize_t err;
> +
> +	err = drm_dp_dpcd_write(aux, DP_CEC_TX_MESSAGE_BUFFER,
> +				msg->msg, msg->len);
> +	if (err < 0)
> +		return err;
> +
> +	if (retries > 5)
> +		retries = 5;
> +	err = drm_dp_dpcd_writeb(aux, DP_CEC_TX_MESSAGE_INFO,
> +				 (msg->len - 1) | (retries << 4) |
> +				 DP_CEC_TX_MESSAGE_SEND);
> +	return err < 0 ? err : 0;
> +}
> +
> +static int drm_dp_cec_adap_monitor_all_enable(struct cec_adapter
> *adap,
> +					      bool enable)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	ssize_t err;
> +	u8 val;
> +
> +	if (!(adap->capabilities & CEC_CAP_MONITOR_ALL))
> +		return 0;
> +
> +	err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CONTROL,
> &val);
> +	if (err >= 0) {
> +		if (enable)
> +			val |= DP_CEC_SNOOPING_ENABLE;
> +		else
> +			val &= ~DP_CEC_SNOOPING_ENABLE;
> +		err = drm_dp_dpcd_writeb(aux,
> DP_CEC_TUNNELING_CONTROL, val);
> +	}
> +	return (enable && err < 0) ? err : 0;
> +}
> +
> +static void drm_dp_cec_adap_status(struct cec_adapter *adap,
> +				   struct seq_file *file)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	u8 buf[DP_AUX_MAX_PAYLOAD_BYTES];
> +	u8 hw_rev;
> +
> +	if (drm_dp_dpcd_read(aux, DP_BRANCH_OUI,
> +			     buf, sizeof(buf)) != sizeof(buf))
> +		return;
> +	hw_rev = buf[9];
> +	buf[9] = 0;
> +	seq_printf(file, "OUI: %02x-%02x-%02x\n",
> +		   buf[0], buf[1], buf[2]);
> +	seq_printf(file, "ID: %s\n", buf + 3);
> +	seq_printf(file, "HW Rev: %d.%d\n", hw_rev >> 4, hw_rev &
> 0xf);
> +	/*
> +	 * Show this both in decimal and hex: at least one vendor
> +	 * always reports this in hex.
> +	 */
> +	seq_printf(file, "FW/SW Rev: %d.%d (0x%02x.0x%02x)\n",
> +		   buf[10], buf[11], buf[10], buf[11]);
> +}
> +
> +static const struct cec_adap_ops drm_dp_cec_adap_ops = {
> +	.adap_enable = drm_dp_cec_adap_enable,
> +	.adap_log_addr = drm_dp_cec_adap_log_addr,
> +	.adap_transmit = drm_dp_cec_adap_transmit,
> +	.adap_monitor_all_enable =
> drm_dp_cec_adap_monitor_all_enable,
> +	.adap_status = drm_dp_cec_adap_status,
> +};
> +
> +static int drm_dp_cec_received(struct drm_dp_aux *aux)
> +{
> +	struct cec_adapter *adap = aux->cec_adap;
> +	struct cec_msg msg;
> +	u8 rx_msg_info;
> +	ssize_t err;
> +
> +	err = drm_dp_dpcd_readb(aux, DP_CEC_RX_MESSAGE_INFO,
> &rx_msg_info);
> +	if (err < 0)
> +		return err;
> +	if (!(rx_msg_info & DP_CEC_RX_MESSAGE_ENDED))
> +		return 0;
> +	msg.len = (rx_msg_info & DP_CEC_RX_MESSAGE_LEN_MASK) + 1;
> +	err = drm_dp_dpcd_read(aux, DP_CEC_RX_MESSAGE_BUFFER,
> msg.msg, msg.len);
> +	if (err < 0)
> +		return err;
> +	cec_received_msg(adap, &msg);
> +	return 0;
> +}
> +
> +static int drm_dp_cec_handle_irq(struct drm_dp_aux *aux)
> +{
> +	struct cec_adapter *adap = aux->cec_adap;
> +	u8 flags;
> +	ssize_t err;
> +
> +	err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_IRQ_FLAGS,
> &flags);
> +	if (err < 0)
> +		return err;
> +
> +	if (flags & DP_CEC_RX_MESSAGE_INFO_VALID)
> +		drm_dp_cec_received(aux);
> +
> +	if (flags & DP_CEC_TX_MESSAGE_SENT)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK);
> +	else if (flags & DP_CEC_TX_LINE_ERROR)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR
> |
> +						CEC_TX_STATUS_MAX_RE
> TRIES);
> +	else if (flags &
> +		 (DP_CEC_TX_ADDRESS_NACK_ERROR |
> DP_CEC_TX_DATA_NACK_ERROR))
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK |
> +						CEC_TX_STATUS_MAX_RE
> TRIES);
> +	drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, flags);
> +	return 0;
> +}
> +
> +/**
> + * drm_dp_cec_irq() - handle CEC interrupt, if any
> + * @aux: DisplayPort AUX channel
> + *
> + * Should be called when handling an IRQ_HPD request. If CEC-
> tunneling-over-AUX
> + * is present, then it will check for a CEC_IRQ and handle it
> accordingly.
> + *
> + * Returns true if an interrupt was handled successfully or false
> otherwise.
> + */
> +bool drm_dp_cec_irq(struct drm_dp_aux *aux)
> +{
> +	int attempts = 4;
> +	bool handled = false;
> +
> +	if (!aux->cec_adap)
> +		return false;
> +
> +	while (attempts--) {
> +		u8 cec_irq;
> +		int ret;
> +
> +		ret = drm_dp_dpcd_readb(aux,
> DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> +					&cec_irq);
> +		if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
> +			break;
> +
> +		if (!drm_dp_cec_handle_irq(aux))
> +			handled = true;
> +
> +		ret = drm_dp_dpcd_writeb(aux,
> DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> +					 DP_CEC_IRQ);
> +		if (ret < 0)
> +			break;
> +	}
> +	return handled;
> +}
> +EXPORT_SYMBOL(drm_dp_cec_irq);
> +
> +/**
> + * drm_dp_cec_configure_adapter() - configure the CEC adapter
> + * @aux: DisplayPort AUX channel
> + * @name: name of the CEC device
> + * @parent: parent device
> + *
> + * Checks if this is a DisplayPort-to-HDMI adapter that supports
> + * CEC-tunneling-over-AUX, and if so it creates a CEC device.
> + *
> + * If a CEC device was already created, then check if the
> capabilities
> + * have changed. If not, then do nothing. Otherwise destroy the old
> + * CEC device and create a new CEC device.
> + *
> + * This can happen when one DP-to-HDMI adapter is disconnected and
> + * replaced by another adapter with different CEC capabilities.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char
> *name,
> +				 struct device *parent)
> +{
> +	u32 cec_caps = CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> +		       CEC_CAP_PASSTHROUGH | CEC_CAP_RC |
> CEC_CAP_NEEDS_HPD;
> +	unsigned int num_las = 1;
> +	int err;
> +	u8 cap;
> +
> +	if (drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CAPABILITY,
> &cap) != 1 ||
> +	    !(cap & DP_CEC_TUNNELING_CAPABLE)) {
> +		cec_unregister_adapter(aux->cec_adap);
> +		aux->cec_adap = NULL;
> +		return -ENODEV;
> +	}
> +
> +	if (cap & DP_CEC_SNOOPING_CAPABLE)
> +		cec_caps |= CEC_CAP_MONITOR_ALL;
> +	if (cap & DP_CEC_MULTIPLE_LA_CAPABLE)
> +		num_las = CEC_MAX_LOG_ADDRS;
> +
> +	if (!IS_ERR_OR_NULL(aux->cec_adap)) {
> +		if (aux->cec_adap->capabilities == cec_caps &&
> +		    aux->cec_adap->available_log_addrs == num_las)
> +			return 0;
> +		cec_unregister_adapter(aux->cec_adap);
> +	}
> +
> +	aux->cec_adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
> +			 aux, name, cec_caps, num_las);
> +	if (IS_ERR(aux->cec_adap)) {
> +		err = PTR_ERR(aux->cec_adap);
> +		aux->cec_adap = NULL;
> +		return err;
> +	}
> +	err = cec_register_adapter(aux->cec_adap, parent);
> +	if (err) {
> +		cec_delete_adapter(aux->cec_adap);
> +		aux->cec_adap = NULL;
> +	}
> +	return err;
> +}
> +EXPORT_SYMBOL(drm_dp_cec_configure_adapter);
> diff --git a/include/drm/drm_dp_helper.h
> b/include/drm/drm_dp_helper.h
> index b17476a6909c..0e236dd40b42 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -952,6 +952,8 @@ struct drm_dp_aux_msg {
>  	size_t size;
>  };
>  
> +struct cec_adapter;
> +
>  /**
>   * struct drm_dp_aux - DisplayPort AUX channel
>   * @name: user-visible name of this AUX channel and the I2C-over-AUX 
> adapter
> @@ -1010,6 +1012,10 @@ struct drm_dp_aux {
>  	 * @i2c_defer_count: Counts I2C DEFERs, used for DP
> validation.
>  	 */
>  	unsigned i2c_defer_count;
> +	/**
> +	 * @cec_adap: the CEC adapter for CEC-Tunneling-over-AUX
> support.
> +	 */
> +	struct cec_adapter *cec_adap;
>  };
>  
>  ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int
> offset,
> @@ -1132,4 +1138,22 @@ drm_dp_has_quirk(const struct drm_dp_desc
> *desc, enum drm_dp_quirk quirk)
>  	return desc->quirks & BIT(quirk);
>  }
>  
> +#ifdef CONFIG_DRM_DP_CEC
> +bool drm_dp_cec_irq(struct drm_dp_aux *aux);
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char
> *name,
> +				 struct device *parent);
> +#else
> +static inline bool drm_dp_cec_irq(struct drm_dp_aux *aux)
> +{
> +	return false;
> +}
> +
> +static inline int drm_dp_cec_configure_adapter(struct drm_dp_aux
> *aux,
> +					       const char *name,
> +					       struct device
> *parent)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>  #endif /* _DRM_DP_HELPER_H_ */

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
  2017-07-11 13:30 ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
  2017-07-12 20:07   ` [PATCH for 4.13] cec: cec_transmit_attempt_done: ignore CEC_TX_STATUS_MAX_RETRIES Hans Verkuil
@ 2017-08-10  0:39   ` Carlos Santa
  1 sibling, 0 replies; 9+ messages in thread
From: Carlos Santa @ 2017-08-10  0:39 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Daniel Vetter, Hans Verkuil, dri-devel

On Tue, 2017-07-11 at 15:30 +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Implement support for this DisplayPort feature.
> 
> The cec device is created whenever it detects an adapter that
> has this feature. It is only removed when a new adapter is connected
> that does not support this. If a new adapter is connected that has
> different properties than the previous one, then the old cec device
> is
> unregistered and a new one is registered to replace the old one.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---

This was tested internally using the EE-PW700 adapter (DisplayPort/USB-
C to HDMI) from Samsung on a Skylake platform.

Several commands were tested including: 

# cec-ctl --playback

# turns the TV off
# cec-ctl -t=0 --standby

# turns the TV on
# cec-ctl -t=0 --image-view-on

The cec-compliance binary was also run and it reported no failures.

Tested-by: Carlos Santa <carlos.santa@intel.com>

>  drivers/gpu/drm/i915/intel_dp.c | 18 ++++++++++++++----
>  1 file changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c
> b/drivers/gpu/drm/i915/intel_dp.c
> index 64fa774c855b..fdb853d2c458 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -32,6 +32,7 @@
>  #include <linux/notifier.h>
>  #include <linux/reboot.h>
>  #include <asm/byteorder.h>
> +#include <media/cec.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
> @@ -1449,6 +1450,7 @@ static void intel_aux_reg_init(struct intel_dp
> *intel_dp)
>  static void
>  intel_dp_aux_fini(struct intel_dp *intel_dp)
>  {
> +	cec_unregister_adapter(intel_dp->aux.cec_adap);
>  	kfree(intel_dp->aux.name);
>  }
>  
> @@ -4587,6 +4589,7 @@ intel_dp_set_edid(struct intel_dp *intel_dp)
>  	intel_connector->detect_edid = edid;
>  
>  	intel_dp->has_audio = drm_detect_monitor_audio(edid);
> +	cec_s_phys_addr_from_edid(intel_dp->aux.cec_adap, edid);
>  }
>  
>  static void
> @@ -4596,6 +4599,7 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
>  
>  	kfree(intel_connector->detect_edid);
>  	intel_connector->detect_edid = NULL;
> +	cec_phys_addr_invalidate(intel_dp->aux.cec_adap);
>  
>  	intel_dp->has_audio = false;
>  }
> @@ -4616,13 +4620,17 @@ intel_dp_long_pulse(struct intel_connector
> *intel_connector)
>  	intel_display_power_get(to_i915(dev), intel_dp-
> >aux_power_domain);
>  
>  	/* Can't disconnect eDP, but you can close the lid... */
> -	if (is_edp(intel_dp))
> +	if (is_edp(intel_dp)) {
>  		status = edp_detect(intel_dp);
> -	else if (intel_digital_port_connected(to_i915(dev),
> -					      dp_to_dig_port(intel_d
> p)))
> +	} else if (intel_digital_port_connected(to_i915(dev),
> +						dp_to_dig_port(intel
> _dp))) {
>  		status = intel_dp_detect_dpcd(intel_dp);
> -	else
> +		if (status == connector_status_connected)
> +			drm_dp_cec_configure_adapter(&intel_dp->aux,
> +				     intel_dp->aux.name, dev->dev);
> +	} else {
>  		status = connector_status_disconnected;
> +	}
>  
>  	if (status == connector_status_disconnected) {
>  		memset(&intel_dp->compliance, 0, sizeof(intel_dp-
> >compliance));
> @@ -5011,6 +5019,8 @@ intel_dp_hpd_pulse(struct intel_digital_port
> *intel_dig_port, bool long_hpd)
>  
>  	intel_display_power_get(dev_priv, intel_dp-
> >aux_power_domain);
>  
> +	drm_dp_cec_irq(&intel_dp->aux);
> +
>  	if (intel_dp->is_mst) {
>  		if (intel_dp_check_mst_status(intel_dp) == -EINVAL)
> {
>  			/*

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX
  2017-07-11 13:30 ` [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX Hans Verkuil
  2017-08-10  0:34   ` Carlos Santa
@ 2017-08-10 19:19   ` Sean Paul
  1 sibling, 0 replies; 9+ messages in thread
From: Sean Paul @ 2017-08-10 19:19 UTC (permalink / raw)
  To: Hans Verkuil; +Cc: linux-media, Daniel Vetter, Hans Verkuil, dri-devel

On Tue, Jul 11, 2017 at 03:30:09PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> This adds support for the DisplayPort CEC-Tunneling-over-AUX
> feature that is part of the DisplayPort 1.3 standard.

Hi Hans,
Thank you for the patch, it looks great, I just have a few minor nits below. 

> 
> Unfortunately, not all DisplayPort/USB-C to HDMI adapters with a
> chip that has this capability actually hook up the CEC pin, so
> even though a CEC device is created, it may not actually work.

Boo :(

> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  drivers/gpu/drm/Kconfig      |  10 ++
>  drivers/gpu/drm/Makefile     |   1 +
>  drivers/gpu/drm/drm_dp_cec.c | 308 +++++++++++++++++++++++++++++++++++++++++++
>  include/drm/drm_dp_helper.h  |  24 ++++
>  4 files changed, 343 insertions(+)
>  create mode 100644 drivers/gpu/drm/drm_dp_cec.c
> 
> diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
> index 83cb2a88c204..1f2708df5c4e 100644
> --- a/drivers/gpu/drm/Kconfig
> +++ b/drivers/gpu/drm/Kconfig
> @@ -120,6 +120,16 @@ config DRM_LOAD_EDID_FIRMWARE
>  	  default case is N. Details and instructions how to build your own
>  	  EDID data are given in Documentation/EDID/HOWTO.txt.
>  
> +config DRM_DP_CEC
> +	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
> +	select CEC_CORE
> +	help
> +	  Choose this option if you want to enable HDMI CEC support for
> +	  DisplayPort/USB-C to HDMI adapters.
> +
> +	  Note: not all adapters support this feature, and even for those
> +	  that do support this they often do not hook up the CEC pin.
> +
>  config DRM_TTM
>  	tristate
>  	depends on DRM && MMU
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index 24a066e1841c..c6552c62049e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -40,6 +40,7 @@ drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
>  drm_kms_helper-$(CONFIG_DRM_FBDEV_EMULATION) += drm_fb_helper.o
>  drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
>  drm_kms_helper-$(CONFIG_DRM_DP_AUX_CHARDEV) += drm_dp_aux_dev.o
> +drm_kms_helper-$(CONFIG_DRM_DP_CEC) += drm_dp_cec.o
>  
>  obj-$(CONFIG_DRM_KMS_HELPER) += drm_kms_helper.o
>  obj-$(CONFIG_DRM_DEBUG_MM_SELFTEST) += selftests/
> diff --git a/drivers/gpu/drm/drm_dp_cec.c b/drivers/gpu/drm/drm_dp_cec.c
> new file mode 100644
> index 000000000000..7f2e298d45c0
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_cec.c
> @@ -0,0 +1,308 @@
> +/*
> + * DisplayPort CEC-Tunneling-over-AUX support
> + *
> + * Copyright 2017 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
> + *
> + * This program is free software; you may redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; version 2 of the License.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
> + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS
> + * BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN
> + * ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN
> + * CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
> + * SOFTWARE.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/slab.h>
> +#include <drm/drm_dp_helper.h>
> +#include <media/cec.h>
> +
> +/*
> + * Unfortunately it turns out that we have a chicken-and-egg situation
> + * here. Quite a few active (mini-)DP-to-HDMI or USB-C-to-HDMI adapters
> + * have a converter chip that supports CEC-Tunneling-over-AUX (usually the
> + * Parade PS176), but they do not wire up the CEC pin, thus making CEC
> + * useless.
> + *
> + * Sadly there is no way for this driver to know this. What happens is
> + * that a /dev/cecX device is created that is isolated and unable to see
> + * any of the other CEC devices. Quite literally the CEC wire is cut
> + * (or in this case, never connected in the first place).
> + *
> + * I suspect that the reason so few adapters support this is that this
> + * tunneling protocol was never supported by any OS. So there was no
> + * easy way of testing it, and no incentive to correctly wire up the
> + * CEC pin.
> + *
> + * Hopefully by creating this driver it will be easier for vendors to
> + * finally fix their adapters and test the CEC functionality.
> + *
> + * I keep a list of known working adapters here:
> + *
> + * https://hverkuil.home.xs4all.nl/cec-status.txt
> + *
> + * Please mail me (hverkuil@xs4all.nl) if you find an adapter that works
> + * and is not yet listed there.

According to the schematics, the Google USB-C -> HDMI adapter[1] has CEC wired
up.

[1]- https://store.google.com/product/usb_type_c_to_hdmi_adapter

> + */
> +
> +/**
> + * DOC: dp cec helpers
> + *
> + * These functions take care of supporting the CEC-Tunneling-over-AUX
> + * feature of DisplayPort-to-HDMI adapters.
> + */
> +
> +static int drm_dp_cec_adap_enable(struct cec_adapter *adap, bool enable)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	ssize_t err = 0;
> +
> +	if (enable)
> +		err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL,
> +					 DP_CEC_TUNNELING_ENABLE);
> +	else
> +		err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, 0);

nit: The pattern for this type of thing is usually,

u32 val = 0;

if (enable)
        val = DP_CEC_TUNNELING_ENABLE;

err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, val);

> +	return (enable && err < 0) ? err : 0;
> +}
> +
> +static int drm_dp_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	u8 mask[2] = { 0x00, 0x80 };
> +	ssize_t err;
> +
> +	if (addr != CEC_LOG_ADDR_INVALID) {


Save yourself a bit of work and do:

if (addr == CEC_LOG_ADDR_INVALID)
        return 0;

> +		u16 la_mask = adap->log_addrs.log_addr_mask;
> +
> +		la_mask |= 0x8000 | (1 << addr);

What's the 0x8000? Can you make it a little more clear by doing:

la_mask |= 1 << SOME_DESCRIPTIVE_NAME | (1 << addr);


> +		mask[0] = la_mask & 0xff;
> +		mask[1] = la_mask >> 8;
> +	}
> +	err = drm_dp_dpcd_write(aux, DP_CEC_LOGICAL_ADDRESS_MASK, mask, 2);
> +	return (addr != CEC_LOG_ADDR_INVALID && err < 0) ? err : 0;
> +}
> +
> +static int drm_dp_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
> +				    u32 signal_free_time, struct cec_msg *msg)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	unsigned int retries = attempts - 1;

unsigned int retries = min(5, attempts - 1);

> +	ssize_t err;
> +
> +	err = drm_dp_dpcd_write(aux, DP_CEC_TX_MESSAGE_BUFFER,
> +				msg->msg, msg->len);
> +	if (err < 0)
> +		return err;
> +
> +	if (retries > 5)
> +		retries = 5;
> +	err = drm_dp_dpcd_writeb(aux, DP_CEC_TX_MESSAGE_INFO,
> +				 (msg->len - 1) | (retries << 4) |
> +				 DP_CEC_TX_MESSAGE_SEND);
> +	return err < 0 ? err : 0;
> +}
> +
> +static int drm_dp_cec_adap_monitor_all_enable(struct cec_adapter *adap,
> +					      bool enable)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	ssize_t err;
> +	u8 val;
> +
> +	if (!(adap->capabilities & CEC_CAP_MONITOR_ALL))
> +		return 0;
> +
> +	err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CONTROL, &val);
> +	if (err >= 0) {
> +		if (enable)
> +			val |= DP_CEC_SNOOPING_ENABLE;
> +		else
> +			val &= ~DP_CEC_SNOOPING_ENABLE;
> +		err = drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_CONTROL, val);
> +	}
> +	return (enable && err < 0) ? err : 0;
> +}
> +
> +static void drm_dp_cec_adap_status(struct cec_adapter *adap,
> +				   struct seq_file *file)
> +{
> +	struct drm_dp_aux *aux = cec_get_drvdata(adap);
> +	u8 buf[DP_AUX_MAX_PAYLOAD_BYTES];
> +	u8 hw_rev;
> +
> +	if (drm_dp_dpcd_read(aux, DP_BRANCH_OUI,
> +			     buf, sizeof(buf)) != sizeof(buf))
> +		return;
> +	hw_rev = buf[9];
> +	buf[9] = 0;
> +	seq_printf(file, "OUI: %02x-%02x-%02x\n",
> +		   buf[0], buf[1], buf[2]);
> +	seq_printf(file, "ID: %s\n", buf + 3);
> +	seq_printf(file, "HW Rev: %d.%d\n", hw_rev >> 4, hw_rev & 0xf);
> +	/*
> +	 * Show this both in decimal and hex: at least one vendor
> +	 * always reports this in hex.
> +	 */
> +	seq_printf(file, "FW/SW Rev: %d.%d (0x%02x.0x%02x)\n",
> +		   buf[10], buf[11], buf[10], buf[11]);
> +}
> +
> +static const struct cec_adap_ops drm_dp_cec_adap_ops = {
> +	.adap_enable = drm_dp_cec_adap_enable,
> +	.adap_log_addr = drm_dp_cec_adap_log_addr,
> +	.adap_transmit = drm_dp_cec_adap_transmit,
> +	.adap_monitor_all_enable = drm_dp_cec_adap_monitor_all_enable,
> +	.adap_status = drm_dp_cec_adap_status,
> +};
> +
> +static int drm_dp_cec_received(struct drm_dp_aux *aux)
> +{
> +	struct cec_adapter *adap = aux->cec_adap;
> +	struct cec_msg msg;
> +	u8 rx_msg_info;
> +	ssize_t err;
> +
> +	err = drm_dp_dpcd_readb(aux, DP_CEC_RX_MESSAGE_INFO, &rx_msg_info);
> +	if (err < 0)
> +		return err;
> +	if (!(rx_msg_info & DP_CEC_RX_MESSAGE_ENDED))
> +		return 0;
> +	msg.len = (rx_msg_info & DP_CEC_RX_MESSAGE_LEN_MASK) + 1;
> +	err = drm_dp_dpcd_read(aux, DP_CEC_RX_MESSAGE_BUFFER, msg.msg, msg.len);
> +	if (err < 0)
> +		return err;
> +	cec_received_msg(adap, &msg);
> +	return 0;
> +}
> +
> +static int drm_dp_cec_handle_irq(struct drm_dp_aux *aux)
> +{
> +	struct cec_adapter *adap = aux->cec_adap;
> +	u8 flags;
> +	ssize_t err;
> +
> +	err = drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, &flags);
> +	if (err < 0)
> +		return err;
> +
> +	if (flags & DP_CEC_RX_MESSAGE_INFO_VALID)
> +		drm_dp_cec_received(aux);
> +
> +	if (flags & DP_CEC_TX_MESSAGE_SENT)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK);
> +	else if (flags & DP_CEC_TX_LINE_ERROR)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR |
> +						CEC_TX_STATUS_MAX_RETRIES);
> +	else if (flags &
> +		 (DP_CEC_TX_ADDRESS_NACK_ERROR | DP_CEC_TX_DATA_NACK_ERROR))
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK |
> +						CEC_TX_STATUS_MAX_RETRIES);
> +	drm_dp_dpcd_writeb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, flags);
> +	return 0;
> +}
> +
> +/**
> + * drm_dp_cec_irq() - handle CEC interrupt, if any
> + * @aux: DisplayPort AUX channel
> + *
> + * Should be called when handling an IRQ_HPD request. If CEC-tunneling-over-AUX
> + * is present, then it will check for a CEC_IRQ and handle it accordingly.
> + *
> + * Returns true if an interrupt was handled successfully or false otherwise.
> + */
> +bool drm_dp_cec_irq(struct drm_dp_aux *aux)
> +{
> +	int attempts = 4;
> +	bool handled = false;
> +
> +	if (!aux->cec_adap)
> +		return false;
> +
> +	while (attempts--) {

nit: this seems more natural as a for loop:

for (int attempts = 5; attempts > 0; attempts--)

> +		u8 cec_irq;
> +		int ret;
> +
> +		ret = drm_dp_dpcd_readb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> +					&cec_irq);
> +		if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
> +			break;
> +
> +		if (!drm_dp_cec_handle_irq(aux))
> +			handled = true;
> +
> +		ret = drm_dp_dpcd_writeb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
> +					 DP_CEC_IRQ);
> +		if (ret < 0)
> +			break;
> +	}
> +	return handled;
> +}
> +EXPORT_SYMBOL(drm_dp_cec_irq);
> +
> +/**
> + * drm_dp_cec_configure_adapter() - configure the CEC adapter
> + * @aux: DisplayPort AUX channel
> + * @name: name of the CEC device
> + * @parent: parent device
> + *
> + * Checks if this is a DisplayPort-to-HDMI adapter that supports
> + * CEC-tunneling-over-AUX, and if so it creates a CEC device.
> + *
> + * If a CEC device was already created, then check if the capabilities
> + * have changed. If not, then do nothing. Otherwise destroy the old
> + * CEC device and create a new CEC device.
> + *
> + * This can happen when one DP-to-HDMI adapter is disconnected and
> + * replaced by another adapter with different CEC capabilities.
> + *
> + * Returns 0 on success or a negative error code on failure.
> + */
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char *name,
> +				 struct device *parent)
> +{
> +	u32 cec_caps = CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> +		       CEC_CAP_PASSTHROUGH | CEC_CAP_RC | CEC_CAP_NEEDS_HPD;
> +	unsigned int num_las = 1;
> +	int err;
> +	u8 cap;
> +
> +	if (drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_CAPABILITY, &cap) != 1 ||
> +	    !(cap & DP_CEC_TUNNELING_CAPABLE)) {
> +		cec_unregister_adapter(aux->cec_adap);
> +		aux->cec_adap = NULL;
> +		return -ENODEV;
> +	}
> +
> +	if (cap & DP_CEC_SNOOPING_CAPABLE)
> +		cec_caps |= CEC_CAP_MONITOR_ALL;
> +	if (cap & DP_CEC_MULTIPLE_LA_CAPABLE)
> +		num_las = CEC_MAX_LOG_ADDRS;
> +
> +	if (!IS_ERR_OR_NULL(aux->cec_adap)) {
> +		if (aux->cec_adap->capabilities == cec_caps &&
> +		    aux->cec_adap->available_log_addrs == num_las)
> +			return 0;
> +		cec_unregister_adapter(aux->cec_adap);
> +	}
> +
> +	aux->cec_adap = cec_allocate_adapter(&drm_dp_cec_adap_ops,
> +			 aux, name, cec_caps, num_las);
> +	if (IS_ERR(aux->cec_adap)) {
> +		err = PTR_ERR(aux->cec_adap);
> +		aux->cec_adap = NULL;
> +		return err;
> +	}
> +	err = cec_register_adapter(aux->cec_adap, parent);
> +	if (err) {
> +		cec_delete_adapter(aux->cec_adap);
> +		aux->cec_adap = NULL;
> +	}
> +	return err;
> +}
> +EXPORT_SYMBOL(drm_dp_cec_configure_adapter);
> diff --git a/include/drm/drm_dp_helper.h b/include/drm/drm_dp_helper.h
> index b17476a6909c..0e236dd40b42 100644
> --- a/include/drm/drm_dp_helper.h
> +++ b/include/drm/drm_dp_helper.h
> @@ -952,6 +952,8 @@ struct drm_dp_aux_msg {
>  	size_t size;
>  };
>  
> +struct cec_adapter;
> +
>  /**
>   * struct drm_dp_aux - DisplayPort AUX channel
>   * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
> @@ -1010,6 +1012,10 @@ struct drm_dp_aux {
>  	 * @i2c_defer_count: Counts I2C DEFERs, used for DP validation.
>  	 */
>  	unsigned i2c_defer_count;
> +	/**
> +	 * @cec_adap: the CEC adapter for CEC-Tunneling-over-AUX support.
> +	 */
> +	struct cec_adapter *cec_adap;
>  };
>  
>  ssize_t drm_dp_dpcd_read(struct drm_dp_aux *aux, unsigned int offset,
> @@ -1132,4 +1138,22 @@ drm_dp_has_quirk(const struct drm_dp_desc *desc, enum drm_dp_quirk quirk)
>  	return desc->quirks & BIT(quirk);
>  }
>  
> +#ifdef CONFIG_DRM_DP_CEC
> +bool drm_dp_cec_irq(struct drm_dp_aux *aux);
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char *name,
> +				 struct device *parent);
> +#else
> +static inline bool drm_dp_cec_irq(struct drm_dp_aux *aux)
> +{
> +	return false;
> +}
> +
> +static inline int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux,
> +					       const char *name,
> +					       struct device *parent)
> +{
> +	return -ENODEV;
> +}
> +#endif
> +
>  #endif /* _DRM_DP_HELPER_H_ */
> -- 
> 2.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Sean Paul, Software Engineer, Google / Chromium OS

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-08-10 19:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-07-11 13:30 [PATCH 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
2017-07-11 13:30 ` [PATCH 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX Hans Verkuil
2017-08-10  0:34   ` Carlos Santa
2017-08-10 19:19   ` Sean Paul
2017-07-11 13:30 ` [PATCH 2/3] drm-kms-helpers.rst: document the DP CEC helpers Hans Verkuil
2017-07-11 13:30 ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Hans Verkuil
2017-07-12 20:07   ` [PATCH for 4.13] cec: cec_transmit_attempt_done: ignore CEC_TX_STATUS_MAX_RETRIES Hans Verkuil
2017-08-10  0:39   ` [PATCH 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support Carlos Santa
2017-08-09 10:01 ` [PATCH 0/3] " Hans Verkuil

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).