[PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
feature. This patch series is based on the 4.14 mainline release but applies
as well to drm-next.

This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
adapter (where the CEC pin is wired up).

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 or MegaChips MCDP2900), 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.

Note that the current implementation does not support CEC over an MST hub.
As far as I can see there is no mechanism defined in the DisplayPort
standard to transport CEC interrupts over an MST device. It might be
possible to do this through polling, but I have not been able to get that
to work.
----------------------------------------------------------------------

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).

Hopefully if this gets merged there will be an incentive for vendors
to make adapters where this actually works. It is a very nice feature
for HTPC boxes.

The main reason why this v5 is delayed by 2 months is due to the fact
that I needed some dedicated time to investigate what happens when an
MST hub is in use. It turns out that this is not working. There is no
mechanism defined in the DisplayPort standard to transport the CEC
interrupt back up the MST chain. I was actually able to send a CEC
message but the interrupt that tells when the transmit finished is
unavailable.

I attempted to implement this via polling, but I got weird errors
and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
register. I decided to give up on this for now and just disable CEC
for DP-to-HDMI adapters after an MST hub. I plan to revisit this
later since it would be neat to make this work as well. Although it
might not be possible at all.

If anyone is interested, work-in-progress for this is here:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst

Note that I removed the Tested-by tag from Carlos Santa due to the
almost complete rework of the third patch. Carlos, can you test this again?

Regards,

        Hans

Changes since v4:

- Updated comment at the start of drm_dp_cec.c
- Add edid pointer to drm_dp_cec_configure_adapter
- Reworked the last patch (adding CEC to i915) based on Ville's comments
  and my MST testing:
	- register/unregister CEC in intel_dp_connector_register/unregister
	- add comment and check if connector is registered in long_pulse
	- unregister CEC if an MST 'connector' is detected.

[PATCHv5 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

[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>
---
Changes since v4:

- Updated comment at the start of drm_dp_cec.c
- Add edid pointer to drm_dp_cec_configure_adapter
---
 drivers/gpu/drm/Kconfig      |  10 ++
 drivers/gpu/drm/Makefile     |   1 +
 drivers/gpu/drm/drm_dp_cec.c | 304 +++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_dp_helper.h  |  27 ++++
 4 files changed, 342 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 8ce07039bb89..6f0cd526703e 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -41,6 +41,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..e72051bfa18c
--- /dev/null
+++ b/drivers/gpu/drm/drm_dp_cec.c
@@ -0,0 +1,304 @@
+/*
+ * 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 or MegaChips MCDP2900), 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.
+ *
+ * Note that the current implementation does not support CEC over an MST hub.
+ * As far as I can see there is no mechanism defined in the DisplayPort
+ * standard to transport CEC interrupts over an MST device. It might be
+ * possible to do this through polling, but I have not been able to get that
+ * to work.
+ */
+
+/**
+ * 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);
+	u32 val = enable ? DP_CEC_TUNNELING_ENABLE : 0;
+	ssize_t err = 0;
+
+	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);
+	/* Bit 15 (logical address 15) should always be set */
+	u16 la_mask = 1 << CEC_LOG_ADDR_BROADCAST;
+	u8 mask[2];
+	ssize_t err;
+
+	if (addr != CEC_LOG_ADDR_INVALID)
+		la_mask |= adap->log_addrs.log_addr_mask | (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 = 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;
+
+	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);
+	struct drm_dp_desc desc;
+	struct drm_dp_dpcd_ident *id = &desc.ident;
+
+	if (drm_dp_read_desc(aux, &desc, true))
+		return;
+	seq_printf(file, "OUI: %*pdH\n",
+		   (int)sizeof(id->oui), id->oui);
+	seq_printf(file, "ID: %*pE\n",
+		   (int)strnlen(id->device_id, sizeof(id->device_id)),
+		   id->device_id);
+	seq_printf(file, "HW Rev: %d.%d\n", id->hw_rev >> 4, id->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",
+		   id->sw_major_rev, id->sw_minor_rev,
+		   id->sw_major_rev, id->sw_minor_rev);
+}
+
+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 bool drm_dp_cec_handle_irq(struct drm_dp_aux *aux)
+{
+	struct cec_adapter *adap = aux->cec_adap;
+	u8 flags;
+
+	if (drm_dp_dpcd_readb(aux, DP_CEC_TUNNELING_IRQ_FLAGS, &flags) < 0)
+		return false;
+
+	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 true;
+}
+
+/**
+ * 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)
+{
+	bool handled;
+	u8 cec_irq;
+	int ret;
+
+	if (!aux->cec_adap)
+		return false;
+
+	ret = drm_dp_dpcd_readb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1,
+				&cec_irq);
+	if (ret < 0 || !(cec_irq & DP_CEC_IRQ))
+		return false;
+
+	handled = drm_dp_cec_handle_irq(aux);
+	drm_dp_dpcd_writeb(aux, DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1, DP_CEC_IRQ);
+	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 adapter
+ * @parent: parent device
+ * @edid: initial EDID, may be NULL (== invalid physical address)
+ *
+ * Checks if this is a DisplayPort-to-HDMI adapter that supports
+ * CEC-tunneling-over-AUX, and if so it creates a CEC adapter.
+ *
+ * If a CEC adapter was already created, then check if the capabilities
+ * have changed. If not, then only update the EDID for the existing
+ * CEC adapter. Otherwise destroy the old CEC adapter (if any) and create
+ * a new CEC adapter and set the initial EDID for that adapter.
+ *
+ * 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, const struct edid *edid)
+{
+	u32 cec_caps = CEC_CAP_DEFAULTS | 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 (aux->cec_adap) {
+		if (aux->cec_adap->capabilities == cec_caps &&
+		    aux->cec_adap->available_log_addrs == num_las) {
+			cec_s_phys_addr_from_edid(aux->cec_adap, edid);
+			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;
+	}
+	cec_s_phys_addr_from_edid(aux->cec_adap, edid);
+	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..9bac0c47f62b 100644
--- a/include/drm/drm_dp_helper.h
+++ b/include/drm/drm_dp_helper.h
@@ -952,6 +952,9 @@ struct drm_dp_aux_msg {
 	size_t size;
 };
 
+struct cec_adapter;
+struct edid;
+
 /**
  * struct drm_dp_aux - DisplayPort AUX channel
  * @name: user-visible name of this AUX channel and the I2C-over-AUX adapter
@@ -1010,6 +1013,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 +1139,24 @@ 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,
+				 const struct edid *edid);
+#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,
+					       const struct edid *edid)
+{
+	return -ENODEV;
+}
+#endif
+
 #endif /* _DRM_DP_HELPER_H_ */
-- 
2.14.1

Re: [PATCHv5, 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

[Dariusz Marcinkiewicz]

Hello, pretty late here but I have a small comment.


> 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.

....
> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char
*name,
> +                                struct device *parent, const struct edid
*edid)
> +{
> +       u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
It seems there is a slight issue here when kernel is compiled w/o
CONFIG_MEDIA_CEC_RC, in such case
https://github.com/torvalds/linux/blob/master/drivers/media/cec/cec-core.c#L255
strips CEC_CAP_RC from the adapter's caps. As a result the below check
always fails and a new adapter is created every time this is run.
....
> +               if (aux->cec_adap->capabilities == cec_caps &&
> +                   aux->cec_adap->available_log_addrs == num_las) {
> +                       cec_s_phys_addr_from_edid(aux->cec_adap, edid);
> +                       return 0;
> +               }
> +               cec_unregister_adapter(aux->cec_adap);
> +       }
> +
...

Thank you and best regards.

Re: [PATCHv5, 1/3] drm: add support for DisplayPort CEC-Tunneling-over-AUX

[Hans Verkuil]

On 02/05/18 10:24, Dariusz Marcinkiewicz wrote:
> Hello, pretty late here but I have a small comment.
> 
> 
>> 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.
> 
> ....
>> +int drm_dp_cec_configure_adapter(struct drm_dp_aux *aux, const char
> *name,
>> +                                struct device *parent, const struct edid
> *edid)
>> +{
>> +       u32 cec_caps = CEC_CAP_DEFAULTS | CEC_CAP_NEEDS_HPD;
> It seems there is a slight issue here when kernel is compiled w/o
> CONFIG_MEDIA_CEC_RC, in such case
> https://github.com/torvalds/linux/blob/master/drivers/media/cec/cec-core.c#L255
> strips CEC_CAP_RC from the adapter's caps. As a result the below check
> always fails and a new adapter is created every time this is run.

Ah, good one. I missed that.

I've fixed it in my tree.

I still haven't had the time to finish this patch series :-(
It's high on my TODO list, but not high enough yet...

Regards,

	Hans

> ....
>> +               if (aux->cec_adap->capabilities == cec_caps &&
>> +                   aux->cec_adap->available_log_addrs == num_las) {
>> +                       cec_s_phys_addr_from_edid(aux->cec_adap, edid);
>> +                       return 0;
>> +               }
>> +               cec_unregister_adapter(aux->cec_adap);
>> +       }
>> +
> ...
> 
> Thank you and best regards.
> 

[PATCHv5 2/3] drm-kms-helpers.rst: document the DP CEC helpers

[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>
---
No changes since v4.
---
 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 13dd237418cc..a8c2f43c6f1a 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.14.1

[PATCHv5 3/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[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>
---
Changes since v4:

Reworked the last patch (adding CEC to i915) based on Ville's comments
and my MST testing:
	- register/unregister CEC in intel_dp_connector_register/unregister
	- add comment and check if connector is registered in long_pulse
	- unregister CEC if an MST 'connector' is detected.
---
 drivers/gpu/drm/i915/intel_dp.c | 47 ++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 09f274419eea..853346e8c5e9 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>
@@ -4690,6 +4691,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
@@ -4699,6 +4701,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;
 }
@@ -4773,6 +4776,14 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		 * with EDID on it
 		 */
 		status = connector_status_disconnected;
+		if (connector->registered) {
+			/*
+			 * CEC is not supported for an MST device, unregister
+			 * the existing CEC adapter, if any.
+			 */
+			cec_unregister_adapter(intel_dp->aux.cec_adap);
+			intel_dp->aux.cec_adap = NULL;
+		}
 		goto out;
 	} else {
 		/*
@@ -4822,6 +4833,25 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
+	if (status == connector_status_connected && connector->registered) {
+		/*
+		 * A new DP-to-HDMI adapter could have been plugged in, so
+		 * call drm_dp_cec_configure_adapter to check if a CEC device
+		 * should be unregistered and/or registered, depending on the
+		 * CEC capabilities of the adapter.
+		 *
+		 * The CEC device is associated with the connector, so it
+		 * sticks around when the adapter is unplugged and is only
+		 * unregistered if the connector is unregistered or if another
+		 * adapter is plugged in with no or different CEC capabilities.
+		 *
+		 * This is what CEC applications expect.
+		 */
+		drm_dp_cec_configure_adapter(&intel_dp->aux,
+					     connector->name, dev->dev,
+					     intel_connector->detect_edid);
+	}
+
 	intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
 	return status;
 }
@@ -4902,6 +4932,7 @@ static int
 intel_dp_connector_register(struct drm_connector *connector)
 {
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
+	struct drm_device *dev = connector->dev;
 	int ret;
 
 	ret = intel_connector_register(connector);
@@ -4913,6 +4944,15 @@ intel_dp_connector_register(struct drm_connector *connector)
 	DRM_DEBUG_KMS("registering %s bus for %s\n",
 		      intel_dp->aux.name, connector->kdev->kobj.name);
 
+	if (connector->status == connector_status_connected &&
+	    !intel_dp->is_mst && !intel_dp->aux.cec_adap) {
+		struct intel_connector *intel_connector =
+			intel_dp->attached_connector;
+
+		drm_dp_cec_configure_adapter(&intel_dp->aux,
+					     connector->name, dev->dev,
+					     intel_connector->detect_edid);
+	}
 	intel_dp->aux.dev = connector->kdev;
 	return drm_dp_aux_register(&intel_dp->aux);
 }
@@ -4920,7 +4960,11 @@ intel_dp_connector_register(struct drm_connector *connector)
 static void
 intel_dp_connector_unregister(struct drm_connector *connector)
 {
-	drm_dp_aux_unregister(&intel_attached_dp(connector)->aux);
+	struct intel_dp *intel_dp = intel_attached_dp(connector);
+
+	cec_unregister_adapter(intel_dp->aux.cec_adap);
+	intel_dp->aux.cec_adap = NULL;
+	drm_dp_aux_unregister(&intel_dp->aux);
 	intel_connector_unregister(connector);
 }
 
@@ -5129,6 +5173,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	}
 
 	if (!intel_dp->is_mst) {
+		drm_dp_cec_irq(&intel_dp->aux);
 		if (!intel_dp_short_pulse(intel_dp)) {
 			intel_dp->detect_done = false;
 			goto put_power;
-- 
2.14.1

Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

Ping!

I really like to get this in for 4.16 so I can move forward with hooking
this up for nouveau/amd.

Regards,

	Hans

On 11/20/2017 12:42 PM, Hans Verkuil wrote:
> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
> feature. This patch series is based on the 4.14 mainline release but applies
> as well to drm-next.
> 
> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
> adapter (where the CEC pin is wired up).
> 
> 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 or MegaChips MCDP2900), 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.
> 
> Note that the current implementation does not support CEC over an MST hub.
> As far as I can see there is no mechanism defined in the DisplayPort
> standard to transport CEC interrupts over an MST device. It might be
> possible to do this through polling, but I have not been able to get that
> to work.
> ----------------------------------------------------------------------
> 
> 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).
> 
> Hopefully if this gets merged there will be an incentive for vendors
> to make adapters where this actually works. It is a very nice feature
> for HTPC boxes.
> 
> The main reason why this v5 is delayed by 2 months is due to the fact
> that I needed some dedicated time to investigate what happens when an
> MST hub is in use. It turns out that this is not working. There is no
> mechanism defined in the DisplayPort standard to transport the CEC
> interrupt back up the MST chain. I was actually able to send a CEC
> message but the interrupt that tells when the transmit finished is
> unavailable.
> 
> I attempted to implement this via polling, but I got weird errors
> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
> register. I decided to give up on this for now and just disable CEC
> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
> later since it would be neat to make this work as well. Although it
> might not be possible at all.
> 
> If anyone is interested, work-in-progress for this is here:
> 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
> 
> Note that I removed the Tested-by tag from Carlos Santa due to the
> almost complete rework of the third patch. Carlos, can you test this again?
> 
> Regards,
> 
>         Hans
> 
> Changes since v4:
> 
> - Updated comment at the start of drm_dp_cec.c
> - Add edid pointer to drm_dp_cec_configure_adapter
> - Reworked the last patch (adding CEC to i915) based on Ville's comments
>   and my MST testing:
> 	- register/unregister CEC in intel_dp_connector_register/unregister
> 	- add comment and check if connector is registered in long_pulse
> 	- unregister CEC if an MST 'connector' is detected.
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

Ping again. Added a CC to Ville whom I inexplicably forgot to add when
I sent the v5 patch series.

Regards,

	Hans

On 01/12/17 08:23, Hans Verkuil wrote:
> Ping!
> 
> I really like to get this in for 4.16 so I can move forward with hooking
> this up for nouveau/amd.
> 
> Regards,
> 
> 	Hans
> 
> On 11/20/2017 12:42 PM, Hans Verkuil wrote:
>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
>> feature. This patch series is based on the 4.14 mainline release but applies
>> as well to drm-next.
>>
>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
>> adapter (where the CEC pin is wired up).
>>
>> 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 or MegaChips MCDP2900), 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.
>>
>> Note that the current implementation does not support CEC over an MST hub.
>> As far as I can see there is no mechanism defined in the DisplayPort
>> standard to transport CEC interrupts over an MST device. It might be
>> possible to do this through polling, but I have not been able to get that
>> to work.
>> ----------------------------------------------------------------------
>>
>> 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).
>>
>> Hopefully if this gets merged there will be an incentive for vendors
>> to make adapters where this actually works. It is a very nice feature
>> for HTPC boxes.
>>
>> The main reason why this v5 is delayed by 2 months is due to the fact
>> that I needed some dedicated time to investigate what happens when an
>> MST hub is in use. It turns out that this is not working. There is no
>> mechanism defined in the DisplayPort standard to transport the CEC
>> interrupt back up the MST chain. I was actually able to send a CEC
>> message but the interrupt that tells when the transmit finished is
>> unavailable.
>>
>> I attempted to implement this via polling, but I got weird errors
>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
>> register. I decided to give up on this for now and just disable CEC
>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
>> later since it would be neat to make this work as well. Although it
>> might not be possible at all.
>>
>> If anyone is interested, work-in-progress for this is here:
>>
>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
>>
>> Note that I removed the Tested-by tag from Carlos Santa due to the
>> almost complete rework of the third patch. Carlos, can you test this again?
>>
>> Regards,
>>
>>         Hans
>>
>> Changes since v4:
>>
>> - Updated comment at the start of drm_dp_cec.c
>> - Add edid pointer to drm_dp_cec_configure_adapter
>> - Reworked the last patch (adding CEC to i915) based on Ville's comments
>>   and my MST testing:
>> 	- register/unregister CEC in intel_dp_connector_register/unregister
>> 	- add comment and check if connector is registered in long_pulse
>> 	- unregister CEC if an MST 'connector' is detected.
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

First of all a Happy New Year for all of you!

And secondly: can this v5 patch series be reviewed/merged? It's been waiting
for that for a very long time now...

Regards,

	Hans

On 12/11/17 09:57, Hans Verkuil wrote:
> Ping again. Added a CC to Ville whom I inexplicably forgot to add when
> I sent the v5 patch series.
> 
> Regards,
> 
> 	Hans
> 
> On 01/12/17 08:23, Hans Verkuil wrote:
>> Ping!
>>
>> I really like to get this in for 4.16 so I can move forward with hooking
>> this up for nouveau/amd.
>>
>> Regards,
>>
>> 	Hans
>>
>> On 11/20/2017 12:42 PM, Hans Verkuil wrote:
>>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
>>> feature. This patch series is based on the 4.14 mainline release but applies
>>> as well to drm-next.
>>>
>>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
>>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
>>> adapter (where the CEC pin is wired up).
>>>
>>> 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 or MegaChips MCDP2900), 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.
>>>
>>> Note that the current implementation does not support CEC over an MST hub.
>>> As far as I can see there is no mechanism defined in the DisplayPort
>>> standard to transport CEC interrupts over an MST device. It might be
>>> possible to do this through polling, but I have not been able to get that
>>> to work.
>>> ----------------------------------------------------------------------
>>>
>>> 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).
>>>
>>> Hopefully if this gets merged there will be an incentive for vendors
>>> to make adapters where this actually works. It is a very nice feature
>>> for HTPC boxes.
>>>
>>> The main reason why this v5 is delayed by 2 months is due to the fact
>>> that I needed some dedicated time to investigate what happens when an
>>> MST hub is in use. It turns out that this is not working. There is no
>>> mechanism defined in the DisplayPort standard to transport the CEC
>>> interrupt back up the MST chain. I was actually able to send a CEC
>>> message but the interrupt that tells when the transmit finished is
>>> unavailable.
>>>
>>> I attempted to implement this via polling, but I got weird errors
>>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
>>> register. I decided to give up on this for now and just disable CEC
>>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
>>> later since it would be neat to make this work as well. Although it
>>> might not be possible at all.
>>>
>>> If anyone is interested, work-in-progress for this is here:
>>>
>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
>>>
>>> Note that I removed the Tested-by tag from Carlos Santa due to the
>>> almost complete rework of the third patch. Carlos, can you test this again?
>>>
>>> Regards,
>>>
>>>         Hans
>>>
>>> Changes since v4:
>>>
>>> - Updated comment at the start of drm_dp_cec.c
>>> - Add edid pointer to drm_dp_cec_configure_adapter
>>> - Reworked the last patch (adding CEC to i915) based on Ville's comments
>>>   and my MST testing:
>>> 	- register/unregister CEC in intel_dp_connector_register/unregister
>>> 	- add comment and check if connector is registered in long_pulse
>>> 	- unregister CEC if an MST 'connector' is detected.
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Ville Syrjälä]

On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote:
> Hi Ville,
> 
> For some strange reason your email disappeared from the Cc list. Perhaps it's the
> ä that confuses something somewhere.
> 
> So I'll just forward this directly to you.
> 
> Can you please take a look? This patch series has been in limbo for too long.

IIRC last I looked we still had some ragistration race to deal with.
Was that fixed?

Also I think we got stuck on leaving the zombie device lingering around
when the display is disconnected. I couldn't understand why that is
at all useful since you anyway remove the device eventually.

Adding the lists back to cc so I don't have to repeat myself there...

> 
> Regards,
> 
> 	Hans
> 
> 
> -------- Forwarded Message --------
> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
> Date: Tue, 9 Jan 2018 13:46:44 +0100
> From: Hans Verkuil <hverkuil@xs4all.nl>
> To: linux-media@vger.kernel.org
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>, Carlos Santa <carlos.santa@intel.com>, dri-devel@lists.freedesktop.org
> 
> First of all a Happy New Year for all of you!
> 
> And secondly: can this v5 patch series be reviewed/merged? It's been waiting
> for that for a very long time now...
> 
> Regards,
> 
> 	Hans
> 
> On 12/11/17 09:57, Hans Verkuil wrote:
> > Ping again. Added a CC to Ville whom I inexplicably forgot to add when
> > I sent the v5 patch series.
> > 
> > Regards,
> > 
> > 	Hans
> > 
> > On 01/12/17 08:23, Hans Verkuil wrote:
> >> Ping!
> >>
> >> I really like to get this in for 4.16 so I can move forward with hooking
> >> this up for nouveau/amd.
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >> On 11/20/2017 12:42 PM, Hans Verkuil wrote:
> >>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
> >>> feature. This patch series is based on the 4.14 mainline release but applies
> >>> as well to drm-next.
> >>>
> >>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
> >>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
> >>> adapter (where the CEC pin is wired up).
> >>>
> >>> 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 or MegaChips MCDP2900), 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.
> >>>
> >>> Note that the current implementation does not support CEC over an MST hub.
> >>> As far as I can see there is no mechanism defined in the DisplayPort
> >>> standard to transport CEC interrupts over an MST device. It might be
> >>> possible to do this through polling, but I have not been able to get that
> >>> to work.
> >>> ----------------------------------------------------------------------
> >>>
> >>> 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).
> >>>
> >>> Hopefully if this gets merged there will be an incentive for vendors
> >>> to make adapters where this actually works. It is a very nice feature
> >>> for HTPC boxes.
> >>>
> >>> The main reason why this v5 is delayed by 2 months is due to the fact
> >>> that I needed some dedicated time to investigate what happens when an
> >>> MST hub is in use. It turns out that this is not working. There is no
> >>> mechanism defined in the DisplayPort standard to transport the CEC
> >>> interrupt back up the MST chain. I was actually able to send a CEC
> >>> message but the interrupt that tells when the transmit finished is
> >>> unavailable.
> >>>
> >>> I attempted to implement this via polling, but I got weird errors
> >>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
> >>> register. I decided to give up on this for now and just disable CEC
> >>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
> >>> later since it would be neat to make this work as well. Although it
> >>> might not be possible at all.
> >>>
> >>> If anyone is interested, work-in-progress for this is here:
> >>>
> >>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
> >>>
> >>> Note that I removed the Tested-by tag from Carlos Santa due to the
> >>> almost complete rework of the third patch. Carlos, can you test this again?
> >>>
> >>> Regards,
> >>>
> >>>         Hans
> >>>
> >>> Changes since v4:
> >>>
> >>> - Updated comment at the start of drm_dp_cec.c
> >>> - Add edid pointer to drm_dp_cec_configure_adapter
> >>> - Reworked the last patch (adding CEC to i915) based on Ville's comments
> >>>   and my MST testing:
> >>> 	- register/unregister CEC in intel_dp_connector_register/unregister
> >>> 	- add comment and check if connector is registered in long_pulse
> >>> 	- unregister CEC if an MST 'connector' is detected.
> >>>
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> dri-devel@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>
> > 
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Ville Syrjälä
Intel OTC

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

On 01/12/2018 05:30 PM, Ville Syrjälä wrote:
> On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote:
>> Hi Ville,
>>
>> For some strange reason your email disappeared from the Cc list. Perhaps it's the
>> ä that confuses something somewhere.
>>
>> So I'll just forward this directly to you.
>>
>> Can you please take a look? This patch series has been in limbo for too long.
> 
> IIRC last I looked we still had some ragistration race to deal with.
> Was that fixed?

That was fixed in v5.

> 
> Also I think we got stuck on leaving the zombie device lingering around
> when the display is disconnected. I couldn't understand why that is
> at all useful since you anyway remove the device eventually.

It's not a zombie device. If you disconnect and reconnect the display then the
application using the CEC device will see the display disappear and reappear
as expected.

It helps if you think of the normal situation (as is present in most ARM SoCs)
where CEC is integral to the HDMI transmitter. I.e. it is not functionality that
can be removed. So the cec device is always there and an application opens the
device and can use it, regardless of whether a display is connected or not.

If a display is detected, the EDID will be read and the CEC physical address is
set. The application is informed of that through an event and the CEC adapter
can be used. If the HPD disappears the physical address is reset to f.f.f.f and
again the application is informed. And in fact it still has to be able to use
the CEC adapter even if there is no HPD since some displays turn off the HPD when
in standby, but CEC can still be used to power them up again.

Now consider a future Intel NUC with an HDMI connector on the backplane and
working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the
CEC support is always there (it's built in), but only becomes visible to the
kernel when you connect a display. You don't want the cec device to disappear
whenever you unplug the display, that makes no sense. Applications would
loose the CEC configuration and have to close and reopen (when it reappears)
the cec device for no good reason since it is built in.

The same situation is valid when using a USB-C to HDMI adapter: disconnecting
or reconnecting a display should not lead to the removal of the CEC device.
Only when an adapter with different CEC capabilities is detected is there a
need to actually unregister the CEC device.

All this is really a workaround of the fact that when the HPD disappears the
DP-to-HDMI adapter (either external or built-in) also disappears from the
topology, even though it is physically still there. If there was a way to
detect the adapter when there is no display connected, then this workaround
wouldn't be needed.

This situation is specific to DisplayPort, this is the only case where the
HDMI connector disappears in a puff of smoke when you disconnect the HDMI
cable, even though the actual physical connector is obviously still there.

Regards,

	Hans

> 
> Adding the lists back to cc so I don't have to repeat myself there...
> 
>>
>> Regards,
>>
>> 	Hans
>>
>>
>> -------- Forwarded Message --------
>> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
>> Date: Tue, 9 Jan 2018 13:46:44 +0100
>> From: Hans Verkuil <hverkuil@xs4all.nl>
>> To: linux-media@vger.kernel.org
>> CC: Daniel Vetter <daniel.vetter@ffwll.ch>, Carlos Santa <carlos.santa@intel.com>, dri-devel@lists.freedesktop.org
>>
>> First of all a Happy New Year for all of you!
>>
>> And secondly: can this v5 patch series be reviewed/merged? It's been waiting
>> for that for a very long time now...
>>
>> Regards,
>>
>> 	Hans
>>
>> On 12/11/17 09:57, Hans Verkuil wrote:
>>> Ping again. Added a CC to Ville whom I inexplicably forgot to add when
>>> I sent the v5 patch series.
>>>
>>> Regards,
>>>
>>> 	Hans
>>>
>>> On 01/12/17 08:23, Hans Verkuil wrote:
>>>> Ping!
>>>>
>>>> I really like to get this in for 4.16 so I can move forward with hooking
>>>> this up for nouveau/amd.
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>> On 11/20/2017 12:42 PM, Hans Verkuil wrote:
>>>>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
>>>>> feature. This patch series is based on the 4.14 mainline release but applies
>>>>> as well to drm-next.
>>>>>
>>>>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
>>>>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
>>>>> adapter (where the CEC pin is wired up).
>>>>>
>>>>> 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 or MegaChips MCDP2900), 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.
>>>>>
>>>>> Note that the current implementation does not support CEC over an MST hub.
>>>>> As far as I can see there is no mechanism defined in the DisplayPort
>>>>> standard to transport CEC interrupts over an MST device. It might be
>>>>> possible to do this through polling, but I have not been able to get that
>>>>> to work.
>>>>> ----------------------------------------------------------------------
>>>>>
>>>>> 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).
>>>>>
>>>>> Hopefully if this gets merged there will be an incentive for vendors
>>>>> to make adapters where this actually works. It is a very nice feature
>>>>> for HTPC boxes.
>>>>>
>>>>> The main reason why this v5 is delayed by 2 months is due to the fact
>>>>> that I needed some dedicated time to investigate what happens when an
>>>>> MST hub is in use. It turns out that this is not working. There is no
>>>>> mechanism defined in the DisplayPort standard to transport the CEC
>>>>> interrupt back up the MST chain. I was actually able to send a CEC
>>>>> message but the interrupt that tells when the transmit finished is
>>>>> unavailable.
>>>>>
>>>>> I attempted to implement this via polling, but I got weird errors
>>>>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
>>>>> register. I decided to give up on this for now and just disable CEC
>>>>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
>>>>> later since it would be neat to make this work as well. Although it
>>>>> might not be possible at all.
>>>>>
>>>>> If anyone is interested, work-in-progress for this is here:
>>>>>
>>>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
>>>>>
>>>>> Note that I removed the Tested-by tag from Carlos Santa due to the
>>>>> almost complete rework of the third patch. Carlos, can you test this again?
>>>>>
>>>>> Regards,
>>>>>
>>>>>         Hans
>>>>>
>>>>> Changes since v4:
>>>>>
>>>>> - Updated comment at the start of drm_dp_cec.c
>>>>> - Add edid pointer to drm_dp_cec_configure_adapter
>>>>> - Reworked the last patch (adding CEC to i915) based on Ville's comments
>>>>>   and my MST testing:
>>>>> 	- register/unregister CEC in intel_dp_connector_register/unregister
>>>>> 	- add comment and check if connector is registered in long_pulse
>>>>> 	- unregister CEC if an MST 'connector' is detected.
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>
>>>
>>> _______________________________________________
>>> dri-devel mailing list
>>> dri-devel@lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> 

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Ville Syrjälä]

On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote:
> On 01/12/2018 05:30 PM, Ville Syrjälä wrote:
> > On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote:
> >> Hi Ville,
> >>
> >> For some strange reason your email disappeared from the Cc list. Perhaps it's the
> >> ä that confuses something somewhere.
> >>
> >> So I'll just forward this directly to you.
> >>
> >> Can you please take a look? This patch series has been in limbo for too long.
> > 
> > IIRC last I looked we still had some ragistration race to deal with.
> > Was that fixed?
> 
> That was fixed in v5.
> 
> > 
> > Also I think we got stuck on leaving the zombie device lingering around
> > when the display is disconnected. I couldn't understand why that is
> > at all useful since you anyway remove the device eventually.
> 
> It's not a zombie device. If you disconnect and reconnect the display then the
> application using the CEC device will see the display disappear and reappear
> as expected.
> 
> It helps if you think of the normal situation (as is present in most ARM SoCs)
> where CEC is integral to the HDMI transmitter. I.e. it is not functionality that
> can be removed. So the cec device is always there and an application opens the
> device and can use it, regardless of whether a display is connected or not.
> 
> If a display is detected, the EDID will be read and the CEC physical address is
> set. The application is informed of that through an event and the CEC adapter
> can be used. If the HPD disappears the physical address is reset to f.f.f.f and
> again the application is informed. And in fact it still has to be able to use
> the CEC adapter even if there is no HPD since some displays turn off the HPD when
> in standby, but CEC can still be used to power them up again.

Hmm. So you're saying there are DP devices out there that release HPD
but still respond to DPCD accesses? That sounds... wrong.

In general I don't think we can assume that a device has retained its
state if it has deasserted HPD, thus we have to reconfigure everything
again anyway.

> 
> Now consider a future Intel NUC with an HDMI connector on the backplane and
> working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the
> CEC support is always there (it's built in), but only becomes visible to the
> kernel when you connect a display. You don't want the cec device to disappear
> whenever you unplug the display, that makes no sense. Applications would
> loose the CEC configuration and have to close and reopen (when it reappears)
> the cec device for no good reason since it is built in.

If the application can't remember its settings across a disconnect it
sounds broken anwyay. This would anyway happen when you disconenct the
entire dongle.

> 
> The same situation is valid when using a USB-C to HDMI adapter: disconnecting
> or reconnecting a display should not lead to the removal of the CEC device.
> Only when an adapter with different CEC capabilities is detected is there a
> need to actually unregister the CEC device.
> 
> All this is really a workaround of the fact that when the HPD disappears the
> DP-to-HDMI adapter (either external or built-in) also disappears from the
> topology, even though it is physically still there.

The dongles I've seen do not disappear. The downstream hpd is
signalled with short hpd pulses + SINK_COUNT instead.

But I've not actually seen a dongle that implements the
BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would
actually do. The spec does say they should default to using long
hpd for downstream hpd handling.

> If there was a way to
> detect the adapter when there is no display connected, then this workaround
> wouldn't be needed.
> 
> This situation is specific to DisplayPort, this is the only case where the
> HDMI connector disappears in a puff of smoke when you disconnect the HDMI
> cable, even though the actual physical connector is obviously still there.
> 
> Regards,
> 
> 	Hans
> 
> > 
> > Adding the lists back to cc so I don't have to repeat myself there...
> > 
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >>
> >> -------- Forwarded Message --------
> >> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
> >> Date: Tue, 9 Jan 2018 13:46:44 +0100
> >> From: Hans Verkuil <hverkuil@xs4all.nl>
> >> To: linux-media@vger.kernel.org
> >> CC: Daniel Vetter <daniel.vetter@ffwll.ch>, Carlos Santa <carlos.santa@intel.com>, dri-devel@lists.freedesktop.org
> >>
> >> First of all a Happy New Year for all of you!
> >>
> >> And secondly: can this v5 patch series be reviewed/merged? It's been waiting
> >> for that for a very long time now...
> >>
> >> Regards,
> >>
> >> 	Hans
> >>
> >> On 12/11/17 09:57, Hans Verkuil wrote:
> >>> Ping again. Added a CC to Ville whom I inexplicably forgot to add when
> >>> I sent the v5 patch series.
> >>>
> >>> Regards,
> >>>
> >>> 	Hans
> >>>
> >>> On 01/12/17 08:23, Hans Verkuil wrote:
> >>>> Ping!
> >>>>
> >>>> I really like to get this in for 4.16 so I can move forward with hooking
> >>>> this up for nouveau/amd.
> >>>>
> >>>> Regards,
> >>>>
> >>>> 	Hans
> >>>>
> >>>> On 11/20/2017 12:42 PM, Hans Verkuil wrote:
> >>>>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
> >>>>> feature. This patch series is based on the 4.14 mainline release but applies
> >>>>> as well to drm-next.
> >>>>>
> >>>>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
> >>>>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
> >>>>> adapter (where the CEC pin is wired up).
> >>>>>
> >>>>> 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 or MegaChips MCDP2900), 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.
> >>>>>
> >>>>> Note that the current implementation does not support CEC over an MST hub.
> >>>>> As far as I can see there is no mechanism defined in the DisplayPort
> >>>>> standard to transport CEC interrupts over an MST device. It might be
> >>>>> possible to do this through polling, but I have not been able to get that
> >>>>> to work.
> >>>>> ----------------------------------------------------------------------
> >>>>>
> >>>>> 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).
> >>>>>
> >>>>> Hopefully if this gets merged there will be an incentive for vendors
> >>>>> to make adapters where this actually works. It is a very nice feature
> >>>>> for HTPC boxes.
> >>>>>
> >>>>> The main reason why this v5 is delayed by 2 months is due to the fact
> >>>>> that I needed some dedicated time to investigate what happens when an
> >>>>> MST hub is in use. It turns out that this is not working. There is no
> >>>>> mechanism defined in the DisplayPort standard to transport the CEC
> >>>>> interrupt back up the MST chain. I was actually able to send a CEC
> >>>>> message but the interrupt that tells when the transmit finished is
> >>>>> unavailable.
> >>>>>
> >>>>> I attempted to implement this via polling, but I got weird errors
> >>>>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
> >>>>> register. I decided to give up on this for now and just disable CEC
> >>>>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
> >>>>> later since it would be neat to make this work as well. Although it
> >>>>> might not be possible at all.
> >>>>>
> >>>>> If anyone is interested, work-in-progress for this is here:
> >>>>>
> >>>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
> >>>>>
> >>>>> Note that I removed the Tested-by tag from Carlos Santa due to the
> >>>>> almost complete rework of the third patch. Carlos, can you test this again?
> >>>>>
> >>>>> Regards,
> >>>>>
> >>>>>         Hans
> >>>>>
> >>>>> Changes since v4:
> >>>>>
> >>>>> - Updated comment at the start of drm_dp_cec.c
> >>>>> - Add edid pointer to drm_dp_cec_configure_adapter
> >>>>> - Reworked the last patch (adding CEC to i915) based on Ville's comments
> >>>>>   and my MST testing:
> >>>>> 	- register/unregister CEC in intel_dp_connector_register/unregister
> >>>>> 	- add comment and check if connector is registered in long_pulse
> >>>>> 	- unregister CEC if an MST 'connector' is detected.
> >>>>>
> >>>>> _______________________________________________
> >>>>> dri-devel mailing list
> >>>>> dri-devel@lists.freedesktop.org
> >>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>>>
> >>>>
> >>>> _______________________________________________
> >>>> dri-devel mailing list
> >>>> dri-devel@lists.freedesktop.org
> >>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>>
> >>>
> >>> _______________________________________________
> >>> dri-devel mailing list
> >>> dri-devel@lists.freedesktop.org
> >>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> >>>
> >>
> >> _______________________________________________
> >> dri-devel mailing list
> >> dri-devel@lists.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/dri-devel
> > 

-- 
Ville Syrjälä
Intel OTC

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

On 01/12/2018 06:52 PM, Ville Syrjälä wrote:
> On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote:
>> On 01/12/2018 05:30 PM, Ville Syrjälä wrote:
>>> On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote:
>>>> Hi Ville,
>>>>
>>>> For some strange reason your email disappeared from the Cc list. Perhaps it's the
>>>> ä that confuses something somewhere.
>>>>
>>>> So I'll just forward this directly to you.
>>>>
>>>> Can you please take a look? This patch series has been in limbo for too long.
>>>
>>> IIRC last I looked we still had some ragistration race to deal with.
>>> Was that fixed?
>>
>> That was fixed in v5.
>>
>>>
>>> Also I think we got stuck on leaving the zombie device lingering around
>>> when the display is disconnected. I couldn't understand why that is
>>> at all useful since you anyway remove the device eventually.
>>
>> It's not a zombie device. If you disconnect and reconnect the display then the
>> application using the CEC device will see the display disappear and reappear
>> as expected.
>>
>> It helps if you think of the normal situation (as is present in most ARM SoCs)
>> where CEC is integral to the HDMI transmitter. I.e. it is not functionality that
>> can be removed. So the cec device is always there and an application opens the
>> device and can use it, regardless of whether a display is connected or not.
>>
>> If a display is detected, the EDID will be read and the CEC physical address is
>> set. The application is informed of that through an event and the CEC adapter
>> can be used. If the HPD disappears the physical address is reset to f.f.f.f and
>> again the application is informed. And in fact it still has to be able to use
>> the CEC adapter even if there is no HPD since some displays turn off the HPD when
>> in standby, but CEC can still be used to power them up again.
> 
> Hmm. So you're saying there are DP devices out there that release HPD
> but still respond to DPCD accesses? That sounds... wrong.

Not quite. To be precise: there are HDMI displays that release HPD when in standby
but still respond to CEC commands.

Such displays are still being made today so if you are building a product like
a media streaming box, then this is something to take into account.

However, for this specific case (CEC tunneling) it is a non-issue since the
DP CEC protocol simply doesn't support sending CEC commands without HPD.

> In general I don't think we can assume that a device has retained its
> state if it has deasserted HPD, thus we have to reconfigure everything
> again anyway.
> 
>>
>> Now consider a future Intel NUC with an HDMI connector on the backplane and
>> working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the
>> CEC support is always there (it's built in), but only becomes visible to the
>> kernel when you connect a display. You don't want the cec device to disappear
>> whenever you unplug the display, that makes no sense. Applications would
>> loose the CEC configuration and have to close and reopen (when it reappears)
>> the cec device for no good reason since it is built in.
> 
> If the application can't remember its settings across a disconnect it
> sounds broken anwyay. This would anyway happen when you disconenct the
> entire dongle.

Huh?

> 
>>
>> The same situation is valid when using a USB-C to HDMI adapter: disconnecting
>> or reconnecting a display should not lead to the removal of the CEC device.
>> Only when an adapter with different CEC capabilities is detected is there a
>> need to actually unregister the CEC device.
>>
>> All this is really a workaround of the fact that when the HPD disappears the
>> DP-to-HDMI adapter (either external or built-in) also disappears from the
>> topology, even though it is physically still there.
> 
> The dongles I've seen do not disappear. The downstream hpd is
> signalled with short hpd pulses + SINK_COUNT instead.
> 
> But I've not actually seen a dongle that implements the
> BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would
> actually do. The spec does say they should default to using long
> hpd for downstream hpd handling.

I did a few more experiments and it appears that someone somewhere keeps
track of DP branch devices. I.e. after disconnecting my usb-c to hdmi adapter
it still appears in i915_display_info. At least until something else is
connected. I basically need to hook into the DP branch detection.

Something to look at this weekend.

Regards,

	Hans

> 
>> If there was a way to
>> detect the adapter when there is no display connected, then this workaround
>> wouldn't be needed.
>>
>> This situation is specific to DisplayPort, this is the only case where the
>> HDMI connector disappears in a puff of smoke when you disconnect the HDMI
>> cable, even though the actual physical connector is obviously still there.
>>
>> Regards,
>>
>> 	Hans
>>
>>>
>>> Adding the lists back to cc so I don't have to repeat myself there...
>>>
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>>
>>>> -------- Forwarded Message --------
>>>> Subject: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support
>>>> Date: Tue, 9 Jan 2018 13:46:44 +0100
>>>> From: Hans Verkuil <hverkuil@xs4all.nl>
>>>> To: linux-media@vger.kernel.org
>>>> CC: Daniel Vetter <daniel.vetter@ffwll.ch>, Carlos Santa <carlos.santa@intel.com>, dri-devel@lists.freedesktop.org
>>>>
>>>> First of all a Happy New Year for all of you!
>>>>
>>>> And secondly: can this v5 patch series be reviewed/merged? It's been waiting
>>>> for that for a very long time now...
>>>>
>>>> Regards,
>>>>
>>>> 	Hans
>>>>
>>>> On 12/11/17 09:57, Hans Verkuil wrote:
>>>>> Ping again. Added a CC to Ville whom I inexplicably forgot to add when
>>>>> I sent the v5 patch series.
>>>>>
>>>>> Regards,
>>>>>
>>>>> 	Hans
>>>>>
>>>>> On 01/12/17 08:23, Hans Verkuil wrote:
>>>>>> Ping!
>>>>>>
>>>>>> I really like to get this in for 4.16 so I can move forward with hooking
>>>>>> this up for nouveau/amd.
>>>>>>
>>>>>> Regards,
>>>>>>
>>>>>> 	Hans
>>>>>>
>>>>>> On 11/20/2017 12:42 PM, Hans Verkuil wrote:
>>>>>>> This patch series adds support for the DisplayPort CEC-Tunneling-over-AUX
>>>>>>> feature. This patch series is based on the 4.14 mainline release but applies
>>>>>>> as well to drm-next.
>>>>>>>
>>>>>>> This patch series has been tested with my NUC7i5BNK, a Samsung USB-C to 
>>>>>>> HDMI adapter and a Club 3D DisplayPort MST Hub + modified UpTab DP-to-HDMI
>>>>>>> adapter (where the CEC pin is wired up).
>>>>>>>
>>>>>>> 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 or MegaChips MCDP2900), 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.
>>>>>>>
>>>>>>> Note that the current implementation does not support CEC over an MST hub.
>>>>>>> As far as I can see there is no mechanism defined in the DisplayPort
>>>>>>> standard to transport CEC interrupts over an MST device. It might be
>>>>>>> possible to do this through polling, but I have not been able to get that
>>>>>>> to work.
>>>>>>> ----------------------------------------------------------------------
>>>>>>>
>>>>>>> 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).
>>>>>>>
>>>>>>> Hopefully if this gets merged there will be an incentive for vendors
>>>>>>> to make adapters where this actually works. It is a very nice feature
>>>>>>> for HTPC boxes.
>>>>>>>
>>>>>>> The main reason why this v5 is delayed by 2 months is due to the fact
>>>>>>> that I needed some dedicated time to investigate what happens when an
>>>>>>> MST hub is in use. It turns out that this is not working. There is no
>>>>>>> mechanism defined in the DisplayPort standard to transport the CEC
>>>>>>> interrupt back up the MST chain. I was actually able to send a CEC
>>>>>>> message but the interrupt that tells when the transmit finished is
>>>>>>> unavailable.
>>>>>>>
>>>>>>> I attempted to implement this via polling, but I got weird errors
>>>>>>> and was not able to read the DP_DEVICE_SERVICE_IRQ_VECTOR_ESI1
>>>>>>> register. I decided to give up on this for now and just disable CEC
>>>>>>> for DP-to-HDMI adapters after an MST hub. I plan to revisit this
>>>>>>> later since it would be neat to make this work as well. Although it
>>>>>>> might not be possible at all.
>>>>>>>
>>>>>>> If anyone is interested, work-in-progress for this is here:
>>>>>>>
>>>>>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=dp-cec-mst
>>>>>>>
>>>>>>> Note that I removed the Tested-by tag from Carlos Santa due to the
>>>>>>> almost complete rework of the third patch. Carlos, can you test this again?
>>>>>>>
>>>>>>> Regards,
>>>>>>>
>>>>>>>         Hans
>>>>>>>
>>>>>>> Changes since v4:
>>>>>>>
>>>>>>> - Updated comment at the start of drm_dp_cec.c
>>>>>>> - Add edid pointer to drm_dp_cec_configure_adapter
>>>>>>> - Reworked the last patch (adding CEC to i915) based on Ville's comments
>>>>>>>   and my MST testing:
>>>>>>> 	- register/unregister CEC in intel_dp_connector_register/unregister
>>>>>>> 	- add comment and check if connector is registered in long_pulse
>>>>>>> 	- unregister CEC if an MST 'connector' is detected.
>>>>>>>
>>>>>>> _______________________________________________
>>>>>>> dri-devel mailing list
>>>>>>> dri-devel@lists.freedesktop.org
>>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>>
>>>>>>
>>>>>> _______________________________________________
>>>>>> dri-devel mailing list
>>>>>> dri-devel@lists.freedesktop.org
>>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> dri-devel mailing list
>>>>> dri-devel@lists.freedesktop.org
>>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>>>
>>>>
>>>> _______________________________________________
>>>> dri-devel mailing list
>>>> dri-devel@lists.freedesktop.org
>>>> https://lists.freedesktop.org/mailman/listinfo/dri-devel
>>>
> 

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

On 01/12/2018 07:12 PM, Hans Verkuil wrote:
> On 01/12/2018 06:52 PM, Ville Syrjälä wrote:
>> On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote:
>>> On 01/12/2018 05:30 PM, Ville Syrjälä wrote:
>>>> On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote:
>>>>> Hi Ville,
>>>>>
>>>>> For some strange reason your email disappeared from the Cc list. Perhaps it's the
>>>>> ä that confuses something somewhere.
>>>>>
>>>>> So I'll just forward this directly to you.
>>>>>
>>>>> Can you please take a look? This patch series has been in limbo for too long.
>>>>
>>>> IIRC last I looked we still had some ragistration race to deal with.
>>>> Was that fixed?
>>>
>>> That was fixed in v5.
>>>
>>>>
>>>> Also I think we got stuck on leaving the zombie device lingering around
>>>> when the display is disconnected. I couldn't understand why that is
>>>> at all useful since you anyway remove the device eventually.
>>>
>>> It's not a zombie device. If you disconnect and reconnect the display then the
>>> application using the CEC device will see the display disappear and reappear
>>> as expected.
>>>
>>> It helps if you think of the normal situation (as is present in most ARM SoCs)
>>> where CEC is integral to the HDMI transmitter. I.e. it is not functionality that
>>> can be removed. So the cec device is always there and an application opens the
>>> device and can use it, regardless of whether a display is connected or not.
>>>
>>> If a display is detected, the EDID will be read and the CEC physical address is
>>> set. The application is informed of that through an event and the CEC adapter
>>> can be used. If the HPD disappears the physical address is reset to f.f.f.f and
>>> again the application is informed. And in fact it still has to be able to use
>>> the CEC adapter even if there is no HPD since some displays turn off the HPD when
>>> in standby, but CEC can still be used to power them up again.
>>
>> Hmm. So you're saying there are DP devices out there that release HPD
>> but still respond to DPCD accesses? That sounds... wrong.
> 
> Not quite. To be precise: there are HDMI displays that release HPD when in standby
> but still respond to CEC commands.
> 
> Such displays are still being made today so if you are building a product like
> a media streaming box, then this is something to take into account.
> 
> However, for this specific case (CEC tunneling) it is a non-issue since the
> DP CEC protocol simply doesn't support sending CEC commands without HPD.
> 
>> In general I don't think we can assume that a device has retained its
>> state if it has deasserted HPD, thus we have to reconfigure everything
>> again anyway.
>>
>>>
>>> Now consider a future Intel NUC with an HDMI connector on the backplane and
>>> working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the
>>> CEC support is always there (it's built in), but only becomes visible to the
>>> kernel when you connect a display. You don't want the cec device to disappear
>>> whenever you unplug the display, that makes no sense. Applications would
>>> loose the CEC configuration and have to close and reopen (when it reappears)
>>> the cec device for no good reason since it is built in.
>>
>> If the application can't remember its settings across a disconnect it
>> sounds broken anwyay. This would anyway happen when you disconenct the
>> entire dongle.
> 
> Huh?
> 
>>
>>>
>>> The same situation is valid when using a USB-C to HDMI adapter: disconnecting
>>> or reconnecting a display should not lead to the removal of the CEC device.
>>> Only when an adapter with different CEC capabilities is detected is there a
>>> need to actually unregister the CEC device.
>>>
>>> All this is really a workaround of the fact that when the HPD disappears the
>>> DP-to-HDMI adapter (either external or built-in) also disappears from the
>>> topology, even though it is physically still there.
>>
>> The dongles I've seen do not disappear. The downstream hpd is
>> signalled with short hpd pulses + SINK_COUNT instead.
>>
>> But I've not actually seen a dongle that implements the
>> BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would
>> actually do. The spec does say they should default to using long
>> hpd for downstream hpd handling.
> 
> I did a few more experiments and it appears that someone somewhere keeps
> track of DP branch devices. I.e. after disconnecting my usb-c to hdmi adapter
> it still appears in i915_display_info. At least until something else is
> connected. I basically need to hook into the DP branch detection.
> 
> Something to look at this weekend.

I found a better place to do the CEC (un)registration: a long HPD pulse
indicates that the DPCD registers have changed, so that's when I should
detect whether there is a new branch device with CEC capabilities.

Regards,

	Hans

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

Hi Ville,

On 12/01/18 20:08, Hans Verkuil wrote:
> On 01/12/2018 07:12 PM, Hans Verkuil wrote:
>> On 01/12/2018 06:52 PM, Ville Syrjälä wrote:
>>> On Fri, Jan 12, 2018 at 06:14:53PM +0100, Hans Verkuil wrote:
>>>> On 01/12/2018 05:30 PM, Ville Syrjälä wrote:
>>>>> On Fri, Jan 12, 2018 at 05:19:44PM +0100, Hans Verkuil wrote:
>>>>>> Hi Ville,
>>>>>>
>>>>>> For some strange reason your email disappeared from the Cc list. Perhaps it's the
>>>>>> ä that confuses something somewhere.
>>>>>>
>>>>>> So I'll just forward this directly to you.
>>>>>>
>>>>>> Can you please take a look? This patch series has been in limbo for too long.
>>>>>
>>>>> IIRC last I looked we still had some ragistration race to deal with.
>>>>> Was that fixed?
>>>>
>>>> That was fixed in v5.
>>>>
>>>>>
>>>>> Also I think we got stuck on leaving the zombie device lingering around
>>>>> when the display is disconnected. I couldn't understand why that is
>>>>> at all useful since you anyway remove the device eventually.
>>>>
>>>> It's not a zombie device. If you disconnect and reconnect the display then the
>>>> application using the CEC device will see the display disappear and reappear
>>>> as expected.
>>>>
>>>> It helps if you think of the normal situation (as is present in most ARM SoCs)
>>>> where CEC is integral to the HDMI transmitter. I.e. it is not functionality that
>>>> can be removed. So the cec device is always there and an application opens the
>>>> device and can use it, regardless of whether a display is connected or not.
>>>>
>>>> If a display is detected, the EDID will be read and the CEC physical address is
>>>> set. The application is informed of that through an event and the CEC adapter
>>>> can be used. If the HPD disappears the physical address is reset to f.f.f.f and
>>>> again the application is informed. And in fact it still has to be able to use
>>>> the CEC adapter even if there is no HPD since some displays turn off the HPD when
>>>> in standby, but CEC can still be used to power them up again.
>>>
>>> Hmm. So you're saying there are DP devices out there that release HPD
>>> but still respond to DPCD accesses? That sounds... wrong.
>>
>> Not quite. To be precise: there are HDMI displays that release HPD when in standby
>> but still respond to CEC commands.
>>
>> Such displays are still being made today so if you are building a product like
>> a media streaming box, then this is something to take into account.
>>
>> However, for this specific case (CEC tunneling) it is a non-issue since the
>> DP CEC protocol simply doesn't support sending CEC commands without HPD.
>>
>>> In general I don't think we can assume that a device has retained its
>>> state if it has deasserted HPD, thus we have to reconfigure everything
>>> again anyway.
>>>
>>>>
>>>> Now consider a future Intel NUC with an HDMI connector on the backplane and
>>>> working DP CEC-Tunneling-over-AUX support (e.g. the Megachips MCDP2900): the
>>>> CEC support is always there (it's built in), but only becomes visible to the
>>>> kernel when you connect a display. You don't want the cec device to disappear
>>>> whenever you unplug the display, that makes no sense. Applications would
>>>> loose the CEC configuration and have to close and reopen (when it reappears)
>>>> the cec device for no good reason since it is built in.
>>>
>>> If the application can't remember its settings across a disconnect it
>>> sounds broken anwyay. This would anyway happen when you disconenct the
>>> entire dongle.
>>
>> Huh?
>>
>>>
>>>>
>>>> The same situation is valid when using a USB-C to HDMI adapter: disconnecting
>>>> or reconnecting a display should not lead to the removal of the CEC device.
>>>> Only when an adapter with different CEC capabilities is detected is there a
>>>> need to actually unregister the CEC device.
>>>>
>>>> All this is really a workaround of the fact that when the HPD disappears the
>>>> DP-to-HDMI adapter (either external or built-in) also disappears from the
>>>> topology, even though it is physically still there.
>>>
>>> The dongles I've seen do not disappear. The downstream hpd is
>>> signalled with short hpd pulses + SINK_COUNT instead.
>>>
>>> But I've not actually seen a dongle that implements the
>>> BRANCH_DEVICE_CTRL DPCD register, so not quite sure what those would
>>> actually do. The spec does say they should default to using long
>>> hpd for downstream hpd handling.
>>
>> I did a few more experiments and it appears that someone somewhere keeps
>> track of DP branch devices. I.e. after disconnecting my usb-c to hdmi adapter
>> it still appears in i915_display_info. At least until something else is
>> connected. I basically need to hook into the DP branch detection.
>>
>> Something to look at this weekend.
> 
> I found a better place to do the CEC (un)registration: a long HPD pulse
> indicates that the DPCD registers have changed, so that's when I should
> detect whether there is a new branch device with CEC capabilities.

Just FYI: unfortunately this is delayed due to a lot of other work.
I will get back to this as soon as have some time.

Regards,

	Hans

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

Hi Ville,

I finally found some time to dig deeper into when a CEC device should be registered.

Since it's been a long while since we discussed this let me just recap the situation
and then propose three solutions:
CEC is implemented for DP-to-HDMI branch devices through DPCD CEC registers. When
HPD is high we can read these registers and check if CEC is supported or not.

If an external USB-C/DP/mini-DP to HDMI adapter is used, then when the HPD goes low
you lose access to the DPCD registers since that is (I think) powered by the HPD.

If an integrated branch device is used (such as for the HDMI connector on the NUC)
the DPCD registers will remain available even if the display is disconnected.

The problem is with external adapters since if the HPD goes low you do not know
if the adapter has been disconnected, or if the display just pulled the HPD low for a
short time (updating the EDID, HDCP changes). In the latter case you do not want to
unregister and reregister the cec device.

I see three options:

1) register a cec device when a connector is registered and keep it for the life time
of the connector. If HPD goes low, or the branch device doesn't support CEC, then just
set the physical address of the CEC adapter to f.f.f.f.

This is simple, but the disadvantage is that there is a CEC device around, even if the
branch device doesn't support CEC.

2) register a cec device when HPD goes high and if a branch device that supports CEC is
detected. Unregister the cec device when the HPD goes low and stays low for more than
a second. This prevents a cec device from disappearing whenever the display toggles
the HPD. It does require a delayed workqueue to handle this delay.

It would be nice if there is a way to avoid a delayed workqueue, but I didn't see
anything in the i915 code.

3) A hybrid option where the cec device is only registered when a CEC capable branch
device is detected, but then we keep it for the remaining lifetime of the connector.
This avoids the delayed workqueue, and avoids creating cec devices if the branch
device doesn't support CEC. But once it is created it won't be removed until the
connector is also unregistered.

I'm leaning towards the second or third option.

Opinions? Other ideas?

Regards,

	Hans

Re: Fwd: Re: [PATCHv5 0/3] drm/i915: add DisplayPort CEC-Tunneling-over-AUX support

[Hans Verkuil]

On 11/06/18 14:20, Hans Verkuil wrote:
> Hi Ville,
> 
> I finally found some time to dig deeper into when a CEC device should be registered.
> 
> Since it's been a long while since we discussed this let me just recap the situation
> and then propose three solutions:
> CEC is implemented for DP-to-HDMI branch devices through DPCD CEC registers. When
> HPD is high we can read these registers and check if CEC is supported or not.
> 
> If an external USB-C/DP/mini-DP to HDMI adapter is used, then when the HPD goes low
> you lose access to the DPCD registers since that is (I think) powered by the HPD.
> 
> If an integrated branch device is used (such as for the HDMI connector on the NUC)
> the DPCD registers will remain available even if the display is disconnected.
> 
> The problem is with external adapters since if the HPD goes low you do not know
> if the adapter has been disconnected, or if the display just pulled the HPD low for a
> short time (updating the EDID, HDCP changes). In the latter case you do not want to
> unregister and reregister the cec device.
> 
> I see three options:
> 
> 1) register a cec device when a connector is registered and keep it for the life time
> of the connector. If HPD goes low, or the branch device doesn't support CEC, then just
> set the physical address of the CEC adapter to f.f.f.f.
> 
> This is simple, but the disadvantage is that there is a CEC device around, even if the
> branch device doesn't support CEC.
> 
> 2) register a cec device when HPD goes high and if a branch device that supports CEC is
> detected. Unregister the cec device when the HPD goes low and stays low for more than
> a second. This prevents a cec device from disappearing whenever the display toggles
> the HPD. It does require a delayed workqueue to handle this delay.
> 
> It would be nice if there is a way to avoid a delayed workqueue, but I didn't see
> anything in the i915 code.
> 
> 3) A hybrid option where the cec device is only registered when a CEC capable branch
> device is detected, but then we keep it for the remaining lifetime of the connector.
> This avoids the delayed workqueue, and avoids creating cec devices if the branch
> device doesn't support CEC. But once it is created it won't be removed until the
> connector is also unregistered.
> 
> I'm leaning towards the second or third option.
> 
> Opinions? Other ideas?

A quick update: I've been working on a next version of this patch series that combines
options 2 and 3 and moves the logic out of the i915 driver into the core drm CEC code
since all DP drivers will have the same problem.

I hope to post the new series later today.

Regards,

	Hans