[PATCH 0/9] cec improvements

[PATCH 0/9] cec improvements

[Hans Verkuil]

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

This patch series adds several helper functions to ease writing
drm or media CEC drivers.

It also adds support for the CEC_CAP_NEEDS_HPD capability which is
needed for hardware that turns off the CEC support if the hotplug
detect signal is low. Usually because the HPD is connected to some
'power' or 'active' pin of the CEC hardware.

One of those is the Odroid-U3, so add a 'needs-hpd' property that
tells the driver whether or not CEC is available without HPD.

Regards,

	Hans

Hans Verkuil (9):
  cec: add cec_s_phys_addr_from_edid helper function
  cec: add cec_phys_addr_invalidate() helper function
  cec: add cec_transmit_attempt_done helper function
  stih-cec/vivid/pulse8/rainshadow: use cec_transmit_attempt_done
  cec: add CEC_CAP_NEEDS_HPD
  cec-ioc-adap-g-caps.rst: document CEC_CAP_NEEDS_HPD
  dt-bindings: media/s5p-cec.txt: document needs-hpd property
  s5p_cec: set the CEC_CAP_NEEDS_HPD flag if needed
  ARM: dts: exynos: add needs-hpd to &hdmicec for Odroid-U3

 .../devicetree/bindings/media/s5p-cec.txt          |  6 +++
 Documentation/media/kapi/cec-core.rst              | 18 +++++++
 .../media/uapi/cec/cec-ioc-adap-g-caps.rst         |  8 +++
 arch/arm/boot/dts/exynos4412-odroidu3.dts          |  4 ++
 drivers/media/cec/cec-adap.c                       | 60 +++++++++++++++++++---
 drivers/media/cec/cec-api.c                        |  5 +-
 drivers/media/cec/cec-core.c                       |  1 +
 drivers/media/platform/s5p-cec/s5p_cec.c           |  4 +-
 drivers/media/platform/sti/cec/stih-cec.c          |  9 ++--
 drivers/media/platform/vivid/vivid-cec.c           |  6 +--
 drivers/media/usb/pulse8-cec/pulse8-cec.c          |  9 ++--
 drivers/media/usb/rainshadow-cec/rainshadow-cec.c  |  9 ++--
 include/media/cec.h                                | 29 +++++++++++
 include/uapi/linux/cec.h                           |  2 +
 14 files changed, 142 insertions(+), 28 deletions(-)

-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/9] cec: add cec_s_phys_addr_from_edid helper function

[Hans Verkuil]

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

This function simplifies the integration of CEC in DRM drivers.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/media/kapi/cec-core.rst |  8 ++++++++
 drivers/media/cec/cec-adap.c          | 14 ++++++++++++++
 include/media/cec.h                   |  9 +++++++++
 3 files changed, 31 insertions(+)

diff --git a/Documentation/media/kapi/cec-core.rst b/Documentation/media/kapi/cec-core.rst
index 7a04c5386dc8..278b358b2f2e 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -307,6 +307,14 @@ to another valid physical address, then this function will first set the
 address to CEC_PHYS_ADDR_INVALID before enabling the new physical address.
 
 .. c:function::
+	void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
+				       const struct edid *edid);
+
+A helper function that extracts the physical address from the edid struct
+and calls cec_s_phys_addr() with that address, or CEC_PHYS_ADDR_INVALID
+if the EDID did not contain a physical address or edid was a NULL pointer.
+
+.. c:function::
 	int cec_s_log_addrs(struct cec_adapter *adap,
 			    struct cec_log_addrs *log_addrs, bool block);
 
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index fd6d9cccade7..61e39bbe3cf9 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -28,6 +28,8 @@
 #include <linux/string.h>
 #include <linux/types.h>
 
+#include <drm/drm_edid.h>
+
 #include "cec-priv.h"
 
 static void cec_fill_msg_report_features(struct cec_adapter *adap,
@@ -1408,6 +1410,18 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 }
 EXPORT_SYMBOL_GPL(cec_s_phys_addr);
 
+void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
+			       const struct edid *edid)
+{
+	u16 pa = CEC_PHYS_ADDR_INVALID;
+
+	if (edid && edid->extensions)
+		pa = cec_get_edid_phys_addr((const u8 *)edid,
+				EDID_LENGTH * (edid->extensions + 1), NULL);
+	cec_s_phys_addr(adap, pa, false);
+}
+EXPORT_SYMBOL_GPL(cec_s_phys_addr_from_edid);
+
 /*
  * Called from either the ioctl or a driver to set the logical addresses.
  *
diff --git a/include/media/cec.h b/include/media/cec.h
index bfa88d4d67e1..a548b292eeb1 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -206,6 +206,8 @@ static inline bool cec_is_sink(const struct cec_adapter *adap)
 #define cec_phys_addr_exp(pa) \
 	((pa) >> 12), ((pa) >> 8) & 0xf, ((pa) >> 4) & 0xf, (pa) & 0xf
 
+struct edid;
+
 #if IS_ENABLED(CONFIG_CEC_CORE)
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 		void *priv, const char *name, u32 caps, u8 available_las);
@@ -217,6 +219,8 @@ int cec_s_log_addrs(struct cec_adapter *adap, struct cec_log_addrs *log_addrs,
 		    bool block);
 void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr,
 		     bool block);
+void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
+			       const struct edid *edid);
 int cec_transmit_msg(struct cec_adapter *adap, struct cec_msg *msg,
 		     bool block);
 
@@ -326,6 +330,11 @@ static inline void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr,
 {
 }
 
+static inline void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
+					     const struct edid *edid)
+{
+}
+
 static inline u16 cec_get_edid_phys_addr(const u8 *edid, unsigned int size,
 					 unsigned int *offset)
 {
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/9] cec: add cec_phys_addr_invalidate() helper function

[Hans Verkuil]

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

Simplifies setting the physical address to CEC_PHYS_ADDR_INVALID.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 include/media/cec.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/media/cec.h b/include/media/cec.h
index a548b292eeb1..3ce73951591e 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -360,4 +360,17 @@ static inline int cec_phys_addr_validate(u16 phys_addr, u16 *parent, u16 *port)
 
 #endif
 
+/**
+ * cec_phys_addr_invalidate() - set the physical address to INVALID
+ *
+ * @adap:	the CEC adapter
+ *
+ * This is a simple helper function to invalidate the physical
+ * address.
+ */
+static inline void cec_phys_addr_invalidate(struct cec_adapter *adap)
+{
+	cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
+}
+
 #endif /* _MEDIA_CEC_H */
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/9] cec: add cec_transmit_attempt_done helper function

[Hans Verkuil]

A simpler variant of cec_transmit_done to be used where the HW does
just a single attempt at a transmit. So if the status indicates an
error, then the corresponding error count will always be 1 and this
function figures that out based on the status argument.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 Documentation/media/kapi/cec-core.rst | 10 ++++++++++
 drivers/media/cec/cec-adap.c          | 26 ++++++++++++++++++++++++++
 include/media/cec.h                   |  6 ++++++
 3 files changed, 42 insertions(+)

diff --git a/Documentation/media/kapi/cec-core.rst b/Documentation/media/kapi/cec-core.rst
index 278b358b2f2e..8a65c69ed071 100644
--- a/Documentation/media/kapi/cec-core.rst
+++ b/Documentation/media/kapi/cec-core.rst
@@ -194,6 +194,11 @@ When a transmit finished (successfully or otherwise):
 	void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
 		       u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
 
+or:
+
+.. c:function::
+	void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status);
+
 The status can be one of:
 
 CEC_TX_STATUS_OK:
@@ -231,6 +236,11 @@ to 1, if the hardware does support retry then either set these counters to
 0 if the hardware provides no feedback of which errors occurred and how many
 times, or fill in the correct values as reported by the hardware.
 
+The cec_transmit_attempt_done() function is a helper for cases where the
+hardware never retries, so the transmit is always for just a single
+attempt. It will call cec_transmit_done() in turn, filling in 1 for the
+count argument corresponding to the status. Or all 0 if the status was OK.
+
 When a CEC message was received:
 
 .. c:function::
diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index 61e39bbe3cf9..bd76c15ade4f 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -553,6 +553,32 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
 }
 EXPORT_SYMBOL_GPL(cec_transmit_done);
 
+void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status)
+{
+	switch (status) {
+	case CEC_TX_STATUS_OK:
+		cec_transmit_done(adap, status, 0, 0, 0, 0);
+		return;
+	case CEC_TX_STATUS_ARB_LOST:
+		cec_transmit_done(adap, status, 1, 0, 0, 0);
+		return;
+	case CEC_TX_STATUS_NACK:
+		cec_transmit_done(adap, status, 0, 1, 0, 0);
+		return;
+	case CEC_TX_STATUS_LOW_DRIVE:
+		cec_transmit_done(adap, status, 0, 0, 1, 0);
+		return;
+	case CEC_TX_STATUS_ERROR:
+		cec_transmit_done(adap, status, 0, 0, 0, 1);
+		return;
+	default:
+		/* Should never happen */
+		WARN(1, "cec-%s: invalid status 0x%02x\n", adap->name, status);
+		return;
+	}
+}
+EXPORT_SYMBOL_GPL(cec_transmit_attempt_done);
+
 /*
  * Called when waiting for a reply times out.
  */
diff --git a/include/media/cec.h b/include/media/cec.h
index 3ce73951591e..a2e184d1df00 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -227,6 +227,12 @@ int cec_transmit_msg(struct cec_adapter *adap, struct cec_msg *msg,
 /* Called by the adapter */
 void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
 		       u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
+/*
+ * Simplified version of cec_transmit_done for hardware that doesn't retry
+ * failed transmits. So this is always just one attempt in which case
+ * the status is sufficient.
+ */
+void cec_transmit_attempt_done(struct cec_adapter *adap, u8 status);
 void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
 
 /**
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/9] stih-cec/vivid/pulse8/rainshadow: use cec_transmit_attempt_done

[Hans Verkuil]

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

Use the helper function cec_transmit_attempt_done instead of
cec_transmit_done to simplify the code.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
---
 drivers/media/platform/sti/cec/stih-cec.c         | 9 ++++-----
 drivers/media/platform/vivid/vivid-cec.c          | 6 +++---
 drivers/media/usb/pulse8-cec/pulse8-cec.c         | 9 +++------
 drivers/media/usb/rainshadow-cec/rainshadow-cec.c | 9 +++------
 4 files changed, 13 insertions(+), 20 deletions(-)

diff --git a/drivers/media/platform/sti/cec/stih-cec.c b/drivers/media/platform/sti/cec/stih-cec.c
index 6f9f03670b56..dccbdaebb7a8 100644
--- a/drivers/media/platform/sti/cec/stih-cec.c
+++ b/drivers/media/platform/sti/cec/stih-cec.c
@@ -226,22 +226,21 @@ static int stih_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
 static void stih_tx_done(struct stih_cec *cec, u32 status)
 {
 	if (status & CEC_TX_ERROR) {
-		cec_transmit_done(cec->adap, CEC_TX_STATUS_ERROR, 0, 0, 0, 1);
+		cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_ERROR);
 		return;
 	}
 
 	if (status & CEC_TX_ARB_ERROR) {
-		cec_transmit_done(cec->adap,
-				  CEC_TX_STATUS_ARB_LOST, 1, 0, 0, 0);
+		cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_ARB_LOST);
 		return;
 	}
 
 	if (!(status & CEC_TX_ACK_GET_STS)) {
-		cec_transmit_done(cec->adap, CEC_TX_STATUS_NACK, 0, 1, 0, 0);
+		cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_NACK);
 		return;
 	}
 
-	cec_transmit_done(cec->adap, CEC_TX_STATUS_OK, 0, 0, 0, 0);
+	cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_OK);
 }
 
 static void stih_rx_done(struct stih_cec *cec, u32 status)
diff --git a/drivers/media/platform/vivid/vivid-cec.c b/drivers/media/platform/vivid/vivid-cec.c
index 653f4099f737..e15705758969 100644
--- a/drivers/media/platform/vivid/vivid-cec.c
+++ b/drivers/media/platform/vivid/vivid-cec.c
@@ -34,7 +34,7 @@ void vivid_cec_bus_free_work(struct vivid_dev *dev)
 		cancel_delayed_work_sync(&cw->work);
 		spin_lock(&dev->cec_slock);
 		list_del(&cw->list);
-		cec_transmit_done(cw->adap, CEC_TX_STATUS_LOW_DRIVE, 0, 0, 1, 0);
+		cec_transmit_attempt_done(cw->adap, CEC_TX_STATUS_LOW_DRIVE);
 		kfree(cw);
 	}
 	spin_unlock(&dev->cec_slock);
@@ -84,7 +84,7 @@ static void vivid_cec_xfer_done_worker(struct work_struct *work)
 	dev->cec_xfer_start_jiffies = 0;
 	list_del(&cw->list);
 	spin_unlock(&dev->cec_slock);
-	cec_transmit_done(cw->adap, cw->tx_status, 0, valid_dest ? 0 : 1, 0, 0);
+	cec_transmit_attempt_done(cw->adap, cw->tx_status);
 
 	/* Broadcast message */
 	if (adap != dev->cec_rx_adap)
@@ -105,7 +105,7 @@ static void vivid_cec_xfer_try_worker(struct work_struct *work)
 	if (dev->cec_xfer_time_jiffies) {
 		list_del(&cw->list);
 		spin_unlock(&dev->cec_slock);
-		cec_transmit_done(cw->adap, CEC_TX_STATUS_ARB_LOST, 1, 0, 0, 0);
+		cec_transmit_attempt_done(cw->adap, CEC_TX_STATUS_ARB_LOST);
 		kfree(cw);
 	} else {
 		INIT_DELAYED_WORK(&cw->work, vivid_cec_xfer_done_worker);
diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c b/drivers/media/usb/pulse8-cec/pulse8-cec.c
index 1dfc2de1fe77..c843070f24c1 100644
--- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
+++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
@@ -148,18 +148,15 @@ static void pulse8_irq_work_handler(struct work_struct *work)
 		cec_received_msg(pulse8->adap, &pulse8->rx_msg);
 		break;
 	case MSGCODE_TRANSMIT_SUCCEEDED:
-		cec_transmit_done(pulse8->adap, CEC_TX_STATUS_OK,
-				  0, 0, 0, 0);
+		cec_transmit_attempt_done(pulse8->adap, CEC_TX_STATUS_OK);
 		break;
 	case MSGCODE_TRANSMIT_FAILED_ACK:
-		cec_transmit_done(pulse8->adap, CEC_TX_STATUS_NACK,
-				  0, 1, 0, 0);
+		cec_transmit_attempt_done(pulse8->adap, CEC_TX_STATUS_NACK);
 		break;
 	case MSGCODE_TRANSMIT_FAILED_LINE:
 	case MSGCODE_TRANSMIT_FAILED_TIMEOUT_DATA:
 	case MSGCODE_TRANSMIT_FAILED_TIMEOUT_LINE:
-		cec_transmit_done(pulse8->adap, CEC_TX_STATUS_ERROR,
-				  0, 0, 0, 1);
+		cec_transmit_attempt_done(pulse8->adap, CEC_TX_STATUS_ERROR);
 		break;
 	}
 }
diff --git a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
index ad468efc4399..f203699e9c1b 100644
--- a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
+++ b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
@@ -98,16 +98,13 @@ static void rain_process_msg(struct rain *rain)
 
 	switch (stat) {
 	case 1:
-		cec_transmit_done(rain->adap, CEC_TX_STATUS_OK,
-				  0, 0, 0, 0);
+		cec_transmit_attempt_done(rain->adap, CEC_TX_STATUS_OK);
 		break;
 	case 2:
-		cec_transmit_done(rain->adap, CEC_TX_STATUS_NACK,
-				  0, 1, 0, 0);
+		cec_transmit_attempt_done(rain->adap, CEC_TX_STATUS_NACK);
 		break;
 	default:
-		cec_transmit_done(rain->adap, CEC_TX_STATUS_LOW_DRIVE,
-				  0, 0, 0, 1);
+		cec_transmit_attempt_done(rain->adap, CEC_TX_STATUS_LOW_DRIVE);
 		break;
 	}
 }
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/9] cec: add CEC_CAP_NEEDS_HPD

[Hans Verkuil]

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

Add a new capability CEC_CAP_NEEDS_HPD. If this capability is set
then the hardware can only use CEC if the HDMI Hotplug Detect pin
is high. Such hardware cannot handle the corner case in the CEC specification
where it is possible to transmit messages even if no hotplug signal is
present (needed for some displays that turn off the HPD when in standby,
but still have CEC enabled).

Typically hardware that needs this capability have the HPD wired to the CEC
block, often to a 'power' or 'active' pin.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
---
 drivers/media/cec/cec-adap.c | 20 ++++++++++++++------
 drivers/media/cec/cec-api.c  |  5 ++++-
 drivers/media/cec/cec-core.c |  1 +
 include/media/cec.h          |  1 +
 include/uapi/linux/cec.h     |  2 ++
 5 files changed, 22 insertions(+), 7 deletions(-)

diff --git a/drivers/media/cec/cec-adap.c b/drivers/media/cec/cec-adap.c
index bd76c15ade4f..bf45977b2823 100644
--- a/drivers/media/cec/cec-adap.c
+++ b/drivers/media/cec/cec-adap.c
@@ -368,6 +368,8 @@ int cec_thread_func(void *_adap)
 			 * transmit should be canceled.
 			 */
 			err = wait_event_interruptible_timeout(adap->kthread_waitq,
+				(adap->needs_hpd &&
+				 (!adap->is_configured && !adap->is_configuring)) ||
 				kthread_should_stop() ||
 				(!adap->transmitting &&
 				 !list_empty(&adap->transmit_queue)),
@@ -383,7 +385,9 @@ int cec_thread_func(void *_adap)
 
 		mutex_lock(&adap->lock);
 
-		if (kthread_should_stop()) {
+		if ((adap->needs_hpd &&
+		     (!adap->is_configured && !adap->is_configuring)) ||
+		    kthread_should_stop()) {
 			cec_flush(adap);
 			goto unlock;
 		}
@@ -682,7 +686,7 @@ int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
 		return -EINVAL;
 	}
 	if (!adap->is_configured && !adap->is_configuring) {
-		if (msg->msg[0] != 0xf0) {
+		if (adap->needs_hpd || msg->msg[0] != 0xf0) {
 			dprintk(1, "%s: adapter is unconfigured\n", __func__);
 			return -ENONET;
 		}
@@ -1158,7 +1162,9 @@ static int cec_config_log_addr(struct cec_adapter *adap,
  */
 static void cec_adap_unconfigure(struct cec_adapter *adap)
 {
-	WARN_ON(adap->ops->adap_log_addr(adap, CEC_LOG_ADDR_INVALID));
+	if (!adap->needs_hpd ||
+	    adap->phys_addr != CEC_PHYS_ADDR_INVALID)
+		WARN_ON(adap->ops->adap_log_addr(adap, CEC_LOG_ADDR_INVALID));
 	adap->log_addrs.log_addr_mask = 0;
 	adap->is_configuring = false;
 	adap->is_configured = false;
@@ -1387,6 +1393,8 @@ void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 	if (phys_addr == adap->phys_addr || adap->devnode.unregistered)
 		return;
 
+	dprintk(1, "new physical address %x.%x.%x.%x\n",
+		cec_phys_addr_exp(phys_addr));
 	if (phys_addr == CEC_PHYS_ADDR_INVALID ||
 	    adap->phys_addr != CEC_PHYS_ADDR_INVALID) {
 		adap->phys_addr = CEC_PHYS_ADDR_INVALID;
@@ -1396,7 +1404,7 @@ void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 		if (adap->monitor_all_cnt)
 			WARN_ON(call_op(adap, adap_monitor_all_enable, false));
 		mutex_lock(&adap->devnode.lock);
-		if (list_empty(&adap->devnode.fhs))
+		if (adap->needs_hpd || list_empty(&adap->devnode.fhs))
 			WARN_ON(adap->ops->adap_enable(adap, false));
 		mutex_unlock(&adap->devnode.lock);
 		if (phys_addr == CEC_PHYS_ADDR_INVALID)
@@ -1404,7 +1412,7 @@ void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 	}
 
 	mutex_lock(&adap->devnode.lock);
-	if (list_empty(&adap->devnode.fhs) &&
+	if ((adap->needs_hpd || list_empty(&adap->devnode.fhs)) &&
 	    adap->ops->adap_enable(adap, true)) {
 		mutex_unlock(&adap->devnode.lock);
 		return;
@@ -1412,7 +1420,7 @@ void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 
 	if (adap->monitor_all_cnt &&
 	    call_op(adap, adap_monitor_all_enable, true)) {
-		if (list_empty(&adap->devnode.fhs))
+		if (adap->needs_hpd || list_empty(&adap->devnode.fhs))
 			WARN_ON(adap->ops->adap_enable(adap, false));
 		mutex_unlock(&adap->devnode.lock);
 		return;
diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c
index 0860fb458757..1359c3977101 100644
--- a/drivers/media/cec/cec-api.c
+++ b/drivers/media/cec/cec-api.c
@@ -202,7 +202,8 @@ static long cec_transmit(struct cec_adapter *adap, struct cec_fh *fh,
 		err = -EPERM;
 	else if (adap->is_configuring)
 		err = -ENONET;
-	else if (!adap->is_configured && msg.msg[0] != 0xf0)
+	else if (!adap->is_configured &&
+		 (adap->needs_hpd || msg.msg[0] != 0xf0))
 		err = -ENONET;
 	else if (cec_is_busy(adap, fh))
 		err = -EBUSY;
@@ -521,6 +522,7 @@ static int cec_open(struct inode *inode, struct file *filp)
 
 	mutex_lock(&devnode->lock);
 	if (list_empty(&devnode->fhs) &&
+	    !adap->needs_hpd &&
 	    adap->phys_addr == CEC_PHYS_ADDR_INVALID) {
 		err = adap->ops->adap_enable(adap, true);
 		if (err) {
@@ -565,6 +567,7 @@ static int cec_release(struct inode *inode, struct file *filp)
 	mutex_lock(&devnode->lock);
 	list_del(&fh->list);
 	if (list_empty(&devnode->fhs) &&
+	    !adap->needs_hpd &&
 	    adap->phys_addr == CEC_PHYS_ADDR_INVALID) {
 		WARN_ON(adap->ops->adap_enable(adap, false));
 	}
diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index 2f87748ba4fc..b516d599d6c4 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -230,6 +230,7 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
 	adap->log_addrs.vendor_id = CEC_VENDOR_ID_NONE;
 	adap->capabilities = caps;
+	adap->needs_hpd = caps & CEC_CAP_NEEDS_HPD;
 	adap->available_log_addrs = available_las;
 	adap->sequence = 0;
 	adap->ops = ops;
diff --git a/include/media/cec.h b/include/media/cec.h
index a2e184d1df00..7e32e80b243e 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -164,6 +164,7 @@ struct cec_adapter {
 	u8 available_log_addrs;
 
 	u16 phys_addr;
+	bool needs_hpd;
 	bool is_configuring;
 	bool is_configured;
 	u32 monitor_all_cnt;
diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
index a0dfe27bc6c7..44579a24f95d 100644
--- a/include/uapi/linux/cec.h
+++ b/include/uapi/linux/cec.h
@@ -336,6 +336,8 @@ static inline int cec_is_unconfigured(__u16 log_addr_mask)
 #define CEC_CAP_RC		(1 << 4)
 /* Hardware can monitor all messages, not just directed and broadcast. */
 #define CEC_CAP_MONITOR_ALL	(1 << 5)
+/* Hardware can use CEC only if the HDMI HPD pin is high. */
+#define CEC_CAP_NEEDS_HPD	(1 << 6)
 
 /**
  * struct cec_caps - CEC capabilities structure.
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/9] cec-ioc-adap-g-caps.rst: document CEC_CAP_NEEDS_HPD

[Hans Verkuil]

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

Document the new CEC_CAP_NEEDS_HPD capability.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
---
 Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst b/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
index a0e961f11017..6d7bf7bef3eb 100644
--- a/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
+++ b/Documentation/media/uapi/cec/cec-ioc-adap-g-caps.rst
@@ -113,6 +113,14 @@ returns the information to the application. The ioctl never fails.
       - 0x00000020
       - The CEC hardware can monitor all messages, not just directed and
 	broadcast messages.
+    * .. _`CEC-CAP-NEEDS-HPD`:
+
+      - ``CEC_CAP_NEEDS_HPD``
+      - 0x00000040
+      - The CEC hardware is only active if the HDMI Hotplug Detect pin is
+        high. This makes it impossible to use CEC to wake up displays that
+	set the HPD pin low when in standby mode, but keep the CEC bus
+	alive.
 
 
 
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 7/9] dt-bindings: media/s5p-cec.txt: document needs-hpd property

[Hans Verkuil]

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

Needed for boards that wire the CEC pin in such a way that it
is unavailable when the HPD is low.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: devicetree@vger.kernel.org
---
 Documentation/devicetree/bindings/media/s5p-cec.txt | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 4bb08d9d940b..261af4d1a791 100644
--- a/Documentation/devicetree/bindings/media/s5p-cec.txt
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -17,6 +17,12 @@ Required properties:
   - samsung,syscon-phandle - phandle to the PMU system controller
   - hdmi-phandle - phandle to the HDMI controller
 
+Optional:
+  - needs-hpd : if present the CEC support is only available when the HPD
+    is high. Some boards only let the CEC pin through if the HPD is high, for
+    example if there is a level converter that uses the HPD to power up
+    or down.
+
 Example:
 
 hdmicec: cec@100B0000 {
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 8/9] s5p_cec: set the CEC_CAP_NEEDS_HPD flag if needed

[Hans Verkuil]

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

Use the needs-hpd DT property to determine if the CEC_CAP_NEEDS_HPD
should be set.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: devicetree@vger.kernel.org
---
 drivers/media/platform/s5p-cec/s5p_cec.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
index 65a223e578ed..8e06071a7977 100644
--- a/drivers/media/platform/s5p-cec/s5p_cec.c
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -173,6 +173,7 @@ static int s5p_cec_probe(struct platform_device *pdev)
 	struct platform_device *hdmi_dev;
 	struct resource *res;
 	struct s5p_cec_dev *cec;
+	bool needs_hpd = of_property_read_bool(pdev->dev.of_node, "needs-hpd");
 	int ret;
 
 	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
@@ -221,7 +222,8 @@ static int s5p_cec_probe(struct platform_device *pdev)
 	cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
 		CEC_NAME,
 		CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
-		CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
+		CEC_CAP_PASSTHROUGH | CEC_CAP_RC |
+		(needs_hpd ? CEC_CAP_NEEDS_HPD : 0), 1);
 	ret = PTR_ERR_OR_ZERO(cec->adap);
 	if (ret)
 		return ret;
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 9/9] ARM: dts: exynos: add needs-hpd to &hdmicec for Odroid-U3

[Hans Verkuil]

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

The Odroid-U3 board has an IP4791CZ12 level shifter that is
disabled if the HPD is low, which means that the CEC pin is
disabled as well.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Andrzej Hajda <a.hajda@samsung.com>
Cc: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/exynos4412-odroidu3.dts | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
index 7504a5aa538e..7209cb48fc2a 100644
--- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
+++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
@@ -131,3 +131,7 @@
 	cs-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
 	status = "okay";
 };
+
+&hdmicec {
+	needs-hpd;
+};
-- 
2.11.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 9/9] ARM: dts: exynos: add needs-hpd to &hdmicec for Odroid-U3

[Krzysztof Kozlowski]

On Wed, Jun 07, 2017 at 04:46:16PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> The Odroid-U3 board has an IP4791CZ12 level shifter that is
> disabled if the HPD is low, which means that the CEC pin is
> disabled as well.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: devicetree@vger.kernel.org
> ---
>  arch/arm/boot/dts/exynos4412-odroidu3.dts | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
> index 7504a5aa538e..7209cb48fc2a 100644
> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
> @@ -131,3 +131,7 @@
>  	cs-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
>  	status = "okay";
>  };
> +
> +&hdmicec {
> +	needs-hpd;
> +};

All good, except we try to keep them sorted alphabetically (helps
avoiding conflicts and makes things easier to find)... which for this
particular file will be difficult as it is semi-sorted. :)
Anyway, how about putting this new node after &buck?

Best regards,
Krzysztof

Re: [PATCH 9/9] ARM: dts: exynos: add needs-hpd to &hdmicec for Odroid-U3

[Hans Verkuil]

On 07/06/17 20:36, Krzysztof Kozlowski wrote:
> On Wed, Jun 07, 2017 at 04:46:16PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>>
>> The Odroid-U3 board has an IP4791CZ12 level shifter that is
>> disabled if the HPD is low, which means that the CEC pin is
>> disabled as well.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>> Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>> Cc: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> ---
>>  arch/arm/boot/dts/exynos4412-odroidu3.dts | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/arch/arm/boot/dts/exynos4412-odroidu3.dts b/arch/arm/boot/dts/exynos4412-odroidu3.dts
>> index 7504a5aa538e..7209cb48fc2a 100644
>> --- a/arch/arm/boot/dts/exynos4412-odroidu3.dts
>> +++ b/arch/arm/boot/dts/exynos4412-odroidu3.dts
>> @@ -131,3 +131,7 @@
>>  	cs-gpios = <&gpb 5 GPIO_ACTIVE_HIGH>;
>>  	status = "okay";
>>  };
>> +
>> +&hdmicec {
>> +	needs-hpd;
>> +};
> 
> All good, except we try to keep them sorted alphabetically (helps
> avoiding conflicts and makes things easier to find)... which for this
> particular file will be difficult as it is semi-sorted. :)
> Anyway, how about putting this new node after &buck?

Done!

	Hans

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

Re: [PATCH 7/9] dt-bindings: media/s5p-cec.txt: document needs-hpd property

[Rob Herring]

On Wed, Jun 07, 2017 at 04:46:14PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Needed for boards that wire the CEC pin in such a way that it
> is unavailable when the HPD is low.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Krzysztof Kozlowski <krzk@kernel.org>
> Cc: Andrzej Hajda <a.hajda@samsung.com>
> Cc: devicetree@vger.kernel.org
> ---
>  Documentation/devicetree/bindings/media/s5p-cec.txt | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
> index 4bb08d9d940b..261af4d1a791 100644
> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
> @@ -17,6 +17,12 @@ Required properties:
>    - samsung,syscon-phandle - phandle to the PMU system controller
>    - hdmi-phandle - phandle to the HDMI controller
>  
> +Optional:
> +  - needs-hpd : if present the CEC support is only available when the HPD
> +    is high. Some boards only let the CEC pin through if the HPD is high, for
> +    example if there is a level converter that uses the HPD to power up
> +    or down.

Seems like something common. Can you document in a common location?

> +
>  Example:
>  
>  hdmicec: cec@100B0000 {
> -- 
> 2.11.0
> 
> _______________________________________________
> 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: [PATCH 7/9] dt-bindings: media/s5p-cec.txt: document needs-hpd property

[Hans Verkuil]

On 09/06/17 16:07, Rob Herring wrote:
> On Wed, Jun 07, 2017 at 04:46:14PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Needed for boards that wire the CEC pin in such a way that it
>> is unavailable when the HPD is low.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Cc: Krzysztof Kozlowski <krzk@kernel.org>
>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>> Cc: devicetree@vger.kernel.org
>> ---
>>  Documentation/devicetree/bindings/media/s5p-cec.txt | 6 ++++++
>>  1 file changed, 6 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
>> index 4bb08d9d940b..261af4d1a791 100644
>> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
>> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
>> @@ -17,6 +17,12 @@ Required properties:
>>    - samsung,syscon-phandle - phandle to the PMU system controller
>>    - hdmi-phandle - phandle to the HDMI controller
>>  
>> +Optional:
>> +  - needs-hpd : if present the CEC support is only available when the HPD
>> +    is high. Some boards only let the CEC pin through if the HPD is high, for
>> +    example if there is a level converter that uses the HPD to power up
>> +    or down.
> 
> Seems like something common. Can you document in a common location?

Should we do the same with hdmi-phandle? It is also used by CEC drivers to find
the HDMI driver.

Currently only used by s5p-cec and stih-cec, but there will be more.

I guess this would be a sensible place to document this:

Documentation/devicetree/bindings/media/cec.txt

Regards,

	Hans

> 
>> +
>>  Example:
>>  
>>  hdmicec: cec@100B0000 {
>> -- 
>> 2.11.0
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/9] dt-bindings: media/s5p-cec.txt: document needs-hpd property

[Rob Herring]

On Fri, Jun 9, 2017 at 9:11 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
> On 09/06/17 16:07, Rob Herring wrote:
>> On Wed, Jun 07, 2017 at 04:46:14PM +0200, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Needed for boards that wire the CEC pin in such a way that it
>>> is unavailable when the HPD is low.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>
>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>> Cc: devicetree@vger.kernel.org
>>> ---
>>>  Documentation/devicetree/bindings/media/s5p-cec.txt | 6 ++++++
>>>  1 file changed, 6 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
>>> index 4bb08d9d940b..261af4d1a791 100644
>>> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
>>> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
>>> @@ -17,6 +17,12 @@ Required properties:
>>>    - samsung,syscon-phandle - phandle to the PMU system controller
>>>    - hdmi-phandle - phandle to the HDMI controller
>>>
>>> +Optional:
>>> +  - needs-hpd : if present the CEC support is only available when the HPD
>>> +    is high. Some boards only let the CEC pin through if the HPD is high, for
>>> +    example if there is a level converter that uses the HPD to power up
>>> +    or down.
>>
>> Seems like something common. Can you document in a common location?
>
> Should we do the same with hdmi-phandle? It is also used by CEC drivers to find
> the HDMI driver.

Yes.

> Currently only used by s5p-cec and stih-cec, but there will be more.
>
> I guess this would be a sensible place to document this:
>
> Documentation/devicetree/bindings/media/cec.txt

Sounds good. You can do this as a follow-up to this patch if you want.
For this one:

Acked-by: Rob Herring <robh@kernel.org>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 7/9] dt-bindings: media/s5p-cec.txt: document needs-hpd property

[Hans Verkuil]

On 06/09/2017 05:31 PM, Rob Herring wrote:
> On Fri, Jun 9, 2017 at 9:11 AM, Hans Verkuil <hverkuil@xs4all.nl> wrote:
>> On 09/06/17 16:07, Rob Herring wrote:
>>> On Wed, Jun 07, 2017 at 04:46:14PM +0200, Hans Verkuil wrote:
>>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>>
>>>> Needed for boards that wire the CEC pin in such a way that it
>>>> is unavailable when the HPD is low.
>>>>
>>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>>> Cc: Krzysztof Kozlowski <krzk@kernel.org>
>>>> Cc: Andrzej Hajda <a.hajda@samsung.com>
>>>> Cc: devicetree@vger.kernel.org
>>>> ---
>>>>  Documentation/devicetree/bindings/media/s5p-cec.txt | 6 ++++++
>>>>  1 file changed, 6 insertions(+)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
>>>> index 4bb08d9d940b..261af4d1a791 100644
>>>> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
>>>> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
>>>> @@ -17,6 +17,12 @@ Required properties:
>>>>    - samsung,syscon-phandle - phandle to the PMU system controller
>>>>    - hdmi-phandle - phandle to the HDMI controller
>>>>
>>>> +Optional:
>>>> +  - needs-hpd : if present the CEC support is only available when the HPD
>>>> +    is high. Some boards only let the CEC pin through if the HPD is high, for
>>>> +    example if there is a level converter that uses the HPD to power up
>>>> +    or down.
>>>
>>> Seems like something common. Can you document in a common location?
>>
>> Should we do the same with hdmi-phandle? It is also used by CEC drivers to find
>> the HDMI driver.
> 
> Yes.
> 
>> Currently only used by s5p-cec and stih-cec, but there will be more.
>>
>> I guess this would be a sensible place to document this:
>>
>> Documentation/devicetree/bindings/media/cec.txt
> 
> Sounds good. You can do this as a follow-up to this patch if you want.
> For this one:
> 
> Acked-by: Rob Herring <robh@kernel.org>
> 

Sorry, I have what might be a stupid question: should I update the s5p-cec.txt
to refer to the cec.txt bindings file for the hdmi-phandle and needs-hpd instead
of describing it here? It seems pointless to do that for the hdmi-phandle, but
it might make more sense for the needs-hpd property.

E.g.:

  - needs-hpd: CEC support is only present if HPD is high. See cec.txt for more
    details.


Proposed text for cec.txt:

-------------------------------------------------
Common bindings for HDMI CEC adapters

- hdmi-phandle: phandle to the HDMI controller.

- needs-hpd: if present the CEC support is only available when the HPD
  is high. Some boards only let the CEC pin through if the HPD is high, for
  example if there is a level converter that uses the HPD to power up
  or down.
-------------------------------------------------

Regards,

	Hans

Re: [PATCH 7/9] dt-bindings: media/s5p-cec.txt: document needs-hpd property

[Rob Herring]

On Fri, Jun 9, 2017 at 10:55 AM, Hans Verkuil <hansverk-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org> wrote:
> On 06/09/2017 05:31 PM, Rob Herring wrote:
>> On Fri, Jun 9, 2017 at 9:11 AM, Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org> wrote:
>>> On 09/06/17 16:07, Rob Herring wrote:
>>>> On Wed, Jun 07, 2017 at 04:46:14PM +0200, Hans Verkuil wrote:
>>>>> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>>>>>
>>>>> Needed for boards that wire the CEC pin in such a way that it
>>>>> is unavailable when the HPD is low.
>>>>>
>>>>> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
>>>>> Cc: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>>>> Cc: Andrzej Hajda <a.hajda-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>>>> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>>>>> ---
>>>>>  Documentation/devicetree/bindings/media/s5p-cec.txt | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
>>>>> index 4bb08d9d940b..261af4d1a791 100644
>>>>> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
>>>>> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
>>>>> @@ -17,6 +17,12 @@ Required properties:
>>>>>    - samsung,syscon-phandle - phandle to the PMU system controller
>>>>>    - hdmi-phandle - phandle to the HDMI controller
>>>>>
>>>>> +Optional:
>>>>> +  - needs-hpd : if present the CEC support is only available when the HPD
>>>>> +    is high. Some boards only let the CEC pin through if the HPD is high, for
>>>>> +    example if there is a level converter that uses the HPD to power up
>>>>> +    or down.
>>>>
>>>> Seems like something common. Can you document in a common location?
>>>
>>> Should we do the same with hdmi-phandle? It is also used by CEC drivers to find
>>> the HDMI driver.
>>
>> Yes.
>>
>>> Currently only used by s5p-cec and stih-cec, but there will be more.
>>>
>>> I guess this would be a sensible place to document this:
>>>
>>> Documentation/devicetree/bindings/media/cec.txt
>>
>> Sounds good. You can do this as a follow-up to this patch if you want.
>> For this one:
>>
>> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
>>
>
> Sorry, I have what might be a stupid question: should I update the s5p-cec.txt
> to refer to the cec.txt bindings file for the hdmi-phandle and needs-hpd instead
> of describing it here? It seems pointless to do that for the hdmi-phandle, but
> it might make more sense for the needs-hpd property.

Yes. Just make both say "see ./cec.txt". You're right it doesn't gain
much for hdmi-phandle, but at least indicates it is a standard
property.

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

Re: [PATCH 8/9] s5p_cec: set the CEC_CAP_NEEDS_HPD flag if needed

[Sylwester Nawrocki]

On 06/07/2017 04:46 PM, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
> 
> Use the needs-hpd DT property to determine if the CEC_CAP_NEEDS_HPD
> should be set.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Acked-by: Sylwester Nawrocki <s.nawrocki-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>

> ---
>   drivers/media/platform/s5p-cec/s5p_cec.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/media/platform/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
> index 65a223e578ed..8e06071a7977 100644
> --- a/drivers/media/platform/s5p-cec/s5p_cec.c
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
> @@ -173,6 +173,7 @@ static int s5p_cec_probe(struct platform_device *pdev)
>   	struct platform_device *hdmi_dev;
>   	struct resource *res;
>   	struct s5p_cec_dev *cec;
> +	bool needs_hpd = of_property_read_bool(pdev->dev.of_node, "needs-hpd");

dev->of_node could also be used instead of pdev->dev.of_node.

>   	int ret;
>   
>   	np = of_parse_phandle(pdev->dev.of_node, "hdmi-phandle", 0);
> @@ -221,7 +222,8 @@ static int s5p_cec_probe(struct platform_device *pdev)
>   	cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
>   		CEC_NAME,
>   		CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> -		CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
> +		CEC_CAP_PASSTHROUGH | CEC_CAP_RC |
> +		(needs_hpd ? CEC_CAP_NEEDS_HPD : 0), 1);
>   	ret = PTR_ERR_OR_ZERO(cec->adap);
>   	if (ret)
>   		return ret; 

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

Re: [PATCH 4/9] stih-cec/vivid/pulse8/rainshadow: use cec_transmit_attempt_done

[Benjamin Gaignard]

2017-06-07 16:46 GMT+02:00 Hans Verkuil <hverkuil@xs4all.nl>:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Use the helper function cec_transmit_attempt_done instead of
> cec_transmit_done to simplify the code.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> ---
>  drivers/media/platform/sti/cec/stih-cec.c         | 9 ++++-----
>  drivers/media/platform/vivid/vivid-cec.c          | 6 +++---
>  drivers/media/usb/pulse8-cec/pulse8-cec.c         | 9 +++------
>  drivers/media/usb/rainshadow-cec/rainshadow-cec.c | 9 +++------
>  4 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/drivers/media/platform/sti/cec/stih-cec.c b/drivers/media/platform/sti/cec/stih-cec.c
> index 6f9f03670b56..dccbdaebb7a8 100644
> --- a/drivers/media/platform/sti/cec/stih-cec.c
> +++ b/drivers/media/platform/sti/cec/stih-cec.c
> @@ -226,22 +226,21 @@ static int stih_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
>  static void stih_tx_done(struct stih_cec *cec, u32 status)
>  {
>         if (status & CEC_TX_ERROR) {
> -               cec_transmit_done(cec->adap, CEC_TX_STATUS_ERROR, 0, 0, 0, 1);
> +               cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_ERROR);
>                 return;
>         }
>
>         if (status & CEC_TX_ARB_ERROR) {
> -               cec_transmit_done(cec->adap,
> -                                 CEC_TX_STATUS_ARB_LOST, 1, 0, 0, 0);
> +               cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_ARB_LOST);
>                 return;
>         }
>
>         if (!(status & CEC_TX_ACK_GET_STS)) {
> -               cec_transmit_done(cec->adap, CEC_TX_STATUS_NACK, 0, 1, 0, 0);
> +               cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_NACK);
>                 return;
>         }
>
> -       cec_transmit_done(cec->adap, CEC_TX_STATUS_OK, 0, 0, 0, 0);
> +       cec_transmit_attempt_done(cec->adap, CEC_TX_STATUS_OK);
>  }
>
>  static void stih_rx_done(struct stih_cec *cec, u32 status)
> diff --git a/drivers/media/platform/vivid/vivid-cec.c b/drivers/media/platform/vivid/vivid-cec.c
> index 653f4099f737..e15705758969 100644
> --- a/drivers/media/platform/vivid/vivid-cec.c
> +++ b/drivers/media/platform/vivid/vivid-cec.c
> @@ -34,7 +34,7 @@ void vivid_cec_bus_free_work(struct vivid_dev *dev)
>                 cancel_delayed_work_sync(&cw->work);
>                 spin_lock(&dev->cec_slock);
>                 list_del(&cw->list);
> -               cec_transmit_done(cw->adap, CEC_TX_STATUS_LOW_DRIVE, 0, 0, 1, 0);
> +               cec_transmit_attempt_done(cw->adap, CEC_TX_STATUS_LOW_DRIVE);
>                 kfree(cw);
>         }
>         spin_unlock(&dev->cec_slock);
> @@ -84,7 +84,7 @@ static void vivid_cec_xfer_done_worker(struct work_struct *work)
>         dev->cec_xfer_start_jiffies = 0;
>         list_del(&cw->list);
>         spin_unlock(&dev->cec_slock);
> -       cec_transmit_done(cw->adap, cw->tx_status, 0, valid_dest ? 0 : 1, 0, 0);
> +       cec_transmit_attempt_done(cw->adap, cw->tx_status);
>
>         /* Broadcast message */
>         if (adap != dev->cec_rx_adap)
> @@ -105,7 +105,7 @@ static void vivid_cec_xfer_try_worker(struct work_struct *work)
>         if (dev->cec_xfer_time_jiffies) {
>                 list_del(&cw->list);
>                 spin_unlock(&dev->cec_slock);
> -               cec_transmit_done(cw->adap, CEC_TX_STATUS_ARB_LOST, 1, 0, 0, 0);
> +               cec_transmit_attempt_done(cw->adap, CEC_TX_STATUS_ARB_LOST);
>                 kfree(cw);
>         } else {
>                 INIT_DELAYED_WORK(&cw->work, vivid_cec_xfer_done_worker);
> diff --git a/drivers/media/usb/pulse8-cec/pulse8-cec.c b/drivers/media/usb/pulse8-cec/pulse8-cec.c
> index 1dfc2de1fe77..c843070f24c1 100644
> --- a/drivers/media/usb/pulse8-cec/pulse8-cec.c
> +++ b/drivers/media/usb/pulse8-cec/pulse8-cec.c
> @@ -148,18 +148,15 @@ static void pulse8_irq_work_handler(struct work_struct *work)
>                 cec_received_msg(pulse8->adap, &pulse8->rx_msg);
>                 break;
>         case MSGCODE_TRANSMIT_SUCCEEDED:
> -               cec_transmit_done(pulse8->adap, CEC_TX_STATUS_OK,
> -                                 0, 0, 0, 0);
> +               cec_transmit_attempt_done(pulse8->adap, CEC_TX_STATUS_OK);
>                 break;
>         case MSGCODE_TRANSMIT_FAILED_ACK:
> -               cec_transmit_done(pulse8->adap, CEC_TX_STATUS_NACK,
> -                                 0, 1, 0, 0);
> +               cec_transmit_attempt_done(pulse8->adap, CEC_TX_STATUS_NACK);
>                 break;
>         case MSGCODE_TRANSMIT_FAILED_LINE:
>         case MSGCODE_TRANSMIT_FAILED_TIMEOUT_DATA:
>         case MSGCODE_TRANSMIT_FAILED_TIMEOUT_LINE:
> -               cec_transmit_done(pulse8->adap, CEC_TX_STATUS_ERROR,
> -                                 0, 0, 0, 1);
> +               cec_transmit_attempt_done(pulse8->adap, CEC_TX_STATUS_ERROR);
>                 break;
>         }
>  }
> diff --git a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
> index ad468efc4399..f203699e9c1b 100644
> --- a/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
> +++ b/drivers/media/usb/rainshadow-cec/rainshadow-cec.c
> @@ -98,16 +98,13 @@ static void rain_process_msg(struct rain *rain)
>
>         switch (stat) {
>         case 1:
> -               cec_transmit_done(rain->adap, CEC_TX_STATUS_OK,
> -                                 0, 0, 0, 0);
> +               cec_transmit_attempt_done(rain->adap, CEC_TX_STATUS_OK);
>                 break;
>         case 2:
> -               cec_transmit_done(rain->adap, CEC_TX_STATUS_NACK,
> -                                 0, 1, 0, 0);
> +               cec_transmit_attempt_done(rain->adap, CEC_TX_STATUS_NACK);
>                 break;
>         default:
> -               cec_transmit_done(rain->adap, CEC_TX_STATUS_LOW_DRIVE,
> -                                 0, 0, 0, 1);
> +               cec_transmit_attempt_done(rain->adap, CEC_TX_STATUS_LOW_DRIVE);
>                 break;
>         }
>  }
> --
> 2.11.0
>

Acked-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>