[PATCH v8 0/5] ASoC: MT8173 HDMI jack detection

[PATCH v8 0/5] ASoC: MT8173 HDMI jack detection

[Philipp Zabel]

Hi,

now that kernel v4.8-rc1 is released and includes the new CEC framework in
drivers/staging/media/cec, I wonder how to proceed with this. To allow the
ASoC drivers to present HDMI cable connection state as an ALSA jack, and
to allow them to update sink capabilities when it signals an EDID change,
we need a notification mechanism between CEC/HDMI, and ALSA drivers.

I had used Russell's hdmi notification prototype to let the CEC/HDMI
drivers notify the hdmi-codec and ASoC machine drivers of changes. The
mtk_cec driver currently is just a placeholder that only handles the HPD
interrupts.

Should the notifications be integrated with the CEC framework for those
devices that do have CEC support?

Changes since v7 ("ASoC: MT8173 HDMI codec support"):
 - Dropped the already applied ELD control and machine driver patches
 - Dropped the N/CTS helper patch for now, as the helpers don't exist yet.
 - Lock ELD mutex while copying ELD in hdmi_eld_ctl_get

regards
Philipp

Philipp Zabel (5):
  video: rmk's HDMI notification prototype
  ASoC: hdmi-codec: Use HDMI notifications to add jack support
  ASoC: mediatek: Add jack detection support to mt8173-rt5650-rt5676
    machine driver
  ASoC: mediatek: Add jack detection support to the mt8173-rt5650
    machine driver
  drm/mediatek: hdmi: issue notifications

 drivers/gpu/drm/mediatek/mtk_cec.c               | 11 +++
 drivers/gpu/drm/mediatek/mtk_hdmi.c              |  3 +
 drivers/video/Kconfig                            |  3 +
 drivers/video/Makefile                           |  1 +
 drivers/video/hdmi-notifier.c                    | 61 ++++++++++++++++
 include/linux/hdmi-notifier.h                    | 44 ++++++++++++
 include/sound/hdmi-codec.h                       |  6 ++
 sound/soc/codecs/Kconfig                         |  1 +
 sound/soc/codecs/hdmi-codec.c                    | 88 +++++++++++++++++++++---
 sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c | 21 ++++++
 sound/soc/mediatek/mt8173/mt8173-rt5650.c        | 21 ++++++
 11 files changed, 252 insertions(+), 8 deletions(-)
 create mode 100644 drivers/video/hdmi-notifier.c
 create mode 100644 include/linux/hdmi-notifier.h

-- 
2.8.1

[PATCH v8 1/5] video: rmk's HDMI notification prototype

[Philipp Zabel]

This is Russell's HDMI notification prototype [1], currently waiting
for the HDMI CEC situation to resolve.

The use case for the notifications on MediaTek MT8173 is to let the
(dis)connection notifications control an ALSA jack object.

No Signed-off-by since this is not my code, and still up for discussion.

[1] https://patchwork.kernel.org/patch/8351501/
---
 drivers/video/Kconfig         |  3 +++
 drivers/video/Makefile        |  1 +
 drivers/video/hdmi-notifier.c | 61 +++++++++++++++++++++++++++++++++++++++++++
 include/linux/hdmi-notifier.h | 44 +++++++++++++++++++++++++++++++
 4 files changed, 109 insertions(+)
 create mode 100644 drivers/video/hdmi-notifier.c
 create mode 100644 include/linux/hdmi-notifier.h

diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
index 3c20af9..1ee7b9f 100644
--- a/drivers/video/Kconfig
+++ b/drivers/video/Kconfig
@@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
 config HDMI
 	bool
 
+config HDMI_NOTIFIERS
+	bool
+
 if VT
 	source "drivers/video/console/Kconfig"
 endif
diff --git a/drivers/video/Makefile b/drivers/video/Makefile
index 9ad3c17..65f5649 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_HDMI_NOTIFIERS)      += hdmi-notifier.o
 
 obj-$(CONFIG_VT)		  += console/
 obj-$(CONFIG_LOGO)		  += logo/
diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
new file mode 100644
index 0000000..a233359
--- /dev/null
+++ b/drivers/video/hdmi-notifier.c
@@ -0,0 +1,61 @@
+#include <linux/export.h>
+#include <linux/hdmi-notifier.h>
+#include <linux/notifier.h>
+#include <linux/string.h>
+
+static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
+
+int hdmi_register_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_register(&hdmi_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(hdmi_register_notifier);
+
+int hdmi_unregister_notifier(struct notifier_block *nb)
+{
+	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
+}
+EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
+
+void hdmi_event_connect(struct device *dev)
+{
+	struct hdmi_event_base base;
+
+	base.source = dev;
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_connect);
+
+void hdmi_event_disconnect(struct device *dev)
+{
+	struct hdmi_event_base base;
+
+	base.source = dev;
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
+
+void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)
+{
+	struct hdmi_event_new_edid new_edid;
+
+	new_edid.base.source = dev;
+	new_edid.edid = edid;
+	new_edid.size = size;
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_EDID, &new_edid);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_new_edid);
+
+void hdmi_event_new_eld(struct device *dev, const void *eld)
+{
+	struct hdmi_event_new_eld new_eld;
+
+	new_eld.base.source = dev;
+	memcpy(new_eld.eld, eld, sizeof(new_eld.eld));
+
+	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_ELD, &new_eld);
+}
+EXPORT_SYMBOL_GPL(hdmi_event_new_eld);
diff --git a/include/linux/hdmi-notifier.h b/include/linux/hdmi-notifier.h
new file mode 100644
index 0000000..9c5ad49
--- /dev/null
+++ b/include/linux/hdmi-notifier.h
@@ -0,0 +1,44 @@
+#ifndef LINUX_HDMI_NOTIFIER_H
+#define LINUX_HDMI_NOTIFIER_H
+
+#include <linux/types.h>
+
+enum {
+	HDMI_CONNECTED,
+	HDMI_DISCONNECTED,
+	HDMI_NEW_EDID,
+	HDMI_NEW_ELD,
+};
+
+struct hdmi_event_base {
+	struct device *source;
+};
+
+struct hdmi_event_new_edid {
+	struct hdmi_event_base base;
+	const void *edid;
+	size_t size;
+};
+
+struct hdmi_event_new_eld {
+	struct hdmi_event_base base;
+	unsigned char eld[128];
+};
+
+union hdmi_event {
+	struct hdmi_event_base base;
+	struct hdmi_event_new_edid edid;
+	struct hdmi_event_new_eld eld;
+};
+
+struct notifier_block;
+
+int hdmi_register_notifier(struct notifier_block *nb);
+int hdmi_unregister_notifier(struct notifier_block *nb);
+
+void hdmi_event_connect(struct device *dev);
+void hdmi_event_disconnect(struct device *dev);
+void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size);
+void hdmi_event_new_eld(struct device *dev, const void *eld);
+
+#endif
-- 
2.8.1

[PATCH v8 2/5] ASoC: hdmi-codec: Use HDMI notifications to add jack support

[Philipp Zabel]

Use HDMI connection / disconnection notifications to update an ALSA
jack object. Also make a copy of the ELD block after every change.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
Changes since v7:
 - Lock ELD mutex while copying ELD in hdmi_eld_ctl_get
---
 include/sound/hdmi-codec.h    |  6 +++
 sound/soc/codecs/Kconfig      |  1 +
 sound/soc/codecs/hdmi-codec.c | 88 +++++++++++++++++++++++++++++++++++++++----
 3 files changed, 87 insertions(+), 8 deletions(-)

diff --git a/include/sound/hdmi-codec.h b/include/sound/hdmi-codec.h
index 530c57b..0fd2e38 100644
--- a/include/sound/hdmi-codec.h
+++ b/include/sound/hdmi-codec.h
@@ -98,6 +98,12 @@ struct hdmi_codec_pdata {
 	void *data;
 };
 
+struct snd_soc_codec;
+struct snd_soc_jack;
+
+int hdmi_codec_set_jack_detect(struct snd_soc_codec *codec,
+			       struct snd_soc_jack *jack);
+
 #define HDMI_CODEC_DRV_NAME "hdmi-audio-codec"
 
 #endif /* __HDMI_CODEC_H__ */
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index 1cd6ab3..f8c5e3e 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -508,6 +508,7 @@ config SND_SOC_HDMI_CODEC
 	select SND_PCM_ELD
 	select SND_PCM_IEC958
 	select HDMI
+	select HDMI_NOTIFIERS
 
 config SND_SOC_ES8328
 	tristate "Everest Semi ES8328 CODEC"
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index f27d115..973892b 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -12,9 +12,12 @@
  * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.	 See the GNU
  * General Public License for more details.
  */
+#include <linux/hdmi-notifier.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/string.h>
 #include <sound/core.h>
+#include <sound/jack.h>
 #include <sound/pcm.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
@@ -27,11 +30,16 @@
 struct hdmi_codec_priv {
 	struct hdmi_codec_pdata hcd;
 	struct snd_soc_dai_driver *daidrv;
+	struct snd_soc_jack *jack;
 	struct hdmi_codec_daifmt daifmt[2];
 	struct mutex current_stream_lock;
 	struct snd_pcm_substream *current_stream;
 	struct snd_pcm_hw_constraint_list ratec;
+	struct mutex eld_lock;
 	uint8_t eld[MAX_ELD_BYTES];
+	struct device *dev;
+	struct notifier_block nb;
+	unsigned int jack_status;
 };
 
 static const struct snd_soc_dapm_widget hdmi_widgets[] = {
@@ -65,7 +73,9 @@ static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
 	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
 	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
 
+	mutex_lock(&hcp->eld_lock);
 	memcpy(ucontrol->value.bytes.data, hcp->eld, sizeof(hcp->eld));
+	mutex_unlock(&hcp->eld_lock);
 
 	return 0;
 }
@@ -103,7 +113,7 @@ static int hdmi_codec_startup(struct snd_pcm_substream *substream,
 			      struct snd_soc_dai *dai)
 {
 	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
-	int ret = 0;
+	int ret;
 
 	dev_dbg(dai->dev, "%s()\n", __func__);
 
@@ -122,17 +132,15 @@ static int hdmi_codec_startup(struct snd_pcm_substream *substream,
 	}
 
 	if (hcp->hcd.ops->get_eld) {
+		mutex_lock(&hcp->eld_lock);
 		ret = hcp->hcd.ops->get_eld(dai->dev->parent, hcp->hcd.data,
 					    hcp->eld, sizeof(hcp->eld));
-
-		if (!ret) {
+		if (!ret)
 			ret = snd_pcm_hw_constraint_eld(substream->runtime,
 							hcp->eld);
-			if (ret)
-				return ret;
-		}
+		mutex_unlock(&hcp->eld_lock);
 	}
-	return 0;
+	return ret;
 }
 
 static void hdmi_codec_shutdown(struct snd_pcm_substream *substream,
@@ -355,6 +363,63 @@ static struct snd_soc_codec_driver hdmi_codec = {
 	.num_dapm_routes = ARRAY_SIZE(hdmi_routes),
 };
 
+static void hdmi_codec_jack_report(struct hdmi_codec_priv *hcp,
+				   unsigned int jack_status)
+{
+	if (!hcp->jack)
+		return;
+
+	if (jack_status != hcp->jack_status) {
+		snd_soc_jack_report(hcp->jack, jack_status, SND_JACK_LINEOUT);
+		hcp->jack_status = jack_status;
+	}
+}
+
+static int hdmi_codec_notify(struct notifier_block *nb, unsigned long event,
+			     void *data)
+{
+	struct hdmi_codec_priv *hcp = container_of(nb, struct hdmi_codec_priv,
+						   nb);
+	union hdmi_event *event_block = data;
+
+	if (hcp->dev->parent != event_block->base.source)
+		return NOTIFY_OK;
+
+	if (!hcp->jack)
+		return NOTIFY_OK;
+
+	switch (event) {
+	case HDMI_CONNECTED:
+		hdmi_codec_jack_report(hcp, SND_JACK_LINEOUT);
+		break;
+	case HDMI_DISCONNECTED:
+		hdmi_codec_jack_report(hcp, 0);
+		break;
+	case HDMI_NEW_ELD:
+		hdmi_codec_jack_report(hcp, SND_JACK_LINEOUT);
+		mutex_lock(&hcp->eld_lock);
+		memcpy(hcp->eld, event_block->eld.eld, sizeof(hcp->eld));
+		mutex_unlock(&hcp->eld_lock);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+int hdmi_codec_set_jack_detect(struct snd_soc_codec *codec,
+			       struct snd_soc_jack *jack)
+{
+	struct hdmi_codec_priv *hcp = snd_soc_codec_get_drvdata(codec);
+
+	hcp->jack = jack;
+	hcp->nb.notifier_call = hdmi_codec_notify;
+
+	hdmi_register_notifier(&hcp->nb);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(hdmi_codec_set_jack_detect);
+
 static int hdmi_codec_probe(struct platform_device *pdev)
 {
 	struct hdmi_codec_pdata *hcd = pdev->dev.platform_data;
@@ -383,6 +448,7 @@ static int hdmi_codec_probe(struct platform_device *pdev)
 
 	hcp->hcd = *hcd;
 	mutex_init(&hcp->current_stream_lock);
+	mutex_init(&hcp->eld_lock);
 
 	hcp->daidrv = devm_kzalloc(dev, dai_count * sizeof(*hcp->daidrv),
 				   GFP_KERNEL);
@@ -399,6 +465,8 @@ static int hdmi_codec_probe(struct platform_device *pdev)
 	if (hcd->spdif)
 		hcp->daidrv[i] = hdmi_spdif_dai;
 
+	dev_set_drvdata(dev, hcp);
+
 	ret = snd_soc_register_codec(dev, &hdmi_codec, hcp->daidrv,
 				     dai_count);
 	if (ret) {
@@ -407,12 +475,16 @@ static int hdmi_codec_probe(struct platform_device *pdev)
 		return ret;
 	}
 
-	dev_set_drvdata(dev, hcp);
+	hcp->dev = dev;
+
 	return 0;
 }
 
 static int hdmi_codec_remove(struct platform_device *pdev)
 {
+	struct hdmi_codec_priv *hcp = platform_get_drvdata(pdev);
+
+	hdmi_unregister_notifier(&hcp->nb);
 	snd_soc_unregister_codec(&pdev->dev);
 	return 0;
 }
-- 
2.8.1

[PATCH v8 3/5] ASoC: mediatek: Add jack detection support to mt8173-rt5650-rt5676 machine driver

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c b/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c
index 1b8b2a7..707eeaf 100644
--- a/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c
+++ b/sound/soc/mediatek/mt8173/mt8173-rt5650-rt5676.c
@@ -18,6 +18,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <sound/soc.h>
+#include <sound/hdmi-codec.h>
 #include <sound/jack.h>
 #include "../../codecs/rt5645.h"
 #include "../../codecs/rt5677.h"
@@ -131,6 +132,25 @@ static struct snd_soc_dai_link_component mt8173_rt5650_rt5676_codecs[] = {
 	},
 };
 
+static struct snd_soc_jack mt8173_hdmi_card_jack;
+
+static int mt8173_hdmi_init(struct snd_soc_pcm_runtime *runtime)
+{
+	struct snd_soc_card *card = runtime->card;
+	struct snd_soc_codec *codec = runtime->codec;
+	int ret;
+
+	/* enable jack detection */
+	ret = snd_soc_card_jack_new(card, "HDMI Jack", SND_JACK_LINEOUT,
+				    &mt8173_hdmi_card_jack, NULL, 0);
+	if (ret) {
+		dev_err(card->dev, "Can't new HDMI Jack %d\n", ret);
+		return ret;
+	}
+
+	return hdmi_codec_set_jack_detect(codec, &mt8173_hdmi_card_jack);
+}
+
 enum {
 	DAI_LINK_PLAYBACK,
 	DAI_LINK_CAPTURE,
@@ -195,6 +215,7 @@ static struct snd_soc_dai_link mt8173_rt5650_rt5676_dais[] = {
 		.no_pcm = 1,
 		.codec_dai_name = "i2s-hifi",
 		.dpcm_playback = 1,
+		.init = mt8173_hdmi_init,
 	},
 	/* rt5676 <-> rt5650 intercodec link: Sets rt5676 I2S2 as master */
 	[DAI_LINK_INTERCODEC] = {
-- 
2.8.1

[PATCH v8 4/5] ASoC: mediatek: Add jack detection support to the mt8173-rt5650 machine driver

[Philipp Zabel]

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 sound/soc/mediatek/mt8173/mt8173-rt5650.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)

diff --git a/sound/soc/mediatek/mt8173/mt8173-rt5650.c b/sound/soc/mediatek/mt8173/mt8173-rt5650.c
index ba65f41..64ae9ad 100644
--- a/sound/soc/mediatek/mt8173/mt8173-rt5650.c
+++ b/sound/soc/mediatek/mt8173/mt8173-rt5650.c
@@ -18,6 +18,7 @@
 #include <linux/gpio.h>
 #include <linux/of_gpio.h>
 #include <sound/soc.h>
+#include <sound/hdmi-codec.h>
 #include <sound/jack.h>
 #include "../../codecs/rt5645.h"
 
@@ -166,6 +167,25 @@ static struct snd_soc_dai_link_component mt8173_rt5650_codecs[] = {
 	},
 };
 
+static struct snd_soc_jack mt8173_hdmi_card_jack;
+
+static int mt8173_hdmi_init(struct snd_soc_pcm_runtime *runtime)
+{
+	struct snd_soc_card *card = runtime->card;
+	struct snd_soc_codec *codec = runtime->codec;
+	int ret;
+
+	/* enable jack detection */
+	ret = snd_soc_card_jack_new(card, "HDMI Jack", SND_JACK_LINEOUT,
+				    &mt8173_hdmi_card_jack, NULL, 0);
+	if (ret) {
+		dev_err(card->dev, "Can't new HDMI Jack %d\n", ret);
+		return ret;
+	}
+
+	return hdmi_codec_set_jack_detect(codec, &mt8173_hdmi_card_jack);
+}
+
 enum {
 	DAI_LINK_PLAYBACK,
 	DAI_LINK_CAPTURE,
@@ -228,6 +248,7 @@ static struct snd_soc_dai_link mt8173_rt5650_dais[] = {
 		.no_pcm = 1,
 		.codec_dai_name = "i2s-hifi",
 		.dpcm_playback = 1,
+		.init = mt8173_hdmi_init,
 	},
 };
 
-- 
2.8.1

[PATCH v8 5/5] drm/mediatek: hdmi: issue notifications

[Philipp Zabel]

Issue hot-plug detection, EDID update, and ELD update notifications
from the CEC and HDMI drivers.

Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
 drivers/gpu/drm/mediatek/mtk_cec.c  | 11 +++++++++++
 drivers/gpu/drm/mediatek/mtk_hdmi.c |  3 +++
 2 files changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/mediatek/mtk_cec.c b/drivers/gpu/drm/mediatek/mtk_cec.c
index 7a3eb8c..f78a73c 100644
--- a/drivers/gpu/drm/mediatek/mtk_cec.c
+++ b/drivers/gpu/drm/mediatek/mtk_cec.c
@@ -13,6 +13,7 @@
  */
 #include <linux/clk.h>
 #include <linux/delay.h>
+#include <linux/hdmi-notifier.h>
 #include <linux/io.h>
 #include <linux/interrupt.h>
 #include <linux/platform_device.h>
@@ -105,6 +106,12 @@ void mtk_cec_set_hpd_event(struct device *dev,
 	cec->hdmi_dev = hdmi_dev;
 	cec->hpd_event = hpd_event;
 	spin_unlock_irqrestore(&cec->lock, flags);
+
+	/* Initial notification event to set jack state */
+	if (mtk_cec_hpd_high(dev))
+		hdmi_event_connect(hdmi_dev);
+	else
+		hdmi_event_disconnect(hdmi_dev);
 }
 
 bool mtk_cec_hpd_high(struct device *dev)
@@ -179,6 +186,10 @@ static irqreturn_t mtk_cec_htplg_isr_thread(int irq, void *arg)
 	if (cec->hpd != hpd) {
 		dev_dbg(dev, "hotplug event! cur hpd = %d, hpd = %d\n",
 			cec->hpd, hpd);
+		if (hpd)
+			hdmi_event_connect(cec->hdmi_dev);
+		else
+			hdmi_event_disconnect(cec->hdmi_dev);
 		cec->hpd = hpd;
 		mtk_cec_hpd_event(cec, hpd);
 	}
diff --git a/drivers/gpu/drm/mediatek/mtk_hdmi.c b/drivers/gpu/drm/mediatek/mtk_hdmi.c
index 334562d..478aa73 100644
--- a/drivers/gpu/drm/mediatek/mtk_hdmi.c
+++ b/drivers/gpu/drm/mediatek/mtk_hdmi.c
@@ -20,6 +20,7 @@
 #include <linux/clk.h>
 #include <linux/delay.h>
 #include <linux/hdmi.h>
+#include <linux/hdmi-notifier.h>
 #include <linux/i2c.h>
 #include <linux/io.h>
 #include <linux/kernel.h>
@@ -1225,9 +1226,11 @@ static int mtk_hdmi_conn_get_modes(struct drm_connector *conn)
 	hdmi->dvi_mode = !drm_detect_monitor_audio(edid);
 
 	drm_mode_connector_update_edid_property(conn, edid);
+	hdmi_event_new_edid(hdmi->dev, edid, sizeof(*edid));
 
 	ret = drm_add_edid_modes(conn, edid);
 	drm_edid_to_eld(conn, edid);
+	hdmi_event_new_eld(hdmi->dev, conn->eld);
 	kfree(edid);
 	return ret;
 }
-- 
2.8.1

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Russell King - ARM Linux]

On Thu, Aug 11, 2016 at 11:20:23AM +0200, Philipp Zabel wrote:
> This is Russell's HDMI notification prototype [1], currently waiting
> for the HDMI CEC situation to resolve.
> 
> The use case for the notifications on MediaTek MT8173 is to let the
> (dis)connection notifications control an ALSA jack object.
> 
> No Signed-off-by since this is not my code, and still up for discussion.

Well, I have two drivers (both CEC drivers) which use this, and I still
don't see any alternative solution to the problem that this patch is
solving.

I don't think it's really a CEC problem - there's three bits to HDMI
that need to track each others state - the video, audio and CEC paths.

While the video and audio paths may be one block, the CEC path may
actually be a separate block.  For example, the TDA998x devices
integrate the HDMI video/audio block along with a TDA9950 on the
same device - the TDA9950 being a CEC engine.  The TDA9950 is also
available as a separate device, and even when integrated with HDMI,
it appears on the I2C bus as a seperate device.

So, splitting the functionality is definitely the right model.  We
just need some way to keep each blocks state in sync.  What's provided
in this patch is the simple solution which seems to work for the use
cases we have.

I think, in light of no comments against this approach, and no other
approach being available, that this is good enough justification to
merge this, especially as it is blocking other work.

So... if people want to give me reviewed-by/acked-bys, I'll add them
to my patch, and I'll post that and the dw-hdmi and tda9950 CEC drivers.

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

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Hans Verkuil]

Hi Russell,

I like this approach and it is something I will need for other CEC drivers
(not yet merged) as well.

Just a few comments:

On 08/11/16 11:20, Philipp Zabel wrote:
> This is Russell's HDMI notification prototype [1], currently waiting
> for the HDMI CEC situation to resolve.
> 
> The use case for the notifications on MediaTek MT8173 is to let the
> (dis)connection notifications control an ALSA jack object.
> 
> No Signed-off-by since this is not my code, and still up for discussion.
> 
> [1] https://patchwork.kernel.org/patch/8351501/
> ---
>  drivers/video/Kconfig         |  3 +++
>  drivers/video/Makefile        |  1 +
>  drivers/video/hdmi-notifier.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>  include/linux/hdmi-notifier.h | 44 +++++++++++++++++++++++++++++++
>  4 files changed, 109 insertions(+)
>  create mode 100644 drivers/video/hdmi-notifier.c
>  create mode 100644 include/linux/hdmi-notifier.h
> 
> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> index 3c20af9..1ee7b9f 100644
> --- a/drivers/video/Kconfig
> +++ b/drivers/video/Kconfig
> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>  config HDMI
>  	bool
>  
> +config HDMI_NOTIFIERS
> +	bool
> +
>  if VT
>  	source "drivers/video/console/Kconfig"
>  endif
> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> index 9ad3c17..65f5649 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_HDMI_NOTIFIERS)      += hdmi-notifier.o
>  
>  obj-$(CONFIG_VT)		  += console/
>  obj-$(CONFIG_LOGO)		  += logo/
> diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
> new file mode 100644
> index 0000000..a233359
> --- /dev/null
> +++ b/drivers/video/hdmi-notifier.c
> @@ -0,0 +1,61 @@
> +#include <linux/export.h>
> +#include <linux/hdmi-notifier.h>
> +#include <linux/notifier.h>
> +#include <linux/string.h>
> +
> +static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
> +
> +int hdmi_register_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_register(&hdmi_notifier, nb);
> +}
> +EXPORT_SYMBOL_GPL(hdmi_register_notifier);
> +
> +int hdmi_unregister_notifier(struct notifier_block *nb)
> +{
> +	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
> +}
> +EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
> +
> +void hdmi_event_connect(struct device *dev)
> +{
> +	struct hdmi_event_base base;
> +
> +	base.source = dev;
> +
> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
> +}
> +EXPORT_SYMBOL_GPL(hdmi_event_connect);
> +
> +void hdmi_event_disconnect(struct device *dev)
> +{
> +	struct hdmi_event_base base;
> +
> +	base.source = dev;
> +
> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
> +}
> +EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
> +
> +void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)

I would prefer const u8 *edid. It is after all just a u8 array and not some
opaque data structure.

> +{
> +	struct hdmi_event_new_edid new_edid;
> +
> +	new_edid.base.source = dev;
> +	new_edid.edid = edid;
> +	new_edid.size = size;
> +
> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_EDID, &new_edid);
> +}
> +EXPORT_SYMBOL_GPL(hdmi_event_new_edid);
> +
> +void hdmi_event_new_eld(struct device *dev, const void *eld)

Stupid question: what does ELD stand for? I've no idea...

And is void the right choice here or should it also be u8?

> +{
> +	struct hdmi_event_new_eld new_eld;
> +
> +	new_eld.base.source = dev;
> +	memcpy(new_eld.eld, eld, sizeof(new_eld.eld));
> +
> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_ELD, &new_eld);
> +}
> +EXPORT_SYMBOL_GPL(hdmi_event_new_eld);
> diff --git a/include/linux/hdmi-notifier.h b/include/linux/hdmi-notifier.h
> new file mode 100644
> index 0000000..9c5ad49
> --- /dev/null
> +++ b/include/linux/hdmi-notifier.h
> @@ -0,0 +1,44 @@
> +#ifndef LINUX_HDMI_NOTIFIER_H
> +#define LINUX_HDMI_NOTIFIER_H
> +
> +#include <linux/types.h>
> +
> +enum {
> +	HDMI_CONNECTED,
> +	HDMI_DISCONNECTED,
> +	HDMI_NEW_EDID,
> +	HDMI_NEW_ELD,
> +};
> +
> +struct hdmi_event_base {
> +	struct device *source;
> +};
> +
> +struct hdmi_event_new_edid {
> +	struct hdmi_event_base base;
> +	const void *edid;
> +	size_t size;
> +};
> +
> +struct hdmi_event_new_eld {
> +	struct hdmi_event_base base;
> +	unsigned char eld[128];
> +};
> +
> +union hdmi_event {
> +	struct hdmi_event_base base;
> +	struct hdmi_event_new_edid edid;
> +	struct hdmi_event_new_eld eld;
> +};
> +
> +struct notifier_block;
> +
> +int hdmi_register_notifier(struct notifier_block *nb);
> +int hdmi_unregister_notifier(struct notifier_block *nb);
> +
> +void hdmi_event_connect(struct device *dev);
> +void hdmi_event_disconnect(struct device *dev);
> +void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size);
> +void hdmi_event_new_eld(struct device *dev, const void *eld);
> +
> +#endif
> 

Regards,

	Hans

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Russell King - ARM Linux]

On Thu, Aug 11, 2016 at 12:30:03PM +0200, Hans Verkuil wrote:
> Hi Russell,
> 
> I like this approach and it is something I will need for other CEC drivers
> (not yet merged) as well.
> 
> Just a few comments:
> 
> On 08/11/16 11:20, Philipp Zabel wrote:
> > This is Russell's HDMI notification prototype [1], currently waiting
> > for the HDMI CEC situation to resolve.
> > 
> > The use case for the notifications on MediaTek MT8173 is to let the
> > (dis)connection notifications control an ALSA jack object.
> > 
> > No Signed-off-by since this is not my code, and still up for discussion.
> > 
> > [1] https://patchwork.kernel.org/patch/8351501/
> > ---
> >  drivers/video/Kconfig         |  3 +++
> >  drivers/video/Makefile        |  1 +
> >  drivers/video/hdmi-notifier.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >  include/linux/hdmi-notifier.h | 44 +++++++++++++++++++++++++++++++
> >  4 files changed, 109 insertions(+)
> >  create mode 100644 drivers/video/hdmi-notifier.c
> >  create mode 100644 include/linux/hdmi-notifier.h
> > 
> > diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> > index 3c20af9..1ee7b9f 100644
> > --- a/drivers/video/Kconfig
> > +++ b/drivers/video/Kconfig
> > @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
> >  config HDMI
> >  	bool
> >  
> > +config HDMI_NOTIFIERS
> > +	bool
> > +
> >  if VT
> >  	source "drivers/video/console/Kconfig"
> >  endif
> > diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> > index 9ad3c17..65f5649 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_HDMI_NOTIFIERS)      += hdmi-notifier.o
> >  
> >  obj-$(CONFIG_VT)		  += console/
> >  obj-$(CONFIG_LOGO)		  += logo/
> > diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
> > new file mode 100644
> > index 0000000..a233359
> > --- /dev/null
> > +++ b/drivers/video/hdmi-notifier.c
> > @@ -0,0 +1,61 @@
> > +#include <linux/export.h>
> > +#include <linux/hdmi-notifier.h>
> > +#include <linux/notifier.h>
> > +#include <linux/string.h>
> > +
> > +static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
> > +
> > +int hdmi_register_notifier(struct notifier_block *nb)
> > +{
> > +	return blocking_notifier_chain_register(&hdmi_notifier, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(hdmi_register_notifier);
> > +
> > +int hdmi_unregister_notifier(struct notifier_block *nb)
> > +{
> > +	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
> > +}
> > +EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
> > +
> > +void hdmi_event_connect(struct device *dev)
> > +{
> > +	struct hdmi_event_base base;
> > +
> > +	base.source = dev;
> > +
> > +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
> > +}
> > +EXPORT_SYMBOL_GPL(hdmi_event_connect);
> > +
> > +void hdmi_event_disconnect(struct device *dev)
> > +{
> > +	struct hdmi_event_base base;
> > +
> > +	base.source = dev;
> > +
> > +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
> > +}
> > +EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
> > +
> > +void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)
> 
> I would prefer const u8 *edid. It is after all just a u8 array and not some
> opaque data structure.

In DRM, the edid is of type "struct edid" (defined in
include/drm/drm_edid.h).  So, making it "const u8 *" means that all
DRM drivers will have to explicitly cast it - not something I'm in
favour of.

> > +{
> > +	struct hdmi_event_new_edid new_edid;
> > +
> > +	new_edid.base.source = dev;
> > +	new_edid.edid = edid;
> > +	new_edid.size = size;
> > +
> > +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_EDID, &new_edid);
> > +}
> > +EXPORT_SYMBOL_GPL(hdmi_event_new_edid);
> > +
> > +void hdmi_event_new_eld(struct device *dev, const void *eld)
> 
> Stupid question: what does ELD stand for? I've no idea...
> 
> And is void the right choice here or should it also be u8?

The ELD stands for EDID-like data.  See information on HDA039.  It's a
method of conveying the EDID data specific to audio drivers to them
without having the complexities of parsing the full EDID each time.
It's also exported to userland so that userland can determine the
capabilities of the audio path, again, without having to have full
EDID parsers and exporting the full EDID data block through audio
drivers.

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

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Hans Verkuil]

On 08/11/16 12:39, Russell King - ARM Linux wrote:
> On Thu, Aug 11, 2016 at 12:30:03PM +0200, Hans Verkuil wrote:
>> Hi Russell,
>>
>> I like this approach and it is something I will need for other CEC drivers
>> (not yet merged) as well.
>>
>> Just a few comments:
>>
>> On 08/11/16 11:20, Philipp Zabel wrote:
>>> This is Russell's HDMI notification prototype [1], currently waiting
>>> for the HDMI CEC situation to resolve.
>>>
>>> The use case for the notifications on MediaTek MT8173 is to let the
>>> (dis)connection notifications control an ALSA jack object.
>>>
>>> No Signed-off-by since this is not my code, and still up for discussion.
>>>
>>> [1] https://patchwork.kernel.org/patch/8351501/
>>> ---
>>>  drivers/video/Kconfig         |  3 +++
>>>  drivers/video/Makefile        |  1 +
>>>  drivers/video/hdmi-notifier.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>>>  include/linux/hdmi-notifier.h | 44 +++++++++++++++++++++++++++++++
>>>  4 files changed, 109 insertions(+)
>>>  create mode 100644 drivers/video/hdmi-notifier.c
>>>  create mode 100644 include/linux/hdmi-notifier.h
>>>
>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>> index 3c20af9..1ee7b9f 100644
>>> --- a/drivers/video/Kconfig
>>> +++ b/drivers/video/Kconfig
>>> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>>>  config HDMI
>>>  	bool
>>>  
>>> +config HDMI_NOTIFIERS
>>> +	bool
>>> +
>>>  if VT
>>>  	source "drivers/video/console/Kconfig"
>>>  endif
>>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
>>> index 9ad3c17..65f5649 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_HDMI_NOTIFIERS)      += hdmi-notifier.o
>>>  
>>>  obj-$(CONFIG_VT)		  += console/
>>>  obj-$(CONFIG_LOGO)		  += logo/
>>> diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
>>> new file mode 100644
>>> index 0000000..a233359
>>> --- /dev/null
>>> +++ b/drivers/video/hdmi-notifier.c
>>> @@ -0,0 +1,61 @@
>>> +#include <linux/export.h>
>>> +#include <linux/hdmi-notifier.h>
>>> +#include <linux/notifier.h>
>>> +#include <linux/string.h>
>>> +
>>> +static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
>>> +
>>> +int hdmi_register_notifier(struct notifier_block *nb)
>>> +{
>>> +	return blocking_notifier_chain_register(&hdmi_notifier, nb);
>>> +}
>>> +EXPORT_SYMBOL_GPL(hdmi_register_notifier);
>>> +
>>> +int hdmi_unregister_notifier(struct notifier_block *nb)
>>> +{
>>> +	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
>>> +}
>>> +EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
>>> +
>>> +void hdmi_event_connect(struct device *dev)
>>> +{
>>> +	struct hdmi_event_base base;
>>> +
>>> +	base.source = dev;
>>> +
>>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
>>> +}
>>> +EXPORT_SYMBOL_GPL(hdmi_event_connect);
>>> +
>>> +void hdmi_event_disconnect(struct device *dev)
>>> +{
>>> +	struct hdmi_event_base base;
>>> +
>>> +	base.source = dev;
>>> +
>>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
>>> +}
>>> +EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
>>> +
>>> +void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)
>>
>> I would prefer const u8 *edid. It is after all just a u8 array and not some
>> opaque data structure.
> 
> In DRM, the edid is of type "struct edid" (defined in
> include/drm/drm_edid.h).  So, making it "const u8 *" means that all
> DRM drivers will have to explicitly cast it - not something I'm in
> favour of.

I thought this was the raw EDID data. But if you pass a struct edid around,
then why not make this const struct edid *edid? I fail to see why you would
want to use a void pointer here.

Also struct edid is for the first EDID block only, whereas CEC drivers need
the CEA-861 block to get the physical address.

This could of course be a separate event that just notifies clients of the
physical address.

> 
>>> +{
>>> +	struct hdmi_event_new_edid new_edid;
>>> +
>>> +	new_edid.base.source = dev;
>>> +	new_edid.edid = edid;
>>> +	new_edid.size = size;
>>> +
>>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_NEW_EDID, &new_edid);
>>> +}
>>> +EXPORT_SYMBOL_GPL(hdmi_event_new_edid);
>>> +
>>> +void hdmi_event_new_eld(struct device *dev, const void *eld)
>>
>> Stupid question: what does ELD stand for? I've no idea...
>>
>> And is void the right choice here or should it also be u8?
> 
> The ELD stands for EDID-like data.  See information on HDA039.  It's a
> method of conveying the EDID data specific to audio drivers to them
> without having the complexities of parsing the full EDID each time.
> It's also exported to userland so that userland can determine the
> capabilities of the audio path, again, without having to have full
> EDID parsers and exporting the full EDID data block through audio
> drivers.
> 

OK. Since eld is an unsigned char array I would suggest changing void to
unsigned char here.

Or perhaps const unsigned char eld[128].

Regards,

	Hans

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Russell King - ARM Linux]

On Thu, Aug 11, 2016 at 12:49:21PM +0200, Hans Verkuil wrote:
> On 08/11/16 12:39, Russell King - ARM Linux wrote:
> > On Thu, Aug 11, 2016 at 12:30:03PM +0200, Hans Verkuil wrote:
> >> Hi Russell,
> >>
> >> I like this approach and it is something I will need for other CEC drivers
> >> (not yet merged) as well.
> >>
> >> Just a few comments:
> >>
> >> On 08/11/16 11:20, Philipp Zabel wrote:
> >>> This is Russell's HDMI notification prototype [1], currently waiting
> >>> for the HDMI CEC situation to resolve.
> >>>
> >>> The use case for the notifications on MediaTek MT8173 is to let the
> >>> (dis)connection notifications control an ALSA jack object.
> >>>
> >>> No Signed-off-by since this is not my code, and still up for discussion.
> >>>
> >>> [1] https://patchwork.kernel.org/patch/8351501/
> >>> ---
> >>>  drivers/video/Kconfig         |  3 +++
> >>>  drivers/video/Makefile        |  1 +
> >>>  drivers/video/hdmi-notifier.c | 61 +++++++++++++++++++++++++++++++++++++++++++
> >>>  include/linux/hdmi-notifier.h | 44 +++++++++++++++++++++++++++++++
> >>>  4 files changed, 109 insertions(+)
> >>>  create mode 100644 drivers/video/hdmi-notifier.c
> >>>  create mode 100644 include/linux/hdmi-notifier.h
> >>>
> >>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
> >>> index 3c20af9..1ee7b9f 100644
> >>> --- a/drivers/video/Kconfig
> >>> +++ b/drivers/video/Kconfig
> >>> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
> >>>  config HDMI
> >>>  	bool
> >>>  
> >>> +config HDMI_NOTIFIERS
> >>> +	bool
> >>> +
> >>>  if VT
> >>>  	source "drivers/video/console/Kconfig"
> >>>  endif
> >>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
> >>> index 9ad3c17..65f5649 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_HDMI_NOTIFIERS)      += hdmi-notifier.o
> >>>  
> >>>  obj-$(CONFIG_VT)		  += console/
> >>>  obj-$(CONFIG_LOGO)		  += logo/
> >>> diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
> >>> new file mode 100644
> >>> index 0000000..a233359
> >>> --- /dev/null
> >>> +++ b/drivers/video/hdmi-notifier.c
> >>> @@ -0,0 +1,61 @@
> >>> +#include <linux/export.h>
> >>> +#include <linux/hdmi-notifier.h>
> >>> +#include <linux/notifier.h>
> >>> +#include <linux/string.h>
> >>> +
> >>> +static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
> >>> +
> >>> +int hdmi_register_notifier(struct notifier_block *nb)
> >>> +{
> >>> +	return blocking_notifier_chain_register(&hdmi_notifier, nb);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(hdmi_register_notifier);
> >>> +
> >>> +int hdmi_unregister_notifier(struct notifier_block *nb)
> >>> +{
> >>> +	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
> >>> +
> >>> +void hdmi_event_connect(struct device *dev)
> >>> +{
> >>> +	struct hdmi_event_base base;
> >>> +
> >>> +	base.source = dev;
> >>> +
> >>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(hdmi_event_connect);
> >>> +
> >>> +void hdmi_event_disconnect(struct device *dev)
> >>> +{
> >>> +	struct hdmi_event_base base;
> >>> +
> >>> +	base.source = dev;
> >>> +
> >>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
> >>> +}
> >>> +EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
> >>> +
> >>> +void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)
> >>
> >> I would prefer const u8 *edid. It is after all just a u8 array and not some
> >> opaque data structure.
> > 
> > In DRM, the edid is of type "struct edid" (defined in
> > include/drm/drm_edid.h).  So, making it "const u8 *" means that all
> > DRM drivers will have to explicitly cast it - not something I'm in
> > favour of.
> 
> I thought this was the raw EDID data. But if you pass a struct edid around,
> then why not make this const struct edid *edid? I fail to see why you would
> want to use a void pointer here.

struct edid is a DRM thing - it's not generic.  Using struct edid here
would force everyone to use the DRM structure, whereas other subsystems
use, eg, unsigned char.

> Also struct edid is for the first EDID block only, whereas CEC drivers need
> the CEA-861 block to get the physical address.

That may be, but DRM parses the HDMI vendor block too while still using
struct edid to represent it.  See drm_find_edid_extension() in
drivers/gpu/drm/drm_edid.c

This is just how DRM is... I'm not sure what the design decisions were
that came up with this, but it's what we have, and I'm not sure whether
DRM would be open to changing it.

> > The ELD stands for EDID-like data.  See information on HDA039.  It's a
> > method of conveying the EDID data specific to audio drivers to them
> > without having the complexities of parsing the full EDID each time.
> > It's also exported to userland so that userland can determine the
> > capabilities of the audio path, again, without having to have full
> > EDID parsers and exporting the full EDID data block through audio
> > drivers.
> > 
> 
> OK. Since eld is an unsigned char array I would suggest changing void to
> unsigned char here.
> 
> Or perhaps const unsigned char eld[128].

Yes, I'll make that change.

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

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Philipp Zabel]

Am Donnerstag, den 11.08.2016, 11:18 +0100 schrieb Russell King - ARM
Linux:
> On Thu, Aug 11, 2016 at 11:20:23AM +0200, Philipp Zabel wrote:
> > This is Russell's HDMI notification prototype [1], currently waiting
> > for the HDMI CEC situation to resolve.
> > 
> > The use case for the notifications on MediaTek MT8173 is to let the
> > (dis)connection notifications control an ALSA jack object.
> > 
> > No Signed-off-by since this is not my code, and still up for discussion.
> 
> Well, I have two drivers (both CEC drivers) which use this, and I still
> don't see any alternative solution to the problem that this patch is
> solving.
> 
> I don't think it's really a CEC problem - there's three bits to HDMI
> that need to track each others state - the video, audio and CEC paths.
> 
> While the video and audio paths may be one block, the CEC path may
> actually be a separate block.  For example, the TDA998x devices
> integrate the HDMI video/audio block along with a TDA9950 on the
> same device - the TDA9950 being a CEC engine.  The TDA9950 is also
> available as a separate device, and even when integrated with HDMI,
> it appears on the I2C bus as a seperate device.
> 
> So, splitting the functionality is definitely the right model.  We
> just need some way to keep each blocks state in sync.  What's provided
> in this patch is the simple solution which seems to work for the use
> cases we have.

Yes, it works fine for the MT8173 use case.

> I think, in light of no comments against this approach, and no other
> approach being available, that this is good enough justification to
> merge this, especially as it is blocking other work.
> 
> So... if people want to give me reviewed-by/acked-bys, I'll add them
> to my patch, and I'll post that and the dw-hdmi and tda9950 CEC drivers.

Acked-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

regards
Philipp

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Hans Verkuil]

On 08/11/16 16:16, Russell King - ARM Linux wrote:
> On Thu, Aug 11, 2016 at 12:49:21PM +0200, Hans Verkuil wrote:
>> On 08/11/16 12:39, Russell King - ARM Linux wrote:
>>> On Thu, Aug 11, 2016 at 12:30:03PM +0200, Hans Verkuil wrote:
>>>> Hi Russell,
>>>>
>>>> I like this approach and it is something I will need for other CEC drivers
>>>> (not yet merged) as well.
>>>>
>>>> Just a few comments:
>>>>
>>>> On 08/11/16 11:20, Philipp Zabel wrote:
>>>>> This is Russell's HDMI notification prototype [1], currently waiting
>>>>> for the HDMI CEC situation to resolve.
>>>>>
>>>>> The use case for the notifications on MediaTek MT8173 is to let the
>>>>> (dis)connection notifications control an ALSA jack object.
>>>>>
>>>>> No Signed-off-by since this is not my code, and still up for discussion.
>>>>>
>>>>> [1] https://patchwork.kernel.org/patch/8351501/
>>>>> ---
>>>>>  drivers/video/Kconfig         |  3 +++
>>>>>  drivers/video/Makefile        |  1 +
>>>>>  drivers/video/hdmi-notifier.c | 61 +++++++++++++++++++++++++++++++++++++++++++
>>>>>  include/linux/hdmi-notifier.h | 44 +++++++++++++++++++++++++++++++
>>>>>  4 files changed, 109 insertions(+)
>>>>>  create mode 100644 drivers/video/hdmi-notifier.c
>>>>>  create mode 100644 include/linux/hdmi-notifier.h
>>>>>
>>>>> diff --git a/drivers/video/Kconfig b/drivers/video/Kconfig
>>>>> index 3c20af9..1ee7b9f 100644
>>>>> --- a/drivers/video/Kconfig
>>>>> +++ b/drivers/video/Kconfig
>>>>> @@ -36,6 +36,9 @@ config VIDEOMODE_HELPERS
>>>>>  config HDMI
>>>>>  	bool
>>>>>  
>>>>> +config HDMI_NOTIFIERS
>>>>> +	bool
>>>>> +
>>>>>  if VT
>>>>>  	source "drivers/video/console/Kconfig"
>>>>>  endif
>>>>> diff --git a/drivers/video/Makefile b/drivers/video/Makefile
>>>>> index 9ad3c17..65f5649 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_HDMI_NOTIFIERS)      += hdmi-notifier.o
>>>>>  
>>>>>  obj-$(CONFIG_VT)		  += console/
>>>>>  obj-$(CONFIG_LOGO)		  += logo/
>>>>> diff --git a/drivers/video/hdmi-notifier.c b/drivers/video/hdmi-notifier.c
>>>>> new file mode 100644
>>>>> index 0000000..a233359
>>>>> --- /dev/null
>>>>> +++ b/drivers/video/hdmi-notifier.c
>>>>> @@ -0,0 +1,61 @@
>>>>> +#include <linux/export.h>
>>>>> +#include <linux/hdmi-notifier.h>
>>>>> +#include <linux/notifier.h>
>>>>> +#include <linux/string.h>
>>>>> +
>>>>> +static BLOCKING_NOTIFIER_HEAD(hdmi_notifier);
>>>>> +
>>>>> +int hdmi_register_notifier(struct notifier_block *nb)
>>>>> +{
>>>>> +	return blocking_notifier_chain_register(&hdmi_notifier, nb);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(hdmi_register_notifier);
>>>>> +
>>>>> +int hdmi_unregister_notifier(struct notifier_block *nb)
>>>>> +{
>>>>> +	return blocking_notifier_chain_unregister(&hdmi_notifier, nb);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(hdmi_unregister_notifier);
>>>>> +
>>>>> +void hdmi_event_connect(struct device *dev)
>>>>> +{
>>>>> +	struct hdmi_event_base base;
>>>>> +
>>>>> +	base.source = dev;
>>>>> +
>>>>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_CONNECTED, &base);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(hdmi_event_connect);
>>>>> +
>>>>> +void hdmi_event_disconnect(struct device *dev)
>>>>> +{
>>>>> +	struct hdmi_event_base base;
>>>>> +
>>>>> +	base.source = dev;
>>>>> +
>>>>> +	blocking_notifier_call_chain(&hdmi_notifier, HDMI_DISCONNECTED, &base);
>>>>> +}
>>>>> +EXPORT_SYMBOL_GPL(hdmi_event_disconnect);
>>>>> +
>>>>> +void hdmi_event_new_edid(struct device *dev, const void *edid, size_t size)
>>>>
>>>> I would prefer const u8 *edid. It is after all just a u8 array and not some
>>>> opaque data structure.
>>>
>>> In DRM, the edid is of type "struct edid" (defined in
>>> include/drm/drm_edid.h).  So, making it "const u8 *" means that all
>>> DRM drivers will have to explicitly cast it - not something I'm in
>>> favour of.
>>
>> I thought this was the raw EDID data. But if you pass a struct edid around,
>> then why not make this const struct edid *edid? I fail to see why you would
>> want to use a void pointer here.
> 
> struct edid is a DRM thing - it's not generic.  Using struct edid here
> would force everyone to use the DRM structure, whereas other subsystems
> use, eg, unsigned char.

So how the edid pointer should be interpreted depends on which subsystem
sends it? That doesn't sound right. It makes it really hard for the drivers
receiving the notifications.

I am aware of only two subsystems that deal with EDIDs: drm and v4l2 (well,
fbdev as well, but I assume we can ignore that one).

drm uses struct edid, v4l2 just a u8 array. Receivers typically do not need
to decode an EDID, so that's why no effort went into decoding the array on
the v4l2 side. This might change, though. Making struct edid usable in v4l2
would be nice going forward.

But I understand that currently the main purpose of the hdmi_event_new_edid
is to get the physical address from the EDID for CEC? Or do you have other
uses as well?

If it is only the physical address, then I would just make a simple event
that passes that on as a u16 value. This can then be used by both drm and
v4l2.

Note that v4l2 primarily deals with video receivers, and the EDID for
receivers is typically only set once on boot and not changed. So an
hdmi_event_new_phys_addr call would usually only be made once when the
EDID is loaded.

> 
>> Also struct edid is for the first EDID block only, whereas CEC drivers need
>> the CEA-861 block to get the physical address.
> 
> That may be, but DRM parses the HDMI vendor block too while still using
> struct edid to represent it.  See drm_find_edid_extension() in
> drivers/gpu/drm/drm_edid.c
> 
> This is just how DRM is... I'm not sure what the design decisions were
> that came up with this, but it's what we have, and I'm not sure whether
> DRM would be open to changing it.
> 
>>> The ELD stands for EDID-like data.  See information on HDA039.  It's a
>>> method of conveying the EDID data specific to audio drivers to them
>>> without having the complexities of parsing the full EDID each time.
>>> It's also exported to userland so that userland can determine the
>>> capabilities of the audio path, again, without having to have full
>>> EDID parsers and exporting the full EDID data block through audio
>>> drivers.
>>>
>>
>> OK. Since eld is an unsigned char array I would suggest changing void to
>> unsigned char here.
>>
>> Or perhaps const unsigned char eld[128].
> 
> Yes, I'll make that change.
> 

Regards,

	Hans

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Russell King - ARM Linux]

On Thu, Aug 11, 2016 at 04:49:28PM +0200, Hans Verkuil wrote:
> On 08/11/16 16:16, Russell King - ARM Linux wrote:
> > On Thu, Aug 11, 2016 at 12:49:21PM +0200, Hans Verkuil wrote:
> >> I thought this was the raw EDID data. But if you pass a struct edid around,
> >> then why not make this const struct edid *edid? I fail to see why you would
> >> want to use a void pointer here.
> > 
> > struct edid is a DRM thing - it's not generic.  Using struct edid here
> > would force everyone to use the DRM structure, whereas other subsystems
> > use, eg, unsigned char.
> 
> So how the edid pointer should be interpreted depends on which subsystem
> sends it? That doesn't sound right. It makes it really hard for the drivers
> receiving the notifications.

No, that's wrong... and I really don't see why you're trying to make a
meal of this.

In both cases, it's the EDID as received from the device.  One subsystem
happens to represent it internally as a "struct edid" in its drivers,
another subsystem uses "unsigned char []".

In order to _cleanly_ accept both styles, without having to resort to
driver authors needing to litter their code with lots of idiotic casts,
I'm accepting a void * pointer, so that we can accept _both_ a struct
edid and an unsigned char array.

Both are compatible with each other.  Both have the same bytes at the
same offset.  There is no difference between them other than the C
types that are used by each subsystem.

> But I understand that currently the main purpose of the hdmi_event_new_edid
> is to get the physical address from the EDID for CEC? Or do you have other
> uses as well?

That is one use, but if an audio driver wants to parse the raw EDID
instead of the ELD, it can.

What I want to avoid is having the video driver parse lots of different
bits from the EDID and then send lots of events "just in case" something
wants some of the parsed information - it's much more efficient to give
drivers the raw information and allow them to parse what they need from
the EDID.  I'm not convinced that parsing out the physical address, and
then passing that from the video side is a good idea.

The reason I made an exception for the ELD is because (a) it's already
passed between video and audio drivers, (b) the code we have present to
convert an EDID to ELD is in DRM, and requires DRM data structures
which won't be available in an audio driver.

If you want a mechanism to pass just the physical address, we could add
it, and require CEC drivers to make use of both the EDID or physical
address events.  That would mean situations where the physical address
is known to be fixed before hand can be easily catered for.

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

Re: [PATCH v8 1/5] video: rmk's HDMI notification prototype

[Hans Verkuil]

On 08/11/2016 05:03 PM, Russell King - ARM Linux wrote:
> On Thu, Aug 11, 2016 at 04:49:28PM +0200, Hans Verkuil wrote:
>> On 08/11/16 16:16, Russell King - ARM Linux wrote:
>>> On Thu, Aug 11, 2016 at 12:49:21PM +0200, Hans Verkuil wrote:
>>>> I thought this was the raw EDID data. But if you pass a struct edid around,
>>>> then why not make this const struct edid *edid? I fail to see why you would
>>>> want to use a void pointer here.
>>>
>>> struct edid is a DRM thing - it's not generic.  Using struct edid here
>>> would force everyone to use the DRM structure, whereas other subsystems
>>> use, eg, unsigned char.
>>
>> So how the edid pointer should be interpreted depends on which subsystem
>> sends it? That doesn't sound right. It makes it really hard for the drivers
>> receiving the notifications.
>
> No, that's wrong... and I really don't see why you're trying to make a
> meal of this.

It's a misunderstanding on my side. I thought that struct edid was an 'unpacked'
version of the EDID, but it is just an 'overlay' of the first block of the EDID.

Sorry about that, I never worked much with drm :-) And just forget about that
idea of an hdmi_event_new_phys_addr, that too came out of my misunderstanding.

Anyway, what do you think of using this:

struct hdmi_event_new_edid {
	struct hdmi_event_base base;
	union {
		const struct edid *drm_edid;
		const u8 *edid;
	};
	size_t size;
};

hdmi_event_new_edid is fine using a void pointer (well, I hate void pointers,
but I see no clean way around it).

Regards,

	Hans

Re: [PATCH v8 2/5] ASoC: hdmi-codec: Use HDMI notifications to add jack support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 274 bytes --]

On Thu, Aug 11, 2016 at 11:20:24AM +0200, Philipp Zabel wrote:
> Use HDMI connection / disconnection notifications to update an ALSA
> jack object. Also make a copy of the ELD block after every change.

Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v8 3/5] ASoC: mediatek: Add jack detection support to mt8173-rt5650-rt5676 machine driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 216 bytes --]

On Thu, Aug 11, 2016 at 11:20:25AM +0200, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek

Re: [PATCH v8 4/5] ASoC: mediatek: Add jack detection support to the mt8173-rt5650 machine driver

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 216 bytes --]

On Thu, Aug 11, 2016 at 11:20:26AM +0200, Philipp Zabel wrote:
> Signed-off-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>

Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 200 bytes --]

_______________________________________________
Linux-mediatek mailing list
Linux-mediatek-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
http://lists.infradead.org/mailman/listinfo/linux-mediatek