All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCHv4 0/9] video/exynos/sti/cec: add HPD state notifier & use in drivers
@ 2017-02-06 10:29 ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard

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.

Also included is similar code for the STI platform, contributed by
Benjamin Gaignard.

Tested the exynos code with my Odroid U3 exynos4 devboard.

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

Daniel, who should merge this? The HPD notifier code is in drivers/video
(correctly, I think), but it is initially only used by CEC drivers which
are in drivers/media (since CEC is part of the media subsystem).

It would be easiest to merge it all through the media subsystem, but in
that case I need Acks from you.

The alternative is to merge the drivers/video and drivers/gpu patches
via you and the others via media. It's a bit painful though due to the
dependencies.

Regards,

	Hans

Changes since v3:
- Added the STI patches
- Split the exynos4 binding patches in one for documentation and one
  for the dts change itself, also use the correct subject and CC to
  the correct mailinglists (I hope :-) )

Changes since v2:
- Split off the dts changes of the s5p-cec patch into a separate patch
- Renamed HPD_NOTIFIERS to HPD_NOTIFIER to be consistent with the name
  of the source.

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.


Benjamin Gaignard (3):
  sti: hdmi: add HPD notifier support
  stih-cec: add HPD notifier support
  arm: sti: update sti-cec for HPD notifier support

Hans Verkuil (6):
  video: add hotplug detect notifier support
  exynos_hdmi: add HPD notifier support
  cec: integrate HPD notifier support
  ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi
  s5p-cec.txt: document the HDMI controller phandle
  s5p-cec: add hpd-notifier support, move out of staging

 .../devicetree/bindings/media/s5p-cec.txt          |   2 +
 .../devicetree/bindings/media/stih-cec.txt         |   2 +
 arch/arm/boot/dts/exynos4.dtsi                     |   1 +
 arch/arm/boot/dts/stih407-family.dtsi              |  12 --
 arch/arm/boot/dts/stih410.dtsi                     |  13 ++
 drivers/gpu/drm/exynos/Kconfig                     |   1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c               |  23 +++-
 drivers/gpu/drm/sti/Kconfig                        |   1 +
 drivers/gpu/drm/sti/sti_hdmi.c                     |  14 +++
 drivers/gpu/drm/sti/sti_hdmi.h                     |   3 +
 drivers/media/cec/cec-core.c                       |  50 ++++++++
 drivers/media/platform/Kconfig                     |  28 +++++
 drivers/media/platform/Makefile                    |   2 +
 .../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 +
 .../st-cec => media/platform/sti/cec}/Makefile     |   0
 .../st-cec => media/platform/sti/cec}/stih-cec.c   |  31 ++++-
 drivers/staging/media/Kconfig                      |   4 -
 drivers/staging/media/Makefile                     |   2 -
 drivers/staging/media/s5p-cec/Kconfig              |   9 --
 drivers/staging/media/s5p-cec/TODO                 |   7 --
 drivers/staging/media/st-cec/Kconfig               |   8 --
 drivers/staging/media/st-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 +++
 32 files changed, 460 insertions(+), 60 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%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%)
 delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
 delete mode 100644 drivers/staging/media/s5p-cec/TODO
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/TODO
 create mode 100644 drivers/video/hpd-notifier.c
 create mode 100644 include/linux/hpd-notifier.h

-- 
2.11.0


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

* [PATCHv4 0/9] video/exynos/sti/cec: add HPD state notifier & use in drivers
@ 2017-02-06 10:29 ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, dri-devel, Daniel Vetter,
	Marek Szyprowski

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.

Also included is similar code for the STI platform, contributed by
Benjamin Gaignard.

Tested the exynos code with my Odroid U3 exynos4 devboard.

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

Daniel, who should merge this? The HPD notifier code is in drivers/video
(correctly, I think), but it is initially only used by CEC drivers which
are in drivers/media (since CEC is part of the media subsystem).

It would be easiest to merge it all through the media subsystem, but in
that case I need Acks from you.

The alternative is to merge the drivers/video and drivers/gpu patches
via you and the others via media. It's a bit painful though due to the
dependencies.

Regards,

	Hans

Changes since v3:
- Added the STI patches
- Split the exynos4 binding patches in one for documentation and one
  for the dts change itself, also use the correct subject and CC to
  the correct mailinglists (I hope :-) )

Changes since v2:
- Split off the dts changes of the s5p-cec patch into a separate patch
- Renamed HPD_NOTIFIERS to HPD_NOTIFIER to be consistent with the name
  of the source.

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.


Benjamin Gaignard (3):
  sti: hdmi: add HPD notifier support
  stih-cec: add HPD notifier support
  arm: sti: update sti-cec for HPD notifier support

Hans Verkuil (6):
  video: add hotplug detect notifier support
  exynos_hdmi: add HPD notifier support
  cec: integrate HPD notifier support
  ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi
  s5p-cec.txt: document the HDMI controller phandle
  s5p-cec: add hpd-notifier support, move out of staging

 .../devicetree/bindings/media/s5p-cec.txt          |   2 +
 .../devicetree/bindings/media/stih-cec.txt         |   2 +
 arch/arm/boot/dts/exynos4.dtsi                     |   1 +
 arch/arm/boot/dts/stih407-family.dtsi              |  12 --
 arch/arm/boot/dts/stih410.dtsi                     |  13 ++
 drivers/gpu/drm/exynos/Kconfig                     |   1 +
 drivers/gpu/drm/exynos/exynos_hdmi.c               |  23 +++-
 drivers/gpu/drm/sti/Kconfig                        |   1 +
 drivers/gpu/drm/sti/sti_hdmi.c                     |  14 +++
 drivers/gpu/drm/sti/sti_hdmi.h                     |   3 +
 drivers/media/cec/cec-core.c                       |  50 ++++++++
 drivers/media/platform/Kconfig                     |  28 +++++
 drivers/media/platform/Makefile                    |   2 +
 .../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 +
 .../st-cec => media/platform/sti/cec}/Makefile     |   0
 .../st-cec => media/platform/sti/cec}/stih-cec.c   |  31 ++++-
 drivers/staging/media/Kconfig                      |   4 -
 drivers/staging/media/Makefile                     |   2 -
 drivers/staging/media/s5p-cec/Kconfig              |   9 --
 drivers/staging/media/s5p-cec/TODO                 |   7 --
 drivers/staging/media/st-cec/Kconfig               |   8 --
 drivers/staging/media/st-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 +++
 32 files changed, 460 insertions(+), 60 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%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%)
 delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
 delete mode 100644 drivers/staging/media/s5p-cec/TODO
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/TODO
 create mode 100644 drivers/video/hpd-notifier.c
 create mode 100644 include/linux/hpd-notifier.h

-- 
2.11.0

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

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

* [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-06 10:29 ` Hans Verkuil
  (?)
@ 2017-02-06 10:29 ` Hans Verkuil
  2017-02-06 13:08     ` Andrzej Hajda
  2017-02-27 16:08     ` Daniel Vetter
  -1 siblings, 2 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, 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 3c20af999893..a3a58d8481e9 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
 config HDMI
 	bool
 
+config HPD_NOTIFIER
+	bool
+
 if VT
 	source "drivers/video/console/Kconfig"
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9ad3c17d6456..2697ae5c4166 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_NOTIFIER)        += 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 000000000000..971e823ead00
--- /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);
+
+	list_del(&n->head);
+	kfree(n->edid);
+	kfree(n);
+}
+
+void hpd_notifier_put(struct hpd_notifier *n)
+{
+	mutex_lock(&hpd_notifiers_lock);
+	kref_put(&n->kref, hpd_notifier_release);
+	mutex_unlock(&hpd_notifiers_lock);
+}
+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 000000000000..4dcb4515d2b2
--- /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.11.0


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

* [PATCHv4 2/9] exynos_hdmi: add HPD notifier support
  2017-02-06 10:29 ` Hans Verkuil
@ 2017-02-06 10:29   ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, 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 | 23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index d706ca4e2f02..50309409d450 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_NOTIFIER
 	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 5ed8b1effe71..8d48a0a21565 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,8 @@ 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_new_edid(hdata->notifier, edid,
+			    EDID_LENGTH * (1 + edid->extensions));
 
 	ret = drm_add_edid_modes(connector, edid);
 
@@ -1483,6 +1490,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 +1840,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 +1874,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.11.0


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

* [PATCHv4 2/9] exynos_hdmi: add HPD notifier support
@ 2017-02-06 10:29   ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski

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 | 23 ++++++++++++++++++++---
 2 files changed, 21 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index d706ca4e2f02..50309409d450 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_NOTIFIER
 	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 5ed8b1effe71..8d48a0a21565 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,8 @@ 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_new_edid(hdata->notifier, edid,
+			    EDID_LENGTH * (1 + edid->extensions));
 
 	ret = drm_add_edid_modes(connector, edid);
 
@@ -1483,6 +1490,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 +1840,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 +1874,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.11.0

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

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

* [PATCHv4 3/9] cec: integrate HPD notifier support
  2017-02-06 10:29 ` Hans Verkuil
@ 2017-02-06 10:29   ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, 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 37217e205040..f055720a1c65 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_NOTIFIER
+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)
@@ -343,6 +389,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
 	adap->rc = NULL;
 #endif
 	debugfs_remove_recursive(adap->cec_dir);
+#ifdef CONFIG_HPD_NOTIFIER
+	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 96a0aa770d61..f87a07ee36b3 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_NOTIFIER
+#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_NOTIFIER
+	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_NOTIFIER
+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.11.0


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

* [PATCHv4 3/9] cec: integrate HPD notifier support
@ 2017-02-06 10:29   ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski

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 37217e205040..f055720a1c65 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_NOTIFIER
+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)
@@ -343,6 +389,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
 	adap->rc = NULL;
 #endif
 	debugfs_remove_recursive(adap->cec_dir);
+#ifdef CONFIG_HPD_NOTIFIER
+	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 96a0aa770d61..f87a07ee36b3 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_NOTIFIER
+#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_NOTIFIER
+	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_NOTIFIER
+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.11.0

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

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

* [PATCHv4 4/9] ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi
  2017-02-06 10:29 ` Hans Verkuil
@ 2017-02-06 10:29   ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: devicetree, linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski

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

Add the new hdmi phandle to exynos4.dtsi. This phandle is needed by the
s5p-cec driver to initialize the HPD notifier framework.

Tested with my Odroid U3.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
CC: linux-samsung-soc@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index c64737baa45e..51dfcbb51b6b 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";
-- 
2.11.0

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

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

* [PATCHv4 4/9] ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi
@ 2017-02-06 10:29   ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, Hans Verkuil,
	devicetree

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

Add the new hdmi phandle to exynos4.dtsi. This phandle is needed by the
s5p-cec driver to initialize the HPD notifier framework.

Tested with my Odroid U3.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
CC: linux-samsung-soc@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: Krzysztof Kozlowski <krzk@kernel.org>
---
 arch/arm/boot/dts/exynos4.dtsi | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index c64737baa45e..51dfcbb51b6b 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";
-- 
2.11.0


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

* [PATCHv4 5/9] s5p-cec.txt: document the HDMI controller phandle
  2017-02-06 10:29 ` Hans Verkuil
@ 2017-02-06 10:29     ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Daniel Vetter, Russell King,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
	Inki Dae, Marek Szyprowski, Javier Martinez Canillas,
	Benjamin Gaignard, Hans Verkuil,
	devicetree-u79uwXL29TY76Z2rM5mHXA

From: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>

Update the bindings documenting the new hdmi phandle.

Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
CC: linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
CC: Krzysztof Kozlowski <krzk-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
---
 Documentation/devicetree/bindings/media/s5p-cec.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 925ab4d72eaa..710fc005150c 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";
-- 
2.11.0

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

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

* [PATCHv4 5/9] s5p-cec.txt: document the HDMI controller phandle
@ 2017-02-06 10:29     ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, Hans Verkuil,
	devicetree

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

Update the bindings documenting the new hdmi phandle.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
CC: linux-samsung-soc@vger.kernel.org
CC: devicetree@vger.kernel.org
CC: Krzysztof Kozlowski <krzk@kernel.org>
---
 Documentation/devicetree/bindings/media/s5p-cec.txt | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 925ab4d72eaa..710fc005150c 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";
-- 
2.11.0


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

* [PATCHv4 6/9] s5p-cec: add hpd-notifier support, move out of staging
  2017-02-06 10:29 ` Hans Verkuil
                   ` (4 preceding siblings ...)
  (?)
@ 2017-02-06 10:29 ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, 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>
CC: linux-samsung-soc@vger.kernel.org
CC: Krzysztof Kozlowski <krzk@kernel.org>
---
 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 -----
 12 files changed, 52 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/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 0245af0b76e0..9920726f14d2 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -406,6 +406,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_NOTIFIER
+       ---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 5b3cb271d2b8..ad3bf22bfeae 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 2a07968b5ac6..2014f792eceb 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 03732c13d19f..a6f5af6619a4 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 ffb8fa72c3da..1b7804cf4c51 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 a28e82cf6447..e11afbf99452 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 7a3489df3e70..000000000000
--- 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 && (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 64f21bab38f5..000000000000
--- 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.11.0


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

* [PATCHv4 7/9] sti: hdmi: add HPD notifier support
  2017-02-06 10:29 ` Hans Verkuil
                   ` (5 preceding siblings ...)
  (?)
@ 2017-02-06 10:29 ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, Hans Verkuil

From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

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: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 drivers/gpu/drm/sti/Kconfig    |  1 +
 drivers/gpu/drm/sti/sti_hdmi.c | 14 ++++++++++++++
 drivers/gpu/drm/sti/sti_hdmi.h |  3 +++
 3 files changed, 18 insertions(+)

diff --git a/drivers/gpu/drm/sti/Kconfig b/drivers/gpu/drm/sti/Kconfig
index acd72865feac..f5c9572b4169 100644
--- a/drivers/gpu/drm/sti/Kconfig
+++ b/drivers/gpu/drm/sti/Kconfig
@@ -8,5 +8,6 @@ config DRM_STI
 	select DRM_PANEL
 	select FW_LOADER
 	select SND_SOC_HDMI_CODEC if SND_SOC
+	select HPD_NOTIFIER
 	help
 	  Choose this option to enable DRM on STM stiH4xx chipset
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 376b0763c874..d32a38362391 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -786,6 +786,8 @@ static void sti_hdmi_disable(struct drm_bridge *bridge)
 	clk_disable_unprepare(hdmi->clk_pix);
 
 	hdmi->enabled = false;
+
+	hpd_event_disconnect(hdmi->notifier);
 }
 
 static void sti_hdmi_pre_enable(struct drm_bridge *bridge)
@@ -892,6 +894,9 @@ static int sti_hdmi_connector_get_modes(struct drm_connector *connector)
 	if (!edid)
 		goto fail;
 
+	hpd_event_new_edid(hdmi->notifier, edid,
+			   EDID_LENGTH * (edid->extensions + 1));
+
 	count = drm_add_edid_modes(connector, edid);
 	drm_mode_connector_update_edid_property(connector, edid);
 	drm_edid_to_eld(connector, edid);
@@ -949,10 +954,12 @@ sti_hdmi_connector_detect(struct drm_connector *connector, bool force)
 
 	if (hdmi->hpd) {
 		DRM_DEBUG_DRIVER("hdmi cable connected\n");
+		hpd_event_connect(hdmi->notifier);
 		return connector_status_connected;
 	}
 
 	DRM_DEBUG_DRIVER("hdmi cable disconnected\n");
+	hpd_event_disconnect(hdmi->notifier);
 	return connector_status_disconnected;
 }
 
@@ -1464,6 +1471,10 @@ static int sti_hdmi_probe(struct platform_device *pdev)
 		goto release_adapter;
 	}
 
+	hdmi->notifier = hpd_notifier_get(&pdev->dev);
+	if (!hdmi->notifier)
+		goto release_adapter;
+
 	hdmi->reset = devm_reset_control_get(dev, "hdmi");
 	/* Take hdmi out of reset */
 	if (!IS_ERR(hdmi->reset))
@@ -1483,11 +1494,14 @@ static int sti_hdmi_remove(struct platform_device *pdev)
 {
 	struct sti_hdmi *hdmi = dev_get_drvdata(&pdev->dev);
 
+	hpd_event_disconnect(hdmi->notifier);
+
 	i2c_put_adapter(hdmi->ddc_adapt);
 	if (hdmi->audio_pdev)
 		platform_device_unregister(hdmi->audio_pdev);
 	component_del(&pdev->dev, &sti_hdmi_ops);
 
+	hpd_notifier_put(hdmi->notifier);
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/sti/sti_hdmi.h b/drivers/gpu/drm/sti/sti_hdmi.h
index 119bc3582ac7..2109c97eb933 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.h
+++ b/drivers/gpu/drm/sti/sti_hdmi.h
@@ -8,6 +8,7 @@
 #define _STI_HDMI_H_
 
 #include <linux/hdmi.h>
+#include <linux/hpd-notifier.h>
 #include <linux/platform_device.h>
 
 #include <drm/drmP.h>
@@ -77,6 +78,7 @@ static const struct drm_prop_enum_list colorspace_mode_names[] = {
  * @audio_pdev: ASoC hdmi-codec platform device
  * @audio: hdmi audio parameters.
  * @drm_connector: hdmi connector
+ * @notifier: hotplug detect notifier
  */
 struct sti_hdmi {
 	struct device dev;
@@ -102,6 +104,7 @@ struct sti_hdmi {
 	struct platform_device *audio_pdev;
 	struct hdmi_audio_params audio;
 	struct drm_connector *drm_connector;
+	struct hpd_notifier *notifier;
 };
 
 u32 hdmi_read(struct sti_hdmi *hdmi, int offset);
-- 
2.11.0


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

* [PATCHv4 8/9] stih-cec: add HPD notifier support
  2017-02-06 10:29 ` Hans Verkuil
@ 2017-02-06 10:29     ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Daniel Vetter, Russell King,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
	Inki Dae, Marek Szyprowski, Javier Martinez Canillas,
	Benjamin Gaignard, Hans Verkuil,
	devicetree-u79uwXL29TY76Z2rM5mHXA

From: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

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 documentation the new hdmi phandle.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 .../devicetree/bindings/media/stih-cec.txt         |  2 ++
 drivers/media/platform/Kconfig                     | 10 +++++++
 drivers/media/platform/Makefile                    |  1 +
 .../st-cec => media/platform/sti/cec}/Makefile     |  0
 .../st-cec => media/platform/sti/cec}/stih-cec.c   | 31 +++++++++++++++++++---
 drivers/staging/media/Kconfig                      |  2 --
 drivers/staging/media/Makefile                     |  1 -
 drivers/staging/media/st-cec/Kconfig               |  8 ------
 drivers/staging/media/st-cec/TODO                  |  7 -----
 9 files changed, 41 insertions(+), 21 deletions(-)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%)
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/TODO

diff --git a/Documentation/devicetree/bindings/media/stih-cec.txt b/Documentation/devicetree/bindings/media/stih-cec.txt
index 71c4b2f4bcef..7d82121d148a 100644
--- a/Documentation/devicetree/bindings/media/stih-cec.txt
+++ b/Documentation/devicetree/bindings/media/stih-cec.txt
@@ -9,6 +9,7 @@ Required properties:
  - pinctrl-names: Contains only one value - "default"
  - pinctrl-0: Specifies the pin control groups used for CEC hardware.
  - resets: Reference to a reset controller
+ - st,hdmi-handle: Phandle to the HMDI controller
 
 Example for STIH407:
 
@@ -22,4 +23,5 @@ sti-cec@094a087c {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cec0_default>;
 	resets = <&softreset STIH407_LPM_SOFTRESET>;
+	st,hdmi-handle = <&hdmi>;
 };
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9920726f14d2..46db8a32a931 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -422,6 +422,16 @@ config VIDEO_SAMSUNG_S5P_CEC
          CEC bus is present in the HDMI connector and enables communication
          between compatible devices.
 
+config VIDEO_STI_HDMI_CEC
+       tristate "STMicroelectronics STiH4xx HDMI CEC driver"
+       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (ARCH_STI || COMPILE_TEST)
+       select HPD_NOTIFIER
+       ---help---
+         This is a driver for STIH4xx 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
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index ad3bf22bfeae..01b689c10f69 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
 obj-$(CONFIG_VIDEO_STI_BDISP)		+= sti/bdisp/
 obj-$(CONFIG_VIDEO_STI_HVA)		+= sti/hva/
 obj-$(CONFIG_DVB_C8SECTPFE)		+= sti/c8sectpfe/
+obj-$(CONFIG_VIDEO_STI_HDMI_CEC) 	+= sti/cec/
 
 obj-$(CONFIG_BLACKFIN)                  += blackfin/
 
diff --git a/drivers/staging/media/st-cec/Makefile b/drivers/media/platform/sti/cec/Makefile
similarity index 100%
rename from drivers/staging/media/st-cec/Makefile
rename to drivers/media/platform/sti/cec/Makefile
diff --git a/drivers/staging/media/st-cec/stih-cec.c b/drivers/media/platform/sti/cec/stih-cec.c
similarity index 93%
rename from drivers/staging/media/st-cec/stih-cec.c
rename to drivers/media/platform/sti/cec/stih-cec.c
index 3c25638a9610..e50e916837fd 100644
--- a/drivers/staging/media/st-cec/stih-cec.c
+++ b/drivers/media/platform/sti/cec/stih-cec.c
@@ -1,6 +1,4 @@
 /*
- * drivers/staging/media/st-cec/stih-cec.c
- *
  * STIH4xx CEC driver
  * Copyright (C) STMicroelectronic SA 2016
  *
@@ -10,11 +8,13 @@
  * (at your option) any later version.
  */
 #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 <media/cec.h>
@@ -129,6 +129,7 @@ struct stih_cec {
 	void __iomem		*regs;
 	int			irq;
 	u32			irq_status;
+	struct hpd_notifier	*notifier;
 };
 
 static int stih_cec_adap_enable(struct cec_adapter *adap, bool enable)
@@ -303,12 +304,29 @@ static int stih_cec_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct stih_cec *cec;
+	struct device_node *np;
+	struct platform_device *hdmi_dev;
 	int ret;
 
 	cec = devm_kzalloc(dev, sizeof(*cec), GFP_KERNEL);
 	if (!cec)
 		return -ENOMEM;
 
+	np = of_parse_phandle(pdev->dev.of_node, "st,hdmi-handle", 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)
+		return -EPROBE_DEFER;
+
+	cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
+	if (!cec->notifier)
+		return -ENOMEM;
+
 	cec->dev = dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -335,7 +353,7 @@ static int stih_cec_probe(struct platform_device *pdev)
 	cec->adap = cec_allocate_adapter(&sti_cec_adap_ops, cec,
 			CEC_NAME,
 			CEC_CAP_LOG_ADDRS | CEC_CAP_PASSTHROUGH |
-			CEC_CAP_PHYS_ADDR | CEC_CAP_TRANSMIT, 1);
+			CEC_CAP_TRANSMIT, 1);
 	ret = PTR_ERR_OR_ZERO(cec->adap);
 	if (ret)
 		return ret;
@@ -346,12 +364,19 @@ static int stih_cec_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	cec_register_hpd_notifier(cec->adap, cec->notifier);
+
 	platform_set_drvdata(pdev, cec);
 	return 0;
 }
 
 static int stih_cec_remove(struct platform_device *pdev)
 {
+	struct stih_cec *cec = platform_get_drvdata(pdev);
+
+	cec_unregister_adapter(cec->adap);
+	hpd_notifier_put(cec->notifier);
+
 	return 0;
 }
 
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index 1b7804cf4c51..d4f50f02c76e 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -30,6 +30,4 @@ source "drivers/staging/media/omap4iss/Kconfig"
 # Keep LIRC at the end, as it has sub-menus
 source "drivers/staging/media/lirc/Kconfig"
 
-source "drivers/staging/media/st-cec/Kconfig"
-
 endif
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index e11afbf99452..4c9bb23c9628 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -3,4 +3,3 @@ obj-$(CONFIG_DVB_CXD2099)	+= cxd2099/
 obj-$(CONFIG_LIRC_STAGING)	+= lirc/
 obj-$(CONFIG_VIDEO_DM365_VPFE)	+= davinci_vpfe/
 obj-$(CONFIG_VIDEO_OMAP4)	+= omap4iss/
-obj-$(CONFIG_VIDEO_STI_HDMI_CEC) += st-cec/
diff --git a/drivers/staging/media/st-cec/Kconfig b/drivers/staging/media/st-cec/Kconfig
deleted file mode 100644
index c04283db58d6..000000000000
--- a/drivers/staging/media/st-cec/Kconfig
+++ /dev/null
@@ -1,8 +0,0 @@
-config VIDEO_STI_HDMI_CEC
-       tristate "STMicroelectronics STiH4xx HDMI CEC driver"
-       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (ARCH_STI || COMPILE_TEST)
-       ---help---
-         This is a driver for STIH4xx 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/st-cec/TODO b/drivers/staging/media/st-cec/TODO
deleted file mode 100644
index c61289742c5c..000000000000
--- a/drivers/staging/media/st-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
-ST 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.11.0

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

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

* [PATCHv4 8/9] stih-cec: add HPD notifier support
@ 2017-02-06 10:29     ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, Hans Verkuil,
	devicetree

From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

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 documentation the new hdmi phandle.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
CC: devicetree@vger.kernel.org
---
 .../devicetree/bindings/media/stih-cec.txt         |  2 ++
 drivers/media/platform/Kconfig                     | 10 +++++++
 drivers/media/platform/Makefile                    |  1 +
 .../st-cec => media/platform/sti/cec}/Makefile     |  0
 .../st-cec => media/platform/sti/cec}/stih-cec.c   | 31 +++++++++++++++++++---
 drivers/staging/media/Kconfig                      |  2 --
 drivers/staging/media/Makefile                     |  1 -
 drivers/staging/media/st-cec/Kconfig               |  8 ------
 drivers/staging/media/st-cec/TODO                  |  7 -----
 9 files changed, 41 insertions(+), 21 deletions(-)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
 rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%)
 delete mode 100644 drivers/staging/media/st-cec/Kconfig
 delete mode 100644 drivers/staging/media/st-cec/TODO

diff --git a/Documentation/devicetree/bindings/media/stih-cec.txt b/Documentation/devicetree/bindings/media/stih-cec.txt
index 71c4b2f4bcef..7d82121d148a 100644
--- a/Documentation/devicetree/bindings/media/stih-cec.txt
+++ b/Documentation/devicetree/bindings/media/stih-cec.txt
@@ -9,6 +9,7 @@ Required properties:
  - pinctrl-names: Contains only one value - "default"
  - pinctrl-0: Specifies the pin control groups used for CEC hardware.
  - resets: Reference to a reset controller
+ - st,hdmi-handle: Phandle to the HMDI controller
 
 Example for STIH407:
 
@@ -22,4 +23,5 @@ sti-cec@094a087c {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_cec0_default>;
 	resets = <&softreset STIH407_LPM_SOFTRESET>;
+	st,hdmi-handle = <&hdmi>;
 };
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index 9920726f14d2..46db8a32a931 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -422,6 +422,16 @@ config VIDEO_SAMSUNG_S5P_CEC
          CEC bus is present in the HDMI connector and enables communication
          between compatible devices.
 
+config VIDEO_STI_HDMI_CEC
+       tristate "STMicroelectronics STiH4xx HDMI CEC driver"
+       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (ARCH_STI || COMPILE_TEST)
+       select HPD_NOTIFIER
+       ---help---
+         This is a driver for STIH4xx 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
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index ad3bf22bfeae..01b689c10f69 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -39,6 +39,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC)	+= exynos-gsc/
 obj-$(CONFIG_VIDEO_STI_BDISP)		+= sti/bdisp/
 obj-$(CONFIG_VIDEO_STI_HVA)		+= sti/hva/
 obj-$(CONFIG_DVB_C8SECTPFE)		+= sti/c8sectpfe/
+obj-$(CONFIG_VIDEO_STI_HDMI_CEC) 	+= sti/cec/
 
 obj-$(CONFIG_BLACKFIN)                  += blackfin/
 
diff --git a/drivers/staging/media/st-cec/Makefile b/drivers/media/platform/sti/cec/Makefile
similarity index 100%
rename from drivers/staging/media/st-cec/Makefile
rename to drivers/media/platform/sti/cec/Makefile
diff --git a/drivers/staging/media/st-cec/stih-cec.c b/drivers/media/platform/sti/cec/stih-cec.c
similarity index 93%
rename from drivers/staging/media/st-cec/stih-cec.c
rename to drivers/media/platform/sti/cec/stih-cec.c
index 3c25638a9610..e50e916837fd 100644
--- a/drivers/staging/media/st-cec/stih-cec.c
+++ b/drivers/media/platform/sti/cec/stih-cec.c
@@ -1,6 +1,4 @@
 /*
- * drivers/staging/media/st-cec/stih-cec.c
- *
  * STIH4xx CEC driver
  * Copyright (C) STMicroelectronic SA 2016
  *
@@ -10,11 +8,13 @@
  * (at your option) any later version.
  */
 #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 <media/cec.h>
@@ -129,6 +129,7 @@ struct stih_cec {
 	void __iomem		*regs;
 	int			irq;
 	u32			irq_status;
+	struct hpd_notifier	*notifier;
 };
 
 static int stih_cec_adap_enable(struct cec_adapter *adap, bool enable)
@@ -303,12 +304,29 @@ static int stih_cec_probe(struct platform_device *pdev)
 	struct device *dev = &pdev->dev;
 	struct resource *res;
 	struct stih_cec *cec;
+	struct device_node *np;
+	struct platform_device *hdmi_dev;
 	int ret;
 
 	cec = devm_kzalloc(dev, sizeof(*cec), GFP_KERNEL);
 	if (!cec)
 		return -ENOMEM;
 
+	np = of_parse_phandle(pdev->dev.of_node, "st,hdmi-handle", 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)
+		return -EPROBE_DEFER;
+
+	cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
+	if (!cec->notifier)
+		return -ENOMEM;
+
 	cec->dev = dev;
 
 	res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
@@ -335,7 +353,7 @@ static int stih_cec_probe(struct platform_device *pdev)
 	cec->adap = cec_allocate_adapter(&sti_cec_adap_ops, cec,
 			CEC_NAME,
 			CEC_CAP_LOG_ADDRS | CEC_CAP_PASSTHROUGH |
-			CEC_CAP_PHYS_ADDR | CEC_CAP_TRANSMIT, 1);
+			CEC_CAP_TRANSMIT, 1);
 	ret = PTR_ERR_OR_ZERO(cec->adap);
 	if (ret)
 		return ret;
@@ -346,12 +364,19 @@ static int stih_cec_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	cec_register_hpd_notifier(cec->adap, cec->notifier);
+
 	platform_set_drvdata(pdev, cec);
 	return 0;
 }
 
 static int stih_cec_remove(struct platform_device *pdev)
 {
+	struct stih_cec *cec = platform_get_drvdata(pdev);
+
+	cec_unregister_adapter(cec->adap);
+	hpd_notifier_put(cec->notifier);
+
 	return 0;
 }
 
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index 1b7804cf4c51..d4f50f02c76e 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -30,6 +30,4 @@ source "drivers/staging/media/omap4iss/Kconfig"
 # Keep LIRC at the end, as it has sub-menus
 source "drivers/staging/media/lirc/Kconfig"
 
-source "drivers/staging/media/st-cec/Kconfig"
-
 endif
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index e11afbf99452..4c9bb23c9628 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -3,4 +3,3 @@ obj-$(CONFIG_DVB_CXD2099)	+= cxd2099/
 obj-$(CONFIG_LIRC_STAGING)	+= lirc/
 obj-$(CONFIG_VIDEO_DM365_VPFE)	+= davinci_vpfe/
 obj-$(CONFIG_VIDEO_OMAP4)	+= omap4iss/
-obj-$(CONFIG_VIDEO_STI_HDMI_CEC) += st-cec/
diff --git a/drivers/staging/media/st-cec/Kconfig b/drivers/staging/media/st-cec/Kconfig
deleted file mode 100644
index c04283db58d6..000000000000
--- a/drivers/staging/media/st-cec/Kconfig
+++ /dev/null
@@ -1,8 +0,0 @@
-config VIDEO_STI_HDMI_CEC
-       tristate "STMicroelectronics STiH4xx HDMI CEC driver"
-       depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (ARCH_STI || COMPILE_TEST)
-       ---help---
-         This is a driver for STIH4xx 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/st-cec/TODO b/drivers/staging/media/st-cec/TODO
deleted file mode 100644
index c61289742c5c..000000000000
--- a/drivers/staging/media/st-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
-ST 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.11.0


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

* [PATCHv4 9/9] arm: sti: update sti-cec for HPD notifier support
  2017-02-06 10:29 ` Hans Verkuil
@ 2017-02-06 10:29     ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media-u79uwXL29TY76Z2rM5mHXA
  Cc: Daniel Vetter, Russell King,
	dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
	linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Krzysztof Kozlowski,
	Inki Dae, Marek Szyprowski, Javier Martinez Canillas,
	Benjamin Gaignard, Hans Verkuil,
	devicetree-u79uwXL29TY76Z2rM5mHXA

From: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>

To use HPD notifier sti CEC driver needs to get phandle
of the hdmi device.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Signed-off-by: Hans Verkuil <hans.verkuil-FYB4Gu1CFyUAvxtiuMwx3w@public.gmane.org>
CC: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
 arch/arm/boot/dts/stih407-family.dtsi | 12 ------------
 arch/arm/boot/dts/stih410.dtsi        | 13 +++++++++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index c8b2944e304a..592d23538346 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -756,18 +756,6 @@
 				 <&clk_s_c0_flexgen CLK_ETH_PHY>;
 		};
 
-		cec: sti-cec@094a087c {
-			compatible = "st,stih-cec";
-			reg = <0x94a087c 0x64>;
-			clocks = <&clk_sysin>;
-			clock-names = "cec-clk";
-			interrupts = <GIC_SPI 140 IRQ_TYPE_NONE>;
-			interrupt-names = "cec-irq";
-			pinctrl-names = "default";
-			pinctrl-0 = <&pinctrl_cec0_default>;
-			resets = <&softreset STIH407_LPM_SOFTRESET>;
-		};
-
 		rng10: rng@08a89000 {
 			compatible      = "st,rng";
 			reg		= <0x08a89000 0x1000>;
diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index 281a12424cf6..e8c01f77be80 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -259,5 +259,18 @@
 			clocks = <&clk_sysin>;
 			interrupts = <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>;
 		};
+
+		sti-cec@094a087c {
+			compatible = "st,stih-cec";
+			reg = <0x94a087c 0x64>;
+			clocks = <&clk_sysin>;
+			clock-names = "cec-clk";
+			interrupts = <GIC_SPI 140 IRQ_TYPE_NONE>;
+			interrupt-names = "cec-irq";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_cec0_default>;
+			resets = <&softreset STIH407_LPM_SOFTRESET>;
+			st,hdmi-handle = <&sti_hdmi>;
+		};
 	};
 };
-- 
2.11.0

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

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

* [PATCHv4 9/9] arm: sti: update sti-cec for HPD notifier support
@ 2017-02-06 10:29     ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-06 10:29 UTC (permalink / raw)
  To: linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, Hans Verkuil,
	devicetree

From: Benjamin Gaignard <benjamin.gaignard@linaro.org>

To use HPD notifier sti CEC driver needs to get phandle
of the hdmi device.

Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
CC: devicetree@vger.kernel.org
---
 arch/arm/boot/dts/stih407-family.dtsi | 12 ------------
 arch/arm/boot/dts/stih410.dtsi        | 13 +++++++++++++
 2 files changed, 13 insertions(+), 12 deletions(-)

diff --git a/arch/arm/boot/dts/stih407-family.dtsi b/arch/arm/boot/dts/stih407-family.dtsi
index c8b2944e304a..592d23538346 100644
--- a/arch/arm/boot/dts/stih407-family.dtsi
+++ b/arch/arm/boot/dts/stih407-family.dtsi
@@ -756,18 +756,6 @@
 				 <&clk_s_c0_flexgen CLK_ETH_PHY>;
 		};
 
-		cec: sti-cec@094a087c {
-			compatible = "st,stih-cec";
-			reg = <0x94a087c 0x64>;
-			clocks = <&clk_sysin>;
-			clock-names = "cec-clk";
-			interrupts = <GIC_SPI 140 IRQ_TYPE_NONE>;
-			interrupt-names = "cec-irq";
-			pinctrl-names = "default";
-			pinctrl-0 = <&pinctrl_cec0_default>;
-			resets = <&softreset STIH407_LPM_SOFTRESET>;
-		};
-
 		rng10: rng@08a89000 {
 			compatible      = "st,rng";
 			reg		= <0x08a89000 0x1000>;
diff --git a/arch/arm/boot/dts/stih410.dtsi b/arch/arm/boot/dts/stih410.dtsi
index 281a12424cf6..e8c01f77be80 100644
--- a/arch/arm/boot/dts/stih410.dtsi
+++ b/arch/arm/boot/dts/stih410.dtsi
@@ -259,5 +259,18 @@
 			clocks = <&clk_sysin>;
 			interrupts = <GIC_SPI 205 IRQ_TYPE_EDGE_RISING>;
 		};
+
+		sti-cec@094a087c {
+			compatible = "st,stih-cec";
+			reg = <0x94a087c 0x64>;
+			clocks = <&clk_sysin>;
+			clock-names = "cec-clk";
+			interrupts = <GIC_SPI 140 IRQ_TYPE_NONE>;
+			interrupt-names = "cec-irq";
+			pinctrl-names = "default";
+			pinctrl-0 = <&pinctrl_cec0_default>;
+			resets = <&softreset STIH407_LPM_SOFTRESET>;
+			st,hdmi-handle = <&sti_hdmi>;
+		};
 	};
 };
-- 
2.11.0


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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-06 10:29 ` [PATCHv4 1/9] video: add hotplug detect notifier support Hans Verkuil
@ 2017-02-06 13:08     ` Andrzej Hajda
  2017-02-27 16:08     ` Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: Andrzej Hajda @ 2017-02-06 13:08 UTC (permalink / raw)
  To: Hans Verkuil, linux-media
  Cc: Daniel Vetter, Russell King, dri-devel, linux-samsung-soc,
	Krzysztof Kozlowski, Inki Dae, Marek Szyprowski,
	Javier Martinez Canillas, Benjamin Gaignard, Hans Verkuil

On 06.02.2017 11:29, 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>

For patches 1-6:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej



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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
@ 2017-02-06 13:08     ` Andrzej Hajda
  0 siblings, 0 replies; 33+ messages in thread
From: Andrzej Hajda @ 2017-02-06 13:08 UTC (permalink / raw)
  To: Hans Verkuil, linux-media
  Cc: linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski

On 06.02.2017 11:29, 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>

For patches 1-6:
Reviewed-by: Andrzej Hajda <a.hajda@samsung.com>
--
Regards
Andrzej


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

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

* Re: [PATCHv4 8/9] stih-cec: add HPD notifier support
  2017-02-06 10:29     ` Hans Verkuil
@ 2017-02-08 23:17       ` Rob Herring
  -1 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2017-02-08 23:17 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: devicetree, linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski, linux-media

On Mon, Feb 06, 2017 at 11:29:50AM +0100, Hans Verkuil wrote:
> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> 
> 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 documentation the new hdmi phandle.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> CC: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/media/stih-cec.txt         |  2 ++
>  drivers/media/platform/Kconfig                     | 10 +++++++
>  drivers/media/platform/Makefile                    |  1 +
>  .../st-cec => media/platform/sti/cec}/Makefile     |  0
>  .../st-cec => media/platform/sti/cec}/stih-cec.c   | 31 +++++++++++++++++++---
>  drivers/staging/media/Kconfig                      |  2 --
>  drivers/staging/media/Makefile                     |  1 -
>  drivers/staging/media/st-cec/Kconfig               |  8 ------
>  drivers/staging/media/st-cec/TODO                  |  7 -----
>  9 files changed, 41 insertions(+), 21 deletions(-)
>  rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
>  rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%)
>  delete mode 100644 drivers/staging/media/st-cec/Kconfig
>  delete mode 100644 drivers/staging/media/st-cec/TODO
> 
> diff --git a/Documentation/devicetree/bindings/media/stih-cec.txt b/Documentation/devicetree/bindings/media/stih-cec.txt
> index 71c4b2f4bcef..7d82121d148a 100644
> --- a/Documentation/devicetree/bindings/media/stih-cec.txt
> +++ b/Documentation/devicetree/bindings/media/stih-cec.txt
> @@ -9,6 +9,7 @@ Required properties:
>   - pinctrl-names: Contains only one value - "default"
>   - pinctrl-0: Specifies the pin control groups used for CEC hardware.
>   - resets: Reference to a reset controller
> + - st,hdmi-handle: Phandle to the HMDI controller

2 cases in this series. Just drop the vendor prefix on both.

s/HMDI/HDMI/

>  
>  Example for STIH407:
>  
> @@ -22,4 +23,5 @@ sti-cec@094a087c {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_cec0_default>;
>  	resets = <&softreset STIH407_LPM_SOFTRESET>;
> +	st,hdmi-handle = <&hdmi>;
>  };
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCHv4 8/9] stih-cec: add HPD notifier support
@ 2017-02-08 23:17       ` Rob Herring
  0 siblings, 0 replies; 33+ messages in thread
From: Rob Herring @ 2017-02-08 23:17 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, devicetree, linux-samsung-soc, Russell King,
	Krzysztof Kozlowski, Javier Martinez Canillas, Hans Verkuil,
	dri-devel, Daniel Vetter, Marek Szyprowski

On Mon, Feb 06, 2017 at 11:29:50AM +0100, Hans Verkuil wrote:
> From: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> 
> 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 documentation the new hdmi phandle.
> 
> Signed-off-by: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> CC: devicetree@vger.kernel.org
> ---
>  .../devicetree/bindings/media/stih-cec.txt         |  2 ++
>  drivers/media/platform/Kconfig                     | 10 +++++++
>  drivers/media/platform/Makefile                    |  1 +
>  .../st-cec => media/platform/sti/cec}/Makefile     |  0
>  .../st-cec => media/platform/sti/cec}/stih-cec.c   | 31 +++++++++++++++++++---
>  drivers/staging/media/Kconfig                      |  2 --
>  drivers/staging/media/Makefile                     |  1 -
>  drivers/staging/media/st-cec/Kconfig               |  8 ------
>  drivers/staging/media/st-cec/TODO                  |  7 -----
>  9 files changed, 41 insertions(+), 21 deletions(-)
>  rename drivers/{staging/media/st-cec => media/platform/sti/cec}/Makefile (100%)
>  rename drivers/{staging/media/st-cec => media/platform/sti/cec}/stih-cec.c (93%)
>  delete mode 100644 drivers/staging/media/st-cec/Kconfig
>  delete mode 100644 drivers/staging/media/st-cec/TODO
> 
> diff --git a/Documentation/devicetree/bindings/media/stih-cec.txt b/Documentation/devicetree/bindings/media/stih-cec.txt
> index 71c4b2f4bcef..7d82121d148a 100644
> --- a/Documentation/devicetree/bindings/media/stih-cec.txt
> +++ b/Documentation/devicetree/bindings/media/stih-cec.txt
> @@ -9,6 +9,7 @@ Required properties:
>   - pinctrl-names: Contains only one value - "default"
>   - pinctrl-0: Specifies the pin control groups used for CEC hardware.
>   - resets: Reference to a reset controller
> + - st,hdmi-handle: Phandle to the HMDI controller

2 cases in this series. Just drop the vendor prefix on both.

s/HMDI/HDMI/

>  
>  Example for STIH407:
>  
> @@ -22,4 +23,5 @@ sti-cec@094a087c {
>  	pinctrl-names = "default";
>  	pinctrl-0 = <&pinctrl_cec0_default>;
>  	resets = <&softreset STIH407_LPM_SOFTRESET>;
> +	st,hdmi-handle = <&hdmi>;
>  };

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-06 10:29 ` [PATCHv4 1/9] video: add hotplug detect notifier support Hans Verkuil
@ 2017-02-27 16:08     ` Daniel Vetter
  2017-02-27 16:08     ` Daniel Vetter
  1 sibling, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2017-02-27 16:08 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-media, linux-samsung-soc, Russell King,
	Krzysztof Kozlowski, Javier Martinez Canillas, Hans Verkuil,
	dri-devel, Daniel Vetter, Marek Szyprowski

On Mon, Feb 06, 2017 at 11:29:43AM +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.
> 
> 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>

So I'm super late to the party because I kinda ignored all things CEC thus
far. Ḯ'm not sure this is a great design, with two main concerns:

- notifiers have a tendency to make locking utter painful. By design
  notifiers use their own lock to make sure any callbacks don't disappear
  unduly, but then on the other hand the locking order at
  register/unregister time is inverted. Or anything where you need to go
  the other way. For simple things it works, but my experience is that
  sooner or later things stop being simple, and then you're in complete
  pain. Viz fbdev notifier. I much more prefer if we have a direct
  interface, where we can define the lifetime rules and locking rules
  seperately, and if needed separately for each interface. Especially for
  something which is meant to connect different drivers from different
  subsystems so widely.
  
  You could object that this is the only interaction, and therefore there
  can't be loops, but we have dma-buf, reservation_obj and dma_fence
  sharing going on between lots of drivers already, and lots more will
  follow, so there's plenty of other cross-subsystem interactions going on
  to help complete the loop.

- The other concern I have is that we already have interfaces for ELD and
  hotplug notification. Atm their only restricted for use between gfx and
  snd, and e.g. i915 rolls 2 different kinds of its own, but there is a
  semi-standard one:

  include/sound/hdmi-codec.h

  That also deals with ELD and stuff, would be great if we don't force
  drivers to signal ELD changes in multiple different ways. Also, CEC
  should only exist with a HDMI output, so same thing.

- Afaiui we'll always should have a 1:1 mapping between drm HDMI connector
  and a given CEC pin. Notifiers are for n:m, how is this supposed to work
  if you have multiple HDMI ports around? I see that you look up by struct
  device, but it's a bit unclear what kind of device is supposed to be
  used as the key. If we extend the hdmi-codec stuff from sound and make
  it a notch more generic, then we'd already have the arbiter object to
  ties things together.

Just some thoughts on this. And again my apologies for being late with my
input.

Thanks, Daniel

> ---
>  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 3c20af999893..a3a58d8481e9 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>  config HDMI
>  	bool
>  
> +config HPD_NOTIFIER
> +	bool
> +
>  if VT
>  	source "drivers/video/console/Kconfig"
>  endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 9ad3c17d6456..2697ae5c4166 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_NOTIFIER)        += 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 000000000000..971e823ead00
> --- /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);
> +
> +	list_del(&n->head);
> +	kfree(n->edid);
> +	kfree(n);
> +}
> +
> +void hpd_notifier_put(struct hpd_notifier *n)
> +{
> +	mutex_lock(&hpd_notifiers_lock);
> +	kref_put(&n->kref, hpd_notifier_release);
> +	mutex_unlock(&hpd_notifiers_lock);
> +}
> +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 000000000000..4dcb4515d2b2
> --- /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.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
@ 2017-02-27 16:08     ` Daniel Vetter
  0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2017-02-27 16:08 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-samsung-soc, Russell King, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski, linux-media

On Mon, Feb 06, 2017 at 11:29:43AM +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.
> 
> 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>

So I'm super late to the party because I kinda ignored all things CEC thus
far. Ḯ'm not sure this is a great design, with two main concerns:

- notifiers have a tendency to make locking utter painful. By design
  notifiers use their own lock to make sure any callbacks don't disappear
  unduly, but then on the other hand the locking order at
  register/unregister time is inverted. Or anything where you need to go
  the other way. For simple things it works, but my experience is that
  sooner or later things stop being simple, and then you're in complete
  pain. Viz fbdev notifier. I much more prefer if we have a direct
  interface, where we can define the lifetime rules and locking rules
  seperately, and if needed separately for each interface. Especially for
  something which is meant to connect different drivers from different
  subsystems so widely.
  
  You could object that this is the only interaction, and therefore there
  can't be loops, but we have dma-buf, reservation_obj and dma_fence
  sharing going on between lots of drivers already, and lots more will
  follow, so there's plenty of other cross-subsystem interactions going on
  to help complete the loop.

- The other concern I have is that we already have interfaces for ELD and
  hotplug notification. Atm their only restricted for use between gfx and
  snd, and e.g. i915 rolls 2 different kinds of its own, but there is a
  semi-standard one:

  include/sound/hdmi-codec.h

  That also deals with ELD and stuff, would be great if we don't force
  drivers to signal ELD changes in multiple different ways. Also, CEC
  should only exist with a HDMI output, so same thing.

- Afaiui we'll always should have a 1:1 mapping between drm HDMI connector
  and a given CEC pin. Notifiers are for n:m, how is this supposed to work
  if you have multiple HDMI ports around? I see that you look up by struct
  device, but it's a bit unclear what kind of device is supposed to be
  used as the key. If we extend the hdmi-codec stuff from sound and make
  it a notch more generic, then we'd already have the arbiter object to
  ties things together.

Just some thoughts on this. And again my apologies for being late with my
input.

Thanks, Daniel

> ---
>  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 3c20af999893..a3a58d8481e9 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>  config HDMI
>  	bool
>  
> +config HPD_NOTIFIER
> +	bool
> +
>  if VT
>  	source "drivers/video/console/Kconfig"
>  endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 9ad3c17d6456..2697ae5c4166 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_NOTIFIER)        += 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 000000000000..971e823ead00
> --- /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);
> +
> +	list_del(&n->head);
> +	kfree(n->edid);
> +	kfree(n);
> +}
> +
> +void hpd_notifier_put(struct hpd_notifier *n)
> +{
> +	mutex_lock(&hpd_notifiers_lock);
> +	kref_put(&n->kref, hpd_notifier_release);
> +	mutex_unlock(&hpd_notifiers_lock);
> +}
> +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 000000000000..4dcb4515d2b2
> --- /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.11.0
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-27 16:08     ` Daniel Vetter
  (?)
@ 2017-02-27 17:04     ` Russell King - ARM Linux
  2017-02-27 17:21         ` Hans Verkuil
  -1 siblings, 1 reply; 33+ messages in thread
From: Russell King - ARM Linux @ 2017-02-27 17:04 UTC (permalink / raw)
  To: Daniel Vetter
  Cc: Hans Verkuil, linux-media, linux-samsung-soc,
	Krzysztof Kozlowski, Javier Martinez Canillas, Hans Verkuil,
	dri-devel, Daniel Vetter, Marek Szyprowski

On Mon, Feb 27, 2017 at 05:08:41PM +0100, Daniel Vetter wrote:
> On Mon, Feb 06, 2017 at 11:29:43AM +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.
> > 
> > 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>
> 
> So I'm super late to the party because I kinda ignored all things CEC thus
> far. Ḯ'm not sure this is a great design, with two main concerns:

I'm afraid that I walked away from this after it became clear that there
was little hope for any forward progress being made in a timely manner
for multiple reasons (mainly the core CEC code being out of mainline.)

The original notifier was created in August 2015, before there was any
"hdmi codec" support or anything of the like.  At some point (I'm not
sure when) Philipp gave his ack on it, and I definitely know it was
subsequently posted for RFC in August 2016.  We're now 1.5 years after
its creation, 7 months after it was definitely publically posted to
dri-devel, and you've only just said that you don't like the approach...

Anyway, the hdmi-codec header you point at is only relevant when you
have a driver using ASoC and you have the codec part tightly integrated
with your HDMI interface.  That generally works fine there, because
generally they are on the same device, and are very dependent (due to
the need to know the HDMI bus clock.)

The same is not true of CEC though - for example, the TDA998x is
actually two devices - the HDMI bridge, and an entirely separate
TDA9950 CEC device.  They may be in the same package, but the TDA9950
was available as an entirely separate device.  The reason that is the
case is because they are entirely separate entities as far as
functionality goes: nothing on the CEC communication side electrically
depends on the HDMI bus itself.  The only common thing in common is
the connector.

>From the protocol point of view, CEC requires the "physical address"
of a device, and that is part of the EDID information from the HDMI
device - so CEC needs to have access to the EDID.  CEC also needs to
know when if/when the EDID information is updated, or when connection/
disconnection events occur so that it can re-negotiate its "logical
address", and update for any physical address changes.

For example, if you have a CEC device connected to an AV receiver,
which is in turn connected to a TV, and the TV is powered down but
the AV receiver is powered up, then the AV receiver will give all
devices connected to it a physical address to the best of its
knowledge.  Turn the TV on, and the physical address will change
(especially so if the AV receiver has been moved between different
inputs on the TV.)

This all needs the HDMI driver to _notify_ the CEC part of these state
changes - you can't get away from the need to _notify_ these events.

So, what we need is:

(a) some way for CEC to be _notified_ of all HPD change events
(b) some way for CEC to query the EDID in a race free manner w.r.t. HPD

(a) pretty much involves some kind of notification system.  It doesn't
matter whether it's a real notifier, or a struct of function pointers,
the effect is going to be the same no matter what - the basic requirement
is that we run some code in the CEC side when a HPD state change occurs.
Given that, what you seem to be objecting to (wrt locking on this) is
against the fundamental requirement that CEC needs to track the HPD
state.

(b) can be done in other ways, but I'd suggest reversing the design (iow,
having CEC explicitly query the HDMI part for the current EDID) is more
racy than having the HDMI part notify CEC - you have the situation where
CEC could be querying the EDID on one CPU while HDMI on another CPU is
saying that the HPD changed state.

The query approach also carries with it a whole new set of locking issues,
because we can get into this situation:

 HDMI              CEC
  --- HPD insert --->
  <--- EDID read ----

The problem then is that if HDMI holds a lock while sending the HPD insert
message, and it tries to take the same lock when supplying the EDID back
to CEC, you have an immediate deadlock.

So, given that HDMI needs to notify CEC about HPD changes, it also makes
sense to keep the overall flow of data the same for everything - avoid
back-queries, and have HDMI notify CEC of the new EDID.

It also avoids the problem where we may see HPD assert, but it may take
some time for the EDID to become available from HDMI (eg, in the case
of TDA998x, we have to wait a while before even attempting to read the
EDID.)

The last point on EDID is one about the source of the EDID (eg, firmware-
loaded EDID from disk).  That won't work for CEC, since loading a fixed
EDID off disk will not give the correct physical address, and so HDMI
routing will break.  That could be worked around by having userspace
modify the loaded EDID, but that sounds like making the job unnecessarily
hard, when the correct information is only available in the sink's EDID.

When I created the notifier, the obvious problem was how does a driver
receiving a notify message know that it should process that message -
and I chose to supply the source "struct device" with each message.
This would be the HDMI interface itself, and using "struct device"
gives a firmware/no-firmware independent way of identifying the source.

For cases like the TDA998x and dw-hdmi, firmware doesn't get involved
with this: in both cases, the drivers declare their CEC device as a
platform device, so the relationship between the two struct device's
is known.  In the case of a stand-alone TDA9950, then the struct
device for the HDMI side needs to be indicated by firmware, and it's
possible to get the struct device corresponding with a firmware node.

For setups like i915, I would not expect i915 to want to use this, as
I would imagine that (if they did support CEC) CEC would be tightly
integrated, following the pattern of ultra-tight integration of
everything in i915 hardware (it seems to program everything through
the GPU stream.)

If you can think of a better approach, then I'm sure there's lots of
people who'd be willing to do the coding for it... if not, I'm not
sure where we go from here (apart from keeping code in private/vendor
trees.)

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

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-27 17:04     ` Russell King - ARM Linux
@ 2017-02-27 17:21         ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-27 17:21 UTC (permalink / raw)
  To: Russell King - ARM Linux, Daniel Vetter
  Cc: linux-media, linux-samsung-soc, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski

On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> On Mon, Feb 27, 2017 at 05:08:41PM +0100, Daniel Vetter wrote:
>> On Mon, Feb 06, 2017 at 11:29:43AM +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.
>>>
>>> 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>
>>
>> So I'm super late to the party because I kinda ignored all things CEC thus
>> far. Ḯ'm not sure this is a great design, with two main concerns:
> 
> I'm afraid that I walked away from this after it became clear that there
> was little hope for any forward progress being made in a timely manner
> for multiple reasons (mainly the core CEC code being out of mainline.)

In case you missed it: the core CEC code was moved out of staging and into
mainline in 4.10.

> 
> The original notifier was created in August 2015, before there was any
> "hdmi codec" support or anything of the like.  At some point (I'm not
> sure when) Philipp gave his ack on it, and I definitely know it was
> subsequently posted for RFC in August 2016.  We're now 1.5 years after
> its creation, 7 months after it was definitely publically posted to
> dri-devel, and you've only just said that you don't like the approach...
> 
> Anyway, the hdmi-codec header you point at is only relevant when you
> have a driver using ASoC and you have the codec part tightly integrated
> with your HDMI interface.  That generally works fine there, because
> generally they are on the same device, and are very dependent (due to
> the need to know the HDMI bus clock.)
> 
> The same is not true of CEC though - for example, the TDA998x is
> actually two devices - the HDMI bridge, and an entirely separate
> TDA9950 CEC device.  They may be in the same package, but the TDA9950
> was available as an entirely separate device.  The reason that is the
> case is because they are entirely separate entities as far as
> functionality goes: nothing on the CEC communication side electrically
> depends on the HDMI bus itself.  The only common thing in common is
> the connector.
> 
> From the protocol point of view, CEC requires the "physical address"
> of a device, and that is part of the EDID information from the HDMI
> device - so CEC needs to have access to the EDID.  CEC also needs to
> know when if/when the EDID information is updated, or when connection/
> disconnection events occur so that it can re-negotiate its "logical
> address", and update for any physical address changes.
> 
> For example, if you have a CEC device connected to an AV receiver,
> which is in turn connected to a TV, and the TV is powered down but
> the AV receiver is powered up, then the AV receiver will give all
> devices connected to it a physical address to the best of its
> knowledge.  Turn the TV on, and the physical address will change
> (especially so if the AV receiver has been moved between different
> inputs on the TV.)
> 
> This all needs the HDMI driver to _notify_ the CEC part of these state
> changes - you can't get away from the need to _notify_ these events.
> 
> So, what we need is:
> 
> (a) some way for CEC to be _notified_ of all HPD change events
> (b) some way for CEC to query the EDID in a race free manner w.r.t. HPD
> 
> (a) pretty much involves some kind of notification system.  It doesn't
> matter whether it's a real notifier, or a struct of function pointers,
> the effect is going to be the same no matter what - the basic requirement
> is that we run some code in the CEC side when a HPD state change occurs.
> Given that, what you seem to be objecting to (wrt locking on this) is
> against the fundamental requirement that CEC needs to track the HPD
> state.
> 
> (b) can be done in other ways, but I'd suggest reversing the design (iow,
> having CEC explicitly query the HDMI part for the current EDID) is more
> racy than having the HDMI part notify CEC - you have the situation where
> CEC could be querying the EDID on one CPU while HDMI on another CPU is
> saying that the HPD changed state.
> 
> The query approach also carries with it a whole new set of locking issues,
> because we can get into this situation:
> 
>  HDMI              CEC
>   --- HPD insert --->
>   <--- EDID read ----
> 
> The problem then is that if HDMI holds a lock while sending the HPD insert
> message, and it tries to take the same lock when supplying the EDID back
> to CEC, you have an immediate deadlock.
> 
> So, given that HDMI needs to notify CEC about HPD changes, it also makes
> sense to keep the overall flow of data the same for everything - avoid
> back-queries, and have HDMI notify CEC of the new EDID.
> 
> It also avoids the problem where we may see HPD assert, but it may take
> some time for the EDID to become available from HDMI (eg, in the case
> of TDA998x, we have to wait a while before even attempting to read the
> EDID.)
> 
> The last point on EDID is one about the source of the EDID (eg, firmware-
> loaded EDID from disk).  That won't work for CEC, since loading a fixed
> EDID off disk will not give the correct physical address, and so HDMI
> routing will break.  That could be worked around by having userspace
> modify the loaded EDID, but that sounds like making the job unnecessarily
> hard, when the correct information is only available in the sink's EDID.
> 
> When I created the notifier, the obvious problem was how does a driver
> receiving a notify message know that it should process that message -
> and I chose to supply the source "struct device" with each message.
> This would be the HDMI interface itself, and using "struct device"
> gives a firmware/no-firmware independent way of identifying the source.
> 
> For cases like the TDA998x and dw-hdmi, firmware doesn't get involved
> with this: in both cases, the drivers declare their CEC device as a
> platform device, so the relationship between the two struct device's
> is known.  In the case of a stand-alone TDA9950, then the struct
> device for the HDMI side needs to be indicated by firmware, and it's
> possible to get the struct device corresponding with a firmware node.
> 
> For setups like i915, I would not expect i915 to want to use this, as
> I would imagine that (if they did support CEC) CEC would be tightly
> integrated, following the pattern of ultra-tight integration of
> everything in i915 hardware (it seems to program everything through
> the GPU stream.)

Absolutely true. From what I've seen there are two types of CEC hardware
implementations: either tightly integrated in the HDMI IP (in which case
you would call the CEC framework directly, without requiring a notifier
framework), or it is entirely decoupled in which case a notifier is
IMHO the best approach.

> If you can think of a better approach, then I'm sure there's lots of
> people who'd be willing to do the coding for it... if not, I'm not
> sure where we go from here (apart from keeping code in private/vendor
> trees.)

For CEC there are just two things that it needs: the physical address
(contained in the EDID) and it needs to be informed when the EDID disappears
(typically due to a disconnect), in which case the physical address
becomes invalid (f.f.f.f).

Russell, do you have pending code that needs the ELD support in the notifier?
CEC doesn't need it, so from my perspective it can be dropped in the first
version.

It is also possible to have the notifier parse and store the physical address
rather than the whole EDID to slim it down further. Personally I have no
preference. All I need is the physical address :-)

Regards,

	Hans

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
@ 2017-02-27 17:21         ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-27 17:21 UTC (permalink / raw)
  To: Russell King - ARM Linux, Daniel Vetter
  Cc: linux-samsung-soc, dri-devel, Javier Martinez Canillas,
	Hans Verkuil, Krzysztof Kozlowski, Daniel Vetter,
	Marek Szyprowski, linux-media

On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> On Mon, Feb 27, 2017 at 05:08:41PM +0100, Daniel Vetter wrote:
>> On Mon, Feb 06, 2017 at 11:29:43AM +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.
>>>
>>> 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>
>>
>> So I'm super late to the party because I kinda ignored all things CEC thus
>> far. Ḯ'm not sure this is a great design, with two main concerns:
> 
> I'm afraid that I walked away from this after it became clear that there
> was little hope for any forward progress being made in a timely manner
> for multiple reasons (mainly the core CEC code being out of mainline.)

In case you missed it: the core CEC code was moved out of staging and into
mainline in 4.10.

> 
> The original notifier was created in August 2015, before there was any
> "hdmi codec" support or anything of the like.  At some point (I'm not
> sure when) Philipp gave his ack on it, and I definitely know it was
> subsequently posted for RFC in August 2016.  We're now 1.5 years after
> its creation, 7 months after it was definitely publically posted to
> dri-devel, and you've only just said that you don't like the approach...
> 
> Anyway, the hdmi-codec header you point at is only relevant when you
> have a driver using ASoC and you have the codec part tightly integrated
> with your HDMI interface.  That generally works fine there, because
> generally they are on the same device, and are very dependent (due to
> the need to know the HDMI bus clock.)
> 
> The same is not true of CEC though - for example, the TDA998x is
> actually two devices - the HDMI bridge, and an entirely separate
> TDA9950 CEC device.  They may be in the same package, but the TDA9950
> was available as an entirely separate device.  The reason that is the
> case is because they are entirely separate entities as far as
> functionality goes: nothing on the CEC communication side electrically
> depends on the HDMI bus itself.  The only common thing in common is
> the connector.
> 
> From the protocol point of view, CEC requires the "physical address"
> of a device, and that is part of the EDID information from the HDMI
> device - so CEC needs to have access to the EDID.  CEC also needs to
> know when if/when the EDID information is updated, or when connection/
> disconnection events occur so that it can re-negotiate its "logical
> address", and update for any physical address changes.
> 
> For example, if you have a CEC device connected to an AV receiver,
> which is in turn connected to a TV, and the TV is powered down but
> the AV receiver is powered up, then the AV receiver will give all
> devices connected to it a physical address to the best of its
> knowledge.  Turn the TV on, and the physical address will change
> (especially so if the AV receiver has been moved between different
> inputs on the TV.)
> 
> This all needs the HDMI driver to _notify_ the CEC part of these state
> changes - you can't get away from the need to _notify_ these events.
> 
> So, what we need is:
> 
> (a) some way for CEC to be _notified_ of all HPD change events
> (b) some way for CEC to query the EDID in a race free manner w.r.t. HPD
> 
> (a) pretty much involves some kind of notification system.  It doesn't
> matter whether it's a real notifier, or a struct of function pointers,
> the effect is going to be the same no matter what - the basic requirement
> is that we run some code in the CEC side when a HPD state change occurs.
> Given that, what you seem to be objecting to (wrt locking on this) is
> against the fundamental requirement that CEC needs to track the HPD
> state.
> 
> (b) can be done in other ways, but I'd suggest reversing the design (iow,
> having CEC explicitly query the HDMI part for the current EDID) is more
> racy than having the HDMI part notify CEC - you have the situation where
> CEC could be querying the EDID on one CPU while HDMI on another CPU is
> saying that the HPD changed state.
> 
> The query approach also carries with it a whole new set of locking issues,
> because we can get into this situation:
> 
>  HDMI              CEC
>   --- HPD insert --->
>   <--- EDID read ----
> 
> The problem then is that if HDMI holds a lock while sending the HPD insert
> message, and it tries to take the same lock when supplying the EDID back
> to CEC, you have an immediate deadlock.
> 
> So, given that HDMI needs to notify CEC about HPD changes, it also makes
> sense to keep the overall flow of data the same for everything - avoid
> back-queries, and have HDMI notify CEC of the new EDID.
> 
> It also avoids the problem where we may see HPD assert, but it may take
> some time for the EDID to become available from HDMI (eg, in the case
> of TDA998x, we have to wait a while before even attempting to read the
> EDID.)
> 
> The last point on EDID is one about the source of the EDID (eg, firmware-
> loaded EDID from disk).  That won't work for CEC, since loading a fixed
> EDID off disk will not give the correct physical address, and so HDMI
> routing will break.  That could be worked around by having userspace
> modify the loaded EDID, but that sounds like making the job unnecessarily
> hard, when the correct information is only available in the sink's EDID.
> 
> When I created the notifier, the obvious problem was how does a driver
> receiving a notify message know that it should process that message -
> and I chose to supply the source "struct device" with each message.
> This would be the HDMI interface itself, and using "struct device"
> gives a firmware/no-firmware independent way of identifying the source.
> 
> For cases like the TDA998x and dw-hdmi, firmware doesn't get involved
> with this: in both cases, the drivers declare their CEC device as a
> platform device, so the relationship between the two struct device's
> is known.  In the case of a stand-alone TDA9950, then the struct
> device for the HDMI side needs to be indicated by firmware, and it's
> possible to get the struct device corresponding with a firmware node.
> 
> For setups like i915, I would not expect i915 to want to use this, as
> I would imagine that (if they did support CEC) CEC would be tightly
> integrated, following the pattern of ultra-tight integration of
> everything in i915 hardware (it seems to program everything through
> the GPU stream.)

Absolutely true. From what I've seen there are two types of CEC hardware
implementations: either tightly integrated in the HDMI IP (in which case
you would call the CEC framework directly, without requiring a notifier
framework), or it is entirely decoupled in which case a notifier is
IMHO the best approach.

> If you can think of a better approach, then I'm sure there's lots of
> people who'd be willing to do the coding for it... if not, I'm not
> sure where we go from here (apart from keeping code in private/vendor
> trees.)

For CEC there are just two things that it needs: the physical address
(contained in the EDID) and it needs to be informed when the EDID disappears
(typically due to a disconnect), in which case the physical address
becomes invalid (f.f.f.f).

Russell, do you have pending code that needs the ELD support in the notifier?
CEC doesn't need it, so from my perspective it can be dropped in the first
version.

It is also possible to have the notifier parse and store the physical address
rather than the whole EDID to slim it down further. Personally I have no
preference. All I need is the physical address :-)

Regards,

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

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-27 17:21         ` Hans Verkuil
  (?)
@ 2017-02-27 17:46         ` Russell King - ARM Linux
  2017-02-28  8:51             ` Daniel Vetter
  -1 siblings, 1 reply; 33+ messages in thread
From: Russell King - ARM Linux @ 2017-02-27 17:46 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Daniel Vetter, linux-media, linux-samsung-soc,
	Krzysztof Kozlowski, Javier Martinez Canillas, Hans Verkuil,
	dri-devel, Daniel Vetter, Marek Szyprowski

On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > I'm afraid that I walked away from this after it became clear that there
> > was little hope for any forward progress being made in a timely manner
> > for multiple reasons (mainly the core CEC code being out of mainline.)
> 
> In case you missed it: the core CEC code was moved out of staging and into
> mainline in 4.10.

I was aware (even though I've not been publishing anything, I do keep
dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
release.)

> > If you can think of a better approach, then I'm sure there's lots of
> > people who'd be willing to do the coding for it... if not, I'm not
> > sure where we go from here (apart from keeping code in private/vendor
> > trees.)
> 
> For CEC there are just two things that it needs: the physical address
> (contained in the EDID) and it needs to be informed when the EDID disappears
> (typically due to a disconnect), in which case the physical address
> becomes invalid (f.f.f.f).

Yep.  CEC really only needs to know "have new phys address" and
"disconnect" provided that CEC drivers understand that they may receive
a new phys address with no intervening disconnect.  (Consider the case
where EDID changes, but the HDMI driver failed to spot the HPD signal
pulse - unfortunately, there's hardware out there where HPD is next to
useless.)

> Russell, do you have pending code that needs the ELD support in the
> notifier?  CEC doesn't need it, so from my perspective it can be
> dropped in the first version.

I was looking for that while writing the previous mail, and I think
it's time to drop it - I had thought dw-hdmi-*audio would use it, or
the ASoC people, but it's still got no users, so I think it's time
to drop it.

I have seen some patch sets go by which are making use of the notifier,
but I haven't paid close attention to how they're using it or what
they're using it for... as I sort of implied in my previous mail, I
had lost interest in mainline wrt CEC stuff due to the glacial rate
of progress.  (That's not a criticism.)

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

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-27 17:46         ` Russell King - ARM Linux
@ 2017-02-28  8:51             ` Daniel Vetter
  0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2017-02-28  8:51 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: Hans Verkuil, Daniel Vetter, linux-media, linux-samsung-soc,
	Krzysztof Kozlowski, Javier Martinez Canillas, Hans Verkuil,
	dri-devel, Daniel Vetter, Marek Szyprowski

On Mon, Feb 27, 2017 at 05:46:51PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> > On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > > I'm afraid that I walked away from this after it became clear that there
> > > was little hope for any forward progress being made in a timely manner
> > > for multiple reasons (mainly the core CEC code being out of mainline.)
> > 
> > In case you missed it: the core CEC code was moved out of staging and into
> > mainline in 4.10.
> 
> I was aware (even though I've not been publishing anything, I do keep
> dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
> release.)
> 
> > > If you can think of a better approach, then I'm sure there's lots of
> > > people who'd be willing to do the coding for it... if not, I'm not
> > > sure where we go from here (apart from keeping code in private/vendor
> > > trees.)
> > 
> > For CEC there are just two things that it needs: the physical address
> > (contained in the EDID) and it needs to be informed when the EDID disappears
> > (typically due to a disconnect), in which case the physical address
> > becomes invalid (f.f.f.f).
> 
> Yep.  CEC really only needs to know "have new phys address" and
> "disconnect" provided that CEC drivers understand that they may receive
> a new phys address with no intervening disconnect.  (Consider the case
> where EDID changes, but the HDMI driver failed to spot the HPD signal
> pulse - unfortunately, there's hardware out there where HPD is next to
> useless.)

Ok, simplifying the CEC stuff like that would be a lot better I think, to
avoid overlap with other in-kernel interfaces. I still have some
questions, but this might be my misunderstanding of how CEC works:

I thought that CEC is driven through a special separate wire in the HDMI
bus, with the receiver in the TV. Which means that we'd have a 1:1
relationship between HDMI connector and CEC port. That's at least the
use-case I've heard of for Intel boards. Are there other use-cases where
we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
notifier really only makes sense as a quick&dirty hack, or if you really
have n:m for at least one of n,m != 1. Otherwise some very specific
callback service which just provides the information the CEC side needs
seems like a much better idea to me.

> > Russell, do you have pending code that needs the ELD support in the
> > notifier?  CEC doesn't need it, so from my perspective it can be
> > dropped in the first version.
> 
> I was looking for that while writing the previous mail, and I think
> it's time to drop it - I had thought dw-hdmi-*audio would use it, or
> the ASoC people, but it's still got no users, so I think it's time
> to drop it.

For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
in tree, so better to converge on one solution instead of proliferating
even more. The entire sad story of multiple people inventing similar
wheels without talking with each another is water down the river, can't
fix that anymore :(

> I have seen some patch sets go by which are making use of the notifier,
> but I haven't paid close attention to how they're using it or what
> they're using it for... as I sort of implied in my previous mail, I
> had lost interest in mainline wrt CEC stuff due to the glacial rate
> of progress.  (That's not a criticism.)

Maybe some docs that would describe the flow we want to achieve here would
help? Doesn't need to be more than a few lines, but reconstructing that
from the various driver patches later on is indeed hard.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
@ 2017-02-28  8:51             ` Daniel Vetter
  0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2017-02-28  8:51 UTC (permalink / raw)
  To: Russell King - ARM Linux
  Cc: linux-samsung-soc, Krzysztof Kozlowski, Hans Verkuil,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski, linux-media

On Mon, Feb 27, 2017 at 05:46:51PM +0000, Russell King - ARM Linux wrote:
> On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> > On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > > I'm afraid that I walked away from this after it became clear that there
> > > was little hope for any forward progress being made in a timely manner
> > > for multiple reasons (mainly the core CEC code being out of mainline.)
> > 
> > In case you missed it: the core CEC code was moved out of staging and into
> > mainline in 4.10.
> 
> I was aware (even though I've not been publishing anything, I do keep
> dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
> release.)
> 
> > > If you can think of a better approach, then I'm sure there's lots of
> > > people who'd be willing to do the coding for it... if not, I'm not
> > > sure where we go from here (apart from keeping code in private/vendor
> > > trees.)
> > 
> > For CEC there are just two things that it needs: the physical address
> > (contained in the EDID) and it needs to be informed when the EDID disappears
> > (typically due to a disconnect), in which case the physical address
> > becomes invalid (f.f.f.f).
> 
> Yep.  CEC really only needs to know "have new phys address" and
> "disconnect" provided that CEC drivers understand that they may receive
> a new phys address with no intervening disconnect.  (Consider the case
> where EDID changes, but the HDMI driver failed to spot the HPD signal
> pulse - unfortunately, there's hardware out there where HPD is next to
> useless.)

Ok, simplifying the CEC stuff like that would be a lot better I think, to
avoid overlap with other in-kernel interfaces. I still have some
questions, but this might be my misunderstanding of how CEC works:

I thought that CEC is driven through a special separate wire in the HDMI
bus, with the receiver in the TV. Which means that we'd have a 1:1
relationship between HDMI connector and CEC port. That's at least the
use-case I've heard of for Intel boards. Are there other use-cases where
we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
notifier really only makes sense as a quick&dirty hack, or if you really
have n:m for at least one of n,m != 1. Otherwise some very specific
callback service which just provides the information the CEC side needs
seems like a much better idea to me.

> > Russell, do you have pending code that needs the ELD support in the
> > notifier?  CEC doesn't need it, so from my perspective it can be
> > dropped in the first version.
> 
> I was looking for that while writing the previous mail, and I think
> it's time to drop it - I had thought dw-hdmi-*audio would use it, or
> the ASoC people, but it's still got no users, so I think it's time
> to drop it.

For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
in tree, so better to converge on one solution instead of proliferating
even more. The entire sad story of multiple people inventing similar
wheels without talking with each another is water down the river, can't
fix that anymore :(

> I have seen some patch sets go by which are making use of the notifier,
> but I haven't paid close attention to how they're using it or what
> they're using it for... as I sort of implied in my previous mail, I
> had lost interest in mainline wrt CEC stuff due to the glacial rate
> of progress.  (That's not a criticism.)

Maybe some docs that would describe the flow we want to achieve here would
help? Doesn't need to be more than a few lines, but reconstructing that
from the various driver patches later on is indeed hard.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-28  8:51             ` Daniel Vetter
@ 2017-02-28  9:23               ` Hans Verkuil
  -1 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-28  9:23 UTC (permalink / raw)
  To: Daniel Vetter, Russell King - ARM Linux
  Cc: linux-media, linux-samsung-soc, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski

On 02/28/17 09:51, Daniel Vetter wrote:
> On Mon, Feb 27, 2017 at 05:46:51PM +0000, Russell King - ARM Linux wrote:
>> On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
>>> On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
>>>> I'm afraid that I walked away from this after it became clear that there
>>>> was little hope for any forward progress being made in a timely manner
>>>> for multiple reasons (mainly the core CEC code being out of mainline.)
>>>
>>> In case you missed it: the core CEC code was moved out of staging and into
>>> mainline in 4.10.
>>
>> I was aware (even though I've not been publishing anything, I do keep
>> dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
>> release.)
>>
>>>> If you can think of a better approach, then I'm sure there's lots of
>>>> people who'd be willing to do the coding for it... if not, I'm not
>>>> sure where we go from here (apart from keeping code in private/vendor
>>>> trees.)
>>>
>>> For CEC there are just two things that it needs: the physical address
>>> (contained in the EDID) and it needs to be informed when the EDID disappears
>>> (typically due to a disconnect), in which case the physical address
>>> becomes invalid (f.f.f.f).
>>
>> Yep.  CEC really only needs to know "have new phys address" and
>> "disconnect" provided that CEC drivers understand that they may receive
>> a new phys address with no intervening disconnect.  (Consider the case
>> where EDID changes, but the HDMI driver failed to spot the HPD signal
>> pulse - unfortunately, there's hardware out there where HPD is next to
>> useless.)
>
> Ok, simplifying the CEC stuff like that would be a lot better I think, to
> avoid overlap with other in-kernel interfaces. I still have some
> questions, but this might be my misunderstanding of how CEC works:
>
> I thought that CEC is driven through a special separate wire in the HDMI
> bus, with the receiver in the TV. Which means that we'd have a 1:1
> relationship between HDMI connector and CEC port. That's at least the
> use-case I've heard of for Intel boards. Are there other use-cases where
> we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
> notifier really only makes sense as a quick&dirty hack, or if you really
> have n:m for at least one of n,m != 1. Otherwise some very specific
> callback service which just provides the information the CEC side needs
> seems like a much better idea to me.

For the current set of CEC drivers it is 1:1.

I am fairly certain you can get n:1 situations where multiple HDMI connectors
use a single CEC adapter. I'm thinking primarily about HDMI switches here. Also
TVs with multiple inputs (basically a switch as well).

I do not support such hardware yet, though. Or to be more specific: I've never
tested this, so I am not sure if this would work in the current framework, or
if I need to do some more work for that.

That said, each CEC adapter would have only 0 or 1 HDMI outputs and 0 or more
HDMI inputs. More than one HDMI outputs is AFAICT not possible.

>
>>> Russell, do you have pending code that needs the ELD support in the
>>> notifier?  CEC doesn't need it, so from my perspective it can be
>>> dropped in the first version.
>>
>> I was looking for that while writing the previous mail, and I think
>> it's time to drop it - I had thought dw-hdmi-*audio would use it, or
>> the ASoC people, but it's still got no users, so I think it's time
>> to drop it.
>
> For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
> in tree, so better to converge on one solution instead of proliferating
> even more. The entire sad story of multiple people inventing similar
> wheels without talking with each another is water down the river, can't
> fix that anymore :(

I'll drop that in my next patch series.

>
>> I have seen some patch sets go by which are making use of the notifier,
>> but I haven't paid close attention to how they're using it or what
>> they're using it for... as I sort of implied in my previous mail, I
>> had lost interest in mainline wrt CEC stuff due to the glacial rate
>> of progress.  (That's not a criticism.)
>
> Maybe some docs that would describe the flow we want to achieve here would
> help? Doesn't need to be more than a few lines, but reconstructing that
> from the various driver patches later on is indeed hard.

I'll add some more documentation for the next version.

But in a nutshell: each HDMI connector (in or out) has to notify the CEC
driver when the physical address changes (when an EDID is read or set, or
when the HPD goes down). It also needs to provide the current physical
address when the CEC driver is first loaded. This latter requirement is
handled by the notifier framework which remembers the EDID and will create
a notify event as soon as the CEC driver registers itself.

Regards,

	Hans

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
@ 2017-02-28  9:23               ` Hans Verkuil
  0 siblings, 0 replies; 33+ messages in thread
From: Hans Verkuil @ 2017-02-28  9:23 UTC (permalink / raw)
  To: Daniel Vetter, Russell King - ARM Linux
  Cc: linux-samsung-soc, dri-devel, Javier Martinez Canillas,
	Hans Verkuil, Krzysztof Kozlowski, Daniel Vetter,
	Marek Szyprowski, linux-media

On 02/28/17 09:51, Daniel Vetter wrote:
> On Mon, Feb 27, 2017 at 05:46:51PM +0000, Russell King - ARM Linux wrote:
>> On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
>>> On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
>>>> I'm afraid that I walked away from this after it became clear that there
>>>> was little hope for any forward progress being made in a timely manner
>>>> for multiple reasons (mainly the core CEC code being out of mainline.)
>>>
>>> In case you missed it: the core CEC code was moved out of staging and into
>>> mainline in 4.10.
>>
>> I was aware (even though I've not been publishing anything, I do keep
>> dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
>> release.)
>>
>>>> If you can think of a better approach, then I'm sure there's lots of
>>>> people who'd be willing to do the coding for it... if not, I'm not
>>>> sure where we go from here (apart from keeping code in private/vendor
>>>> trees.)
>>>
>>> For CEC there are just two things that it needs: the physical address
>>> (contained in the EDID) and it needs to be informed when the EDID disappears
>>> (typically due to a disconnect), in which case the physical address
>>> becomes invalid (f.f.f.f).
>>
>> Yep.  CEC really only needs to know "have new phys address" and
>> "disconnect" provided that CEC drivers understand that they may receive
>> a new phys address with no intervening disconnect.  (Consider the case
>> where EDID changes, but the HDMI driver failed to spot the HPD signal
>> pulse - unfortunately, there's hardware out there where HPD is next to
>> useless.)
>
> Ok, simplifying the CEC stuff like that would be a lot better I think, to
> avoid overlap with other in-kernel interfaces. I still have some
> questions, but this might be my misunderstanding of how CEC works:
>
> I thought that CEC is driven through a special separate wire in the HDMI
> bus, with the receiver in the TV. Which means that we'd have a 1:1
> relationship between HDMI connector and CEC port. That's at least the
> use-case I've heard of for Intel boards. Are there other use-cases where
> we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
> notifier really only makes sense as a quick&dirty hack, or if you really
> have n:m for at least one of n,m != 1. Otherwise some very specific
> callback service which just provides the information the CEC side needs
> seems like a much better idea to me.

For the current set of CEC drivers it is 1:1.

I am fairly certain you can get n:1 situations where multiple HDMI connectors
use a single CEC adapter. I'm thinking primarily about HDMI switches here. Also
TVs with multiple inputs (basically a switch as well).

I do not support such hardware yet, though. Or to be more specific: I've never
tested this, so I am not sure if this would work in the current framework, or
if I need to do some more work for that.

That said, each CEC adapter would have only 0 or 1 HDMI outputs and 0 or more
HDMI inputs. More than one HDMI outputs is AFAICT not possible.

>
>>> Russell, do you have pending code that needs the ELD support in the
>>> notifier?  CEC doesn't need it, so from my perspective it can be
>>> dropped in the first version.
>>
>> I was looking for that while writing the previous mail, and I think
>> it's time to drop it - I had thought dw-hdmi-*audio would use it, or
>> the ASoC people, but it's still got no users, so I think it's time
>> to drop it.
>
> For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
> in tree, so better to converge on one solution instead of proliferating
> even more. The entire sad story of multiple people inventing similar
> wheels without talking with each another is water down the river, can't
> fix that anymore :(

I'll drop that in my next patch series.

>
>> I have seen some patch sets go by which are making use of the notifier,
>> but I haven't paid close attention to how they're using it or what
>> they're using it for... as I sort of implied in my previous mail, I
>> had lost interest in mainline wrt CEC stuff due to the glacial rate
>> of progress.  (That's not a criticism.)
>
> Maybe some docs that would describe the flow we want to achieve here would
> help? Doesn't need to be more than a few lines, but reconstructing that
> from the various driver patches later on is indeed hard.

I'll add some more documentation for the next version.

But in a nutshell: each HDMI connector (in or out) has to notify the CEC
driver when the physical address changes (when an EDID is read or set, or
when the HPD goes down). It also needs to provide the current physical
address when the CEC driver is first loaded. This latter requirement is
handled by the notifier framework which remembers the EDID and will create
a notify event as soon as the CEC driver registers itself.

Regards,

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

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
  2017-02-28  9:23               ` Hans Verkuil
@ 2017-02-28  9:45                 ` Daniel Vetter
  -1 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2017-02-28  9:45 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: Daniel Vetter, Russell King - ARM Linux, linux-media,
	linux-samsung-soc, Krzysztof Kozlowski, Javier Martinez Canillas,
	Hans Verkuil, dri-devel, Daniel Vetter, Marek Szyprowski

On Tue, Feb 28, 2017 at 10:23:57AM +0100, Hans Verkuil wrote:
> On 02/28/17 09:51, Daniel Vetter wrote:
> > On Mon, Feb 27, 2017 at 05:46:51PM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> > > > On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > > > > I'm afraid that I walked away from this after it became clear that there
> > > > > was little hope for any forward progress being made in a timely manner
> > > > > for multiple reasons (mainly the core CEC code being out of mainline.)
> > > > 
> > > > In case you missed it: the core CEC code was moved out of staging and into
> > > > mainline in 4.10.
> > > 
> > > I was aware (even though I've not been publishing anything, I do keep
> > > dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
> > > release.)
> > > 
> > > > > If you can think of a better approach, then I'm sure there's lots of
> > > > > people who'd be willing to do the coding for it... if not, I'm not
> > > > > sure where we go from here (apart from keeping code in private/vendor
> > > > > trees.)
> > > > 
> > > > For CEC there are just two things that it needs: the physical address
> > > > (contained in the EDID) and it needs to be informed when the EDID disappears
> > > > (typically due to a disconnect), in which case the physical address
> > > > becomes invalid (f.f.f.f).
> > > 
> > > Yep.  CEC really only needs to know "have new phys address" and
> > > "disconnect" provided that CEC drivers understand that they may receive
> > > a new phys address with no intervening disconnect.  (Consider the case
> > > where EDID changes, but the HDMI driver failed to spot the HPD signal
> > > pulse - unfortunately, there's hardware out there where HPD is next to
> > > useless.)
> > 
> > Ok, simplifying the CEC stuff like that would be a lot better I think, to
> > avoid overlap with other in-kernel interfaces. I still have some
> > questions, but this might be my misunderstanding of how CEC works:
> > 
> > I thought that CEC is driven through a special separate wire in the HDMI
> > bus, with the receiver in the TV. Which means that we'd have a 1:1
> > relationship between HDMI connector and CEC port. That's at least the
> > use-case I've heard of for Intel boards. Are there other use-cases where
> > we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
> > notifier really only makes sense as a quick&dirty hack, or if you really
> > have n:m for at least one of n,m != 1. Otherwise some very specific
> > callback service which just provides the information the CEC side needs
> > seems like a much better idea to me.
> 
> For the current set of CEC drivers it is 1:1.
> 
> I am fairly certain you can get n:1 situations where multiple HDMI connectors
> use a single CEC adapter. I'm thinking primarily about HDMI switches here. Also
> TVs with multiple inputs (basically a switch as well).
> 
> I do not support such hardware yet, though. Or to be more specific: I've never
> tested this, so I am not sure if this would work in the current framework, or
> if I need to do some more work for that.
> 
> That said, each CEC adapter would have only 0 or 1 HDMI outputs and 0 or more
> HDMI inputs. More than one HDMI outputs is AFAICT not possible.
> 
> > 
> > > > Russell, do you have pending code that needs the ELD support in the
> > > > notifier?  CEC doesn't need it, so from my perspective it can be
> > > > dropped in the first version.
> > > 
> > > I was looking for that while writing the previous mail, and I think
> > > it's time to drop it - I had thought dw-hdmi-*audio would use it, or
> > > the ASoC people, but it's still got no users, so I think it's time
> > > to drop it.
> > 
> > For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
> > in tree, so better to converge on one solution instead of proliferating
> > even more. The entire sad story of multiple people inventing similar
> > wheels without talking with each another is water down the river, can't
> > fix that anymore :(
> 
> I'll drop that in my next patch series.
> 
> > 
> > > I have seen some patch sets go by which are making use of the notifier,
> > > but I haven't paid close attention to how they're using it or what
> > > they're using it for... as I sort of implied in my previous mail, I
> > > had lost interest in mainline wrt CEC stuff due to the glacial rate
> > > of progress.  (That's not a criticism.)
> > 
> > Maybe some docs that would describe the flow we want to achieve here would
> > help? Doesn't need to be more than a few lines, but reconstructing that
> > from the various driver patches later on is indeed hard.
> 
> I'll add some more documentation for the next version.
> 
> But in a nutshell: each HDMI connector (in or out) has to notify the CEC
> driver when the physical address changes (when an EDID is read or set, or
> when the HPD goes down). It also needs to provide the current physical
> address when the CEC driver is first loaded. This latter requirement is
> handled by the notifier framework which remembers the EDID and will create
> a notify event as soon as the CEC driver registers itself.

[one reply for all blocks]

Ok, if this is all we need, then I think we should do a minimal interface
just for that. I think we should also have an opaque struct to mediate the
1:1 relationship, maybe struct cec_pin or similar? I guess reusing
hdmi_codec is indeed not a perfect fit, since both CEC and snd could be
integrate, or just one, or neither. If we go through an opaque struct we
could also provide platform-specific magic to look them up (like DT/of_).
Internally it could still use notifiers for the first implementation, but
I think it'd be good if we don't expose the n:m semantics to users. So
super rough sketch without looking at anything:

- on the display side (v4l or drm):

cec_(un)register_pin(struct cec_pin *pin);

Registers an unregisters a pin with the CEC subsystem. We'd need platform
data or maybe an entire struct device embedded.

cec_set_address(struct cec_pin *pin, struct cec_address *address);

... or whatever exactly is needed.

- on the CEC driver side:

cec_(un)register_callbacks(struct cec_pin *pin, struct cec_pin_callbacks* cbs);

There wouldn't be any way to get at a cec_pin, that would be done by
platform-specific of_*. Or maybe a fallback using something else, like we
have with gpios or similar. This would also grab refcounts on the device
underlying the cec_pin, to make sure stuff doesn't disappear untimely.

The only callback would be one that gives you the new address. 

How much garbage is this? :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

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

* Re: [PATCHv4 1/9] video: add hotplug detect notifier support
@ 2017-02-28  9:45                 ` Daniel Vetter
  0 siblings, 0 replies; 33+ messages in thread
From: Daniel Vetter @ 2017-02-28  9:45 UTC (permalink / raw)
  To: Hans Verkuil
  Cc: linux-samsung-soc, Russell King - ARM Linux, Krzysztof Kozlowski,
	Javier Martinez Canillas, Hans Verkuil, dri-devel, Daniel Vetter,
	Marek Szyprowski, linux-media

On Tue, Feb 28, 2017 at 10:23:57AM +0100, Hans Verkuil wrote:
> On 02/28/17 09:51, Daniel Vetter wrote:
> > On Mon, Feb 27, 2017 at 05:46:51PM +0000, Russell King - ARM Linux wrote:
> > > On Mon, Feb 27, 2017 at 06:21:05PM +0100, Hans Verkuil wrote:
> > > > On 02/27/2017 06:04 PM, Russell King - ARM Linux wrote:
> > > > > I'm afraid that I walked away from this after it became clear that there
> > > > > was little hope for any forward progress being made in a timely manner
> > > > > for multiple reasons (mainly the core CEC code being out of mainline.)
> > > > 
> > > > In case you missed it: the core CEC code was moved out of staging and into
> > > > mainline in 4.10.
> > > 
> > > I was aware (even though I've not been publishing anything, I do keep
> > > dw-hdmi-cec and tda9950/tda998x up to date with every final kernel
> > > release.)
> > > 
> > > > > If you can think of a better approach, then I'm sure there's lots of
> > > > > people who'd be willing to do the coding for it... if not, I'm not
> > > > > sure where we go from here (apart from keeping code in private/vendor
> > > > > trees.)
> > > > 
> > > > For CEC there are just two things that it needs: the physical address
> > > > (contained in the EDID) and it needs to be informed when the EDID disappears
> > > > (typically due to a disconnect), in which case the physical address
> > > > becomes invalid (f.f.f.f).
> > > 
> > > Yep.  CEC really only needs to know "have new phys address" and
> > > "disconnect" provided that CEC drivers understand that they may receive
> > > a new phys address with no intervening disconnect.  (Consider the case
> > > where EDID changes, but the HDMI driver failed to spot the HPD signal
> > > pulse - unfortunately, there's hardware out there where HPD is next to
> > > useless.)
> > 
> > Ok, simplifying the CEC stuff like that would be a lot better I think, to
> > avoid overlap with other in-kernel interfaces. I still have some
> > questions, but this might be my misunderstanding of how CEC works:
> > 
> > I thought that CEC is driven through a special separate wire in the HDMI
> > bus, with the receiver in the TV. Which means that we'd have a 1:1
> > relationship between HDMI connector and CEC port. That's at least the
> > use-case I've heard of for Intel boards. Are there other use-cases where
> > we do not have a 1:1 relationship between HDMI connector and CEC port? Imo
> > notifier really only makes sense as a quick&dirty hack, or if you really
> > have n:m for at least one of n,m != 1. Otherwise some very specific
> > callback service which just provides the information the CEC side needs
> > seems like a much better idea to me.
> 
> For the current set of CEC drivers it is 1:1.
> 
> I am fairly certain you can get n:1 situations where multiple HDMI connectors
> use a single CEC adapter. I'm thinking primarily about HDMI switches here. Also
> TVs with multiple inputs (basically a switch as well).
> 
> I do not support such hardware yet, though. Or to be more specific: I've never
> tested this, so I am not sure if this would work in the current framework, or
> if I need to do some more work for that.
> 
> That said, each CEC adapter would have only 0 or 1 HDMI outputs and 0 or more
> HDMI inputs. More than one HDMI outputs is AFAICT not possible.
> 
> > 
> > > > Russell, do you have pending code that needs the ELD support in the
> > > > notifier?  CEC doesn't need it, so from my perspective it can be
> > > > dropped in the first version.
> > > 
> > > I was looking for that while writing the previous mail, and I think
> > > it's time to drop it - I had thought dw-hdmi-*audio would use it, or
> > > the ASoC people, but it's still got no users, so I think it's time
> > > to drop it.
> > 
> > For ELD I'd definitely say let's please use the hdmi-codec.h. It's what's
> > in tree, so better to converge on one solution instead of proliferating
> > even more. The entire sad story of multiple people inventing similar
> > wheels without talking with each another is water down the river, can't
> > fix that anymore :(
> 
> I'll drop that in my next patch series.
> 
> > 
> > > I have seen some patch sets go by which are making use of the notifier,
> > > but I haven't paid close attention to how they're using it or what
> > > they're using it for... as I sort of implied in my previous mail, I
> > > had lost interest in mainline wrt CEC stuff due to the glacial rate
> > > of progress.  (That's not a criticism.)
> > 
> > Maybe some docs that would describe the flow we want to achieve here would
> > help? Doesn't need to be more than a few lines, but reconstructing that
> > from the various driver patches later on is indeed hard.
> 
> I'll add some more documentation for the next version.
> 
> But in a nutshell: each HDMI connector (in or out) has to notify the CEC
> driver when the physical address changes (when an EDID is read or set, or
> when the HPD goes down). It also needs to provide the current physical
> address when the CEC driver is first loaded. This latter requirement is
> handled by the notifier framework which remembers the EDID and will create
> a notify event as soon as the CEC driver registers itself.

[one reply for all blocks]

Ok, if this is all we need, then I think we should do a minimal interface
just for that. I think we should also have an opaque struct to mediate the
1:1 relationship, maybe struct cec_pin or similar? I guess reusing
hdmi_codec is indeed not a perfect fit, since both CEC and snd could be
integrate, or just one, or neither. If we go through an opaque struct we
could also provide platform-specific magic to look them up (like DT/of_).
Internally it could still use notifiers for the first implementation, but
I think it'd be good if we don't expose the n:m semantics to users. So
super rough sketch without looking at anything:

- on the display side (v4l or drm):

cec_(un)register_pin(struct cec_pin *pin);

Registers an unregisters a pin with the CEC subsystem. We'd need platform
data or maybe an entire struct device embedded.

cec_set_address(struct cec_pin *pin, struct cec_address *address);

... or whatever exactly is needed.

- on the CEC driver side:

cec_(un)register_callbacks(struct cec_pin *pin, struct cec_pin_callbacks* cbs);

There wouldn't be any way to get at a cec_pin, that would be done by
platform-specific of_*. Or maybe a fallback using something else, like we
have with gpios or similar. This would also grab refcounts on the device
underlying the cec_pin, to make sure stuff doesn't disappear untimely.

The only callback would be one that gives you the new address. 

How much garbage is this? :-)

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

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

end of thread, other threads:[~2017-02-28  9:47 UTC | newest]

Thread overview: 33+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-06 10:29 [PATCHv4 0/9] video/exynos/sti/cec: add HPD state notifier & use in drivers Hans Verkuil
2017-02-06 10:29 ` Hans Verkuil
2017-02-06 10:29 ` [PATCHv4 1/9] video: add hotplug detect notifier support Hans Verkuil
2017-02-06 13:08   ` Andrzej Hajda
2017-02-06 13:08     ` Andrzej Hajda
2017-02-27 16:08   ` Daniel Vetter
2017-02-27 16:08     ` Daniel Vetter
2017-02-27 17:04     ` Russell King - ARM Linux
2017-02-27 17:21       ` Hans Verkuil
2017-02-27 17:21         ` Hans Verkuil
2017-02-27 17:46         ` Russell King - ARM Linux
2017-02-28  8:51           ` Daniel Vetter
2017-02-28  8:51             ` Daniel Vetter
2017-02-28  9:23             ` Hans Verkuil
2017-02-28  9:23               ` Hans Verkuil
2017-02-28  9:45               ` Daniel Vetter
2017-02-28  9:45                 ` Daniel Vetter
2017-02-06 10:29 ` [PATCHv4 2/9] exynos_hdmi: add HPD " Hans Verkuil
2017-02-06 10:29   ` Hans Verkuil
2017-02-06 10:29 ` [PATCHv4 3/9] cec: integrate " Hans Verkuil
2017-02-06 10:29   ` Hans Verkuil
2017-02-06 10:29 ` [PATCHv4 4/9] ARM: dts: exynos: add HDMI controller phandle to exynos4.dtsi Hans Verkuil
2017-02-06 10:29   ` Hans Verkuil
2017-02-06 10:29 ` [PATCHv4 6/9] s5p-cec: add hpd-notifier support, move out of staging Hans Verkuil
2017-02-06 10:29 ` [PATCHv4 7/9] sti: hdmi: add HPD notifier support Hans Verkuil
     [not found] ` <20170206102951.12623-1-hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
2017-02-06 10:29   ` [PATCHv4 5/9] s5p-cec.txt: document the HDMI controller phandle Hans Verkuil
2017-02-06 10:29     ` Hans Verkuil
2017-02-06 10:29   ` [PATCHv4 8/9] stih-cec: add HPD notifier support Hans Verkuil
2017-02-06 10:29     ` Hans Verkuil
2017-02-08 23:17     ` Rob Herring
2017-02-08 23:17       ` Rob Herring
2017-02-06 10:29   ` [PATCHv4 9/9] arm: sti: update sti-cec for " Hans Verkuil
2017-02-06 10:29     ` Hans Verkuil

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.