* [PATCHv2 1/4] video: add hotplug detect notifier support
2017-01-02 14:19 [PATCHv2 0/4] video/exynos/cec: add HPD state notifier & use in s5p-cec Hans Verkuil
@ 2017-01-02 14:19 ` Hans Verkuil
2017-01-03 7:50 ` Andrzej Hajda
2017-03-20 14:26 ` Russell King - ARM Linux
2017-01-02 14:19 ` [PATCHv2 2/4] exynos_hdmi: add HPD " Hans Verkuil
` (2 subsequent siblings)
3 siblings, 2 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-01-02 14:19 UTC (permalink / raw)
To: linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Add support for video hotplug detect and EDID/ELD notifiers, which is used
to convey information from video drivers to their CEC and audio counterparts.
Based on an earlier version from Russell King:
https://patchwork.kernel.org/patch/9277043/
The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
of a video device.
When a new notifier is registered the current state will be reported to
that notifier at registration time.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/video/Kconfig | 3 +
drivers/video/Makefile | 1 +
drivers/video/hpd-notifier.c | 134 +++++++++++++++++++++++++++++++++++++++++++
include/linux/hpd-notifier.h | 109 +++++++++++++++++++++++++++++++++++
4 files changed, 247 insertions(+)
create mode 100644 drivers/video/hpd-notifier.c
create mode 100644 include/linux/hpd-notifier.h
diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3c20af9..cddc860 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
config HDMI
bool
+config HPD_NOTIFIERS
+ bool
+
if VT
source "drivers/video/console/Kconfig"
endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9ad3c17..424698b 100644
--- a/drivers/video/Makefile
+++ b/drivers/video/Makefile
@@ -1,5 +1,6 @@
obj-$(CONFIG_VGASTATE) += vgastate.o
obj-$(CONFIG_HDMI) += hdmi.o
+obj-$(CONFIG_HPD_NOTIFIERS) += hpd-notifier.o
obj-$(CONFIG_VT) += console/
obj-$(CONFIG_LOGO) += logo/
diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
new file mode 100644
index 0000000..54f7a6b
--- /dev/null
+++ b/drivers/video/hpd-notifier.c
@@ -0,0 +1,134 @@
+#include <linux/export.h>
+#include <linux/hpd-notifier.h>
+#include <linux/string.h>
+#include <linux/slab.h>
+#include <linux/list.h>
+
+static LIST_HEAD(hpd_notifiers);
+static DEFINE_MUTEX(hpd_notifiers_lock);
+
+struct hpd_notifier *hpd_notifier_get(struct device *dev)
+{
+ struct hpd_notifier *n;
+
+ mutex_lock(&hpd_notifiers_lock);
+ list_for_each_entry(n, &hpd_notifiers, head) {
+ if (n->dev == dev) {
+ mutex_unlock(&hpd_notifiers_lock);
+ kref_get(&n->kref);
+ return n;
+ }
+ }
+ n = kzalloc(sizeof(*n), GFP_KERNEL);
+ if (!n)
+ goto unlock;
+ n->dev = dev;
+ mutex_init(&n->lock);
+ BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
+ kref_init(&n->kref);
+ list_add_tail(&n->head, &hpd_notifiers);
+unlock:
+ mutex_unlock(&hpd_notifiers_lock);
+ return n;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_get);
+
+static void hpd_notifier_release(struct kref *kref)
+{
+ struct hpd_notifier *n =
+ container_of(kref, struct hpd_notifier, kref);
+
+ mutex_lock(&hpd_notifiers_lock);
+ list_del(&n->head);
+ mutex_unlock(&hpd_notifiers_lock);
+ kfree(n->edid);
+ kfree(n);
+}
+
+void hpd_notifier_put(struct hpd_notifier *n)
+{
+ kref_put(&n->kref, hpd_notifier_release);
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_put);
+
+int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
+{
+ int ret = blocking_notifier_chain_register(&n->notifiers, nb);
+
+ if (ret)
+ return ret;
+ kref_get(&n->kref);
+ mutex_lock(&n->lock);
+ if (n->connected) {
+ blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
+ if (n->edid_size)
+ blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
+ if (n->has_eld)
+ blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
+ }
+ mutex_unlock(&n->lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_register);
+
+int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
+{
+ int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
+
+ if (ret == 0)
+ hpd_notifier_put(n);
+ return ret;
+}
+EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
+
+void hpd_event_connect(struct hpd_notifier *n)
+{
+ mutex_lock(&n->lock);
+ n->connected = true;
+ blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
+ mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_connect);
+
+void hpd_event_disconnect(struct hpd_notifier *n)
+{
+ mutex_lock(&n->lock);
+ n->connected = false;
+ n->has_eld = false;
+ n->edid_size = 0;
+ blocking_notifier_call_chain(&n->notifiers, HPD_DISCONNECTED, n);
+ mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_disconnect);
+
+int hpd_event_new_edid(struct hpd_notifier *n, const void *edid, size_t size)
+{
+ mutex_lock(&n->lock);
+ if (n->edid_allocated_size < size) {
+ void *p = kmalloc(size, GFP_KERNEL);
+
+ if (p == NULL) {
+ mutex_unlock(&n->lock);
+ return -ENOMEM;
+ }
+ kfree(n->edid);
+ n->edid = p;
+ n->edid_allocated_size = size;
+ }
+ memcpy(n->edid, edid, size);
+ n->edid_size = size;
+ blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
+ mutex_unlock(&n->lock);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(hpd_event_new_edid);
+
+void hpd_event_new_eld(struct hpd_notifier *n, const u8 eld[128])
+{
+ mutex_lock(&n->lock);
+ memcpy(n->eld, eld, sizeof(n->eld));
+ n->has_eld = true;
+ blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
+ mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(hpd_event_new_eld);
diff --git a/include/linux/hpd-notifier.h b/include/linux/hpd-notifier.h
new file mode 100644
index 0000000..4dcb451
--- /dev/null
+++ b/include/linux/hpd-notifier.h
@@ -0,0 +1,109 @@
+/*
+ * hpd-notifier.h - notify interested parties of (dis)connect and EDID
+ * events
+ *
+ * Copyright 2016 Russell King <rmk+kernel@arm.linux.org.uk>
+ * Copyright 2016 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+#ifndef LINUX_HPD_NOTIFIER_H
+#define LINUX_HPD_NOTIFIER_H
+
+#include <linux/types.h>
+#include <linux/notifier.h>
+#include <linux/kref.h>
+
+enum {
+ HPD_CONNECTED,
+ HPD_DISCONNECTED,
+ HPD_NEW_EDID,
+ HPD_NEW_ELD,
+};
+
+struct device;
+
+struct hpd_notifier {
+ struct mutex lock;
+ struct list_head head;
+ struct kref kref;
+ struct blocking_notifier_head notifiers;
+ struct device *dev;
+
+ /* Current state */
+ bool connected;
+ bool has_eld;
+ unsigned char eld[128];
+ void *edid;
+ size_t edid_size;
+ size_t edid_allocated_size;
+};
+
+/**
+ * hpd_notifier_get - find or create a new hpd_notifier for the given device.
+ * @dev: device that sends the events.
+ *
+ * If a notifier for device @dev already exists, then increase the refcount
+ * and return that notifier.
+ *
+ * If it doesn't exist, then allocate a new notifier struct and return a
+ * pointer to that new struct.
+ *
+ * Return NULL if the memory could not be allocated.
+ */
+struct hpd_notifier *hpd_notifier_get(struct device *dev);
+
+/**
+ * hpd_notifier_put - decrease refcount and delete when the refcount reaches 0.
+ * @n: notifier
+ */
+void hpd_notifier_put(struct hpd_notifier *n);
+
+/**
+ * hpd_notifier_register - register the notifier with the notifier_block.
+ * @n: the HPD notifier
+ * @nb: the notifier_block
+ */
+int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb);
+
+/**
+ * hpd_notifier_unregister - unregister the notifier with the notifier_block.
+ * @n: the HPD notifier
+ * @nb: the notifier_block
+ */
+int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb);
+
+/**
+ * hpd_event_connect - send a connect event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_CONNECTED event to any registered parties.
+ */
+void hpd_event_connect(struct hpd_notifier *n);
+
+/**
+ * hpd_event_disconnect - send a disconnect event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_DISCONNECTED event to any registered parties.
+ */
+void hpd_event_disconnect(struct hpd_notifier *n);
+
+/**
+ * hpd_event_new_edid - send a new EDID event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_NEW_EDID event to any registered parties.
+ * This function will make a copy the EDID so it can return -ENOMEM if
+ * no memory could be allocated.
+ */
+int hpd_event_new_edid(struct hpd_notifier *n, const void *edid, size_t size);
+
+/**
+ * hpd_event_new_eld - send a new ELD event.
+ * @n: the HPD notifier
+ *
+ * Send an HPD_NEW_ELD event to any registered parties.
+ */
+void hpd_event_new_eld(struct hpd_notifier *n, const u8 eld[128]);
+
+#endif
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCHv2 1/4] video: add hotplug detect notifier support
2017-01-02 14:19 ` [PATCHv2 1/4] video: add hotplug detect notifier support Hans Verkuil
@ 2017-01-03 7:50 ` Andrzej Hajda
2017-01-03 7:54 ` Hans Verkuil
2017-03-20 14:26 ` Russell King - ARM Linux
1 sibling, 1 reply; 17+ messages in thread
From: Andrzej Hajda @ 2017-01-03 7:50 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: linux-samsung-soc, Russell King, dri-devel,
Javier Martinez Canillas, Hans Verkuil, Krzysztof Kozlowski,
Marek Szyprowski
Hi Hans,
On 02.01.2017 15:19, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Add support for video hotplug detect and EDID/ELD notifiers, which is used
> to convey information from video drivers to their CEC and audio counterparts.
>
> Based on an earlier version from Russell King:
>
> https://patchwork.kernel.org/patch/9277043/
>
> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
> of a video device.
>
> When a new notifier is registered the current state will be reported to
> that notifier at registration time.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/video/Kconfig | 3 +
> drivers/video/Makefile | 1 +
> drivers/video/hpd-notifier.c | 134 +++++++++++++++++++++++++++++++++++++++++++
> include/linux/hpd-notifier.h | 109 +++++++++++++++++++++++++++++++++++
> 4 files changed, 247 insertions(+)
> create mode 100644 drivers/video/hpd-notifier.c
> create mode 100644 include/linux/hpd-notifier.h
>
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 3c20af9..cddc860 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
> config HDMI
> bool
>
> +config HPD_NOTIFIERS
> + bool
> +
> if VT
> source "drivers/video/console/Kconfig"
> endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 9ad3c17..424698b 100644
> --- a/drivers/video/Makefile
> +++ b/drivers/video/Makefile
> @@ -1,5 +1,6 @@
> obj-$(CONFIG_VGASTATE) += vgastate.o
> obj-$(CONFIG_HDMI) += hdmi.o
> +obj-$(CONFIG_HPD_NOTIFIERS) += hpd-notifier.o
>
> obj-$(CONFIG_VT) += console/
> obj-$(CONFIG_LOGO) += logo/
> diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
> new file mode 100644
> index 0000000..54f7a6b
> --- /dev/null
> +++ b/drivers/video/hpd-notifier.c
> @@ -0,0 +1,134 @@
> +#include <linux/export.h>
> +#include <linux/hpd-notifier.h>
> +#include <linux/string.h>
> +#include <linux/slab.h>
> +#include <linux/list.h>
> +
> +static LIST_HEAD(hpd_notifiers);
> +static DEFINE_MUTEX(hpd_notifiers_lock);
> +
> +struct hpd_notifier *hpd_notifier_get(struct device *dev)
> +{
> + struct hpd_notifier *n;
> +
> + mutex_lock(&hpd_notifiers_lock);
> + list_for_each_entry(n, &hpd_notifiers, head) {
> + if (n->dev == dev) {
> + mutex_unlock(&hpd_notifiers_lock);
I think this place is racy, we have pointer to unprotected area
(n->kref), so if concurrent thread calls hpd_notifier_put in this moment
&n->kref could be freed and kref_get in the next line will operate on
dangling pointer. Am I right?
Regards
Andrzej
> + kref_get(&n->kref);
> + return n;
> + }
> + }
> + n = kzalloc(sizeof(*n), GFP_KERNEL);
> + if (!n)
> + goto unlock;
> + n->dev = dev;
> + mutex_init(&n->lock);
> + BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
> + kref_init(&n->kref);
> + list_add_tail(&n->head, &hpd_notifiers);
> +unlock:
> + mutex_unlock(&hpd_notifiers_lock);
> + return n;
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_get);
> +
> +static void hpd_notifier_release(struct kref *kref)
> +{
> + struct hpd_notifier *n =
> + container_of(kref, struct hpd_notifier, kref);
> +
> + mutex_lock(&hpd_notifiers_lock);
> + list_del(&n->head);
> + mutex_unlock(&hpd_notifiers_lock);
> + kfree(n->edid);
> + kfree(n);
> +}
> +
> +void hpd_notifier_put(struct hpd_notifier *n)
> +{
> + kref_put(&n->kref, hpd_notifier_release);
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_put);
> +
> +int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
> +{
> + int ret = blocking_notifier_chain_register(&n->notifiers, nb);
> +
> + if (ret)
> + return ret;
> + kref_get(&n->kref);
> + mutex_lock(&n->lock);
> + if (n->connected) {
> + blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
> + if (n->edid_size)
> + blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
> + if (n->has_eld)
> + blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
> + }
> + mutex_unlock(&n->lock);
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_register);
> +
> +int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
> +{
> + int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
> +
> + if (ret == 0)
> + hpd_notifier_put(n);
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
(...)
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCHv2 1/4] video: add hotplug detect notifier support
2017-01-03 7:50 ` Andrzej Hajda
@ 2017-01-03 7:54 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-01-03 7:54 UTC (permalink / raw)
To: Andrzej Hajda, linux-media
Cc: linux-samsung-soc, Russell King, dri-devel,
Javier Martinez Canillas, Hans Verkuil, Krzysztof Kozlowski,
Marek Szyprowski
On 01/03/2017 08:50 AM, Andrzej Hajda wrote:
> Hi Hans,
>
> On 02.01.2017 15:19, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Add support for video hotplug detect and EDID/ELD notifiers, which is used
>> to convey information from video drivers to their CEC and audio counterparts.
>>
>> Based on an earlier version from Russell King:
>>
>> https://patchwork.kernel.org/patch/9277043/
>>
>> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
>> of a video device.
>>
>> When a new notifier is registered the current state will be reported to
>> that notifier at registration time.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/video/Kconfig | 3 +
>> drivers/video/Makefile | 1 +
>> drivers/video/hpd-notifier.c | 134 +++++++++++++++++++++++++++++++++++++++++++
>> include/linux/hpd-notifier.h | 109 +++++++++++++++++++++++++++++++++++
>> 4 files changed, 247 insertions(+)
>> create mode 100644 drivers/video/hpd-notifier.c
>> create mode 100644 include/linux/hpd-notifier.h
>>
>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>> index 3c20af9..cddc860 100644
>> --- a/drivers/video/Kconfig
>> +++ b/drivers/video/Kconfig
>> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>> config HDMI
>> bool
>>
>> +config HPD_NOTIFIERS
>> + bool
>> +
>> if VT
>> source "drivers/video/console/Kconfig"
>> endif
>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
>> index 9ad3c17..424698b 100644
>> --- a/drivers/video/Makefile
>> +++ b/drivers/video/Makefile
>> @@ -1,5 +1,6 @@
>> obj-$(CONFIG_VGASTATE) += vgastate.o
>> obj-$(CONFIG_HDMI) += hdmi.o
>> +obj-$(CONFIG_HPD_NOTIFIERS) += hpd-notifier.o
>>
>> obj-$(CONFIG_VT) += console/
>> obj-$(CONFIG_LOGO) += logo/
>> diff --git a/drivers/video/hpd-notifier.c b/drivers/video/hpd-notifier.c
>> new file mode 100644
>> index 0000000..54f7a6b
>> --- /dev/null
>> +++ b/drivers/video/hpd-notifier.c
>> @@ -0,0 +1,134 @@
>> +#include <linux/export.h>
>> +#include <linux/hpd-notifier.h>
>> +#include <linux/string.h>
>> +#include <linux/slab.h>
>> +#include <linux/list.h>
>> +
>> +static LIST_HEAD(hpd_notifiers);
>> +static DEFINE_MUTEX(hpd_notifiers_lock);
>> +
>> +struct hpd_notifier *hpd_notifier_get(struct device *dev)
>> +{
>> + struct hpd_notifier *n;
>> +
>> + mutex_lock(&hpd_notifiers_lock);
>> + list_for_each_entry(n, &hpd_notifiers, head) {
>> + if (n->dev == dev) {
>> + mutex_unlock(&hpd_notifiers_lock);
>
> I think this place is racy, we have pointer to unprotected area
> (n->kref), so if concurrent thread calls hpd_notifier_put in this moment
> &n->kref could be freed and kref_get in the next line will operate on
> dangling pointer. Am I right?
You're right. I took the hpd_notifiers_lock in hpd_notifier_release,
but I should take it in hpd_notifier_put.
Thanks! I'll fix that.
Regards,
Hans
>
> Regards
> Andrzej
>
>> + kref_get(&n->kref);
>> + return n;
>> + }
>> + }
>> + n = kzalloc(sizeof(*n), GFP_KERNEL);
>> + if (!n)
>> + goto unlock;
>> + n->dev = dev;
>> + mutex_init(&n->lock);
>> + BLOCKING_INIT_NOTIFIER_HEAD(&n->notifiers);
>> + kref_init(&n->kref);
>> + list_add_tail(&n->head, &hpd_notifiers);
>> +unlock:
>> + mutex_unlock(&hpd_notifiers_lock);
>> + return n;
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_get);
>> +
>> +static void hpd_notifier_release(struct kref *kref)
>> +{
>> + struct hpd_notifier *n =
>> + container_of(kref, struct hpd_notifier, kref);
>> +
>> + mutex_lock(&hpd_notifiers_lock);
>> + list_del(&n->head);
>> + mutex_unlock(&hpd_notifiers_lock);
>> + kfree(n->edid);
>> + kfree(n);
>> +}
>> +
>> +void hpd_notifier_put(struct hpd_notifier *n)
>> +{
>> + kref_put(&n->kref, hpd_notifier_release);
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_put);
>> +
>> +int hpd_notifier_register(struct hpd_notifier *n, struct notifier_block *nb)
>> +{
>> + int ret = blocking_notifier_chain_register(&n->notifiers, nb);
>> +
>> + if (ret)
>> + return ret;
>> + kref_get(&n->kref);
>> + mutex_lock(&n->lock);
>> + if (n->connected) {
>> + blocking_notifier_call_chain(&n->notifiers, HPD_CONNECTED, n);
>> + if (n->edid_size)
>> + blocking_notifier_call_chain(&n->notifiers, HPD_NEW_EDID, n);
>> + if (n->has_eld)
>> + blocking_notifier_call_chain(&n->notifiers, HPD_NEW_ELD, n);
>> + }
>> + mutex_unlock(&n->lock);
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_register);
>> +
>> +int hpd_notifier_unregister(struct hpd_notifier *n, struct notifier_block *nb)
>> +{
>> + int ret = blocking_notifier_chain_unregister(&n->notifiers, nb);
>> +
>> + if (ret == 0)
>> + hpd_notifier_put(n);
>> + return ret;
>> +}
>> +EXPORT_SYMBOL_GPL(hpd_notifier_unregister);
> (...)
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 1/4] video: add hotplug detect notifier support
2017-01-02 14:19 ` [PATCHv2 1/4] video: add hotplug detect notifier support Hans Verkuil
2017-01-03 7:50 ` Andrzej Hajda
@ 2017-03-20 14:26 ` Russell King - ARM Linux
2017-03-20 14:27 ` Russell King - ARM Linux
1 sibling, 1 reply; 17+ messages in thread
From: Russell King - ARM Linux @ 2017-03-20 14:26 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Add support for video hotplug detect and EDID/ELD notifiers, which is used
> to convey information from video drivers to their CEC and audio counterparts.
>
> Based on an earlier version from Russell King:
>
> https://patchwork.kernel.org/patch/9277043/
>
> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
> of a video device.
I thought we had decided to drop the ELD bits?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 1/4] video: add hotplug detect notifier support
2017-03-20 14:26 ` Russell King - ARM Linux
@ 2017-03-20 14:27 ` Russell King - ARM Linux
2017-03-20 14:41 ` Hans Verkuil
0 siblings, 1 reply; 17+ messages in thread
From: Russell King - ARM Linux @ 2017-03-20 14:27 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On Mon, Mar 20, 2017 at 02:26:16PM +0000, Russell King - ARM Linux wrote:
> On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
> > From: Hans Verkuil <hans.verkuil@cisco.com>
> >
> > Add support for video hotplug detect and EDID/ELD notifiers, which is used
> > to convey information from video drivers to their CEC and audio counterparts.
> >
> > Based on an earlier version from Russell King:
> >
> > https://patchwork.kernel.org/patch/9277043/
> >
> > The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
> > of a video device.
>
> I thought we had decided to drop the ELD bits?
Ignore that - mailer wrapped around to the first message in my mailbox!
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 1/4] video: add hotplug detect notifier support
2017-03-20 14:27 ` Russell King - ARM Linux
@ 2017-03-20 14:41 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-03-20 14:41 UTC (permalink / raw)
To: Russell King - ARM Linux, Hans Verkuil
Cc: linux-media, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 03/20/2017 03:27 PM, Russell King - ARM Linux wrote:
> On Mon, Mar 20, 2017 at 02:26:16PM +0000, Russell King - ARM Linux wrote:
>> On Mon, Jan 02, 2017 at 03:19:04PM +0100, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> Add support for video hotplug detect and EDID/ELD notifiers, which is used
>>> to convey information from video drivers to their CEC and audio counterparts.
>>>
>>> Based on an earlier version from Russell King:
>>>
>>> https://patchwork.kernel.org/patch/9277043/
>>>
>>> The hpd_notifier is a reference counted object containing the HPD/EDID/ELD state
>>> of a video device.
>>
>> I thought we had decided to drop the ELD bits?
>
> Ignore that - mailer wrapped around to the first message in my mailbox!
>
Just FYI: I've removed anything not needed for CEC in this git repo:
https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=exynos4-cec2
It compiles, but it's otherwise untested.
And I still need to think more about Daniel's comments. I hope to work on this
a bit more next week.
Regards,
Hans
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv2 2/4] exynos_hdmi: add HPD notifier support
2017-01-02 14:19 [PATCHv2 0/4] video/exynos/cec: add HPD state notifier & use in s5p-cec Hans Verkuil
2017-01-02 14:19 ` [PATCHv2 1/4] video: add hotplug detect notifier support Hans Verkuil
@ 2017-01-02 14:19 ` Hans Verkuil
2017-01-03 7:55 ` Andrzej Hajda
2017-01-02 14:19 ` [PATCHv2 3/4] cec: integrate " Hans Verkuil
2017-01-02 14:19 ` [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging Hans Verkuil
3 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2017-01-02 14:19 UTC (permalink / raw)
To: linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Implement the HPD notifier support to allow CEC drivers to
be informed when there is a new EDID and when a connect or
disconnect happens.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/gpu/drm/exynos/Kconfig | 1 +
drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
2 files changed, 22 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
index d706ca4..80bfd1d 100644
--- a/drivers/gpu/drm/exynos/Kconfig
+++ b/drivers/gpu/drm/exynos/Kconfig
@@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
config DRM_EXYNOS_HDMI
bool "HDMI"
depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
+ select HPD_NOTIFIERS
help
Choose this option if you want to use Exynos HDMI for DRM.
diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 5ed8b1e..28bf609 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -31,6 +31,7 @@
#include <linux/pm_runtime.h>
#include <linux/clk.h>
#include <linux/gpio/consumer.h>
+#include <linux/hpd-notifier.h>
#include <linux/regulator/consumer.h>
#include <linux/io.h>
#include <linux/of_address.h>
@@ -118,6 +119,7 @@ struct hdmi_context {
bool dvi_mode;
struct delayed_work hotplug_work;
struct drm_display_mode current_mode;
+ struct hpd_notifier *notifier;
const struct hdmi_driver_data *drv_data;
void __iomem *regs;
@@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
{
struct hdmi_context *hdata = connector_to_hdmi(connector);
- if (gpiod_get_value(hdata->hpd_gpio))
+ if (gpiod_get_value(hdata->hpd_gpio)) {
+ hpd_event_connect(hdata->notifier);
return connector_status_connected;
+ }
+ hpd_event_disconnect(hdata->notifier);
return connector_status_disconnected;
}
@@ -848,6 +853,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
edid->width_cm, edid->height_cm);
drm_mode_connector_update_edid_property(connector, edid);
+ hpd_event_connect(hdata->notifier);
+ hpd_event_new_edid(hdata->notifier, edid,
+ EDID_LENGTH * (1 + edid->extensions));
ret = drm_add_edid_modes(connector, edid);
@@ -1483,6 +1491,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
if (funcs && funcs->disable)
(*funcs->disable)(crtc);
+ hpd_event_disconnect(hdata->notifier);
cancel_delayed_work(&hdata->hotplug_work);
hdmiphy_disable(hdata);
@@ -1832,15 +1841,22 @@ static int hdmi_probe(struct platform_device *pdev)
}
}
+ hdata->notifier = hpd_notifier_get(&pdev->dev);
+ if (hdata->notifier == NULL) {
+ ret = -ENOMEM;
+ goto err_hdmiphy;
+ }
+
pm_runtime_enable(dev);
ret = component_add(&pdev->dev, &hdmi_component_ops);
if (ret)
- goto err_disable_pm_runtime;
+ goto err_notifier_put;
return ret;
-err_disable_pm_runtime:
+err_notifier_put:
+ hpd_notifier_put(hdata->notifier);
pm_runtime_disable(dev);
err_hdmiphy:
@@ -1859,9 +1875,11 @@ static int hdmi_remove(struct platform_device *pdev)
struct hdmi_context *hdata = platform_get_drvdata(pdev);
cancel_delayed_work_sync(&hdata->hotplug_work);
+ hpd_event_disconnect(hdata->notifier);
component_del(&pdev->dev, &hdmi_component_ops);
+ hpd_notifier_put(hdata->notifier);
pm_runtime_disable(&pdev->dev);
if (!IS_ERR(hdata->reg_hdmi_en))
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCHv2 2/4] exynos_hdmi: add HPD notifier support
2017-01-02 14:19 ` [PATCHv2 2/4] exynos_hdmi: add HPD " Hans Verkuil
@ 2017-01-03 7:55 ` Andrzej Hajda
2017-01-03 8:25 ` Hans Verkuil
0 siblings, 1 reply; 17+ messages in thread
From: Andrzej Hajda @ 2017-01-03 7:55 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 02.01.2017 15:19, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> Implement the HPD notifier support to allow CEC drivers to
> be informed when there is a new EDID and when a connect or
> disconnect happens.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> drivers/gpu/drm/exynos/Kconfig | 1 +
> drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
> 2 files changed, 22 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
> index d706ca4..80bfd1d 100644
> --- a/drivers/gpu/drm/exynos/Kconfig
> +++ b/drivers/gpu/drm/exynos/Kconfig
> @@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
> config DRM_EXYNOS_HDMI
> bool "HDMI"
> depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
> + select HPD_NOTIFIERS
> help
> Choose this option if you want to use Exynos HDMI for DRM.
>
> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 5ed8b1e..28bf609 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -31,6 +31,7 @@
> #include <linux/pm_runtime.h>
> #include <linux/clk.h>
> #include <linux/gpio/consumer.h>
> +#include <linux/hpd-notifier.h>
> #include <linux/regulator/consumer.h>
> #include <linux/io.h>
> #include <linux/of_address.h>
> @@ -118,6 +119,7 @@ struct hdmi_context {
> bool dvi_mode;
> struct delayed_work hotplug_work;
> struct drm_display_mode current_mode;
> + struct hpd_notifier *notifier;
> const struct hdmi_driver_data *drv_data;
>
> void __iomem *regs;
> @@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
> {
> struct hdmi_context *hdata = connector_to_hdmi(connector);
>
> - if (gpiod_get_value(hdata->hpd_gpio))
> + if (gpiod_get_value(hdata->hpd_gpio)) {
> + hpd_event_connect(hdata->notifier);
> return connector_status_connected;
> + }
>
> + hpd_event_disconnect(hdata->notifier);
> return connector_status_disconnected;
> }
>
> @@ -848,6 +853,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
> edid->width_cm, edid->height_cm);
>
> drm_mode_connector_update_edid_property(connector, edid);
> + hpd_event_connect(hdata->notifier);
Is there a reason to call hpd_event_connect here? It was called already
from hdmi_detect.
Regards
Andrzej
> + hpd_event_new_edid(hdata->notifier, edid,
> + EDID_LENGTH * (1 + edid->extensions));
>
> ret = drm_add_edid_modes(connector, edid);
>
> @@ -1483,6 +1491,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
> if (funcs && funcs->disable)
> (*funcs->disable)(crtc);
>
> + hpd_event_disconnect(hdata->notifier);
> cancel_delayed_work(&hdata->hotplug_work);
>
> hdmiphy_disable(hdata);
> @@ -1832,15 +1841,22 @@ static int hdmi_probe(struct platform_device *pdev)
> }
> }
>
> + hdata->notifier = hpd_notifier_get(&pdev->dev);
> + if (hdata->notifier == NULL) {
> + ret = -ENOMEM;
> + goto err_hdmiphy;
> + }
> +
> pm_runtime_enable(dev);
>
> ret = component_add(&pdev->dev, &hdmi_component_ops);
> if (ret)
> - goto err_disable_pm_runtime;
> + goto err_notifier_put;
>
> return ret;
>
> -err_disable_pm_runtime:
> +err_notifier_put:
> + hpd_notifier_put(hdata->notifier);
> pm_runtime_disable(dev);
>
> err_hdmiphy:
> @@ -1859,9 +1875,11 @@ static int hdmi_remove(struct platform_device *pdev)
> struct hdmi_context *hdata = platform_get_drvdata(pdev);
>
> cancel_delayed_work_sync(&hdata->hotplug_work);
> + hpd_event_disconnect(hdata->notifier);
>
> component_del(&pdev->dev, &hdmi_component_ops);
>
> + hpd_notifier_put(hdata->notifier);
> pm_runtime_disable(&pdev->dev);
>
> if (!IS_ERR(hdata->reg_hdmi_en))
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCHv2 2/4] exynos_hdmi: add HPD notifier support
2017-01-03 7:55 ` Andrzej Hajda
@ 2017-01-03 8:25 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-01-03 8:25 UTC (permalink / raw)
To: Andrzej Hajda, linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 01/03/2017 08:55 AM, Andrzej Hajda wrote:
> On 02.01.2017 15:19, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Implement the HPD notifier support to allow CEC drivers to
>> be informed when there is a new EDID and when a connect or
>> disconnect happens.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/gpu/drm/exynos/Kconfig | 1 +
>> drivers/gpu/drm/exynos/exynos_hdmi.c | 24 +++++++++++++++++++++---
>> 2 files changed, 22 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/exynos/Kconfig b/drivers/gpu/drm/exynos/Kconfig
>> index d706ca4..80bfd1d 100644
>> --- a/drivers/gpu/drm/exynos/Kconfig
>> +++ b/drivers/gpu/drm/exynos/Kconfig
>> @@ -77,6 +77,7 @@ config DRM_EXYNOS_DP
>> config DRM_EXYNOS_HDMI
>> bool "HDMI"
>> depends on DRM_EXYNOS_MIXER || DRM_EXYNOS5433_DECON
>> + select HPD_NOTIFIERS
>> help
>> Choose this option if you want to use Exynos HDMI for DRM.
>>
>> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> index 5ed8b1e..28bf609 100644
>> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
>> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
>> @@ -31,6 +31,7 @@
>> #include <linux/pm_runtime.h>
>> #include <linux/clk.h>
>> #include <linux/gpio/consumer.h>
>> +#include <linux/hpd-notifier.h>
>> #include <linux/regulator/consumer.h>
>> #include <linux/io.h>
>> #include <linux/of_address.h>
>> @@ -118,6 +119,7 @@ struct hdmi_context {
>> bool dvi_mode;
>> struct delayed_work hotplug_work;
>> struct drm_display_mode current_mode;
>> + struct hpd_notifier *notifier;
>> const struct hdmi_driver_data *drv_data;
>>
>> void __iomem *regs;
>> @@ -807,9 +809,12 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
>> {
>> struct hdmi_context *hdata = connector_to_hdmi(connector);
>>
>> - if (gpiod_get_value(hdata->hpd_gpio))
>> + if (gpiod_get_value(hdata->hpd_gpio)) {
>> + hpd_event_connect(hdata->notifier);
>> return connector_status_connected;
>> + }
>>
>> + hpd_event_disconnect(hdata->notifier);
>> return connector_status_disconnected;
>> }
>>
>> @@ -848,6 +853,9 @@ static int hdmi_get_modes(struct drm_connector *connector)
>> edid->width_cm, edid->height_cm);
>>
>> drm_mode_connector_update_edid_property(connector, edid);
>> + hpd_event_connect(hdata->notifier);
>
> Is there a reason to call hpd_event_connect here? It was called already
> from hdmi_detect.
True. Will drop this.
Hans
>
> Regards
> Andrzej
>
>> + hpd_event_new_edid(hdata->notifier, edid,
>> + EDID_LENGTH * (1 + edid->extensions));
>>
>> ret = drm_add_edid_modes(connector, edid);
>>
>> @@ -1483,6 +1491,7 @@ static void hdmi_disable(struct drm_encoder *encoder)
>> if (funcs && funcs->disable)
>> (*funcs->disable)(crtc);
>>
>> + hpd_event_disconnect(hdata->notifier);
>> cancel_delayed_work(&hdata->hotplug_work);
>>
>> hdmiphy_disable(hdata);
>> @@ -1832,15 +1841,22 @@ static int hdmi_probe(struct platform_device *pdev)
>> }
>> }
>>
>> + hdata->notifier = hpd_notifier_get(&pdev->dev);
>> + if (hdata->notifier == NULL) {
>> + ret = -ENOMEM;
>> + goto err_hdmiphy;
>> + }
>> +
>> pm_runtime_enable(dev);
>>
>> ret = component_add(&pdev->dev, &hdmi_component_ops);
>> if (ret)
>> - goto err_disable_pm_runtime;
>> + goto err_notifier_put;
>>
>> return ret;
>>
>> -err_disable_pm_runtime:
>> +err_notifier_put:
>> + hpd_notifier_put(hdata->notifier);
>> pm_runtime_disable(dev);
>>
>> err_hdmiphy:
>> @@ -1859,9 +1875,11 @@ static int hdmi_remove(struct platform_device *pdev)
>> struct hdmi_context *hdata = platform_get_drvdata(pdev);
>>
>> cancel_delayed_work_sync(&hdata->hotplug_work);
>> + hpd_event_disconnect(hdata->notifier);
>>
>> component_del(&pdev->dev, &hdmi_component_ops);
>>
>> + hpd_notifier_put(hdata->notifier);
>> pm_runtime_disable(&pdev->dev);
>>
>> if (!IS_ERR(hdata->reg_hdmi_en))
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* [PATCHv2 3/4] cec: integrate HPD notifier support
2017-01-02 14:19 [PATCHv2 0/4] video/exynos/cec: add HPD state notifier & use in s5p-cec Hans Verkuil
2017-01-02 14:19 ` [PATCHv2 1/4] video: add hotplug detect notifier support Hans Verkuil
2017-01-02 14:19 ` [PATCHv2 2/4] exynos_hdmi: add HPD " Hans Verkuil
@ 2017-01-02 14:19 ` Hans Verkuil
2017-01-02 14:19 ` [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging Hans Verkuil
3 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-01-02 14:19 UTC (permalink / raw)
To: linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
Support the HPD notifier framework, simplifying drivers that
depend on this.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
drivers/media/cec/cec-core.c | 50 ++++++++++++++++++++++++++++++++++++++++++++
include/media/cec.h | 15 +++++++++++++
2 files changed, 65 insertions(+)
diff --git a/drivers/media/cec/cec-core.c b/drivers/media/cec/cec-core.c
index aca3ab8..a466dbe 100644
--- a/drivers/media/cec/cec-core.c
+++ b/drivers/media/cec/cec-core.c
@@ -195,6 +195,52 @@ static void cec_devnode_unregister(struct cec_devnode *devnode)
put_device(&devnode->dev);
}
+#ifdef CONFIG_HPD_NOTIFIERS
+static u16 parse_hdmi_addr(const struct edid *edid)
+{
+ if (!edid || edid->extensions == 0)
+ return CEC_PHYS_ADDR_INVALID;
+
+ return cec_get_edid_phys_addr((u8 *)edid,
+ EDID_LENGTH * (edid->extensions + 1), NULL);
+}
+
+static int cec_hpd_notify(struct notifier_block *nb, unsigned long event,
+ void *data)
+{
+ struct cec_adapter *adap = container_of(nb, struct cec_adapter, nb);
+ struct hpd_notifier *n = data;
+ unsigned int phys;
+
+ dprintk(1, "event %lu\n", event);
+
+ switch (event) {
+ case HPD_DISCONNECTED:
+ cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
+ break;
+
+ case HPD_NEW_EDID:
+ phys = parse_hdmi_addr(n->edid);
+ cec_s_phys_addr(adap, phys, false);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+void cec_register_hpd_notifier(struct cec_adapter *adap,
+ struct hpd_notifier *notifier)
+{
+ if (WARN_ON(!adap->devnode.registered))
+ return;
+
+ adap->nb.notifier_call = cec_hpd_notify;
+ adap->notifier = notifier;
+ hpd_notifier_register(adap->notifier, &adap->nb);
+}
+EXPORT_SYMBOL_GPL(cec_register_hpd_notifier);
+#endif
+
struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
void *priv, const char *name, u32 caps,
u8 available_las)
@@ -344,6 +390,10 @@ void cec_unregister_adapter(struct cec_adapter *adap)
adap->rc = NULL;
#endif
debugfs_remove_recursive(adap->cec_dir);
+#ifdef CONFIG_HPD_NOTIFIERS
+ if (adap->notifier)
+ hpd_notifier_unregister(adap->notifier, &adap->nb);
+#endif
cec_devnode_unregister(&adap->devnode);
}
EXPORT_SYMBOL_GPL(cec_unregister_adapter);
diff --git a/include/media/cec.h b/include/media/cec.h
index 96a0aa7..d6c7b20 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -28,6 +28,11 @@
#include <linux/kthread.h>
#include <linux/timer.h>
#include <linux/cec-funcs.h>
+#ifdef CONFIG_HPD_NOTIFIERS
+#include <linux/notifier.h>
+#include <linux/hpd-notifier.h>
+#include <drm/drm_edid.h>
+#endif
#include <media/rc-core.h>
#include <media/cec-edid.h>
@@ -173,6 +178,11 @@ struct cec_adapter {
bool passthrough;
struct cec_log_addrs log_addrs;
+#ifdef CONFIG_HPD_NOTIFIERS
+ struct hpd_notifier *notifier;
+ struct notifier_block nb;
+#endif
+
struct dentry *cec_dir;
struct dentry *status_file;
@@ -213,6 +223,11 @@ void cec_transmit_done(struct cec_adapter *adap, u8 status, u8 arb_lost_cnt,
u8 nack_cnt, u8 low_drive_cnt, u8 error_cnt);
void cec_received_msg(struct cec_adapter *adap, struct cec_msg *msg);
+#ifdef CONFIG_HPD_NOTIFIERS
+void cec_register_hpd_notifier(struct cec_adapter *adap,
+ struct hpd_notifier *notifier);
+#endif
+
#else
static inline int cec_register_adapter(struct cec_adapter *adap,
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging
2017-01-02 14:19 [PATCHv2 0/4] video/exynos/cec: add HPD state notifier & use in s5p-cec Hans Verkuil
` (2 preceding siblings ...)
2017-01-02 14:19 ` [PATCHv2 3/4] cec: integrate " Hans Verkuil
@ 2017-01-02 14:19 ` Hans Verkuil
2017-01-02 17:50 ` Krzysztof Kozlowski
2017-01-03 8:00 ` Andrzej Hajda
3 siblings, 2 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-01-02 14:19 UTC (permalink / raw)
To: linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
From: Hans Verkuil <hans.verkuil@cisco.com>
By using the HPD notifier framework there is no longer any reason
to manually set the physical address. This was the one blocking
issue that prevented this driver from going out of staging, so do
this move as well.
Update the bindings documenting the new hdmi phandle and
update exynos4.dtsi accordingly.
Tested with my Odroid U3.
Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
.../devicetree/bindings/media/s5p-cec.txt | 2 ++
arch/arm/boot/dts/exynos4.dtsi | 1 +
drivers/media/platform/Kconfig | 18 +++++++++++
drivers/media/platform/Makefile | 1 +
.../media => media/platform}/s5p-cec/Makefile | 0
.../platform}/s5p-cec/exynos_hdmi_cec.h | 0
.../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0
.../media => media/platform}/s5p-cec/regs-cec.h | 0
.../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++++++++++++++----
.../media => media/platform}/s5p-cec/s5p_cec.h | 3 ++
drivers/staging/media/Kconfig | 2 --
drivers/staging/media/Makefile | 1 -
drivers/staging/media/s5p-cec/Kconfig | 9 ------
drivers/staging/media/s5p-cec/TODO | 7 -----
14 files changed, 55 insertions(+), 24 deletions(-)
rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
delete mode 100644 drivers/staging/media/s5p-cec/TODO
diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
index 925ab4d..710fc00 100644
--- a/Documentation/devicetree/bindings/media/s5p-cec.txt
+++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
@@ -15,6 +15,7 @@ Required properties:
- clock-names : from common clock binding: must contain "hdmicec",
corresponding to entry in the clocks property.
- samsung,syscon-phandle - phandle to the PMU system controller
+ - samsung,hdmi-phandle - phandle to the HDMI controller
Example:
@@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
clocks = <&clock CLK_HDMI_CEC>;
clock-names = "hdmicec";
samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,hdmi-phandle = <&hdmi>;
pinctrl-names = "default";
pinctrl-0 = <&hdmi_cec>;
status = "okay";
diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
index c64737b..51dfcbb 100644
--- a/arch/arm/boot/dts/exynos4.dtsi
+++ b/arch/arm/boot/dts/exynos4.dtsi
@@ -762,6 +762,7 @@
clocks = <&clock CLK_HDMI_CEC>;
clock-names = "hdmicec";
samsung,syscon-phandle = <&pmu_system_controller>;
+ samsung,hdmi-phandle = <&hdmi>;
pinctrl-names = "default";
pinctrl-0 = <&hdmi_cec>;
status = "disabled";
diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
index d944421..cab1637 100644
--- a/drivers/media/platform/Kconfig
+++ b/drivers/media/platform/Kconfig
@@ -392,6 +392,24 @@ config VIDEO_TI_SC
config VIDEO_TI_CSC
tristate
+menuconfig V4L_CEC_DRIVERS
+ bool "Platform HDMI CEC drivers"
+ depends on MEDIA_CEC_SUPPORT
+
+if V4L_CEC_DRIVERS
+
+config VIDEO_SAMSUNG_S5P_CEC
+ tristate "Samsung S5P CEC driver"
+ depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
+ select HPD_NOTIFIERS
+ ---help---
+ This is a driver for Samsung S5P HDMI CEC interface. It uses the
+ generic CEC framework interface.
+ CEC bus is present in the HDMI connector and enables communication
+ between compatible devices.
+
+endif #V4L_CEC_DRIVERS
+
menuconfig V4L_TEST_DRIVERS
bool "Media test drivers"
depends on MEDIA_CAMERA_SUPPORT
diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
index 5b3cb27..ad3bf22 100644
--- a/drivers/media/platform/Makefile
+++ b/drivers/media/platform/Makefile
@@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/
obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D) += s5p-g2d/
+obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/
diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
similarity index 100%
rename from drivers/staging/media/s5p-cec/Makefile
rename to drivers/media/platform/s5p-cec/Makefile
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
similarity index 100%
rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
similarity index 100%
rename from drivers/staging/media/s5p-cec/regs-cec.h
rename to drivers/media/platform/s5p-cec/regs-cec.h
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
similarity index 89%
rename from drivers/staging/media/s5p-cec/s5p_cec.c
rename to drivers/media/platform/s5p-cec/s5p_cec.c
index 2a07968..2014f79 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.c
+++ b/drivers/media/platform/s5p-cec/s5p_cec.c
@@ -14,15 +14,18 @@
*/
#include <linux/clk.h>
+#include <linux/hpd-notifier.h>
#include <linux/interrupt.h>
#include <linux/kernel.h>
#include <linux/mfd/syscon.h>
#include <linux/module.h>
#include <linux/of.h>
+#include <linux/of_platform.h>
#include <linux/platform_device.h>
#include <linux/pm_runtime.h>
#include <linux/timer.h>
#include <linux/workqueue.h>
+#include <media/cec-edid.h>
#include <media/cec.h>
#include "exynos_hdmi_cec.h"
@@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
static int s5p_cec_probe(struct platform_device *pdev)
{
struct device *dev = &pdev->dev;
+ struct device_node *np;
+ struct platform_device *hdmi_dev;
struct resource *res;
struct s5p_cec_dev *cec;
int ret;
+ np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
+
+ if (!np) {
+ dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
+ return -ENODEV;
+ }
+ hdmi_dev = of_find_device_by_node(np);
+ if (hdmi_dev == NULL)
+ return -EPROBE_DEFER;
+
cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
if (!cec)
return -ENOMEM;
@@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
if (IS_ERR(cec->reg))
return PTR_ERR(cec->reg);
+ cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
+ if (cec->notifier == NULL)
+ return -ENOMEM;
+
cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
CEC_NAME,
- CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
+ CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
ret = PTR_ERR_OR_ZERO(cec->adap);
if (ret)
return ret;
+
ret = cec_register_adapter(cec->adap, &pdev->dev);
- if (ret) {
- cec_delete_adapter(cec->adap);
- return ret;
- }
+ if (ret)
+ goto err_delete_adapter;
+
+ cec_register_hpd_notifier(cec->adap, cec->notifier);
platform_set_drvdata(pdev, cec);
pm_runtime_enable(dev);
dev_dbg(dev, "successfuly probed\n");
return 0;
+
+err_delete_adapter:
+ cec_delete_adapter(cec->adap);
+ return ret;
}
static int s5p_cec_remove(struct platform_device *pdev)
@@ -225,6 +249,7 @@ static int s5p_cec_remove(struct platform_device *pdev)
struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
cec_unregister_adapter(cec->adap);
+ hpd_notifier_put(cec->notifier);
pm_runtime_disable(&pdev->dev);
return 0;
}
diff --git a/drivers/staging/media/s5p-cec/s5p_cec.h b/drivers/media/platform/s5p-cec/s5p_cec.h
similarity index 97%
rename from drivers/staging/media/s5p-cec/s5p_cec.h
rename to drivers/media/platform/s5p-cec/s5p_cec.h
index 03732c1..a6f5af6 100644
--- a/drivers/staging/media/s5p-cec/s5p_cec.h
+++ b/drivers/media/platform/s5p-cec/s5p_cec.h
@@ -59,12 +59,15 @@ enum cec_state {
STATE_ERROR
};
+struct hpd_notifier;
+
struct s5p_cec_dev {
struct cec_adapter *adap;
struct clk *clk;
struct device *dev;
struct mutex lock;
struct regmap *pmu;
+ struct hpd_notifier *notifier;
int irq;
void __iomem *reg;
diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
index ffb8fa7..1b7804c 100644
--- a/drivers/staging/media/Kconfig
+++ b/drivers/staging/media/Kconfig
@@ -27,8 +27,6 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
source "drivers/staging/media/omap4iss/Kconfig"
-source "drivers/staging/media/s5p-cec/Kconfig"
-
# Keep LIRC at the end, as it has sub-menus
source "drivers/staging/media/lirc/Kconfig"
diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
index a28e82c..e11afbf 100644
--- a/drivers/staging/media/Makefile
+++ b/drivers/staging/media/Makefile
@@ -1,5 +1,4 @@
obj-$(CONFIG_I2C_BCM2048) += bcm2048/
-obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
obj-$(CONFIG_DVB_CXD2099) += cxd2099/
obj-$(CONFIG_LIRC_STAGING) += lirc/
obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/
diff --git a/drivers/staging/media/s5p-cec/Kconfig b/drivers/staging/media/s5p-cec/Kconfig
deleted file mode 100644
index ddfd955..0000000
--- a/drivers/staging/media/s5p-cec/Kconfig
+++ /dev/null
@@ -1,9 +0,0 @@
-config VIDEO_SAMSUNG_S5P_CEC
- tristate "Samsung S5P CEC driver"
- depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
- ---help---
- This is a driver for Samsung S5P HDMI CEC interface. It uses the
- generic CEC framework interface.
- CEC bus is present in the HDMI connector and enables communication
- between compatible devices.
-
diff --git a/drivers/staging/media/s5p-cec/TODO b/drivers/staging/media/s5p-cec/TODO
deleted file mode 100644
index 64f21ba..0000000
--- a/drivers/staging/media/s5p-cec/TODO
+++ /dev/null
@@ -1,7 +0,0 @@
-This driver requires that userspace sets the physical address.
-However, this should be passed on from the corresponding
-Samsung HDMI driver.
-
-We have to wait until the HDMI notifier framework has been merged
-in order to handle this gracefully, until that time this driver
-has to remain in staging.
--
2.8.1
^ permalink raw reply related [flat|nested] 17+ messages in thread* Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging
2017-01-02 14:19 ` [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging Hans Verkuil
@ 2017-01-02 17:50 ` Krzysztof Kozlowski
2017-01-03 8:00 ` Andrzej Hajda
1 sibling, 0 replies; 17+ messages in thread
From: Krzysztof Kozlowski @ 2017-01-02 17:50 UTC (permalink / raw)
To: Hans Verkuil
Cc: linux-media, Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On Mon, Jan 02, 2017 at 03:19:07PM +0100, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> By using the HPD notifier framework there is no longer any reason
> to manually set the physical address. This was the one blocking
> issue that prevented this driver from going out of staging, so do
> this move as well.
>
> Update the bindings documenting the new hdmi phandle and
> update exynos4.dtsi accordingly.
>
> Tested with my Odroid U3.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> .../devicetree/bindings/media/s5p-cec.txt | 2 ++
> arch/arm/boot/dts/exynos4.dtsi | 1 +
> drivers/media/platform/Kconfig | 18 +++++++++++
> drivers/media/platform/Makefile | 1 +
> .../media => media/platform}/s5p-cec/Makefile | 0
> .../platform}/s5p-cec/exynos_hdmi_cec.h | 0
> .../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0
> .../media => media/platform}/s5p-cec/regs-cec.h | 0
> .../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++++++++++++++----
> .../media => media/platform}/s5p-cec/s5p_cec.h | 3 ++
> drivers/staging/media/Kconfig | 2 --
> drivers/staging/media/Makefile | 1 -
> drivers/staging/media/s5p-cec/Kconfig | 9 ------
> drivers/staging/media/s5p-cec/TODO | 7 -----
> 14 files changed, 55 insertions(+), 24 deletions(-)
> rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
> delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
> delete mode 100644 drivers/staging/media/s5p-cec/TODO
>
> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
> index 925ab4d..710fc00 100644
> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
> @@ -15,6 +15,7 @@ Required properties:
> - clock-names : from common clock binding: must contain "hdmicec",
> corresponding to entry in the clocks property.
> - samsung,syscon-phandle - phandle to the PMU system controller
> + - samsung,hdmi-phandle - phandle to the HDMI controller
>
> Example:
>
> @@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
> clocks = <&clock CLK_HDMI_CEC>;
> clock-names = "hdmicec";
> samsung,syscon-phandle = <&pmu_system_controller>;
> + samsung,hdmi-phandle = <&hdmi>;
> pinctrl-names = "default";
> pinctrl-0 = <&hdmi_cec>;
> status = "okay";
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index c64737b..51dfcbb 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -762,6 +762,7 @@
> clocks = <&clock CLK_HDMI_CEC>;
> clock-names = "hdmicec";
> samsung,syscon-phandle = <&pmu_system_controller>;
> + samsung,hdmi-phandle = <&hdmi>;
> pinctrl-names = "default";
> pinctrl-0 = <&hdmi_cec>;
> status = "disabled";
DTS change has to be a separate patch. It should also go through
arm-soc/samsung-soc tree.
If it is a dependency, then I could provide a tag with it.
Best regards,
Krzysztof
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index d944421..cab1637 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -392,6 +392,24 @@ config VIDEO_TI_SC
> config VIDEO_TI_CSC
> tristate
>
> +menuconfig V4L_CEC_DRIVERS
> + bool "Platform HDMI CEC drivers"
> + depends on MEDIA_CEC_SUPPORT
> +
> +if V4L_CEC_DRIVERS
> +
> +config VIDEO_SAMSUNG_S5P_CEC
> + tristate "Samsung S5P CEC driver"
> + depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
> + select HPD_NOTIFIERS
> + ---help---
> + This is a driver for Samsung S5P HDMI CEC interface. It uses the
> + generic CEC framework interface.
> + CEC bus is present in the HDMI connector and enables communication
> + between compatible devices.
> +
> +endif #V4L_CEC_DRIVERS
> +
> menuconfig V4L_TEST_DRIVERS
> bool "Media test drivers"
> depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 5b3cb27..ad3bf22 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/
>
> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D) += s5p-g2d/
> +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
> obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
>
> obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/
> diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/Makefile
> rename to drivers/media/platform/s5p-cec/Makefile
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/regs-cec.h
> rename to drivers/media/platform/s5p-cec/regs-cec.h
> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
> similarity index 89%
> rename from drivers/staging/media/s5p-cec/s5p_cec.c
> rename to drivers/media/platform/s5p-cec/s5p_cec.c
> index 2a07968..2014f79 100644
> --- a/drivers/staging/media/s5p-cec/s5p_cec.c
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
> @@ -14,15 +14,18 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/hpd-notifier.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/timer.h>
> #include <linux/workqueue.h>
> +#include <media/cec-edid.h>
> #include <media/cec.h>
>
> #include "exynos_hdmi_cec.h"
> @@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
> static int s5p_cec_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct device_node *np;
> + struct platform_device *hdmi_dev;
> struct resource *res;
> struct s5p_cec_dev *cec;
> int ret;
>
> + np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
> +
> + if (!np) {
> + dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
> + return -ENODEV;
> + }
> + hdmi_dev = of_find_device_by_node(np);
> + if (hdmi_dev == NULL)
> + return -EPROBE_DEFER;
> +
> cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
> if (!cec)
> return -ENOMEM;
> @@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
> if (IS_ERR(cec->reg))
> return PTR_ERR(cec->reg);
>
> + cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
> + if (cec->notifier == NULL)
> + return -ENOMEM;
> +
> cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
> CEC_NAME,
> - CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> + CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
> ret = PTR_ERR_OR_ZERO(cec->adap);
> if (ret)
> return ret;
> +
> ret = cec_register_adapter(cec->adap, &pdev->dev);
> - if (ret) {
> - cec_delete_adapter(cec->adap);
> - return ret;
> - }
> + if (ret)
> + goto err_delete_adapter;
> +
> + cec_register_hpd_notifier(cec->adap, cec->notifier);
>
> platform_set_drvdata(pdev, cec);
> pm_runtime_enable(dev);
>
> dev_dbg(dev, "successfuly probed\n");
> return 0;
> +
> +err_delete_adapter:
> + cec_delete_adapter(cec->adap);
> + return ret;
> }
>
> static int s5p_cec_remove(struct platform_device *pdev)
> @@ -225,6 +249,7 @@ static int s5p_cec_remove(struct platform_device *pdev)
> struct s5p_cec_dev *cec = platform_get_drvdata(pdev);
>
> cec_unregister_adapter(cec->adap);
> + hpd_notifier_put(cec->notifier);
> pm_runtime_disable(&pdev->dev);
> return 0;
> }
> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.h b/drivers/media/platform/s5p-cec/s5p_cec.h
> similarity index 97%
> rename from drivers/staging/media/s5p-cec/s5p_cec.h
> rename to drivers/media/platform/s5p-cec/s5p_cec.h
> index 03732c1..a6f5af6 100644
> --- a/drivers/staging/media/s5p-cec/s5p_cec.h
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.h
> @@ -59,12 +59,15 @@ enum cec_state {
> STATE_ERROR
> };
>
> +struct hpd_notifier;
> +
> struct s5p_cec_dev {
> struct cec_adapter *adap;
> struct clk *clk;
> struct device *dev;
> struct mutex lock;
> struct regmap *pmu;
> + struct hpd_notifier *notifier;
> int irq;
> void __iomem *reg;
>
> diff --git a/drivers/staging/media/Kconfig b/drivers/staging/media/Kconfig
> index ffb8fa7..1b7804c 100644
> --- a/drivers/staging/media/Kconfig
> +++ b/drivers/staging/media/Kconfig
> @@ -27,8 +27,6 @@ source "drivers/staging/media/davinci_vpfe/Kconfig"
>
> source "drivers/staging/media/omap4iss/Kconfig"
>
> -source "drivers/staging/media/s5p-cec/Kconfig"
> -
> # Keep LIRC at the end, as it has sub-menus
> source "drivers/staging/media/lirc/Kconfig"
>
> diff --git a/drivers/staging/media/Makefile b/drivers/staging/media/Makefile
> index a28e82c..e11afbf 100644
> --- a/drivers/staging/media/Makefile
> +++ b/drivers/staging/media/Makefile
> @@ -1,5 +1,4 @@
> obj-$(CONFIG_I2C_BCM2048) += bcm2048/
> -obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
> obj-$(CONFIG_DVB_CXD2099) += cxd2099/
> obj-$(CONFIG_LIRC_STAGING) += lirc/
> obj-$(CONFIG_VIDEO_DM365_VPFE) += davinci_vpfe/
> diff --git a/drivers/staging/media/s5p-cec/Kconfig b/drivers/staging/media/s5p-cec/Kconfig
> deleted file mode 100644
> index ddfd955..0000000
> --- a/drivers/staging/media/s5p-cec/Kconfig
> +++ /dev/null
> @@ -1,9 +0,0 @@
> -config VIDEO_SAMSUNG_S5P_CEC
> - tristate "Samsung S5P CEC driver"
> - depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
> - ---help---
> - This is a driver for Samsung S5P HDMI CEC interface. It uses the
> - generic CEC framework interface.
> - CEC bus is present in the HDMI connector and enables communication
> - between compatible devices.
> -
> diff --git a/drivers/staging/media/s5p-cec/TODO b/drivers/staging/media/s5p-cec/TODO
> deleted file mode 100644
> index 64f21ba..0000000
> --- a/drivers/staging/media/s5p-cec/TODO
> +++ /dev/null
> @@ -1,7 +0,0 @@
> -This driver requires that userspace sets the physical address.
> -However, this should be passed on from the corresponding
> -Samsung HDMI driver.
> -
> -We have to wait until the HDMI notifier framework has been merged
> -in order to handle this gracefully, until that time this driver
> -has to remain in staging.
> --
> 2.8.1
>
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging
2017-01-02 14:19 ` [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging Hans Verkuil
2017-01-02 17:50 ` Krzysztof Kozlowski
@ 2017-01-03 8:00 ` Andrzej Hajda
2017-01-03 8:11 ` Hans Verkuil
1 sibling, 1 reply; 17+ messages in thread
From: Andrzej Hajda @ 2017-01-03 8:00 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 02.01.2017 15:19, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
>
> By using the HPD notifier framework there is no longer any reason
> to manually set the physical address. This was the one blocking
> issue that prevented this driver from going out of staging, so do
> this move as well.
>
> Update the bindings documenting the new hdmi phandle and
> update exynos4.dtsi accordingly.
>
> Tested with my Odroid U3.
>
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
> .../devicetree/bindings/media/s5p-cec.txt | 2 ++
> arch/arm/boot/dts/exynos4.dtsi | 1 +
> drivers/media/platform/Kconfig | 18 +++++++++++
> drivers/media/platform/Makefile | 1 +
> .../media => media/platform}/s5p-cec/Makefile | 0
> .../platform}/s5p-cec/exynos_hdmi_cec.h | 0
> .../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0
> .../media => media/platform}/s5p-cec/regs-cec.h | 0
> .../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++++++++++++++----
> .../media => media/platform}/s5p-cec/s5p_cec.h | 3 ++
> drivers/staging/media/Kconfig | 2 --
> drivers/staging/media/Makefile | 1 -
> drivers/staging/media/s5p-cec/Kconfig | 9 ------
> drivers/staging/media/s5p-cec/TODO | 7 -----
> 14 files changed, 55 insertions(+), 24 deletions(-)
> rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
> delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
> delete mode 100644 drivers/staging/media/s5p-cec/TODO
>
> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
> index 925ab4d..710fc00 100644
> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
> @@ -15,6 +15,7 @@ Required properties:
> - clock-names : from common clock binding: must contain "hdmicec",
> corresponding to entry in the clocks property.
> - samsung,syscon-phandle - phandle to the PMU system controller
> + - samsung,hdmi-phandle - phandle to the HDMI controller
>
> Example:
>
> @@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
> clocks = <&clock CLK_HDMI_CEC>;
> clock-names = "hdmicec";
> samsung,syscon-phandle = <&pmu_system_controller>;
> + samsung,hdmi-phandle = <&hdmi>;
> pinctrl-names = "default";
> pinctrl-0 = <&hdmi_cec>;
> status = "okay";
> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
> index c64737b..51dfcbb 100644
> --- a/arch/arm/boot/dts/exynos4.dtsi
> +++ b/arch/arm/boot/dts/exynos4.dtsi
> @@ -762,6 +762,7 @@
> clocks = <&clock CLK_HDMI_CEC>;
> clock-names = "hdmicec";
> samsung,syscon-phandle = <&pmu_system_controller>;
> + samsung,hdmi-phandle = <&hdmi>;
> pinctrl-names = "default";
> pinctrl-0 = <&hdmi_cec>;
> status = "disabled";
> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> index d944421..cab1637 100644
> --- a/drivers/media/platform/Kconfig
> +++ b/drivers/media/platform/Kconfig
> @@ -392,6 +392,24 @@ config VIDEO_TI_SC
> config VIDEO_TI_CSC
> tristate
>
> +menuconfig V4L_CEC_DRIVERS
> + bool "Platform HDMI CEC drivers"
> + depends on MEDIA_CEC_SUPPORT
> +
> +if V4L_CEC_DRIVERS
> +
> +config VIDEO_SAMSUNG_S5P_CEC
> + tristate "Samsung S5P CEC driver"
> + depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
> + select HPD_NOTIFIERS
> + ---help---
> + This is a driver for Samsung S5P HDMI CEC interface. It uses the
> + generic CEC framework interface.
> + CEC bus is present in the HDMI connector and enables communication
> + between compatible devices.
> +
> +endif #V4L_CEC_DRIVERS
> +
> menuconfig V4L_TEST_DRIVERS
> bool "Media test drivers"
> depends on MEDIA_CAMERA_SUPPORT
> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> index 5b3cb27..ad3bf22 100644
> --- a/drivers/media/platform/Makefile
> +++ b/drivers/media/platform/Makefile
> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/
>
> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D) += s5p-g2d/
> +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
> obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
>
> obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/
> diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/Makefile
> rename to drivers/media/platform/s5p-cec/Makefile
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
> diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
> similarity index 100%
> rename from drivers/staging/media/s5p-cec/regs-cec.h
> rename to drivers/media/platform/s5p-cec/regs-cec.h
> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
> similarity index 89%
> rename from drivers/staging/media/s5p-cec/s5p_cec.c
> rename to drivers/media/platform/s5p-cec/s5p_cec.c
> index 2a07968..2014f79 100644
> --- a/drivers/staging/media/s5p-cec/s5p_cec.c
> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
> @@ -14,15 +14,18 @@
> */
>
> #include <linux/clk.h>
> +#include <linux/hpd-notifier.h>
> #include <linux/interrupt.h>
> #include <linux/kernel.h>
> #include <linux/mfd/syscon.h>
> #include <linux/module.h>
> #include <linux/of.h>
> +#include <linux/of_platform.h>
> #include <linux/platform_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/timer.h>
> #include <linux/workqueue.h>
> +#include <media/cec-edid.h>
> #include <media/cec.h>
>
> #include "exynos_hdmi_cec.h"
> @@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
> static int s5p_cec_probe(struct platform_device *pdev)
> {
> struct device *dev = &pdev->dev;
> + struct device_node *np;
> + struct platform_device *hdmi_dev;
> struct resource *res;
> struct s5p_cec_dev *cec;
> int ret;
>
> + np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
> +
> + if (!np) {
> + dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
> + return -ENODEV;
> + }
> + hdmi_dev = of_find_device_by_node(np);
> + if (hdmi_dev == NULL)
> + return -EPROBE_DEFER;
> +
> cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
> if (!cec)
> return -ENOMEM;
> @@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
> if (IS_ERR(cec->reg))
> return PTR_ERR(cec->reg);
>
> + cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
> + if (cec->notifier == NULL)
> + return -ENOMEM;
> +
> cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
> CEC_NAME,
> - CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> + CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
> CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
> ret = PTR_ERR_OR_ZERO(cec->adap);
> if (ret)
> return ret;
> +
> ret = cec_register_adapter(cec->adap, &pdev->dev);
> - if (ret) {
> - cec_delete_adapter(cec->adap);
> - return ret;
> - }
> + if (ret)
> + goto err_delete_adapter;
> +
> + cec_register_hpd_notifier(cec->adap, cec->notifier);
Is there a reason to split registration into two steps?
Wouldn't be better to integrate hpd_notifier_get into
cec_register_hpd_notifier.
Regards
Andrzej
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging
2017-01-03 8:00 ` Andrzej Hajda
@ 2017-01-03 8:11 ` Hans Verkuil
2017-01-04 8:44 ` Andrzej Hajda
0 siblings, 1 reply; 17+ messages in thread
From: Hans Verkuil @ 2017-01-03 8:11 UTC (permalink / raw)
To: Andrzej Hajda, linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
> On 02.01.2017 15:19, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> By using the HPD notifier framework there is no longer any reason
>> to manually set the physical address. This was the one blocking
>> issue that prevented this driver from going out of staging, so do
>> this move as well.
>>
>> Update the bindings documenting the new hdmi phandle and
>> update exynos4.dtsi accordingly.
>>
>> Tested with my Odroid U3.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> .../devicetree/bindings/media/s5p-cec.txt | 2 ++
>> arch/arm/boot/dts/exynos4.dtsi | 1 +
>> drivers/media/platform/Kconfig | 18 +++++++++++
>> drivers/media/platform/Makefile | 1 +
>> .../media => media/platform}/s5p-cec/Makefile | 0
>> .../platform}/s5p-cec/exynos_hdmi_cec.h | 0
>> .../platform}/s5p-cec/exynos_hdmi_cecctrl.c | 0
>> .../media => media/platform}/s5p-cec/regs-cec.h | 0
>> .../media => media/platform}/s5p-cec/s5p_cec.c | 35 ++++++++++++++++++----
>> .../media => media/platform}/s5p-cec/s5p_cec.h | 3 ++
>> drivers/staging/media/Kconfig | 2 --
>> drivers/staging/media/Makefile | 1 -
>> drivers/staging/media/s5p-cec/Kconfig | 9 ------
>> drivers/staging/media/s5p-cec/TODO | 7 -----
>> 14 files changed, 55 insertions(+), 24 deletions(-)
>> rename drivers/{staging/media => media/platform}/s5p-cec/Makefile (100%)
>> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cec.h (100%)
>> rename drivers/{staging/media => media/platform}/s5p-cec/exynos_hdmi_cecctrl.c (100%)
>> rename drivers/{staging/media => media/platform}/s5p-cec/regs-cec.h (100%)
>> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.c (89%)
>> rename drivers/{staging/media => media/platform}/s5p-cec/s5p_cec.h (97%)
>> delete mode 100644 drivers/staging/media/s5p-cec/Kconfig
>> delete mode 100644 drivers/staging/media/s5p-cec/TODO
>>
>> diff --git a/Documentation/devicetree/bindings/media/s5p-cec.txt b/Documentation/devicetree/bindings/media/s5p-cec.txt
>> index 925ab4d..710fc00 100644
>> --- a/Documentation/devicetree/bindings/media/s5p-cec.txt
>> +++ b/Documentation/devicetree/bindings/media/s5p-cec.txt
>> @@ -15,6 +15,7 @@ Required properties:
>> - clock-names : from common clock binding: must contain "hdmicec",
>> corresponding to entry in the clocks property.
>> - samsung,syscon-phandle - phandle to the PMU system controller
>> + - samsung,hdmi-phandle - phandle to the HDMI controller
>>
>> Example:
>>
>> @@ -25,6 +26,7 @@ hdmicec: cec@100B0000 {
>> clocks = <&clock CLK_HDMI_CEC>;
>> clock-names = "hdmicec";
>> samsung,syscon-phandle = <&pmu_system_controller>;
>> + samsung,hdmi-phandle = <&hdmi>;
>> pinctrl-names = "default";
>> pinctrl-0 = <&hdmi_cec>;
>> status = "okay";
>> diff --git a/arch/arm/boot/dts/exynos4.dtsi b/arch/arm/boot/dts/exynos4.dtsi
>> index c64737b..51dfcbb 100644
>> --- a/arch/arm/boot/dts/exynos4.dtsi
>> +++ b/arch/arm/boot/dts/exynos4.dtsi
>> @@ -762,6 +762,7 @@
>> clocks = <&clock CLK_HDMI_CEC>;
>> clock-names = "hdmicec";
>> samsung,syscon-phandle = <&pmu_system_controller>;
>> + samsung,hdmi-phandle = <&hdmi>;
>> pinctrl-names = "default";
>> pinctrl-0 = <&hdmi_cec>;
>> status = "disabled";
>> diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
>> index d944421..cab1637 100644
>> --- a/drivers/media/platform/Kconfig
>> +++ b/drivers/media/platform/Kconfig
>> @@ -392,6 +392,24 @@ config VIDEO_TI_SC
>> config VIDEO_TI_CSC
>> tristate
>>
>> +menuconfig V4L_CEC_DRIVERS
>> + bool "Platform HDMI CEC drivers"
>> + depends on MEDIA_CEC_SUPPORT
>> +
>> +if V4L_CEC_DRIVERS
>> +
>> +config VIDEO_SAMSUNG_S5P_CEC
>> + tristate "Samsung S5P CEC driver"
>> + depends on VIDEO_DEV && MEDIA_CEC_SUPPORT && (PLAT_S5P || ARCH_EXYNOS || COMPILE_TEST)
>> + select HPD_NOTIFIERS
>> + ---help---
>> + This is a driver for Samsung S5P HDMI CEC interface. It uses the
>> + generic CEC framework interface.
>> + CEC bus is present in the HDMI connector and enables communication
>> + between compatible devices.
>> +
>> +endif #V4L_CEC_DRIVERS
>> +
>> menuconfig V4L_TEST_DRIVERS
>> bool "Media test drivers"
>> depends on MEDIA_CAMERA_SUPPORT
>> diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
>> index 5b3cb27..ad3bf22 100644
>> --- a/drivers/media/platform/Makefile
>> +++ b/drivers/media/platform/Makefile
>> @@ -33,6 +33,7 @@ obj-$(CONFIG_VIDEO_SAMSUNG_S5P_JPEG) += s5p-jpeg/
>> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_MFC) += s5p-mfc/
>>
>> obj-$(CONFIG_VIDEO_SAMSUNG_S5P_G2D) += s5p-g2d/
>> +obj-$(CONFIG_VIDEO_SAMSUNG_S5P_CEC) += s5p-cec/
>> obj-$(CONFIG_VIDEO_SAMSUNG_EXYNOS_GSC) += exynos-gsc/
>>
>> obj-$(CONFIG_VIDEO_STI_BDISP) += sti/bdisp/
>> diff --git a/drivers/staging/media/s5p-cec/Makefile b/drivers/media/platform/s5p-cec/Makefile
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/Makefile
>> rename to drivers/media/platform/s5p-cec/Makefile
>> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cec.h b/drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cec.h
>> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cec.h
>> diff --git a/drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c b/drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/exynos_hdmi_cecctrl.c
>> rename to drivers/media/platform/s5p-cec/exynos_hdmi_cecctrl.c
>> diff --git a/drivers/staging/media/s5p-cec/regs-cec.h b/drivers/media/platform/s5p-cec/regs-cec.h
>> similarity index 100%
>> rename from drivers/staging/media/s5p-cec/regs-cec.h
>> rename to drivers/media/platform/s5p-cec/regs-cec.h
>> diff --git a/drivers/staging/media/s5p-cec/s5p_cec.c b/drivers/media/platform/s5p-cec/s5p_cec.c
>> similarity index 89%
>> rename from drivers/staging/media/s5p-cec/s5p_cec.c
>> rename to drivers/media/platform/s5p-cec/s5p_cec.c
>> index 2a07968..2014f79 100644
>> --- a/drivers/staging/media/s5p-cec/s5p_cec.c
>> +++ b/drivers/media/platform/s5p-cec/s5p_cec.c
>> @@ -14,15 +14,18 @@
>> */
>>
>> #include <linux/clk.h>
>> +#include <linux/hpd-notifier.h>
>> #include <linux/interrupt.h>
>> #include <linux/kernel.h>
>> #include <linux/mfd/syscon.h>
>> #include <linux/module.h>
>> #include <linux/of.h>
>> +#include <linux/of_platform.h>
>> #include <linux/platform_device.h>
>> #include <linux/pm_runtime.h>
>> #include <linux/timer.h>
>> #include <linux/workqueue.h>
>> +#include <media/cec-edid.h>
>> #include <media/cec.h>
>>
>> #include "exynos_hdmi_cec.h"
>> @@ -167,10 +170,22 @@ static const struct cec_adap_ops s5p_cec_adap_ops = {
>> static int s5p_cec_probe(struct platform_device *pdev)
>> {
>> struct device *dev = &pdev->dev;
>> + struct device_node *np;
>> + struct platform_device *hdmi_dev;
>> struct resource *res;
>> struct s5p_cec_dev *cec;
>> int ret;
>>
>> + np = of_parse_phandle(pdev->dev.of_node, "samsung,hdmi-phandle", 0);
>> +
>> + if (!np) {
>> + dev_err(&pdev->dev, "Failed to find hdmi node in device tree\n");
>> + return -ENODEV;
>> + }
>> + hdmi_dev = of_find_device_by_node(np);
>> + if (hdmi_dev == NULL)
>> + return -EPROBE_DEFER;
>> +
>> cec = devm_kzalloc(&pdev->dev, sizeof(*cec), GFP_KERNEL);
>> if (!cec)
>> return -ENOMEM;
>> @@ -200,24 +215,33 @@ static int s5p_cec_probe(struct platform_device *pdev)
>> if (IS_ERR(cec->reg))
>> return PTR_ERR(cec->reg);
>>
>> + cec->notifier = hpd_notifier_get(&hdmi_dev->dev);
>> + if (cec->notifier == NULL)
>> + return -ENOMEM;
>> +
>> cec->adap = cec_allocate_adapter(&s5p_cec_adap_ops, cec,
>> CEC_NAME,
>> - CEC_CAP_PHYS_ADDR | CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
>> + CEC_CAP_LOG_ADDRS | CEC_CAP_TRANSMIT |
>> CEC_CAP_PASSTHROUGH | CEC_CAP_RC, 1);
>> ret = PTR_ERR_OR_ZERO(cec->adap);
>> if (ret)
>> return ret;
>> +
>> ret = cec_register_adapter(cec->adap, &pdev->dev);
>> - if (ret) {
>> - cec_delete_adapter(cec->adap);
>> - return ret;
>> - }
>> + if (ret)
>> + goto err_delete_adapter;
>> +
>> + cec_register_hpd_notifier(cec->adap, cec->notifier);
>
> Is there a reason to split registration into two steps?
> Wouldn't be better to integrate hpd_notifier_get into
> cec_register_hpd_notifier.
One problem is that hpd_notifier_get can fail, whereas cec_register_hpd_notifier can't.
And I rather not have to register a CEC device only to unregister it again if the
hpd_notifier_get would fail.
Another reason is that this keeps the responsibility of the hpd_notifier life-time
handling in the driver instead of hiding it in the CEC framework, which is IMHO
unexpected.
I think I want to keep this as-is, at least for now.
Regards,
Hans
^ permalink raw reply [flat|nested] 17+ messages in thread* Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging
2017-01-03 8:11 ` Hans Verkuil
@ 2017-01-04 8:44 ` Andrzej Hajda
2017-01-23 10:14 ` Hans Verkuil
0 siblings, 1 reply; 17+ messages in thread
From: Andrzej Hajda @ 2017-01-04 8:44 UTC (permalink / raw)
To: Hans Verkuil, linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 03.01.2017 09:11, Hans Verkuil wrote:
> On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
>> Is there a reason to split registration into two steps?
>> Wouldn't be better to integrate hpd_notifier_get into
>> cec_register_hpd_notifier.
> One problem is that hpd_notifier_get can fail, whereas cec_register_hpd_notifier can't.
> And I rather not have to register a CEC device only to unregister it again if the
> hpd_notifier_get would fail.
hpd_notifier_get can fail only due to lack of memory for about 150 bytes
so if it happens whole system will probably fail anyway :)
>
> Another reason is that this keeps the responsibility of the hpd_notifier life-time
> handling in the driver instead of hiding it in the CEC framework, which is IMHO
> unexpected.
Notifier is used only by CEC framework, so IMHO it would be desirable to
put CEC specific things into CEC framework.
Drivers duty is just to find notifier device.
Leaving it as is will just put little more burden on drivers, so this is
not big deal, do as you wish :)
Regards
Andrzej
>
> I think I want to keep this as-is, at least for now.
>
> Regards,
>
> Hans
>
>
>
>
^ permalink raw reply [flat|nested] 17+ messages in thread
* Re: [PATCHv2 4/4] s5p-cec: add hpd-notifier support, move out of staging
2017-01-04 8:44 ` Andrzej Hajda
@ 2017-01-23 10:14 ` Hans Verkuil
0 siblings, 0 replies; 17+ messages in thread
From: Hans Verkuil @ 2017-01-23 10:14 UTC (permalink / raw)
To: Andrzej Hajda, linux-media
Cc: Russell King, dri-devel, Marek Szyprowski,
Javier Martinez Canillas, Benjamin Gaignard, linux-samsung-soc,
Krzysztof Kozlowski, Inki Dae, Hans Verkuil
On 01/04/2017 09:44 AM, Andrzej Hajda wrote:
> On 03.01.2017 09:11, Hans Verkuil wrote:
>> On 01/03/2017 09:00 AM, Andrzej Hajda wrote:
>>> Is there a reason to split registration into two steps?
>>> Wouldn't be better to integrate hpd_notifier_get into
>>> cec_register_hpd_notifier.
>> One problem is that hpd_notifier_get can fail, whereas cec_register_hpd_notifier can't.
>> And I rather not have to register a CEC device only to unregister it again if the
>> hpd_notifier_get would fail.
>
> hpd_notifier_get can fail only due to lack of memory for about 150 bytes
> so if it happens whole system will probably fail anyway :)
>
>
>>
>> Another reason is that this keeps the responsibility of the hpd_notifier life-time
>> handling in the driver instead of hiding it in the CEC framework, which is IMHO
>> unexpected.
>
> Notifier is used only by CEC framework, so IMHO it would be desirable to
> put CEC specific things into CEC framework.
The CEC framework is just the first that needs it. But especially audio drivers also
want to use it. It was designed to help out both subsystems since both need the EDID/ELD.
Regards,
Hans
> Drivers duty is just to find notifier device.
> Leaving it as is will just put little more burden on drivers, so this is
> not big deal, do as you wish :)
>
> Regards
> Andrzej
>
>>
>> I think I want to keep this as-is, at least for now.
>>
>> Regards,
>>
>> Hans
>>
>>
>>
>>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 17+ messages in thread