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(+)
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);