[PATCHv2 0/4] video/exynos/cec: add HPD state notifier & use in s5p-cec

[PATCHv2 0/4] video/exynos/cec: add HPD state notifier & use in s5p-cec

[Hans Verkuil]

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

This patch series adds the hotplug detect notifier code, based on Russell's code:

https://patchwork.kernel.org/patch/9277043/

It adds support for it to the exynos_hdmi drm driver, adds support for
it to the CEC framework and finally adds support to the s5p-cec driver,
which now can be moved out of staging.

Tested with my Odroid U3 exynos4 devboard.

Comments are welcome. I'd like to get this in for the 4.11 kernel as
this is a missing piece needed to integrate CEC drivers.

Changes since v1:

Renamed HDMI notifier to HPD (hotplug detect) notifier since this code is
not HDMI specific, but is interesting for any video source that has to
deal with hotplug detect and EDID/ELD (HDMI, DVI, VGA, DP, ....).
Only the use with CEC adapters is HDMI specific, but the HPD notifier
is more generic.

Regards,

	Hans

Hans Verkuil (4):
  video: add hotplug detect notifier support
  exynos_hdmi: add HPD notifier support
  cec: integrate HPD notifier support
  s5p-cec: add hpd-notifier support, move out of staging

 .../devicetree/bindings/media/s5p-cec.txt          |   2 +
 arch/arm/boot/dts/exynos4.dtsi                     |   1 +
 drivers/gpu/drm/exynos/Kconfig                     |   1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c               |  24 +++-
 drivers/media/cec/cec-core.c                       |  50 ++++++++
 drivers/media/platform/Kconfig                     |  18 +++
 drivers/media/platform/Makefile                    |   1 +
 .../media => media/platform}/s5p-cec/Makefile      |   0
 .../platform}/s5p-cec/exynos_hdmi_cec.h            |   0
 .../platform}/s5p-cec/exynos_hdmi_cecctrl.c        |   0
 .../media => media/platform}/s5p-cec/regs-cec.h    |   0
 .../media => media/platform}/s5p-cec/s5p_cec.c     |  35 +++++-
 .../media => media/platform}/s5p-cec/s5p_cec.h     |   3 +
 drivers/staging/media/Kconfig                      |   2 -
 drivers/staging/media/Makefile                     |   1 -
 drivers/staging/media/s5p-cec/Kconfig              |   9 --
 drivers/staging/media/s5p-cec/TODO                 |   7 --
 drivers/video/Kconfig                              |   3 +
 drivers/video/Makefile                             |   1 +
 drivers/video/hpd-notifier.c                       | 134 +++++++++++++++++++++
 include/linux/hpd-notifier.h                       | 109 +++++++++++++++++
 include/media/cec.h                                |  15 +++
 22 files changed, 389 insertions(+), 27 deletions(-)
 rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
 delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
 delete mode 100644 drivers/staging/media/s5p-cec/TODO
 create mode 100644 drivers/video/hpd-notifier.c
 create mode 100644 include/linux/hpd-notifier.h

-- 
2.8.1

[PATCHv2 1/4] video: add hotplug detect notifier support

[Hans Verkuil]

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

Add support for video hotplug detect and EDID/ELD notifiers, which is used
to convey information from video drivers to their CEC and audio counterparts.

Based on an earlier version from Russell King:

https://patchwork.kernel.org/patch/9277043/

The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
of a video device.

When a new notifier is registered the current state will be reported to
that notifier at registration time.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/video/Kconfig        |   3 +
 drivers/video/Makefile       |   1 +
 drivers/video/hpd-notifier.c | 134 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/hpd-notifier.h | 109 +++++++++++++++++++++++++++++++++++
 4 files changed, 247 insertions(+)
 create mode 100644 drivers/video/hpd-notifier.c
 create mode 100644 include/linux/hpd-notifier.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3c20af9..cddc860 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
 config HDMI
 	bool
 
+config HPD_NOTIFIERS
+	bool
+
 if VT
 	source "drivers/video/console/Kconfig"
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9ad3c17..424698b 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -1,5 +1,6 @@
 obj-$(CONFIG_VGASTATE)            += vgastate.o
 obj-$(CONFIG_HDMI)                += hdmi.o
+obj-$(CONFIG_HPD_NOTIFIERS)       += hpd-notifier.o
 
 obj-$(CONFIG_VT)		  += console/
 obj-$(CONFIG_LOGO)		  += logo/
diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
new file mode 100644
index 0000000..54f7a6b
--- /dev/null
+++ b/drivers/video/hpd-notifier.c
@@ -0,0 +1,134 @@
+#include <linux/export.h>
+#include <linux/hpd-notifier.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+
+static LIST_HEAD(hpd_notifiers);
+static DEFINE_MUTEX(hpd_notifiers_lock);
+
+struct hpd_notifier *hpd_notifier_get(struct device *dev)
+{
+	struct hpd_notifier *n;
+
+	mutex_lock(&hpd_notifiers_lock);
+	list_for_each_entry(n, &hpd_notifiers, head) {
+		if (n->dev == dev) {
+			mutex_unlock(&hpd_notifiers_lock);
+			kref_get(&n->kref);
+			return n;
+		}
+	}
+	n = kzalloc(sizeof(*n), GFP_KERNEL);
+	if (!n)
+		goto unlock;
+	n->dev = dev;
+	mutex_init(&n->lock);
+	BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
+	kref_init(&n->kref);
+	list_add_tail(&n->head, &hpd_notifiers);
+unlock:
+	mutex_unlock(&hpd_notifiers_lock);
+	return n;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_get);
+
+static void hpd_notifier_release(struct kref *kref)
+{
+	struct hpd_notifier *n =
+		container_of(kref, struct hpd_notifier, kref);
+
+	mutex_lock(&hpd_notifiers_lock);
+	list_del(&n->head);
+	mutex_unlock(&hpd_notifiers_lock);
+	kfree(n->edid);
+	kfree(n);
+}
+
+void hpd_notifier_put(struct hpd_notifier *n)
+{
+	kref_put(&n->kref, hpd_notifier_release);
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_put);
+
+int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
+{
+	int ret = blocking_notifier_chain_register(&n->notifiers, nb);
+
+	if (ret)
+		return ret;
+	kref_get(&n->kref);
+	mutex_lock(&n->lock);
+	if (n->connected) {
+		blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
+		if (n->edid_size)
+			blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
+		if (n->has_eld)
+			blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
+	}
+	mutex_unlock(&n->lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_register);
+
+int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
+{
+	int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
+
+	if (ret == 0)
+		hpd_notifier_put(n);
+	return ret;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
+
+void hpd_event_connect(struct hpd_notifier *n)
+{
+	mutex_lock(&n->lock);
+	n->connected = true;
+	blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
+	mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_connect);
+
+void hpd_event_disconnect(struct hpd_notifier *n)
+{
+	mutex_lock(&n->lock);
+	n->connected = false;
+	n->has_eld = false;
+	n->edid_size = 0;
+	blocking_notifier_call_chain(&n->notifiers, HPD_DISCONNECTED, n);
+	mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_disconnect);
+
+int hpd_event_new_edid(struct hpd_notifier *n, const void *edid, size_t size)
+{
+	mutex_lock(&n->lock);
+	if (n->edid_allocated_size < size) {
+		void *p = kmalloc(size, GFP_KERNEL);
+
+		if (p == NULL) {
+			mutex_unlock(&n->lock);
+			return -ENOMEM;
+		}
+		kfree(n->edid);
+		n->edid = p;
+		n->edid_allocated_size = size;
+	}
+	memcpy(n->edid, edid, size);
+	n->edid_size = size;
+	blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
+	mutex_unlock(&n->lock);
+	return 0;
+}
+EXPORT_SYMBOL_GPL(hpd_event_new_edid);
+
+void hpd_event_new_eld(struct hpd_notifier *n, const u8 eld[128])
+{
+	mutex_lock(&n->lock);
+	memcpy(n->eld, eld, sizeof(n->eld));
+	n->has_eld = true;
+	blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
+	mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_new_eld);
diff --git a/include/linux/hpd-notifier.h b/include/linux/hpd-notifier.h
new file mode 100644
index 0000000..4dcb451
--- /dev/null
+++ b/include/linux/hpd-notifier.h
@@ -0,0 +1,109 @@
+/*
+ * hpd-notifier.h - notify interested parties of (dis)connect and EDID
+ * events
+ *
+ * Copyright 2016 Russell King <rmk+kernel@arm.linux.org.uk>
+ * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+#ifndef LINUX_HPD_NOTIFIER_H
+#define LINUX_HPD_NOTIFIER_H
+
+#include <linux/types.h>
+#include <linux/notifier.h>
+#include <linux/kref.h>
+
+enum {
+	HPD_CONNECTED,
+	HPD_DISCONNECTED,
+	HPD_NEW_EDID,
+	HPD_NEW_ELD,
+};
+
+struct device;
+
+struct hpd_notifier {
+	struct mutex lock;
+	struct list_head head;
+	struct kref kref;
+	struct blocking_notifier_head notifiers;
+	struct device *dev;
+
+	/* Current state */
+	bool connected;
+	bool has_eld;
+	unsigned char eld[128];
+	void *edid;
+	size_t edid_size;
+	size_t edid_allocated_size;
+};
+
+/**
+ * hpd_notifier_get - find or create a new hpd_notifier for the given device.
+ * @dev: device that sends the events.
+ *
+ * If a notifier for device @dev already exists, then increase the refcount
+ * and return that notifier.
+ *
+ * If it doesn't exist, then allocate a new notifier struct and return a
+ * pointer to that new struct.
+ *
+ * Return NULL if the memory could not be allocated.
+ */
+struct hpd_notifier *hpd_notifier_get(struct device *dev);
+
+/**
+ * hpd_notifier_put - decrease refcount and delete when the refcount reaches 0.
+ * @n: notifier
+ */
+void hpd_notifier_put(struct hpd_notifier *n);
+
+/**
+ * hpd_notifier_register - register the notifier with the notifier_block.
+ * @n: the HPD notifier
+ * @nb: the notifier_block
+ */
+int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb);
+
+/**
+ * hpd_notifier_unregister - unregister the notifier with the notifier_block.
+ * @n: the HPD notifier
+ * @nb: the notifier_block
+ */
+int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb);
+
+/**
+ * hpd_event_connect - send a connect event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_CONNECTED event to any registered parties.
+ */
+void hpd_event_connect(struct hpd_notifier *n);
+
+/**
+ * hpd_event_disconnect - send a disconnect event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_DISCONNECTED event to any registered parties.
+ */
+void hpd_event_disconnect(struct hpd_notifier *n);
+
+/**
+ * hpd_event_new_edid - send a new EDID event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_NEW_EDID event to any registered parties.
+ * This function will make a copy the EDID so it can return -ENOMEM if
+ * no memory could be allocated.
+ */
+int hpd_event_new_edid(struct hpd_notifier *n, const void *edid, size_t size);
+
+/**
+ * hpd_event_new_eld - send a new ELD event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_NEW_ELD event to any registered parties.
+ */
+void hpd_event_new_eld(struct hpd_notifier *n, const u8 eld[128]);
+
+#endif
-- 
2.8.1

[PATCHv2 2/4] exynos_hdmi: add HPD notifier support

[Hans Verkuil]

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

Implement the HPD notifier support to allow CEC drivers to
be informed when there is a new EDID and when a connect or
disconnect happens.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/gpu/drm/exynos/Kconfig       |  1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
 2 files changed, 22 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index d706ca4..80bfd1d 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
 config DRM_EXYNOS_HDMI
 	bool "HDMI"
 	depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
+	select HPD_NOTIFIERS
 	help
 	  Choose this option if you want to use Exynos HDMI for DRM.
 
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8b1e..28bf609 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -31,6 +31,7 @@
 #include <linux/pm_runtime.h>
 #include <linux/clk.h>
 #include <linux/gpio/consumer.h>
+#include <linux/hpd-notifier.h>
 #include <linux/regulator/consumer.h>
 #include <linux/io.h>
 #include <linux/of_address.h>
@@ -118,6 +119,7 @@ struct hdmi_context {
 	bool				dvi_mode;
 	struct delayed_work		hotplug_work;
 	struct drm_display_mode		current_mode;
+	struct hpd_notifier		*notifier;
 	const struct hdmi_driver_data	*drv_data;
 
 	void __iomem			*regs;
@@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
 {
 	struct hdmi_context *hdata = connector_to_hdmi(connector);
 
-	if (gpiod_get_value(hdata->hpd_gpio))
+	if (gpiod_get_value(hdata->hpd_gpio)) {
+		hpd_event_connect(hdata->notifier);
 		return connector_status_connected;
+	}
 
+	hpd_event_disconnect(hdata->notifier);
 	return connector_status_disconnected;
 }
 
@@ -848,6 +853,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
 		edid->width_cm, edid->height_cm);
 
 	drm_mode_connector_update_edid_property(connector, edid);
+	hpd_event_connect(hdata->notifier);
+	hpd_event_new_edid(hdata->notifier, edid,
+			    EDID_LENGTH * (1 + edid->extensions));
 
 	ret = drm_add_edid_modes(connector, edid);
 
@@ -1483,6 +1491,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
 	if (funcs && funcs->disable)
 		(*funcs->disable)(crtc);
 
+	hpd_event_disconnect(hdata->notifier);
 	cancel_delayed_work(&hdata->hotplug_work);
 
 	hdmiphy_disable(hdata);
@@ -1832,15 +1841,22 @@ static int hdmi_probe(struct platform_device *pdev)
 		}
 	}
 
+	hdata->notifier = hpd_notifier_get(&pdev->dev);
+	if (hdata->notifier == NULL) {
+		ret = -ENOMEM;
+		goto err_hdmiphy;
+	}
+
 	pm_runtime_enable(dev);
 
 	ret = component_add(&pdev->dev, &hdmi_component_ops);
 	if (ret)
-		goto err_disable_pm_runtime;
+		goto err_notifier_put;
 
 	return ret;
 
-err_disable_pm_runtime:
+err_notifier_put:
+	hpd_notifier_put(hdata->notifier);
 	pm_runtime_disable(dev);
 
 err_hdmiphy:
@@ -1859,9 +1875,11 @@ static int hdmi_remove(struct platform_device *pdev)
 	struct hdmi_context *hdata = platform_get_drvdata(pdev);
 
 	cancel_delayed_work_sync(&hdata->hotplug_work);
+	hpd_event_disconnect(hdata->notifier);
 
 	component_del(&pdev->dev, &hdmi_component_ops);
 
+	hpd_notifier_put(hdata->notifier);
 	pm_runtime_disable(&pdev->dev);
 
 	if (!IS_ERR(hdata->reg_hdmi_en))
-- 
2.8.1

[PATCHv2 3/4] cec: integrate HPD notifier support

[Hans Verkuil]

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

Support the HPD notifier framework, simplifying drivers that
depend on this.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 drivers/media/cec/cec-core.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
 include/media/cec.h          | 15 +++++++++++++
 2 files changed, 65 insertions(+)

diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index aca3ab8..a466dbe 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -195,6 +195,52 @@ static void cec_devnode_unregister(struct cec_devnode *devnode)
 	put_device(&devnode->dev);
 }
 
+#ifdef CONFIG_HPD_NOTIFIERS
+static u16 parse_hdmi_addr(const struct edid *edid)
+{
+	if (!edid || edid->extensions == 0)
+		return CEC_PHYS_ADDR_INVALID;
+
+	return cec_get_edid_phys_addr((u8 *)edid,
+				EDID_LENGTH * (edid->extensions + 1), NULL);
+}
+
+static int cec_hpd_notify(struct notifier_block *nb, unsigned long event,
+			   void *data)
+{
+	struct cec_adapter *adap = container_of(nb, struct cec_adapter, nb);
+	struct hpd_notifier *n = data;
+	unsigned int phys;
+
+	dprintk(1, "event %lu\n", event);
+
+	switch (event) {
+	case HPD_DISCONNECTED:
+		cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
+		break;
+
+	case HPD_NEW_EDID:
+		phys = parse_hdmi_addr(n->edid);
+		cec_s_phys_addr(adap, phys, false);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+void cec_register_hpd_notifier(struct cec_adapter *adap,
+				struct hpd_notifier *notifier)
+{
+	if (WARN_ON(!adap->devnode.registered))
+		return;
+
+	adap->nb.notifier_call = cec_hpd_notify;
+	adap->notifier = notifier;
+	hpd_notifier_register(adap->notifier, &adap->nb);
+}
+EXPORT_SYMBOL_GPL(cec_register_hpd_notifier);
+#endif
+
 struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 					 void *priv, const char *name, u32 caps,
 					 u8 available_las)
@@ -344,6 +390,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
 	adap->rc = NULL;
 #endif
 	debugfs_remove_recursive(adap->cec_dir);
+#ifdef CONFIG_HPD_NOTIFIERS
+	if (adap->notifier)
+		hpd_notifier_unregister(adap->notifier, &adap->nb);
+#endif
 	cec_devnode_unregister(&adap->devnode);
 }
 EXPORT_SYMBOL_GPL(cec_unregister_adapter);
diff --git a/include/media/cec.h b/include/media/cec.h
index 96a0aa7..d6c7b20 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -28,6 +28,11 @@
 #include <linux/kthread.h>
 #include <linux/timer.h>
 #include <linux/cec-funcs.h>
+#ifdef CONFIG_HPD_NOTIFIERS
+#include <linux/notifier.h>
+#include <linux/hpd-notifier.h>
+#include <drm/drm_edid.h>
+#endif
 #include <media/rc-core.h>
 #include <media/cec-edid.h>
 
@@ -173,6 +178,11 @@ struct cec_adapter {
 	bool passthrough;
 	struct cec_log_addrs log_addrs;
 
+#ifdef CONFIG_HPD_NOTIFIERS
+	struct hpd_notifier	*notifier;
+	struct notifier_block	nb;
+#endif
+
 	struct dentry *cec_dir;
 	struct dentry *status_file;
 
@@ -213,6 +223,11 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
 		       u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
 void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
 
+#ifdef CONFIG_HPD_NOTIFIERS
+void cec_register_hpd_notifier(struct cec_adapter *adap,
+				struct hpd_notifier *notifier);
+#endif
+
 #else
 
 static inline int cec_register_adapter(struct cec_adapter *adap,
-- 
2.8.1

[PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Hans Verkuil]

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

By using the HPD notifier framework there is no longer any reason
to manually set the physical address. This was the one blocking
issue that prevented this driver from going out of staging, so do
this move as well.

Update the bindings documenting the new hdmi phandle and
update exynos4.dtsi accordingly.

Tested with my Odroid U3.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
 .../devicetree/bindings/media/s5p-cec.txt          |  2 ++
 arch/arm/boot/dts/exynos4.dtsi                     |  1 +
 drivers/media/platform/Kconfig                     | 18 +++++++++++
 drivers/media/platform/Makefile                    |  1 +
 .../media => media/platform}/s5p-cec/Makefile      |  0
 .../platform}/s5p-cec/exynos_hdmi_cec.h            |  0
 .../platform}/s5p-cec/exynos_hdmi_cecctrl.c        |  0
 .../media => media/platform}/s5p-cec/regs-cec.h    |  0
 .../media => media/platform}/s5p-cec/s5p_cec.c     | 35 ++++++++++++++++++----
 .../media => media/platform}/s5p-cec/s5p_cec.h     |  3 ++
 drivers/staging/media/Kconfig                      |  2 --
 drivers/staging/media/Makefile                     |  1 -
 drivers/staging/media/s5p-cec/Kconfig              |  9 ------
 drivers/staging/media/s5p-cec/TODO                 |  7 -----
 14 files changed, 55 insertions(+), 24 deletions(-)
 rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
 rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
 delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
 delete mode 100644 drivers/staging/media/s5p-cec/TODO

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 925ab4d..710fc00 100644
--- a/Documentation/devicetree/bindings/media/s5p-cec.txt
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -15,6 +15,7 @@ Required properties:
   - clock-names : from common clock binding: must contain "hdmicec",
 		  corresponding to entry in the clocks property.
   - samsung,syscon-phandle - phandle to the PMU system controller
+  - samsung,hdmi-phandle - phandle to the HDMI controller
 
 Example:
 
@@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
 	clocks = <&clock CLK_HDMI_CEC>;
 	clock-names = "hdmicec";
 	samsung,syscon-phandle = <&pmu_system_controller>;
+	samsung,hdmi-phandle = <&hdmi>;
 	pinctrl-names = "default";
 	pinctrl-0 = <&hdmi_cec>;
 	status = "okay";
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index c64737b..51dfcbb 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -762,6 +762,7 @@
 		clocks = <&clock CLK_HDMI_CEC>;
 		clock-names = "hdmicec";
 		samsung,syscon-phandle = <&pmu_system_controller>;
+		samsung,hdmi-phandle = <&hdmi>;
 		pinctrl-names = "default";
 		pinctrl-0 = <&hdmi_cec>;
 		status = "disabled";
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index d944421..cab1637 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -392,6 +392,24 @@ config VIDEO_TI_SC
 config VIDEO_TI_CSC
 	tristate
 
+menuconfig V4L_CEC_DRIVERS
+	bool "Platform HDMI CEC drivers"
+	depends on MEDIA_CEC_SUPPORT
+
+if V4L_CEC_DRIVERS
+
+config VIDEO_SAMSUNG_S5P_CEC
+       tristate "Samsung S5P CEC driver"
+       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
+       select HPD_NOTIFIERS
+       ---help---
+         This is a driver for Samsung S5P HDMI CEC interface. It uses the
+         generic CEC framework interface.
+         CEC bus is present in the HDMI connector and enables communication
+         between compatible devices.
+
+endif #V4L_CEC_DRIVERS
+
 menuconfig V4L_TEST_DRIVERS
 	bool "Media test drivers"
 	depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 5b3cb27..ad3bf22 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)	+= s5p-jpeg/
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)	+= s5p-mfc/
 
 obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)	+= s5p-g2d/
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)	+= s5p-cec/
 obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
 
 obj-$(CONFIG_VIDEO_STI_BDISP)		+= sti/bdisp/
diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
similarity index 100%
rename from drivers/staging/media/s5p-cec/Makefile
rename to drivers/media/platform/s5p-cec/Makefile
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/regs-cec.h
rename to drivers/media/platform/s5p-cec/regs-cec.h
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
similarity index 89%
rename from drivers/staging/media/s5p-cec/s5p_cec.c
rename to drivers/media/platform/s5p-cec/s5p_cec.c
index 2a07968..2014f79 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.c
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -14,15 +14,18 @@
  */
 
 #include <linux/clk.h>
+#include <linux/hpd-notifier.h>
 #include <linux/interrupt.h>
 #include <linux/kernel.h>
 #include <linux/mfd/syscon.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/pm_runtime.h>
 #include <linux/timer.h>
 #include <linux/workqueue.h>
+#include <media/cec-edid.h>
 #include <media/cec.h>
 
 #include "exynos_hdmi_cec.h"
@@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
 static int s5p_cec_probe(struct platform_device *pdev)
 {
 	struct device *dev = &pdev->dev;
+	struct device_node *np;
+	struct platform_device *hdmi_dev;
 	struct resource *res;
 	struct s5p_cec_dev *cec;
 	int ret;
 
+	np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
+
+	if (!np) {
+		dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
+		return -ENODEV;
+	}
+	hdmi_dev = of_find_device_by_node(np);
+	if (hdmi_dev == NULL)
+		return -EPROBE_DEFER;
+
 	cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
 	if (!cec)
 		return -ENOMEM;
@@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
 	if (IS_ERR(cec->reg))
 		return PTR_ERR(cec->reg);
 
+	cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
+	if (cec->notifier == NULL)
+		return -ENOMEM;
+
 	cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
 		CEC_NAME,
-		CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
+		CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
 		CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
 	ret = PTR_ERR_OR_ZERO(cec->adap);
 	if (ret)
 		return ret;
+
 	ret = cec_register_adapter(cec->adap, &pdev->dev);
-	if (ret) {
-		cec_delete_adapter(cec->adap);
-		return ret;
-	}
+	if (ret)
+		goto err_delete_adapter;
+
+	cec_register_hpd_notifier(cec->adap, cec->notifier);
 
 	platform_set_drvdata(pdev, cec);
 	pm_runtime_enable(dev);
 
 	dev_dbg(dev, "successfuly probed\n");
 	return 0;
+
+err_delete_adapter:
+	cec_delete_adapter(cec->adap);
+	return ret;
 }
 
 static int s5p_cec_remove(struct platform_device *pdev)
@@ -225,6 +249,7 @@ static int s5p_cec_remove(struct platform_device *pdev)
 	struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
 
 	cec_unregister_adapter(cec->adap);
+	hpd_notifier_put(cec->notifier);
 	pm_runtime_disable(&pdev->dev);
 	return 0;
 }
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.h b/drivers/media/platform/s5p-cec/s5p_cec.h
similarity index 97%
rename from drivers/staging/media/s5p-cec/s5p_cec.h
rename to drivers/media/platform/s5p-cec/s5p_cec.h
index 03732c1..a6f5af6 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.h
+++ b/drivers/media/platform/s5p-cec/s5p_cec.h
@@ -59,12 +59,15 @@ enum cec_state {
 	STATE_ERROR
 };
 
+struct hpd_notifier;
+
 struct s5p_cec_dev {
 	struct cec_adapter	*adap;
 	struct clk		*clk;
 	struct device		*dev;
 	struct mutex		lock;
 	struct regmap           *pmu;
+	struct hpd_notifier	*notifier;
 	int			irq;
 	void __iomem		*reg;
 
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index ffb8fa7..1b7804c 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -27,8 +27,6 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
 
 source "drivers/staging/media/omap4iss/Kconfig"
 
-source "drivers/staging/media/s5p-cec/Kconfig"
-
 # Keep LIRC at the end, as it has sub-menus
 source "drivers/staging/media/lirc/Kconfig"
 
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index a28e82c..e11afbf 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -1,5 +1,4 @@
 obj-$(CONFIG_I2C_BCM2048)	+= bcm2048/
-obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
 obj-$(CONFIG_DVB_CXD2099)	+= cxd2099/
 obj-$(CONFIG_LIRC_STAGING)	+= lirc/
 obj-$(CONFIG_VIDEO_DM365_VPFE)	+= davinci_vpfe/
diff --git a/drivers/staging/media/s5p-cec/Kconfig b/drivers/staging/media/s5p-cec/Kconfig
deleted file mode 100644
index ddfd955..0000000
--- a/drivers/staging/media/s5p-cec/Kconfig
+++ /dev/null
@@ -1,9 +0,0 @@
-config VIDEO_SAMSUNG_S5P_CEC
-       tristate "Samsung S5P CEC driver"
-       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
-       ---help---
-         This is a driver for Samsung S5P HDMI CEC interface. It uses the
-         generic CEC framework interface.
-         CEC bus is present in the HDMI connector and enables communication
-         between compatible devices.
-
diff --git a/drivers/staging/media/s5p-cec/TODO b/drivers/staging/media/s5p-cec/TODO
deleted file mode 100644
index 64f21ba..0000000
--- a/drivers/staging/media/s5p-cec/TODO
+++ /dev/null
@@ -1,7 +0,0 @@
-This driver requires that userspace sets the physical address.
-However, this should be passed on from the corresponding
-Samsung HDMI driver.
-
-We have to wait until the HDMI notifier framework has been merged
-in order to handle this gracefully, until that time this driver
-has to remain in staging.
-- 
2.8.1

Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Krzysztof Kozlowski]

On Mon, Jan 02, 2017 at 03:19:07PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> By using the HPD notifier framework there is no longer any reason
> to manually set the physical address. This was the one blocking
> issue that prevented this driver from going out of staging, so do
> this move as well.
> 
> Update the bindings documenting the new hdmi phandle and
> update exynos4.dtsi accordingly.
> 
> Tested with my Odroid U3.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-cec.txt          |  2 ++
>  arch/arm/boot/dts/exynos4.dtsi                     |  1 +
>  drivers/media/platform/Kconfig                     | 18 +++++++++++
>  drivers/media/platform/Makefile                    |  1 +
>  .../media => media/platform}/s5p-cec/Makefile      |  0
>  .../platform}/s5p-cec/exynos_hdmi_cec.h            |  0
>  .../platform}/s5p-cec/exynos_hdmi_cecctrl.c        |  0
>  .../media => media/platform}/s5p-cec/regs-cec.h    |  0
>  .../media => media/platform}/s5p-cec/s5p_cec.c     | 35 ++++++++++++++++++----
>  .../media => media/platform}/s5p-cec/s5p_cec.h     |  3 ++
>  drivers/staging/media/Kconfig                      |  2 --
>  drivers/staging/media/Makefile                     |  1 -
>  drivers/staging/media/s5p-cec/Kconfig              |  9 ------
>  drivers/staging/media/s5p-cec/TODO                 |  7 -----
>  14 files changed, 55 insertions(+), 24 deletions(-)
>  rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
>  delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
>  delete mode 100644 drivers/staging/media/s5p-cec/TODO
> 
> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
> index 925ab4d..710fc00 100644
> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
> @@ -15,6 +15,7 @@ Required properties:
>    - clock-names : from common clock binding: must contain "hdmicec",
>  		  corresponding to entry in the clocks property.
>    - samsung,syscon-phandle - phandle to the PMU system controller
> +  - samsung,hdmi-phandle - phandle to the HDMI controller
>  
>  Example:
>  
> @@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
>  	clocks = <&clock CLK_HDMI_CEC>;
>  	clock-names = "hdmicec";
>  	samsung,syscon-phandle = <&pmu_system_controller>;
> +	samsung,hdmi-phandle = <&hdmi>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&hdmi_cec>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index c64737b..51dfcbb 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -762,6 +762,7 @@
>  		clocks = <&clock CLK_HDMI_CEC>;
>  		clock-names = "hdmicec";
>  		samsung,syscon-phandle = <&pmu_system_controller>;
> +		samsung,hdmi-phandle = <&hdmi>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&hdmi_cec>;
>  		status = "disabled";

DTS change has to be a separate patch. It should also go through
arm-soc/samsung-soc tree.

If it is a dependency, then I could provide a tag with it.

Best regards,
Krzysztof

> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index d944421..cab1637 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -392,6 +392,24 @@ config VIDEO_TI_SC
>  config VIDEO_TI_CSC
>  	tristate
>  
> +menuconfig V4L_CEC_DRIVERS
> +	bool "Platform HDMI CEC drivers"
> +	depends on MEDIA_CEC_SUPPORT
> +
> +if V4L_CEC_DRIVERS
> +
> +config VIDEO_SAMSUNG_S5P_CEC
> +       tristate "Samsung S5P CEC driver"
> +       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
> +       select HPD_NOTIFIERS
> +       ---help---
> +         This is a driver for Samsung S5P HDMI CEC interface. It uses the
> +         generic CEC framework interface.
> +         CEC bus is present in the HDMI connector and enables communication
> +         between compatible devices.
> +
> +endif #V4L_CEC_DRIVERS
> +
>  menuconfig V4L_TEST_DRIVERS
>  	bool "Media test drivers"
>  	depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 5b3cb27..ad3bf22 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)	+= s5p-jpeg/
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)	+= s5p-mfc/
>  
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)	+= s5p-g2d/
> +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)	+= s5p-cec/
>  obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
>  
>  obj-$(CONFIG_VIDEO_STI_BDISP)		+= sti/bdisp/
> diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/Makefile
> rename to drivers/media/platform/s5p-cec/Makefile
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/regs-cec.h
> rename to drivers/media/platform/s5p-cec/regs-cec.h
> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
> similarity index 89%
> rename from drivers/staging/media/s5p-cec/s5p_cec.c
> rename to drivers/media/platform/s5p-cec/s5p_cec.c
> index 2a07968..2014f79 100644
> --- a/drivers/staging/media/s5p-cec/s5p_cec.c
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
> @@ -14,15 +14,18 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/hpd-notifier.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/timer.h>
>  #include <linux/workqueue.h>
> +#include <media/cec-edid.h>
>  #include <media/cec.h>
>  
>  #include "exynos_hdmi_cec.h"
> @@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
>  static int s5p_cec_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *np;
> +	struct platform_device *hdmi_dev;
>  	struct resource *res;
>  	struct s5p_cec_dev *cec;
>  	int ret;
>  
> +	np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
> +
> +	if (!np) {
> +		dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
> +		return -ENODEV;
> +	}
> +	hdmi_dev = of_find_device_by_node(np);
> +	if (hdmi_dev == NULL)
> +		return -EPROBE_DEFER;
> +
>  	cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
>  	if (!cec)
>  		return -ENOMEM;
> @@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
>  	if (IS_ERR(cec->reg))
>  		return PTR_ERR(cec->reg);
>  
> +	cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
> +	if (cec->notifier == NULL)
> +		return -ENOMEM;
> +
>  	cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
>  		CEC_NAME,
> -		CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> +		CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
>  		CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
>  	ret = PTR_ERR_OR_ZERO(cec->adap);
>  	if (ret)
>  		return ret;
> +
>  	ret = cec_register_adapter(cec->adap, &pdev->dev);
> -	if (ret) {
> -		cec_delete_adapter(cec->adap);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_delete_adapter;
> +
> +	cec_register_hpd_notifier(cec->adap, cec->notifier);
>  
>  	platform_set_drvdata(pdev, cec);
>  	pm_runtime_enable(dev);
>  
>  	dev_dbg(dev, "successfuly probed\n");
>  	return 0;
> +
> +err_delete_adapter:
> +	cec_delete_adapter(cec->adap);
> +	return ret;
>  }
>  
>  static int s5p_cec_remove(struct platform_device *pdev)
> @@ -225,6 +249,7 @@ static int s5p_cec_remove(struct platform_device *pdev)
>  	struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
>  
>  	cec_unregister_adapter(cec->adap);
> +	hpd_notifier_put(cec->notifier);
>  	pm_runtime_disable(&pdev->dev);
>  	return 0;
>  }
> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.h b/drivers/media/platform/s5p-cec/s5p_cec.h
> similarity index 97%
> rename from drivers/staging/media/s5p-cec/s5p_cec.h
> rename to drivers/media/platform/s5p-cec/s5p_cec.h
> index 03732c1..a6f5af6 100644
> --- a/drivers/staging/media/s5p-cec/s5p_cec.h
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.h
> @@ -59,12 +59,15 @@ enum cec_state {
>  	STATE_ERROR
>  };
>  
> +struct hpd_notifier;
> +
>  struct s5p_cec_dev {
>  	struct cec_adapter	*adap;
>  	struct clk		*clk;
>  	struct device		*dev;
>  	struct mutex		lock;
>  	struct regmap           *pmu;
> +	struct hpd_notifier	*notifier;
>  	int			irq;
>  	void __iomem		*reg;
>  
> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
> index ffb8fa7..1b7804c 100644
> --- a/drivers/staging/media/Kconfig
> +++ b/drivers/staging/media/Kconfig
> @@ -27,8 +27,6 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
>  
>  source "drivers/staging/media/omap4iss/Kconfig"
>  
> -source "drivers/staging/media/s5p-cec/Kconfig"
> -
>  # Keep LIRC at the end, as it has sub-menus
>  source "drivers/staging/media/lirc/Kconfig"
>  
> diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
> index a28e82c..e11afbf 100644
> --- a/drivers/staging/media/Makefile
> +++ b/drivers/staging/media/Makefile
> @@ -1,5 +1,4 @@
>  obj-$(CONFIG_I2C_BCM2048)	+= bcm2048/
> -obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
>  obj-$(CONFIG_DVB_CXD2099)	+= cxd2099/
>  obj-$(CONFIG_LIRC_STAGING)	+= lirc/
>  obj-$(CONFIG_VIDEO_DM365_VPFE)	+= davinci_vpfe/
> diff --git a/drivers/staging/media/s5p-cec/Kconfig b/drivers/staging/media/s5p-cec/Kconfig
> deleted file mode 100644
> index ddfd955..0000000
> --- a/drivers/staging/media/s5p-cec/Kconfig
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -config VIDEO_SAMSUNG_S5P_CEC
> -       tristate "Samsung S5P CEC driver"
> -       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
> -       ---help---
> -         This is a driver for Samsung S5P HDMI CEC interface. It uses the
> -         generic CEC framework interface.
> -         CEC bus is present in the HDMI connector and enables communication
> -         between compatible devices.
> -
> diff --git a/drivers/staging/media/s5p-cec/TODO b/drivers/staging/media/s5p-cec/TODO
> deleted file mode 100644
> index 64f21ba..0000000
> --- a/drivers/staging/media/s5p-cec/TODO
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -This driver requires that userspace sets the physical address.
> -However, this should be passed on from the corresponding
> -Samsung HDMI driver.
> -
> -We have to wait until the HDMI notifier framework has been merged
> -in order to handle this gracefully, until that time this driver
> -has to remain in staging.
> -- 
> 2.8.1
> 

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Andrzej Hajda]

Hi Hans,

On 02.01.2017 15:19, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Add support for video hotplug detect and EDID/ELD notifiers, which is used
> to convey information from video drivers to their CEC and audio counterparts.
>
> Based on an earlier version from Russell King:
>
> https://patchwork.kernel.org/patch/9277043/
>
> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
> of a video device.
>
> When a new notifier is registered the current state will be reported to
> that notifier at registration time.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/video/Kconfig        |   3 +
>  drivers/video/Makefile       |   1 +
>  drivers/video/hpd-notifier.c | 134 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hpd-notifier.h | 109 +++++++++++++++++++++++++++++++++++
>  4 files changed, 247 insertions(+)
>  create mode 100644 drivers/video/hpd-notifier.c
>  create mode 100644 include/linux/hpd-notifier.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 3c20af9..cddc860 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>  config HDMI
>  	bool
>  
> +config HPD_NOTIFIERS
> +	bool
> +
>  if VT
>  	source "drivers/video/console/Kconfig"
>  endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 9ad3c17..424698b 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -1,5 +1,6 @@
>  obj-$(CONFIG_VGASTATE)            += vgastate.o
>  obj-$(CONFIG_HDMI)                += hdmi.o
> +obj-$(CONFIG_HPD_NOTIFIERS)       += hpd-notifier.o
>  
>  obj-$(CONFIG_VT)		  += console/
>  obj-$(CONFIG_LOGO)		  += logo/
> diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
> new file mode 100644
> index 0000000..54f7a6b
> --- /dev/null
> +++ b/drivers/video/hpd-notifier.c
> @@ -0,0 +1,134 @@
> +#include <linux/export.h>
> +#include <linux/hpd-notifier.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +
> +static LIST_HEAD(hpd_notifiers);
> +static DEFINE_MUTEX(hpd_notifiers_lock);
> +
> +struct hpd_notifier *hpd_notifier_get(struct device *dev)
> +{
> +	struct hpd_notifier *n;
> +
> +	mutex_lock(&hpd_notifiers_lock);
> +	list_for_each_entry(n, &hpd_notifiers, head) {
> +		if (n->dev == dev) {
> +			mutex_unlock(&hpd_notifiers_lock);

I think this place is racy, we have pointer to unprotected area
(n->kref), so if concurrent thread calls hpd_notifier_put in this moment
&n->kref could be freed and kref_get in the next line will operate on
dangling pointer. Am I right?

Regards
Andrzej

> +			kref_get(&n->kref);
> +			return n;
> +		}
> +	}
> +	n = kzalloc(sizeof(*n), GFP_KERNEL);
> +	if (!n)
> +		goto unlock;
> +	n->dev = dev;
> +	mutex_init(&n->lock);
> +	BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
> +	kref_init(&n->kref);
> +	list_add_tail(&n->head, &hpd_notifiers);
> +unlock:
> +	mutex_unlock(&hpd_notifiers_lock);
> +	return n;
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_get);
> +
> +static void hpd_notifier_release(struct kref *kref)
> +{
> +	struct hpd_notifier *n =
> +		container_of(kref, struct hpd_notifier, kref);
> +
> +	mutex_lock(&hpd_notifiers_lock);
> +	list_del(&n->head);
> +	mutex_unlock(&hpd_notifiers_lock);
> +	kfree(n->edid);
> +	kfree(n);
> +}
> +
> +void hpd_notifier_put(struct hpd_notifier *n)
> +{
> +	kref_put(&n->kref, hpd_notifier_release);
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_put);
> +
> +int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
> +{
> +	int ret = blocking_notifier_chain_register(&n->notifiers, nb);
> +
> +	if (ret)
> +		return ret;
> +	kref_get(&n->kref);
> +	mutex_lock(&n->lock);
> +	if (n->connected) {
> +		blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
> +		if (n->edid_size)
> +			blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
> +		if (n->has_eld)
> +			blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
> +	}
> +	mutex_unlock(&n->lock);
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_register);
> +
> +int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
> +{
> +	int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
> +
> +	if (ret == 0)
> +		hpd_notifier_put(n);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
(...)

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Hans Verkuil]

On 01/03/2017 08:50 AM, Andrzej Hajda wrote:
> Hi Hans,
> 
> On 02.01.2017 15:19, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add support for video hotplug detect and EDID/ELD notifiers, which is used
>> to convey information from video drivers to their CEC and audio counterparts.
>>
>> Based on an earlier version from Russell King:
>>
>> https://patchwork.kernel.org/patch/9277043/
>>
>> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
>> of a video device.
>>
>> When a new notifier is registered the current state will be reported to
>> that notifier at registration time.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/video/Kconfig        |   3 +
>>  drivers/video/Makefile       |   1 +
>>  drivers/video/hpd-notifier.c | 134 +++++++++++++++++++++++++++++++++++++++++++
>>  include/linux/hpd-notifier.h | 109 +++++++++++++++++++++++++++++++++++
>>  4 files changed, 247 insertions(+)
>>  create mode 100644 drivers/video/hpd-notifier.c
>>  create mode 100644 include/linux/hpd-notifier.h
>>
>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>> index 3c20af9..cddc860 100644
>> --- a/drivers/video/Kconfig
>> +++ b/drivers/video/Kconfig
>> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>>  config HDMI
>>  	bool
>>  
>> +config HPD_NOTIFIERS
>> +	bool
>> +
>>  if VT
>>  	source "drivers/video/console/Kconfig"
>>  endif
>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
>> index 9ad3c17..424698b 100644
>> --- a/drivers/video/Makefile
>> +++ b/drivers/video/Makefile
>> @@ -1,5 +1,6 @@
>>  obj-$(CONFIG_VGASTATE)            += vgastate.o
>>  obj-$(CONFIG_HDMI)                += hdmi.o
>> +obj-$(CONFIG_HPD_NOTIFIERS)       += hpd-notifier.o
>>  
>>  obj-$(CONFIG_VT)		  += console/
>>  obj-$(CONFIG_LOGO)		  += logo/
>> diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
>> new file mode 100644
>> index 0000000..54f7a6b
>> --- /dev/null
>> +++ b/drivers/video/hpd-notifier.c
>> @@ -0,0 +1,134 @@
>> +#include <linux/export.h>
>> +#include <linux/hpd-notifier.h>
>> +#include <linux/string.h>
>> +#include <linux/slab.h>
>> +#include <linux/list.h>
>> +
>> +static LIST_HEAD(hpd_notifiers);
>> +static DEFINE_MUTEX(hpd_notifiers_lock);
>> +
>> +struct hpd_notifier *hpd_notifier_get(struct device *dev)
>> +{
>> +	struct hpd_notifier *n;
>> +
>> +	mutex_lock(&hpd_notifiers_lock);
>> +	list_for_each_entry(n, &hpd_notifiers, head) {
>> +		if (n->dev == dev) {
>> +			mutex_unlock(&hpd_notifiers_lock);
> 
> I think this place is racy, we have pointer to unprotected area
> (n->kref), so if concurrent thread calls hpd_notifier_put in this moment
> &n->kref could be freed and kref_get in the next line will operate on
> dangling pointer. Am I right?

You're right. I took the hpd_notifiers_lock in hpd_notifier_release,
but I should take it in hpd_notifier_put.

Thanks! I'll fix that.

Regards,

	Hans

> 
> Regards
> Andrzej
> 
>> +			kref_get(&n->kref);
>> +			return n;
>> +		}
>> +	}
>> +	n = kzalloc(sizeof(*n), GFP_KERNEL);
>> +	if (!n)
>> +		goto unlock;
>> +	n->dev = dev;
>> +	mutex_init(&n->lock);
>> +	BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
>> +	kref_init(&n->kref);
>> +	list_add_tail(&n->head, &hpd_notifiers);
>> +unlock:
>> +	mutex_unlock(&hpd_notifiers_lock);
>> +	return n;
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_get);
>> +
>> +static void hpd_notifier_release(struct kref *kref)
>> +{
>> +	struct hpd_notifier *n =
>> +		container_of(kref, struct hpd_notifier, kref);
>> +
>> +	mutex_lock(&hpd_notifiers_lock);
>> +	list_del(&n->head);
>> +	mutex_unlock(&hpd_notifiers_lock);
>> +	kfree(n->edid);
>> +	kfree(n);
>> +}
>> +
>> +void hpd_notifier_put(struct hpd_notifier *n)
>> +{
>> +	kref_put(&n->kref, hpd_notifier_release);
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_put);
>> +
>> +int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
>> +{
>> +	int ret = blocking_notifier_chain_register(&n->notifiers, nb);
>> +
>> +	if (ret)
>> +		return ret;
>> +	kref_get(&n->kref);
>> +	mutex_lock(&n->lock);
>> +	if (n->connected) {
>> +		blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
>> +		if (n->edid_size)
>> +			blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
>> +		if (n->has_eld)
>> +			blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
>> +	}
>> +	mutex_unlock(&n->lock);
>> +	return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_register);
>> +
>> +int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
>> +{
>> +	int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
>> +
>> +	if (ret == 0)
>> +		hpd_notifier_put(n);
>> +	return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
> (...)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCHv2 2/4] exynos_hdmi: add HPD notifier support

[Andrzej Hajda]

On 02.01.2017 15:19, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Implement the HPD notifier support to allow CEC drivers to
> be informed when there is a new EDID and when a connect or
> disconnect happens.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  drivers/gpu/drm/exynos/Kconfig       |  1 +
>  drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
>  2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
> index d706ca4..80bfd1d 100644
> --- a/drivers/gpu/drm/exynos/Kconfig
> +++ b/drivers/gpu/drm/exynos/Kconfig
> @@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
>  config DRM_EXYNOS_HDMI
>  	bool "HDMI"
>  	depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
> +	select HPD_NOTIFIERS
>  	help
>  	  Choose this option if you want to use Exynos HDMI for DRM.
>  
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5ed8b1e..28bf609 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -31,6 +31,7 @@
>  #include <linux/pm_runtime.h>
>  #include <linux/clk.h>
>  #include <linux/gpio/consumer.h>
> +#include <linux/hpd-notifier.h>
>  #include <linux/regulator/consumer.h>
>  #include <linux/io.h>
>  #include <linux/of_address.h>
> @@ -118,6 +119,7 @@ struct hdmi_context {
>  	bool				dvi_mode;
>  	struct delayed_work		hotplug_work;
>  	struct drm_display_mode		current_mode;
> +	struct hpd_notifier		*notifier;
>  	const struct hdmi_driver_data	*drv_data;
>  
>  	void __iomem			*regs;
> @@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
>  {
>  	struct hdmi_context *hdata = connector_to_hdmi(connector);
>  
> -	if (gpiod_get_value(hdata->hpd_gpio))
> +	if (gpiod_get_value(hdata->hpd_gpio)) {
> +		hpd_event_connect(hdata->notifier);
>  		return connector_status_connected;
> +	}
>  
> +	hpd_event_disconnect(hdata->notifier);
>  	return connector_status_disconnected;
>  }
>  
> @@ -848,6 +853,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
>  		edid->width_cm, edid->height_cm);
>  
>  	drm_mode_connector_update_edid_property(connector, edid);
> +	hpd_event_connect(hdata->notifier);

Is there a reason to call hpd_event_connect here? It was called already
from hdmi_detect.

Regards
Andrzej

> +	hpd_event_new_edid(hdata->notifier, edid,
> +			    EDID_LENGTH * (1 + edid->extensions));
>  
>  	ret = drm_add_edid_modes(connector, edid);
>  
> @@ -1483,6 +1491,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
>  	if (funcs && funcs->disable)
>  		(*funcs->disable)(crtc);
>  
> +	hpd_event_disconnect(hdata->notifier);
>  	cancel_delayed_work(&hdata->hotplug_work);
>  
>  	hdmiphy_disable(hdata);
> @@ -1832,15 +1841,22 @@ static int hdmi_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	hdata->notifier = hpd_notifier_get(&pdev->dev);
> +	if (hdata->notifier == NULL) {
> +		ret = -ENOMEM;
> +		goto err_hdmiphy;
> +	}
> +
>  	pm_runtime_enable(dev);
>  
>  	ret = component_add(&pdev->dev, &hdmi_component_ops);
>  	if (ret)
> -		goto err_disable_pm_runtime;
> +		goto err_notifier_put;
>  
>  	return ret;
>  
> -err_disable_pm_runtime:
> +err_notifier_put:
> +	hpd_notifier_put(hdata->notifier);
>  	pm_runtime_disable(dev);
>  
>  err_hdmiphy:
> @@ -1859,9 +1875,11 @@ static int hdmi_remove(struct platform_device *pdev)
>  	struct hdmi_context *hdata = platform_get_drvdata(pdev);
>  
>  	cancel_delayed_work_sync(&hdata->hotplug_work);
> +	hpd_event_disconnect(hdata->notifier);
>  
>  	component_del(&pdev->dev, &hdmi_component_ops);
>  
> +	hpd_notifier_put(hdata->notifier);
>  	pm_runtime_disable(&pdev->dev);
>  
>  	if (!IS_ERR(hdata->reg_hdmi_en))

Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Andrzej Hajda]

On 02.01.2017 15:19, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> By using the HPD notifier framework there is no longer any reason
> to manually set the physical address. This was the one blocking
> issue that prevented this driver from going out of staging, so do
> this move as well.
>
> Update the bindings documenting the new hdmi phandle and
> update exynos4.dtsi accordingly.
>
> Tested with my Odroid U3.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
>  .../devicetree/bindings/media/s5p-cec.txt          |  2 ++
>  arch/arm/boot/dts/exynos4.dtsi                     |  1 +
>  drivers/media/platform/Kconfig                     | 18 +++++++++++
>  drivers/media/platform/Makefile                    |  1 +
>  .../media => media/platform}/s5p-cec/Makefile      |  0
>  .../platform}/s5p-cec/exynos_hdmi_cec.h            |  0
>  .../platform}/s5p-cec/exynos_hdmi_cecctrl.c        |  0
>  .../media => media/platform}/s5p-cec/regs-cec.h    |  0
>  .../media => media/platform}/s5p-cec/s5p_cec.c     | 35 ++++++++++++++++++----
>  .../media => media/platform}/s5p-cec/s5p_cec.h     |  3 ++
>  drivers/staging/media/Kconfig                      |  2 --
>  drivers/staging/media/Makefile                     |  1 -
>  drivers/staging/media/s5p-cec/Kconfig              |  9 ------
>  drivers/staging/media/s5p-cec/TODO                 |  7 -----
>  14 files changed, 55 insertions(+), 24 deletions(-)
>  rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
>  rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
>  delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
>  delete mode 100644 drivers/staging/media/s5p-cec/TODO
>
> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
> index 925ab4d..710fc00 100644
> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
> @@ -15,6 +15,7 @@ Required properties:
>    - clock-names : from common clock binding: must contain "hdmicec",
>  		  corresponding to entry in the clocks property.
>    - samsung,syscon-phandle - phandle to the PMU system controller
> +  - samsung,hdmi-phandle - phandle to the HDMI controller
>  
>  Example:
>  
> @@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
>  	clocks = <&clock CLK_HDMI_CEC>;
>  	clock-names = "hdmicec";
>  	samsung,syscon-phandle = <&pmu_system_controller>;
> +	samsung,hdmi-phandle = <&hdmi>;
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&hdmi_cec>;
>  	status = "okay";
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index c64737b..51dfcbb 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -762,6 +762,7 @@
>  		clocks = <&clock CLK_HDMI_CEC>;
>  		clock-names = "hdmicec";
>  		samsung,syscon-phandle = <&pmu_system_controller>;
> +		samsung,hdmi-phandle = <&hdmi>;
>  		pinctrl-names = "default";
>  		pinctrl-0 = <&hdmi_cec>;
>  		status = "disabled";
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index d944421..cab1637 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -392,6 +392,24 @@ config VIDEO_TI_SC
>  config VIDEO_TI_CSC
>  	tristate
>  
> +menuconfig V4L_CEC_DRIVERS
> +	bool "Platform HDMI CEC drivers"
> +	depends on MEDIA_CEC_SUPPORT
> +
> +if V4L_CEC_DRIVERS
> +
> +config VIDEO_SAMSUNG_S5P_CEC
> +       tristate "Samsung S5P CEC driver"
> +       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
> +       select HPD_NOTIFIERS
> +       ---help---
> +         This is a driver for Samsung S5P HDMI CEC interface. It uses the
> +         generic CEC framework interface.
> +         CEC bus is present in the HDMI connector and enables communication
> +         between compatible devices.
> +
> +endif #V4L_CEC_DRIVERS
> +
>  menuconfig V4L_TEST_DRIVERS
>  	bool "Media test drivers"
>  	depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 5b3cb27..ad3bf22 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)	+= s5p-jpeg/
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)	+= s5p-mfc/
>  
>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)	+= s5p-g2d/
> +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)	+= s5p-cec/
>  obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
>  
>  obj-$(CONFIG_VIDEO_STI_BDISP)		+= sti/bdisp/
> diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/Makefile
> rename to drivers/media/platform/s5p-cec/Makefile
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/regs-cec.h
> rename to drivers/media/platform/s5p-cec/regs-cec.h
> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
> similarity index 89%
> rename from drivers/staging/media/s5p-cec/s5p_cec.c
> rename to drivers/media/platform/s5p-cec/s5p_cec.c
> index 2a07968..2014f79 100644
> --- a/drivers/staging/media/s5p-cec/s5p_cec.c
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
> @@ -14,15 +14,18 @@
>   */
>  
>  #include <linux/clk.h>
> +#include <linux/hpd-notifier.h>
>  #include <linux/interrupt.h>
>  #include <linux/kernel.h>
>  #include <linux/mfd/syscon.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/pm_runtime.h>
>  #include <linux/timer.h>
>  #include <linux/workqueue.h>
> +#include <media/cec-edid.h>
>  #include <media/cec.h>
>  
>  #include "exynos_hdmi_cec.h"
> @@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
>  static int s5p_cec_probe(struct platform_device *pdev)
>  {
>  	struct device *dev = &pdev->dev;
> +	struct device_node *np;
> +	struct platform_device *hdmi_dev;
>  	struct resource *res;
>  	struct s5p_cec_dev *cec;
>  	int ret;
>  
> +	np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
> +
> +	if (!np) {
> +		dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
> +		return -ENODEV;
> +	}
> +	hdmi_dev = of_find_device_by_node(np);
> +	if (hdmi_dev == NULL)
> +		return -EPROBE_DEFER;
> +
>  	cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
>  	if (!cec)
>  		return -ENOMEM;
> @@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
>  	if (IS_ERR(cec->reg))
>  		return PTR_ERR(cec->reg);
>  
> +	cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
> +	if (cec->notifier == NULL)
> +		return -ENOMEM;
> +
>  	cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
>  		CEC_NAME,
> -		CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> +		CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
>  		CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
>  	ret = PTR_ERR_OR_ZERO(cec->adap);
>  	if (ret)
>  		return ret;
> +
>  	ret = cec_register_adapter(cec->adap, &pdev->dev);
> -	if (ret) {
> -		cec_delete_adapter(cec->adap);
> -		return ret;
> -	}
> +	if (ret)
> +		goto err_delete_adapter;
> +
> +	cec_register_hpd_notifier(cec->adap, cec->notifier);

Is there a reason to split registration into two steps?
Wouldn't be better to integrate hpd_notifier_get into
cec_register_hpd_notifier.

Regards
Andrzej

Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Hans Verkuil]

On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
> On 02.01.2017 15:19, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> By using the HPD notifier framework there is no longer any reason
>> to manually set the physical address. This was the one blocking
>> issue that prevented this driver from going out of staging, so do
>> this move as well.
>>
>> Update the bindings documenting the new hdmi phandle and
>> update exynos4.dtsi accordingly.
>>
>> Tested with my Odroid U3.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  .../devicetree/bindings/media/s5p-cec.txt          |  2 ++
>>  arch/arm/boot/dts/exynos4.dtsi                     |  1 +
>>  drivers/media/platform/Kconfig                     | 18 +++++++++++
>>  drivers/media/platform/Makefile                    |  1 +
>>  .../media => media/platform}/s5p-cec/Makefile      |  0
>>  .../platform}/s5p-cec/exynos_hdmi_cec.h            |  0
>>  .../platform}/s5p-cec/exynos_hdmi_cecctrl.c        |  0
>>  .../media => media/platform}/s5p-cec/regs-cec.h    |  0
>>  .../media => media/platform}/s5p-cec/s5p_cec.c     | 35 ++++++++++++++++++----
>>  .../media => media/platform}/s5p-cec/s5p_cec.h     |  3 ++
>>  drivers/staging/media/Kconfig                      |  2 --
>>  drivers/staging/media/Makefile                     |  1 -
>>  drivers/staging/media/s5p-cec/Kconfig              |  9 ------
>>  drivers/staging/media/s5p-cec/TODO                 |  7 -----
>>  14 files changed, 55 insertions(+), 24 deletions(-)
>>  rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
>>  rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
>>  rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
>>  rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
>>  rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
>>  rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
>>  delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
>>  delete mode 100644 drivers/staging/media/s5p-cec/TODO
>>
>> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
>> index 925ab4d..710fc00 100644
>> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
>> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
>> @@ -15,6 +15,7 @@ Required properties:
>>    - clock-names : from common clock binding: must contain "hdmicec",
>>  		  corresponding to entry in the clocks property.
>>    - samsung,syscon-phandle - phandle to the PMU system controller
>> +  - samsung,hdmi-phandle - phandle to the HDMI controller
>>  
>>  Example:
>>  
>> @@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
>>  	clocks = <&clock CLK_HDMI_CEC>;
>>  	clock-names = "hdmicec";
>>  	samsung,syscon-phandle = <&pmu_system_controller>;
>> +	samsung,hdmi-phandle = <&hdmi>;
>>  	pinctrl-names = "default";
>>  	pinctrl-0 = <&hdmi_cec>;
>>  	status = "okay";
>> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
>> index c64737b..51dfcbb 100644
>> --- a/arch/arm/boot/dts/exynos4.dtsi
>> +++ b/arch/arm/boot/dts/exynos4.dtsi
>> @@ -762,6 +762,7 @@
>>  		clocks = <&clock CLK_HDMI_CEC>;
>>  		clock-names = "hdmicec";
>>  		samsung,syscon-phandle = <&pmu_system_controller>;
>> +		samsung,hdmi-phandle = <&hdmi>;
>>  		pinctrl-names = "default";
>>  		pinctrl-0 = <&hdmi_cec>;
>>  		status = "disabled";
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index d944421..cab1637 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -392,6 +392,24 @@ config VIDEO_TI_SC
>>  config VIDEO_TI_CSC
>>  	tristate
>>  
>> +menuconfig V4L_CEC_DRIVERS
>> +	bool "Platform HDMI CEC drivers"
>> +	depends on MEDIA_CEC_SUPPORT
>> +
>> +if V4L_CEC_DRIVERS
>> +
>> +config VIDEO_SAMSUNG_S5P_CEC
>> +       tristate "Samsung S5P CEC driver"
>> +       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
>> +       select HPD_NOTIFIERS
>> +       ---help---
>> +         This is a driver for Samsung S5P HDMI CEC interface. It uses the
>> +         generic CEC framework interface.
>> +         CEC bus is present in the HDMI connector and enables communication
>> +         between compatible devices.
>> +
>> +endif #V4L_CEC_DRIVERS
>> +
>>  menuconfig V4L_TEST_DRIVERS
>>  	bool "Media test drivers"
>>  	depends on MEDIA_CAMERA_SUPPORT
>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
>> index 5b3cb27..ad3bf22 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG)	+= s5p-jpeg/
>>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC)	+= s5p-mfc/
>>  
>>  obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D)	+= s5p-g2d/
>> +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC)	+= s5p-cec/
>>  obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
>>  
>>  obj-$(CONFIG_VIDEO_STI_BDISP)		+= sti/bdisp/
>> diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/Makefile
>> rename to drivers/media/platform/s5p-cec/Makefile
>> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
>> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
>> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
>> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
>> diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/regs-cec.h
>> rename to drivers/media/platform/s5p-cec/regs-cec.h
>> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
>> similarity index 89%
>> rename from drivers/staging/media/s5p-cec/s5p_cec.c
>> rename to drivers/media/platform/s5p-cec/s5p_cec.c
>> index 2a07968..2014f79 100644
>> --- a/drivers/staging/media/s5p-cec/s5p_cec.c
>> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
>> @@ -14,15 +14,18 @@
>>   */
>>  
>>  #include <linux/clk.h>
>> +#include <linux/hpd-notifier.h>
>>  #include <linux/interrupt.h>
>>  #include <linux/kernel.h>
>>  #include <linux/mfd/syscon.h>
>>  #include <linux/module.h>
>>  #include <linux/of.h>
>> +#include <linux/of_platform.h>
>>  #include <linux/platform_device.h>
>>  #include <linux/pm_runtime.h>
>>  #include <linux/timer.h>
>>  #include <linux/workqueue.h>
>> +#include <media/cec-edid.h>
>>  #include <media/cec.h>
>>  
>>  #include "exynos_hdmi_cec.h"
>> @@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
>>  static int s5p_cec_probe(struct platform_device *pdev)
>>  {
>>  	struct device *dev = &pdev->dev;
>> +	struct device_node *np;
>> +	struct platform_device *hdmi_dev;
>>  	struct resource *res;
>>  	struct s5p_cec_dev *cec;
>>  	int ret;
>>  
>> +	np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
>> +
>> +	if (!np) {
>> +		dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
>> +		return -ENODEV;
>> +	}
>> +	hdmi_dev = of_find_device_by_node(np);
>> +	if (hdmi_dev == NULL)
>> +		return -EPROBE_DEFER;
>> +
>>  	cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
>>  	if (!cec)
>>  		return -ENOMEM;
>> @@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
>>  	if (IS_ERR(cec->reg))
>>  		return PTR_ERR(cec->reg);
>>  
>> +	cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
>> +	if (cec->notifier == NULL)
>> +		return -ENOMEM;
>> +
>>  	cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
>>  		CEC_NAME,
>> -		CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
>> +		CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
>>  		CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
>>  	ret = PTR_ERR_OR_ZERO(cec->adap);
>>  	if (ret)
>>  		return ret;
>> +
>>  	ret = cec_register_adapter(cec->adap, &pdev->dev);
>> -	if (ret) {
>> -		cec_delete_adapter(cec->adap);
>> -		return ret;
>> -	}
>> +	if (ret)
>> +		goto err_delete_adapter;
>> +
>> +	cec_register_hpd_notifier(cec->adap, cec->notifier);
> 
> Is there a reason to split registration into two steps?
> Wouldn't be better to integrate hpd_notifier_get into
> cec_register_hpd_notifier.

One problem is that hpd_notifier_get can fail, whereas cec_register_hpd_notifier can't.
And I rather not have to register a CEC device only to unregister it again if the
hpd_notifier_get would fail.

Another reason is that this keeps the responsibility of the hpd_notifier life-time
handling in the driver instead of hiding it in the CEC framework, which is IMHO
unexpected.

I think I want to keep this as-is, at least for now.

Regards,

	Hans

Re: [PATCHv2 2/4] exynos_hdmi: add HPD notifier support

[Hans Verkuil]

On 01/03/2017 08:55 AM, Andrzej Hajda wrote:
> On 02.01.2017 15:19, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Implement the HPD notifier support to allow CEC drivers to
>> be informed when there is a new EDID and when a connect or
>> disconnect happens.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>>  drivers/gpu/drm/exynos/Kconfig       |  1 +
>>  drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
>>  2 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
>> index d706ca4..80bfd1d 100644
>> --- a/drivers/gpu/drm/exynos/Kconfig
>> +++ b/drivers/gpu/drm/exynos/Kconfig
>> @@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
>>  config DRM_EXYNOS_HDMI
>>  	bool "HDMI"
>>  	depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
>> +	select HPD_NOTIFIERS
>>  	help
>>  	  Choose this option if you want to use Exynos HDMI for DRM.
>>  
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 5ed8b1e..28bf609 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -31,6 +31,7 @@
>>  #include <linux/pm_runtime.h>
>>  #include <linux/clk.h>
>>  #include <linux/gpio/consumer.h>
>> +#include <linux/hpd-notifier.h>
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/io.h>
>>  #include <linux/of_address.h>
>> @@ -118,6 +119,7 @@ struct hdmi_context {
>>  	bool				dvi_mode;
>>  	struct delayed_work		hotplug_work;
>>  	struct drm_display_mode		current_mode;
>> +	struct hpd_notifier		*notifier;
>>  	const struct hdmi_driver_data	*drv_data;
>>  
>>  	void __iomem			*regs;
>> @@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
>>  {
>>  	struct hdmi_context *hdata = connector_to_hdmi(connector);
>>  
>> -	if (gpiod_get_value(hdata->hpd_gpio))
>> +	if (gpiod_get_value(hdata->hpd_gpio)) {
>> +		hpd_event_connect(hdata->notifier);
>>  		return connector_status_connected;
>> +	}
>>  
>> +	hpd_event_disconnect(hdata->notifier);
>>  	return connector_status_disconnected;
>>  }
>>  
>> @@ -848,6 +853,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
>>  		edid->width_cm, edid->height_cm);
>>  
>>  	drm_mode_connector_update_edid_property(connector, edid);
>> +	hpd_event_connect(hdata->notifier);
> 
> Is there a reason to call hpd_event_connect here? It was called already
> from hdmi_detect.

True. Will drop this.

	Hans

> 
> Regards
> Andrzej
> 
>> +	hpd_event_new_edid(hdata->notifier, edid,
>> +			    EDID_LENGTH * (1 + edid->extensions));
>>  
>>  	ret = drm_add_edid_modes(connector, edid);
>>  
>> @@ -1483,6 +1491,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
>>  	if (funcs && funcs->disable)
>>  		(*funcs->disable)(crtc);
>>  
>> +	hpd_event_disconnect(hdata->notifier);
>>  	cancel_delayed_work(&hdata->hotplug_work);
>>  
>>  	hdmiphy_disable(hdata);
>> @@ -1832,15 +1841,22 @@ static int hdmi_probe(struct platform_device *pdev)
>>  		}
>>  	}
>>  
>> +	hdata->notifier = hpd_notifier_get(&pdev->dev);
>> +	if (hdata->notifier == NULL) {
>> +		ret = -ENOMEM;
>> +		goto err_hdmiphy;
>> +	}
>> +
>>  	pm_runtime_enable(dev);
>>  
>>  	ret = component_add(&pdev->dev, &hdmi_component_ops);
>>  	if (ret)
>> -		goto err_disable_pm_runtime;
>> +		goto err_notifier_put;
>>  
>>  	return ret;
>>  
>> -err_disable_pm_runtime:
>> +err_notifier_put:
>> +	hpd_notifier_put(hdata->notifier);
>>  	pm_runtime_disable(dev);
>>  
>>  err_hdmiphy:
>> @@ -1859,9 +1875,11 @@ static int hdmi_remove(struct platform_device *pdev)
>>  	struct hdmi_context *hdata = platform_get_drvdata(pdev);
>>  
>>  	cancel_delayed_work_sync(&hdata->hotplug_work);
>> +	hpd_event_disconnect(hdata->notifier);
>>  
>>  	component_del(&pdev->dev, &hdmi_component_ops);
>>  
>> +	hpd_notifier_put(hdata->notifier);
>>  	pm_runtime_disable(&pdev->dev);
>>  
>>  	if (!IS_ERR(hdata->reg_hdmi_en))
> 
> 

Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Andrzej Hajda]

On 03.01.2017 09:11, Hans Verkuil wrote:
> On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
>> Is there a reason to split registration into two steps?
>> Wouldn't be better to integrate hpd_notifier_get into
>> cec_register_hpd_notifier.
> One problem is that hpd_notifier_get can fail, whereas cec_register_hpd_notifier can't.
> And I rather not have to register a CEC device only to unregister it again if the
> hpd_notifier_get would fail.

hpd_notifier_get can fail only due to lack of memory for about 150 bytes
so if it happens whole system will probably fail anyway :)


>
> Another reason is that this keeps the responsibility of the hpd_notifier life-time
> handling in the driver instead of hiding it in the CEC framework, which is IMHO
> unexpected.

Notifier is used only by CEC framework, so IMHO it would be desirable to
put CEC specific things into CEC framework.
Drivers duty is just to find notifier device.
Leaving it as is will just put little more burden on drivers, so this is
not big deal, do as you wish :)

Regards
Andrzej

>
> I think I want to keep this as-is, at least for now.
>
> Regards,
>
> 	Hans
>
>
>
>

Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging

[Hans Verkuil]

On 01/04/2017 09:44 AM, Andrzej Hajda wrote:
> On 03.01.2017 09:11, Hans Verkuil wrote:
>> On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
>>> Is there a reason to split registration into two steps?
>>> Wouldn't be better to integrate hpd_notifier_get into
>>> cec_register_hpd_notifier.
>> One problem is that hpd_notifier_get can fail, whereas cec_register_hpd_notifier can't.
>> And I rather not have to register a CEC device only to unregister it again if the
>> hpd_notifier_get would fail.
> 
> hpd_notifier_get can fail only due to lack of memory for about 150 bytes
> so if it happens whole system will probably fail anyway :)
> 
> 
>>
>> Another reason is that this keeps the responsibility of the hpd_notifier life-time
>> handling in the driver instead of hiding it in the CEC framework, which is IMHO
>> unexpected.
> 
> Notifier is used only by CEC framework, so IMHO it would be desirable to
> put CEC specific things into CEC framework.

The CEC framework is just the first that needs it. But especially audio drivers also
want to use it. It was designed to help out both subsystems since both need the EDID/ELD.

Regards,

	Hans

> Drivers duty is just to find notifier device.
> Leaving it as is will just put little more burden on drivers, so this is
> not big deal, do as you wish :)
> 
> Regards
> Andrzej
> 
>>
>> I think I want to keep this as-is, at least for now.
>>
>> Regards,
>>
>> 	Hans
>>
>>
>>
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Russell King - ARM Linux]

On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Add support for video hotplug detect and EDID/ELD notifiers, which is used
> to convey information from video drivers to their CEC and audio counterparts.
> 
> Based on an earlier version from Russell King:
> 
> https://patchwork.kernel.org/patch/9277043/
> 
> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
> of a video device.

I thought we had decided to drop the ELD bits?

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Russell King - ARM Linux]

On Mon, Mar 20, 2017 at 02:26:16PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> > 
> > Add support for video hotplug detect and EDID/ELD notifiers, which is used
> > to convey information from video drivers to their CEC and audio counterparts.
> > 
> > Based on an earlier version from Russell King:
> > 
> > https://patchwork.kernel.org/patch/9277043/
> > 
> > The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
> > of a video device.
> 
> I thought we had decided to drop the ELD bits?

Ignore that - mailer wrapped around to the first message in my mailbox!

-- 
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

Re: [PATCHv2 1/4] video: add hotplug detect notifier support

[Hans Verkuil]

On 03/20/2017 03:27 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:26:16PM +0000, Russell King - ARM Linux wrote:
>> On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Add support for video hotplug detect and EDID/ELD notifiers, which is used
>>> to convey information from video drivers to their CEC and audio counterparts.
>>>
>>> Based on an earlier version from Russell King:
>>>
>>> https://patchwork.kernel.org/patch/9277043/
>>>
>>> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
>>> of a video device.
>>
>> I thought we had decided to drop the ELD bits?
>
> Ignore that - mailer wrapped around to the first message in my mailbox!
>

Just FYI: I've removed anything not needed for CEC in this git repo:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=exynos4-cec2

It compiles, but it's otherwise untested.

And I still need to think more about Daniel's comments. I hope to work on this
a bit more next week.

Regards,

	Hans