diff mbox

[RFC,V2,0/5] another generic audio hdmi codec proposal

Message ID 20151006185157.GT21513@n2100.arm.linux.org.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Russell King - ARM Linux Oct. 6, 2015, 6:51 p.m. UTC
On Tue, Oct 06, 2015 at 05:46:02PM +0100, Mark Brown wrote:
> On Tue, Oct 06, 2015 at 06:44:34PM +0300, Jyri Sarha wrote:
> > On 10/06/15 12:23, Arnaud Pouliquen wrote:
> 
> > >In a first step, before going deep in discussion on the approach, it
> > >should be interesting to have maintainers feedback, to be sure that my
> > >approach could make sense from DRM and ALSA point of view.
> 
> > Absolutely. In the end the maintainers need to make the final decision any
> > way.
> 
> As I keep saying at least for me a very big part of that is going to be
> seeing some degree of consensus among the people working on HDMI.  I
> really don't want to get something merged and then having people turn up
> shortly afterwards saying that there's some reason to redo everything
> and I don't have much familiarity with the video stuff so I'm largely
> relying on consensus and review from the DRM side (which seems to have
> been absent thus far).

As I've found, having also looked at getting CEC up and running both on
TDA998x and on dw-hdmi, having some kind of fixed structure between the
video side and the audio side doesn't work.

We at least need some kind of notification system from the video part
to the other parts, so that we can sanely transmit connect/disconnect/
edid updates to different parts of the system *not* that I'm suggesting
that in the case of the Dove Cubox, seeing a disconnect on HDMI should
kill the audio - it most _certainly_ _never_ should there, it would be
utterly insane, and I'm going to play hard-ball on that case.

CEC is much more critical to getting these events - knowing when the
sink device has been plugged in or unplugged is critical to proper
operation of CEC, as is knowing the CEC physical bus address - and
that comes from the EDID.  Hence why a notification system that drivers
can hook into is critical.

Audio is just another example of needing to have communication between
the video and audio parts.

I'm not saying that I have something that's a perfect solution; I've
been rapidly losing interest in iMX6 recently due to the glacial
progress of everything in that arena, and the endless stream of new
bugs being introduced (I'm pretty sure Freescale have a bug quota
which must be maintained at a certain level in all software.)  As I
now have a couple of new Armada 388 boards, this has taken my focus
almost completely off iMX6 and onto that right now.

However, here's the kind of thing that I prototyped to get CEC working
on iMX6 and Dove.  It's not a patch set, because that requires me to
pull together patches from two different trees and sort out out properly,
so this is to give some ideas.

It's also pending sorting out what CEC subsystem is going to make it
into the kernel - I've only last night managed to find the time to try
out the framework which Hans Verkuil posted at the start of September,
which I still have a few concerns with, especially with the publication
of the device prior to driver setup having been completed.

Anyway, here's the prototype patch.  Obviously won't apply to any
kernel version anyone has, but should give some food for thought.

 drivers/cec/Makefile             |  1 +
 drivers/cec/cec-dw_hdmi.c        | 90 ++++++++++++++++++++++++++++++++++++++++
 drivers/cec/hdmi-not.c           | 61 +++++++++++++++++++++++++++
 drivers/gpu/drm/bridge/dw_hdmi.c |  9 ++++
 include/linux/hdmi-not.h         | 39 +++++++++++++++++
 5 files changed, 200 insertions(+)

Comments

Jyri Sarha Oct. 7, 2015, 7:36 a.m. UTC | #1
On 10/06/15 21:51, Russell King - ARM Linux wrote:
> We at least need some kind of notification system from the video part
> to the other parts, so that we can sanely transmit connect/disconnect/
> edid updates to different parts of the system*not*  that I'm suggesting
> that in the case of the Dove Cubox, seeing a disconnect on HDMI should
> kill the audio - it most_certainly_  _never_  should there, it would be
> utterly insane, and I'm going to play hard-ball on that case.

I am not proposing any automatic mechanism for killing the audio in the 
my hdmi codec patch set. I am merely providing the means for the video 
driver to gracefully abort the audio stream if the driver decides it can 
not support it any more.

For external HDMI encoders that is hardly ever needed, since the CPU DAI 
is usually the bit- and frame-clock master in those cases. The CPU DAI 
can go on banging the bits even if the encoder isn't listening. So we 
can drop the abort_cb() that if it still annoys you after this explanation.

Best regards,
Jyri
diff mbox

Patch

diff --git a/drivers/cec/Makefile b/drivers/cec/Makefile
index 9e31a9333294..69c354c60b07 100644
--- a/drivers/cec/Makefile
+++ b/drivers/cec/Makefile
@@ -1,3 +1,4 @@ 
+obj-y += hdmi-not.o
 obj-$(CONFIG_HDMI_CEC_CORE) += cec-dev.o
 obj-$(CONFIG_HDMI_CEC_EVDEV) += cec-input.o
 
diff --git a/drivers/cec/cec-dw_hdmi.c b/drivers/cec/cec-dw_hdmi.c
index b56844296d3e..a693808a881a 100644
--- a/drivers/cec/cec-dw_hdmi.c
+++ b/drivers/cec/cec-dw_hdmi.c
@@ -1,14 +1,18 @@ 
 /* http://git.freescale.com/git/cgit.cgi/imx/linux-2.6-imx.git/
  * tree/drivers/mxc/hdmi-cec/mxc_hdmi-cec.c?h=imx_3.0.35_4.1.0 */
 #include <linux/cec-dev.h>
+#include <linux/hdmi-not.h>
 #include <linux/interrupt.h>
 #include <linux/io.h>
 #include <linux/module.h>
+#include <linux/notifier.h>
 #include <linux/platform_data/dw_hdmi-cec.h>
 #include <linux/platform_device.h>
 #include <linux/sched.h>
 #include <linux/slab.h>
 
+#include <drm/drm_edid.h>
+
 #define DEV_NAME "mxc_hdmi_cec"
 
 enum {
@@ -47,6 +51,8 @@  struct dw_hdmi_cec {
 	void *ops_data;
 	int retries;
 	int irq;
+	struct cec_dev *cecdev;
+	struct notifier_block nb;
 };
 
 static void dw_hdmi_set_address(struct cec_dev *cecdev, unsigned addresses)
@@ -162,6 +168,83 @@  static const struct cec_dev_ops dw_hdmi_cec_dev_ops = {
 	.set_address = dw_hdmi_set_address,
 };
 
+static unsigned int cea_len(const u8 *ext, unsigned int offset)
+{
+	return ext[offset] & 0x1f;
+}
+
+static unsigned int cea_tag(const u8 *ext, unsigned int offset)
+{
+	return ext[offset] >> 5;
+}
+
+static unsigned int parse_hdmi_addr(const struct edid *edid)
+{
+	unsigned int i, end;
+	const u8 *edid_ext = NULL;
+
+	if (!edid || edid->extensions == 0)
+		return (u16)~0;
+
+	for (i = 0; i < edid->extensions; i++) {
+		edid_ext = (u8 *)edid + EDID_LENGTH * (i + 1);
+		if (edid_ext[0] == CEA_EXT)
+			break;
+		edid_ext = NULL;
+	}
+
+	if (!edid_ext)
+		return (u16)~0;
+
+	end = edid_ext[2];
+	if (end < 4 || end > 127)
+		return (u16)~0;
+
+	for (i = 4; i < end && i + cea_len(edid_ext, i) < end; i += 1 + cea_len(edid_ext, i)) {
+pr_info("cea tag %u len %u\n", cea_tag(edid_ext, i), cea_len(edid_ext, i));
+		if (cea_tag(edid_ext, i) != 3 || cea_len(edid_ext, i) < 5)
+			continue;
+
+		if (edid_ext[i + 1] != 0x03 ||
+		    edid_ext[i + 2] != 0x0c ||
+		    edid_ext[i + 3] != 0x00)
+			continue;
+
+		return edid_ext[i + 4] << 8 | edid_ext[i + 5];
+	}
+
+	return (u16)~0;
+}
+
+static int dw_hdmi_cec_notify(struct notifier_block *nb, unsigned long event,
+			      void *data)
+{
+	struct dw_hdmi_cec *cec = container_of(nb, struct dw_hdmi_cec, nb);
+	union hdmi_event *event_block = data;
+	unsigned int phys;
+
+	dev_info(cec->cecdev->class_dev.parent, "event %lu\n", event);
+
+	if (event_block->base.source != cec->cecdev->class_dev.parent)
+		return NOTIFY_OK;
+
+	switch (event) {
+	case HDMI_CONNECTED:
+		break;
+
+	case HDMI_DISCONNECTED:
+		cec_dev_disconnect(cec->cecdev);
+		break;
+
+	case HDMI_NEW_EDID:
+		phys = parse_hdmi_addr(event_block->edid.edid);
+		cec_dev_connect(cec->cecdev, phys);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
 static int dw_hdmi_cec_probe(struct platform_device *pdev)
 {
 	struct dw_hdmi_cec_data *data = dev_get_platdata(&pdev->dev);
@@ -186,6 +269,8 @@  static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	cec->irq = data->irq;
 	cec->ops = data->ops;
 	cec->ops_data = data->ops_data;
+	cec->cecdev = cecdev;
+	cec->nb.notifier_call = dw_hdmi_cec_notify;
 
 	cecdev->ops = &dw_hdmi_cec_dev_ops;
 
@@ -197,6 +282,8 @@  static int dw_hdmi_cec_probe(struct platform_device *pdev)
 	writeb_relaxed(~0, cec->base + HDMI_IH_MUTE_CEC_STAT0);
 	writeb_relaxed(0, cec->base + HDMI_CEC_POLARITY);
 
+	hdmi_register_notifier(&cec->nb);
+
 	platform_set_drvdata(pdev, cecdev);
 
 	ret = cec_dev_add(cecdev);
@@ -209,6 +296,9 @@  static int dw_hdmi_cec_probe(struct platform_device *pdev)
 static int dw_hdmi_cec_remove(struct platform_device *pdev)
 {
 	struct cec_dev *cecdev = platform_get_drvdata(pdev);
+	struct dw_hdmi_cec *cec = cecdev_priv(cecdev);
+
+	hdmi_unregister_notifier(&cec->nb);
 
 	cec_dev_remove(cecdev);
 	cec_dev_free(cecdev);
diff --git a/drivers/cec/hdmi-not.c b/drivers/cec/hdmi-not.c
new file mode 100644
index 000000000000..ba3be8afeb8f
--- /dev/null
+++ b/drivers/cec/hdmi-not.c
@@ -0,0 +1,61 @@ 
+#include <linux/export.h>
+#include <linux/hdmi-not.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/drivers/gpu/drm/bridge/dw_hdmi.c b/drivers/gpu/drm/bridge/dw_hdmi.c
index 41d08f0cf88c..cb8764eecd70 100644
--- a/drivers/gpu/drm/bridge/dw_hdmi.c
+++ b/drivers/gpu/drm/bridge/dw_hdmi.c
@@ -16,6 +16,7 @@ 
 #include <linux/err.h>
 #include <linux/clk.h>
 #include <linux/hdmi.h>
+#include <linux/hdmi-not.h>
 #include <linux/mutex.h>
 #include <linux/of_device.h>
 #include <linux/platform_data/dw_hdmi-cec.h>
@@ -1484,9 +1485,11 @@  static int dw_hdmi_connector_get_modes(struct drm_connector *connector)
 		hdmi->sink_is_hdmi = drm_detect_hdmi_monitor(edid);
 		hdmi->sink_has_audio = drm_detect_monitor_audio(edid);
 		drm_mode_connector_update_edid_property(connector, edid);
+		hdmi_event_new_edid(hdmi->dev, edid, 0);
 		ret = drm_add_edid_modes(connector, edid);
 		/* Store the ELD */
 		drm_edid_to_eld(connector, edid);
+		hdmi_event_new_eld(hdmi->dev, connector->eld);
 		kfree(edid);
 	} else {
 		dev_dbg(hdmi->dev, "failed to get edid\n");
@@ -1648,6 +1651,12 @@  static irqreturn_t dw_hdmi_irq(int irq, void *dev_id)
 			dw_hdmi_update_phy_mask(hdmi);
 		}
 		mutex_unlock(&hdmi->mutex);
+
+		if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) == 0)
+			hdmi_event_disconnect(hdmi->dev);
+		else if ((phy_stat & (HDMI_PHY_RX_SENSE | HDMI_PHY_HPD)) ==
+			 (HDMI_IH_PHY_STAT0_RX_SENSE | HDMI_PHY_HPD))
+			hdmi_event_connect(hdmi->dev);
 	}
 
 	if (intr_stat & HDMI_IH_PHY_STAT0_HPD) {
diff --git a/include/linux/hdmi-not.h b/include/linux/hdmi-not.h
new file mode 100644
index 000000000000..940ece45bbaf
--- /dev/null
+++ b/include/linux/hdmi-not.h
@@ -0,0 +1,39 @@ 
+#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);