| Message ID | 20170414102512.48834-9-hverkuil@xs4all.nl (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Tomi, I did some more testing, and I discovered a bug in this code, but I am not sure how to solve it. On 04/14/2017 12:25 PM, Hans Verkuil wrote: > From: Hans Verkuil <hans.verkuil@cisco.com> > > Hook up the HDMI CEC support in the hdmi4 driver. > > It add the CEC irq handler, the CEC (un)init calls and tells the CEC > implementation when the physical address changes. > > Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com> > --- > drivers/gpu/drm/omapdrm/dss/Kconfig | 9 +++++++++ > drivers/gpu/drm/omapdrm/dss/Makefile | 1 + > drivers/gpu/drm/omapdrm/dss/hdmi4.c | 23 ++++++++++++++++++++++- > 3 files changed, 32 insertions(+), 1 deletion(-) > <snip> > diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c > index e371b47ff6ff..ebe5b27cee6f 100644 > --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c > +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c <snip> > @@ -392,6 +401,8 @@ static void hdmi_display_disable(struct omap_dss_device *dssdev) > > DSSDBG("Enter hdmi_display_disable\n"); > > + hdmi4_cec_set_phys_addr(&hdmi.core, CEC_PHYS_ADDR_INVALID); > + > mutex_lock(&hdmi.lock); > > spin_lock_irqsave(&hdmi.audio_playing_lock, flags); My assumption was that hdmi_display_disable() was called when the hotplug would go away. But I discovered that that isn't the case, or at least not when X is running. It seems that the actual HPD check is done in hdmic_detect() in omapdrm/displays/connector-hdmi.c. But there I have no access to hdmi.core (needed for the hdmi4_cec_set_phys_addr() call). Any idea how to solve this? I am not all that familiar with drm, let alone omapdrm, so if you can point me in the right direction, then that would be very helpful. Regards, Hans
On 06/05/17 14:58, Hans Verkuil wrote: > My assumption was that hdmi_display_disable() was called when the hotplug would go > away. But I discovered that that isn't the case, or at least not when X is running. > It seems that the actual HPD check is done in hdmic_detect() in > omapdrm/displays/connector-hdmi.c. For some HW it's done there (in the case there's no IP handling the HPD), but in some cases it's done in tpd12s015 driver (e.g. pandaboard), and in some cases it also could be done in the hdmi driver (if the HPD is handled by the HDMI IP, but at the moment we don't have this case supported in the SW). > But there I have no access to hdmi.core (needed for the hdmi4_cec_set_phys_addr() call). > > Any idea how to solve this? I am not all that familiar with drm, let alone omapdrm, > so if you can point me in the right direction, then that would be very helpful. Hmm, indeed, looks the the output is kept enabled even if HPD drops and the connector status is changed to disconnected. I don't have a very good solution... I think we have to add a function to omapdss_hdmi_ops, which the connector-hdmi and tpd12s015 drivers can call when they detect a HPD change. That call would go to the HDMI IP driver. Peter is about to send hotplug-interrupt-handling series, I think the HPD function work should be done on top of that, as otherwise it'll just conflict horribly. Tomi
On 05/08/2017 12:26 PM, Tomi Valkeinen wrote: > On 06/05/17 14:58, Hans Verkuil wrote: > >> My assumption was that hdmi_display_disable() was called when the hotplug would go >> away. But I discovered that that isn't the case, or at least not when X is running. >> It seems that the actual HPD check is done in hdmic_detect() in >> omapdrm/displays/connector-hdmi.c. > > For some HW it's done there (in the case there's no IP handling the > HPD), but in some cases it's done in tpd12s015 driver (e.g. pandaboard), > and in some cases it also could be done in the hdmi driver (if the HPD > is handled by the HDMI IP, but at the moment we don't have this case > supported in the SW). > >> But there I have no access to hdmi.core (needed for the hdmi4_cec_set_phys_addr() call). >> >> Any idea how to solve this? I am not all that familiar with drm, let alone omapdrm, >> so if you can point me in the right direction, then that would be very helpful. > > Hmm, indeed, looks the the output is kept enabled even if HPD drops and > the connector status is changed to disconnected. > > I don't have a very good solution... I think we have to add a function > to omapdss_hdmi_ops, which the connector-hdmi and tpd12s015 drivers can > call when they detect a HPD change. That call would go to the HDMI IP > driver. Right, I was thinking the same, I just wasn't sure if that was the correct solution. > Peter is about to send hotplug-interrupt-handling series, I think the > HPD function work should be done on top of that, as otherwise it'll just > conflict horribly. OK, I'll do that. I'll get CEC supported on the omap4 eventually! :-) Regards, Hans
Hi Tomi, On 08/05/17 12:26, Tomi Valkeinen wrote: > On 06/05/17 14:58, Hans Verkuil wrote: > >> My assumption was that hdmi_display_disable() was called when the hotplug would go >> away. But I discovered that that isn't the case, or at least not when X is running. >> It seems that the actual HPD check is done in hdmic_detect() in >> omapdrm/displays/connector-hdmi.c. > > For some HW it's done there (in the case there's no IP handling the > HPD), but in some cases it's done in tpd12s015 driver (e.g. pandaboard), > and in some cases it also could be done in the hdmi driver (if the HPD > is handled by the HDMI IP, but at the moment we don't have this case > supported in the SW). > >> But there I have no access to hdmi.core (needed for the hdmi4_cec_set_phys_addr() call). >> >> Any idea how to solve this? I am not all that familiar with drm, let alone omapdrm, >> so if you can point me in the right direction, then that would be very helpful. > > Hmm, indeed, looks the the output is kept enabled even if HPD drops and > the connector status is changed to disconnected. > > I don't have a very good solution... I think we have to add a function > to omapdss_hdmi_ops, which the connector-hdmi and tpd12s015 drivers can > call when they detect a HPD change. That call would go to the HDMI IP > driver. > > Peter is about to send hotplug-interrupt-handling series, I think the > HPD function work should be done on top of that, as otherwise it'll just > conflict horribly. Has that been merged yet? And if so, what git repo/branch should I base my next version of this patch series on? If not, do you know when it is expected? Regards, Hans
On 08/06/17 10:34, Hans Verkuil wrote: >> Peter is about to send hotplug-interrupt-handling series, I think the >> HPD function work should be done on top of that, as otherwise it'll just >> conflict horribly. > > Has that been merged yet? And if so, what git repo/branch should I base > my next version of this patch series on? If not, do you know when it is > expected? No, still pending review. The patches ("[PATCH v2 0/3] drm/omap: Support for hotplug detection") apply on top of latest drm-next, if you want to try. Tomi
Hi Tomi, On 06/08/2017 11:19 AM, Tomi Valkeinen wrote: > On 08/06/17 10:34, Hans Verkuil wrote: > >>> Peter is about to send hotplug-interrupt-handling series, I think the >>> HPD function work should be done on top of that, as otherwise it'll just >>> conflict horribly. >> >> Has that been merged yet? And if so, what git repo/branch should I base >> my next version of this patch series on? If not, do you know when it is >> expected? > > No, still pending review. The patches ("[PATCH v2 0/3] drm/omap: Support > for hotplug detection") apply on top of latest drm-next, if you want to try. I gather[1] that this has been merged? Where can I find a git tree that has these patches? I'd like to get the omap4 CEC support in for 4.14. Regards, Hans [1]: http://www.spinics.net/lists/dri-devel/msg143440.html
diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig index d1fa730c7d54..d18e83902b74 100644 --- a/drivers/gpu/drm/omapdrm/dss/Kconfig +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig @@ -71,9 +71,18 @@ config OMAP4_DSS_HDMI bool "HDMI support for OMAP4" default y select OMAP2_DSS_HDMI_COMMON + select MEDIA_CEC_EDID help HDMI support for OMAP4 based SoCs. +config OMAP4_DSS_HDMI_CEC + bool "Enable HDMI CEC support for OMAP4" + depends on OMAP4_DSS_HDMI + select MEDIA_CEC_SUPPORT + default y + ---help--- + When selected the HDMI transmitter will support the CEC feature. + config OMAP5_DSS_HDMI bool "HDMI support for OMAP5" default n diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile index b651ec9751e6..d1c6acfbd134 100644 --- a/drivers/gpu/drm/omapdrm/dss/Makefile +++ b/drivers/gpu/drm/omapdrm/dss/Makefile @@ -11,5 +11,6 @@ omapdss-$(CONFIG_OMAP2_DSS_DSI) += dsi.o omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_common.o hdmi_wp.o hdmi_pll.o \ hdmi_phy.o omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.o +omapdss-$(CONFIG_OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o omapdss-$(CONFIG_OMAP5_DSS_HDMI) += hdmi5.o hdmi5_core.o ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c index e371b47ff6ff..ebe5b27cee6f 100644 --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c @@ -35,9 +35,11 @@ #include <linux/component.h> #include <linux/of.h> #include <sound/omap-hdmi-audio.h> +#include <media/cec-edid.h> #include "omapdss.h" #include "hdmi4_core.h" +#include "hdmi4_cec.h" #include "dss.h" #include "dss_features.h" #include "hdmi.h" @@ -96,6 +98,13 @@ static irqreturn_t hdmi_irq_handler(int irq, void *data) } else if (irqstatus & HDMI_IRQ_LINK_DISCONNECT) { hdmi_wp_set_phy_pwr(wp, HDMI_PHYPWRCMD_LDOON); } + if (irqstatus & HDMI_IRQ_CORE) { + u32 intr4 = hdmi_read_reg(hdmi->core.base, HDMI_CORE_SYS_INTR4); + + hdmi_write_reg(hdmi->core.base, HDMI_CORE_SYS_INTR4, intr4); + if (intr4 & 8) + hdmi4_cec_irq(&hdmi->core); + } return IRQ_HANDLED; } @@ -392,6 +401,8 @@ static void hdmi_display_disable(struct omap_dss_device *dssdev) DSSDBG("Enter hdmi_display_disable\n"); + hdmi4_cec_set_phys_addr(&hdmi.core, CEC_PHYS_ADDR_INVALID); + mutex_lock(&hdmi.lock); spin_lock_irqsave(&hdmi.audio_playing_lock, flags); @@ -492,7 +503,11 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev, } r = read_edid(edid, len); - + if (r >= 256) + hdmi4_cec_set_phys_addr(&hdmi.core, + cec_get_edid_phys_addr(edid, r, NULL)); + else + hdmi4_cec_set_phys_addr(&hdmi.core, CEC_PHYS_ADDR_INVALID); if (need_enable) hdmi4_core_disable(dssdev); @@ -728,6 +743,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data) if (r) goto err; + r = hdmi4_cec_init(pdev, &hdmi.core, &hdmi.wp); + if (r) + goto err; + irq = platform_get_irq(pdev, 0); if (irq < 0) { DSSERR("platform_get_irq failed\n"); @@ -772,6 +791,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data) hdmi_uninit_output(pdev); + hdmi4_cec_uninit(&hdmi.core); + hdmi_pll_uninit(&hdmi.pll); pm_runtime_disable(&pdev->dev);