All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC PATCH 0/3] OMAP4 HDMI CEC support
@ 2016-04-29  9:39 ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, dri-devel

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series sits on top of my earlier HDMI CEC framework:

http://www.spinics.net/lists/linux-media/msg99847.html

It is an RFC patch for now as I want to clean up hdmi_cec a bit more
and do a bit more testing.

Many thanks to Tomi for finding obscure problems in the omap4 drivers
that prevented CEC from working on my pandaboard.

Feedback is welcome!

Regards,

	Hans

Hans Verkuil (2):
  omap4: add CEC support
  encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid

Tomi Valkeinen (1):
  drm/omap: fix OMAP4 hdmi_core_powerdown_disable()

 arch/arm/boot/dts/omap4-panda-a4.dts               |   2 +-
 arch/arm/boot/dts/omap4-panda-es.dts               |   2 +-
 arch/arm/boot/dts/omap4.dtsi                       |   5 +-
 .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c   |  13 +-
 drivers/gpu/drm/omapdrm/dss/Kconfig                |   8 +
 drivers/gpu/drm/omapdrm/dss/Makefile               |   3 +
 drivers/gpu/drm/omapdrm/dss/hdmi.h                 |  62 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c                |  39 ++-
 drivers/gpu/drm/omapdrm/dss/hdmi4_core.c           |   2 +-
 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c             | 329 +++++++++++++++++++++
 10 files changed, 454 insertions(+), 11 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

-- 
2.8.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [RFC PATCH 0/3] OMAP4 HDMI CEC support
@ 2016-04-29  9:39 ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, dri-devel

From: Hans Verkuil <hans.verkuil@cisco.com>

This patch series sits on top of my earlier HDMI CEC framework:

http://www.spinics.net/lists/linux-media/msg99847.html

It is an RFC patch for now as I want to clean up hdmi_cec a bit more
and do a bit more testing.

Many thanks to Tomi for finding obscure problems in the omap4 drivers
that prevented CEC from working on my pandaboard.

Feedback is welcome!

Regards,

	Hans

Hans Verkuil (2):
  omap4: add CEC support
  encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid

Tomi Valkeinen (1):
  drm/omap: fix OMAP4 hdmi_core_powerdown_disable()

 arch/arm/boot/dts/omap4-panda-a4.dts               |   2 +-
 arch/arm/boot/dts/omap4-panda-es.dts               |   2 +-
 arch/arm/boot/dts/omap4.dtsi                       |   5 +-
 .../gpu/drm/omapdrm/displays/encoder-tpd12s015.c   |  13 +-
 drivers/gpu/drm/omapdrm/dss/Kconfig                |   8 +
 drivers/gpu/drm/omapdrm/dss/Makefile               |   3 +
 drivers/gpu/drm/omapdrm/dss/hdmi.h                 |  62 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c                |  39 ++-
 drivers/gpu/drm/omapdrm/dss/hdmi4_core.c           |   2 +-
 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c             | 329 +++++++++++++++++++++
 10 files changed, 454 insertions(+), 11 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* [RFC PATCH 1/3] drm/omap: fix OMAP4 hdmi_core_powerdown_disable()
  2016-04-29  9:39 ` Hans Verkuil
@ 2016-04-29  9:39   ` Hans Verkuil
  -1 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, dri-devel

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

hdmi_core_powerdown_disable() is supposed to disable HDMI core's
power-down mode. However, the function sets the power-down bit to 0,
which means "enable power-down".

This hasn't caused any issues as the PD seems to affect only interrupts
from HDMI core, and none of those interrupts are used at the moment. CEC
functionality requires core interrupts, and the PD mode needs to be
fixed.

This patch fixes hdmi_core_powerdown_disable() to actually disable the
PD mode.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
index fa72e73..ef3afe9 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
@@ -211,7 +211,7 @@ static void hdmi_core_init(struct hdmi_core_video_config *video_cfg)
 static void hdmi_core_powerdown_disable(struct hdmi_core_data *core)
 {
 	DSSDBG("Enter hdmi_core_powerdown_disable\n");
-	REG_FLD_MOD(core->base, HDMI_CORE_SYS_SYS_CTRL1, 0x0, 0, 0);
+	REG_FLD_MOD(core->base, HDMI_CORE_SYS_SYS_CTRL1, 0x1, 0, 0);
 }
 
 static void hdmi_core_swreset_release(struct hdmi_core_data *core)
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH 1/3] drm/omap: fix OMAP4 hdmi_core_powerdown_disable()
@ 2016-04-29  9:39   ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, dri-devel

From: Tomi Valkeinen <tomi.valkeinen@ti.com>

hdmi_core_powerdown_disable() is supposed to disable HDMI core's
power-down mode. However, the function sets the power-down bit to 0,
which means "enable power-down".

This hasn't caused any issues as the PD seems to affect only interrupts
from HDMI core, and none of those interrupts are used at the moment. CEC
functionality requires core interrupts, and the PD mode needs to be
fixed.

This patch fixes hdmi_core_powerdown_disable() to actually disable the
PD mode.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Reported-by: Hans Verkuil <hverkuil@xs4all.nl>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
index fa72e73..ef3afe9 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_core.c
@@ -211,7 +211,7 @@ static void hdmi_core_init(struct hdmi_core_video_config *video_cfg)
 static void hdmi_core_powerdown_disable(struct hdmi_core_data *core)
 {
 	DSSDBG("Enter hdmi_core_powerdown_disable\n");
-	REG_FLD_MOD(core->base, HDMI_CORE_SYS_SYS_CTRL1, 0x0, 0, 0);
+	REG_FLD_MOD(core->base, HDMI_CORE_SYS_SYS_CTRL1, 0x1, 0, 0);
 }
 
 static void hdmi_core_swreset_release(struct hdmi_core_data *core)
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH 2/3] omap4: add CEC support
  2016-04-29  9:39 ` Hans Verkuil
@ 2016-04-29  9:39   ` Hans Verkuil
  -1 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
 arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
 arch/arm/boot/dts/omap4.dtsi           |   5 +-
 drivers/gpu/drm/omapdrm/dss/Kconfig    |   8 +
 drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
 drivers/gpu/drm/omapdrm/dss/hdmi.h     |  62 ++++++-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c    |  39 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 +++++++++++++++++++++++++++++++++
 8 files changed, 441 insertions(+), 9 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts b/arch/arm/boot/dts/omap4-panda-a4.dts
index 78d3631..f0c1020 100644
--- a/arch/arm/boot/dts/omap4-panda-a4.dts
+++ b/arch/arm/boot/dts/omap4-panda-a4.dts
@@ -13,7 +13,7 @@
 /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
 &dss_hdmi_pins {
 	pinctrl-single,pins = <
-		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
+		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
 		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
 		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
 		>;
diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts
index 119f8e6..940fe4f 100644
--- a/arch/arm/boot/dts/omap4-panda-es.dts
+++ b/arch/arm/boot/dts/omap4-panda-es.dts
@@ -34,7 +34,7 @@
 /* PandaboardES has external pullups on SCL & SDA */
 &dss_hdmi_pins {
 	pinctrl-single,pins = <
-		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
+		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)		/* hdmi_cec.hdmi_cec */
 		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
 		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
 		>;
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 2bd9c83..1bb490f 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -1006,8 +1006,9 @@
 				reg = <0x58006000 0x200>,
 				      <0x58006200 0x100>,
 				      <0x58006300 0x100>,
-				      <0x58006400 0x1000>;
-				reg-names = "wp", "pll", "phy", "core";
+				      <0x58006400 0x900>,
+				      <0x58006D00 0x100>;
+				reg-names = "wp", "pll", "phy", "core", "cec";
 				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 				ti,hwmods = "dss_hdmi";
diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
index d1fa730..69638e9 100644
--- a/drivers/gpu/drm/omapdrm/dss/Kconfig
+++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
@@ -71,9 +71,17 @@ 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 OMAP2_DSS_HDMI_CEC
+	bool "Enable HDMI CEC support for OMAP4"
+	depends on OMAP4_DSS_HDMI && MEDIA_CEC
+	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 b651ec9..37eb597 100644
--- a/drivers/gpu/drm/omapdrm/dss/Makefile
+++ b/drivers/gpu/drm/omapdrm/dss/Makefile
@@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
 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
+ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
+  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
+endif
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.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/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
index 53616b0..7cf8a91 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/hdmi.h>
 #include <video/omapdss.h>
+#include <media/cec.h>
 
 #include "dss.h"
 
@@ -83,6 +84,31 @@
 #define HDMI_TXPHY_PAD_CFG_CTRL			0xC
 #define HDMI_TXPHY_BIST_CONTROL			0x1C
 
+/* HDMI CEC */
+#define HDMI_CEC_DEV_ID                         0x0
+#define HDMI_CEC_SPEC                           0x4
+#define HDMI_CEC_DBG_3                          0x1C
+#define HDMI_CEC_TX_INIT                        0x20
+#define HDMI_CEC_TX_DEST                        0x24
+#define HDMI_CEC_SETUP                          0x38
+#define HDMI_CEC_TX_COMMAND                     0x3C
+#define HDMI_CEC_TX_OPERAND                     0x40
+#define HDMI_CEC_TRANSMIT_DATA                  0x7C
+#define HDMI_CEC_CA_7_0                         0x88
+#define HDMI_CEC_CA_15_8                        0x8C
+#define HDMI_CEC_INT_STATUS_0                   0x98
+#define HDMI_CEC_INT_STATUS_1                   0x9C
+#define HDMI_CEC_INT_ENABLE_0                   0x90
+#define HDMI_CEC_INT_ENABLE_1                   0x94
+#define HDMI_CEC_RX_CONTROL                     0xB0
+#define HDMI_CEC_RX_COUNT                       0xB4
+#define HDMI_CEC_RX_CMD_HEADER                  0xB8
+#define HDMI_CEC_RX_COMMAND                     0xBC
+#define HDMI_CEC_RX_OPERAND                     0xC0
+
+#define HDMI_CEC_TX_FIFO_INT_MASK		0x64
+#define HDMI_CEC_RETRANSMIT_CNT_INT_MASK	0x2
+
 enum hdmi_pll_pwr {
 	HDMI_PLLPWRCMD_ALLOFF = 0,
 	HDMI_PLLPWRCMD_PLLONLY = 1,
@@ -250,6 +276,12 @@ struct hdmi_phy_data {
 	u8 lane_polarity[4];
 };
 
+struct hdmi_cec_data {
+	void __iomem *base;
+	struct cec_adapter *adap;
+	u16 phys_addr;
+};
+
 struct hdmi_core_data {
 	void __iomem *base;
 };
@@ -319,6 +351,33 @@ void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s);
 int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy);
 int hdmi_phy_parse_lanes(struct hdmi_phy_data *phy, const u32 *lanes);
 
+/* HDMI CEC funcs */
+#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
+void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa);
+void hdmi_cec_irq(struct hdmi_cec_data *cec);
+int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec);
+void hdmi_cec_uninit(struct hdmi_cec_data *cec);
+#else
+static inline void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
+{
+}
+
+static inline void hdmi_cec_irq(struct hdmi_cec_data *cec)
+{
+}
+
+static inline int hdmi_cec_init(struct platform_device *pdev,
+				struct hdmi_cec_data *cec)
+{
+	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
+	return 0;
+}
+
+static inline void hdmi_cec_uninit(struct hdmi_cec_data *cec)
+{
+}
+#endif
+
 /* HDMI common funcs */
 int hdmi_parse_lanes_of(struct platform_device *pdev, struct device_node *ep,
 	struct hdmi_phy_data *phy);
@@ -344,6 +403,7 @@ struct omap_hdmi {
 	struct hdmi_wp_data	wp;
 	struct hdmi_pll_data	pll;
 	struct hdmi_phy_data	phy;
+	struct hdmi_cec_data	cec;
 	struct hdmi_core_data	core;
 
 	struct hdmi_config cfg;
@@ -361,7 +421,7 @@ struct omap_hdmi {
 	bool audio_configured;
 	struct omap_dss_audio audio_config;
 
-	/* This lock should be taken when booleans bellow are touched. */
+	/* This lock should be taken when booleans below are touched. */
 	spinlock_t audio_playing_lock;
 	bool audio_playing;
 	bool display_enabled;
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index f892ae15..47a60bf 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
 #include <video/omapdss.h>
+#include <media/cec-edid.h>
 #include <sound/omap-hdmi-audio.h>
 
 #include "hdmi4_core.h"
@@ -69,14 +70,15 @@ static void hdmi_runtime_put(void)
 
 static irqreturn_t hdmi_irq_handler(int irq, void *data)
 {
-	struct hdmi_wp_data *wp = data;
+	struct omap_hdmi *hdmi = data;
+	struct hdmi_wp_data *wp = &hdmi->wp;
 	u32 irqstatus;
 
 	irqstatus = hdmi_wp_get_irqstatus(wp);
 	hdmi_wp_set_irqstatus(wp, irqstatus);
 
 	if ((irqstatus & HDMI_IRQ_LINK_CONNECT) &&
-			irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
+	    (irqstatus & HDMI_IRQ_LINK_DISCONNECT)) {
 		/*
 		 * If we get both connect and disconnect interrupts at the same
 		 * time, turn off the PHY, clear interrupts, and restart, which
@@ -94,6 +96,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)
+			hdmi_cec_irq(&hdmi->cec);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -213,6 +222,12 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 
 	hdmi4_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
 
+	/* Initialize CEC clock divider */
+	/* CEC needs 2MHz clock hence set the devider to 24 to get
+	   48/24=2MHz clock */
+	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_CLK, 0x18, 5, 0);
+	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
+
 	/* bypass TV gamma table */
 	dispc_enable_gamma_table(0);
 
@@ -228,7 +243,11 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 		goto err_vid_enable;
 
 	hdmi_wp_set_irqenable(wp,
-		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
+		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT |
+		HDMI_IRQ_CORE);
+
+	/* Unmask CEC interrupt */
+	REG_FLD_MOD(hdmi.core.base, HDMI_CORE_SYS_INTR_UNMASK4, 0x1, 3, 3);
 
 	return 0;
 
@@ -250,6 +269,8 @@ static void hdmi_power_off_full(struct omap_dss_device *dssdev)
 	enum omap_channel channel = dssdev->dispc_channel;
 
 	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
+	hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
+	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
 
 	hdmi_wp_video_stop(&hdmi.wp);
 
@@ -488,6 +509,10 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev,
 	}
 
 	r = read_edid(edid, len);
+	if (r >= 256)
+		hdmi.cec.phys_addr = cec_get_edid_phys_addr(edid, r, NULL);
+	else
+		hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
 
 	if (need_enable)
 		hdmi_core_disable(dssdev);
@@ -724,6 +749,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		goto err;
 
+	r = hdmi_cec_init(pdev, &hdmi.cec);
+	if (r)
+		goto err;
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		DSSERR("platform_get_irq failed\n");
@@ -733,7 +762,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 
 	r = devm_request_threaded_irq(&pdev->dev, irq,
 			NULL, hdmi_irq_handler,
-			IRQF_ONESHOT, "OMAP HDMI", &hdmi.wp);
+			IRQF_ONESHOT, "OMAP HDMI", &hdmi);
 	if (r) {
 		DSSERR("HDMI IRQ request failed\n");
 		goto err;
@@ -768,6 +797,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
 
 	hdmi_uninit_output(pdev);
 
+	hdmi_cec_uninit(&hdmi.cec);
+
 	hdmi_pll_uninit(&hdmi.pll);
 
 	pm_runtime_disable(&pdev->dev);
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
new file mode 100644
index 0000000..d4309df
--- /dev/null
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
@@ -0,0 +1,329 @@
+/*
+ * HDMI CEC
+ *
+ * Based on the CEC code from hdmi_ti_4xxx_ip.c from Android.
+ *
+ * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Authors: Yong Zhi
+ *	Mythri pk <mythripk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <video/omapdss.h>
+
+#include "dss.h"
+#include "hdmi.h"
+
+#define HDMI_CORE_CEC_RETRY    200
+
+void hdmi_cec_transmit_fifo_empty(struct hdmi_cec_data *cec, u32 stat1)
+{
+	if (stat1 & 2) {
+		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_NACK | CEC_TX_STATUS_MAX_RETRIES,
+				  0, (dbg3 >> 4) & 7, 0, 0);
+	} else if (stat1 & 1) {
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_ARB_LOST | CEC_TX_STATUS_MAX_RETRIES,
+				  0, 0, 0, 0);
+	} else if (stat1 == 0) {
+		cec_transmit_done(cec->adap, CEC_TX_STATUS_OK,
+				  0, 0, 0, 0);
+	}
+}
+
+void hdmi_cec_received_msg(struct hdmi_cec_data *cec)
+{
+	u32 cnt = hdmi_read_reg(cec->base, HDMI_CEC_RX_COUNT) & 0xff;
+
+	/* While there are CEC frames in the FIFO */
+	while (cnt & 0x70) {
+		/* and the frame doesn't have an error */
+		if (!(cnt & 0x80)) {
+			struct cec_msg msg = {};
+			unsigned int i;
+
+			/* then read the message */
+			msg.len = cnt & 0xf;
+			msg.msg[0] = hdmi_read_reg(cec->base, HDMI_CEC_RX_CMD_HEADER) & 0xff;
+			msg.msg[1] = hdmi_read_reg(cec->base, HDMI_CEC_RX_COMMAND) & 0xff;
+			for (i = 0; i < msg.len; i++)
+				msg.msg[2 + i] =
+					hdmi_read_reg(cec->base, HDMI_CEC_RX_OPERAND + i * 4) & 0xff;
+			msg.len += 2;
+			cec_received_msg(cec->adap, &msg);
+		}
+		/* Clear the current frame from the FIFO */
+		hdmi_write_reg(cec->base, HDMI_CEC_RX_CONTROL, 1);
+		/* Wait until the current frame is cleared */
+		while (hdmi_read_reg(cec->base, HDMI_CEC_RX_CONTROL) & 1)
+			udelay(1);
+		/*
+		 * Re-read the count register and loop to see if there are
+		 * more messages in the FIFO.
+		 */
+		cnt = hdmi_read_reg(cec->base, HDMI_CEC_RX_COUNT) & 0xff;
+	}
+}
+
+void hdmi_cec_irq(struct hdmi_cec_data *cec)
+{
+	u32 stat0 = hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_0);
+	u32 stat1 = hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_1);
+
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_0, stat0);
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1, stat1);
+
+	if (stat0 & 0x02)
+		hdmi_cec_received_msg(cec);
+	if (stat0 & 0x40)
+		REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
+	else if (stat0 & 0x24)
+		hdmi_cec_transmit_fifo_empty(cec, stat1);
+	if (stat1 & 2) {
+		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_NACK | CEC_TX_STATUS_MAX_RETRIES,
+				  0, (dbg3 >> 4) & 7, 0, 0);
+	} else if (stat1 & 1) {
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_ARB_LOST | CEC_TX_STATUS_MAX_RETRIES,
+				  0, 0, 0, 0);
+	}
+	if (stat1 & 0x3)
+		REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
+}
+
+static int hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+	struct hdmi_cec_data *cec = adap->priv;
+	int retry = HDMI_CORE_CEC_RETRY;
+	int temp;
+
+	/* Clear TX FIFO */
+	REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
+	while (retry) {
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+		if (FLD_GET(temp, 7, 7) == 0)
+			break;
+		retry--;
+	}
+	if (retry == 0x0) {
+		pr_err("Could not clear TX FIFO");
+		return -EBUSY;
+	}
+
+	/* Clear RX FIFO */
+	hdmi_write_reg(cec->base, HDMI_CEC_RX_CONTROL, 0x3);
+	retry = HDMI_CORE_CEC_RETRY;
+	while (retry) {
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_RX_CONTROL);
+		if (FLD_GET(temp, 1, 0) == 0)
+			break;
+		retry--;
+	}
+	if (retry == 0x0) {
+		pr_err("Could not clear RX FIFO");
+		return -EBUSY;
+	}
+
+	/* Clear CEC interrupts */
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1,
+		hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_1));
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_0,
+		hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_0));
+
+	/* Enable HDMI core interrupts */
+	if (enable) {
+		/* Enable CEC interrupts */
+		/* Transmit FIFO full/empty event */
+		/* Receiver FIFO not empty event */
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_0, 0x26);
+		/* Frame retransmit count exceeded event */
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_1, 0x0f);
+	} else {
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_0, 0);
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_1, 0);
+		return 0;
+	}
+
+	/* Remove BYpass mode */
+
+	/* cec calibration enable (self clearing) */
+	hdmi_write_reg(cec->base, HDMI_CEC_SETUP, 0x03);
+	msleep(10);
+	hdmi_write_reg(cec->base, HDMI_CEC_SETUP, 0x04);
+
+	temp = hdmi_read_reg(cec->base, HDMI_CEC_SETUP);
+	if (FLD_GET(temp, 4, 4) != 0) {
+		temp = FLD_MOD(temp, 0, 4, 4);
+		hdmi_write_reg(cec->base, HDMI_CEC_SETUP, temp);
+
+		/*
+		 * If we enabled CEC in middle of a CEC messages on CEC n/w,
+		 * we could have start bit irregularity and/or short
+		 * pulse event. Clear them now.
+		 */
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_1);
+		temp = FLD_MOD(0x0, 0x5, 2, 0);
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1, temp);
+	}
+	return 0;
+}
+
+static int hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
+{
+	struct hdmi_cec_data *cec = adap->priv;
+	u32 v;
+
+	if (log_addr == CEC_LOG_ADDR_INVALID) {
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_7_0, 0);
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_15_8, 0);
+		return 0;
+	}
+	if (log_addr <= 7) {
+		v = hdmi_read_reg(cec->base, HDMI_CEC_CA_7_0);
+		v |= 1 << log_addr;
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_7_0, v);
+	} else {
+		v = hdmi_read_reg(cec->base, HDMI_CEC_CA_15_8);
+		v |= 1 << (log_addr - 8);
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_15_8, v);
+	}
+	return 0;
+}
+
+static int hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+				   u32 signal_free_time, struct cec_msg *msg)
+{
+	struct hdmi_cec_data *cec = adap->priv;
+	u32 retry = HDMI_CORE_CEC_RETRY;
+	u32 temp, i = 0;
+
+	/*
+	 * 1. Flush TX FIFO - required as change of initiator ID / destination
+	 *    ID while TX is in progress could result in corrupted message.
+	 * 2. Clear interrupt status registers for TX.
+	 * 3. Set initiator Address, set retry count
+	 * 4. Set Destination Address
+	 * 5. Clear TX interrupt flags - if required
+	 * 6. Set the command
+	 * 7. Transmit
+	 * 8. Check for NACK / ACK - report the same.
+	 */
+
+	/* Clear TX FIFO */
+	REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 1, 7, 7);
+
+	while (retry) {
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+		if (FLD_GET(temp, 7, 7) == 0)
+			break;
+		udelay(10);
+		retry--;
+	}
+	if (retry == 0x0) {
+		pr_err("Could not clear TX FIFO");
+		pr_err("\n FIFO Reset - retry  : %d - was %d\n",
+			retry, HDMI_CORE_CEC_RETRY);
+		return -EIO;
+	}
+
+	/* Clear TX interrupts */
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_0,
+		       HDMI_CEC_TX_FIFO_INT_MASK);
+
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1,
+		       HDMI_CEC_RETRANSMIT_CNT_INT_MASK);
+
+	/* Set the retry count */
+	REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, attempts - 1, 6, 4);
+
+	/* Set the initiator addresses */
+	hdmi_write_reg(cec->base, HDMI_CEC_TX_INIT, cec_msg_initiator(msg));
+
+	/* Set destination id */
+	temp = cec_msg_destination(msg);
+	if (msg->len == 1)
+		temp |= 0x80;
+	hdmi_write_reg(cec->base, HDMI_CEC_TX_DEST, temp);
+	if (msg->len == 1)
+		return 0;
+
+	/* Setup command and arguments for the command */
+	hdmi_write_reg(cec->base, HDMI_CEC_TX_COMMAND, msg->msg[1]);
+
+	for (i = 0; i < msg->len - 2; i++)
+		hdmi_write_reg(cec->base, HDMI_CEC_TX_OPERAND + i * 4,
+			       msg->msg[2 + i]);
+
+	/* Operand count */
+	hdmi_write_reg(cec->base, HDMI_CEC_TRANSMIT_DATA,
+		       (msg->len - 2) | 0x10);
+	return 0;
+}
+
+void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
+{
+	cec_s_phys_addr(cec->adap, pa, false);
+}
+
+const struct cec_adap_ops hdmi_cec_adap_ops = {
+	.adap_enable = hdmi_cec_adap_enable,
+	.adap_log_addr = hdmi_cec_adap_log_addr,
+	.adap_transmit = hdmi_cec_adap_transmit,
+};
+
+int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec)
+{
+	struct resource *res;
+	u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
+		CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
+	unsigned int ret;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cec");
+	if (!res) {
+		DSSERR("can't get CEC mem resource\n");
+		return -EINVAL;
+	}
+
+	cec->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(cec->base)) {
+		DSSERR("can't ioremap TX CEC\n");
+		return PTR_ERR(cec->base);
+	}
+
+	cec->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, cec,
+		"omap4", caps, CEC_MAX_LOG_ADDRS, &pdev->dev);
+	ret = PTR_ERR_OR_ZERO(cec->adap);
+	if (ret < 0)
+		return ret;
+	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
+	ret = cec_register_adapter(cec->adap);
+	if (ret < 0) {
+		cec_delete_adapter(cec->adap);
+		return ret;
+	}
+	return 0;
+}
+
+void hdmi_cec_uninit(struct hdmi_cec_data *cec)
+{
+	cec_unregister_adapter(cec->adap);
+}
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH 2/3] omap4: add CEC support
@ 2016-04-29  9:39   ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, Hans Verkuil, dri-devel

From: Hans Verkuil <hans.verkuil@cisco.com>

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
---
 arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
 arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
 arch/arm/boot/dts/omap4.dtsi           |   5 +-
 drivers/gpu/drm/omapdrm/dss/Kconfig    |   8 +
 drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
 drivers/gpu/drm/omapdrm/dss/hdmi.h     |  62 ++++++-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c    |  39 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 +++++++++++++++++++++++++++++++++
 8 files changed, 441 insertions(+), 9 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts b/arch/arm/boot/dts/omap4-panda-a4.dts
index 78d3631..f0c1020 100644
--- a/arch/arm/boot/dts/omap4-panda-a4.dts
+++ b/arch/arm/boot/dts/omap4-panda-a4.dts
@@ -13,7 +13,7 @@
 /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
 &dss_hdmi_pins {
 	pinctrl-single,pins = <
-		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
+		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
 		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
 		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
 		>;
diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts
index 119f8e6..940fe4f 100644
--- a/arch/arm/boot/dts/omap4-panda-es.dts
+++ b/arch/arm/boot/dts/omap4-panda-es.dts
@@ -34,7 +34,7 @@
 /* PandaboardES has external pullups on SCL & SDA */
 &dss_hdmi_pins {
 	pinctrl-single,pins = <
-		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
+		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)		/* hdmi_cec.hdmi_cec */
 		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
 		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
 		>;
diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
index 2bd9c83..1bb490f 100644
--- a/arch/arm/boot/dts/omap4.dtsi
+++ b/arch/arm/boot/dts/omap4.dtsi
@@ -1006,8 +1006,9 @@
 				reg = <0x58006000 0x200>,
 				      <0x58006200 0x100>,
 				      <0x58006300 0x100>,
-				      <0x58006400 0x1000>;
-				reg-names = "wp", "pll", "phy", "core";
+				      <0x58006400 0x900>,
+				      <0x58006D00 0x100>;
+				reg-names = "wp", "pll", "phy", "core", "cec";
 				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
 				status = "disabled";
 				ti,hwmods = "dss_hdmi";
diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
index d1fa730..69638e9 100644
--- a/drivers/gpu/drm/omapdrm/dss/Kconfig
+++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
@@ -71,9 +71,17 @@ 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 OMAP2_DSS_HDMI_CEC
+	bool "Enable HDMI CEC support for OMAP4"
+	depends on OMAP4_DSS_HDMI && MEDIA_CEC
+	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 b651ec9..37eb597 100644
--- a/drivers/gpu/drm/omapdrm/dss/Makefile
+++ b/drivers/gpu/drm/omapdrm/dss/Makefile
@@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
 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
+ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
+  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
+endif
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.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/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
index 53616b0..7cf8a91 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
@@ -24,6 +24,7 @@
 #include <linux/platform_device.h>
 #include <linux/hdmi.h>
 #include <video/omapdss.h>
+#include <media/cec.h>
 
 #include "dss.h"
 
@@ -83,6 +84,31 @@
 #define HDMI_TXPHY_PAD_CFG_CTRL			0xC
 #define HDMI_TXPHY_BIST_CONTROL			0x1C
 
+/* HDMI CEC */
+#define HDMI_CEC_DEV_ID                         0x0
+#define HDMI_CEC_SPEC                           0x4
+#define HDMI_CEC_DBG_3                          0x1C
+#define HDMI_CEC_TX_INIT                        0x20
+#define HDMI_CEC_TX_DEST                        0x24
+#define HDMI_CEC_SETUP                          0x38
+#define HDMI_CEC_TX_COMMAND                     0x3C
+#define HDMI_CEC_TX_OPERAND                     0x40
+#define HDMI_CEC_TRANSMIT_DATA                  0x7C
+#define HDMI_CEC_CA_7_0                         0x88
+#define HDMI_CEC_CA_15_8                        0x8C
+#define HDMI_CEC_INT_STATUS_0                   0x98
+#define HDMI_CEC_INT_STATUS_1                   0x9C
+#define HDMI_CEC_INT_ENABLE_0                   0x90
+#define HDMI_CEC_INT_ENABLE_1                   0x94
+#define HDMI_CEC_RX_CONTROL                     0xB0
+#define HDMI_CEC_RX_COUNT                       0xB4
+#define HDMI_CEC_RX_CMD_HEADER                  0xB8
+#define HDMI_CEC_RX_COMMAND                     0xBC
+#define HDMI_CEC_RX_OPERAND                     0xC0
+
+#define HDMI_CEC_TX_FIFO_INT_MASK		0x64
+#define HDMI_CEC_RETRANSMIT_CNT_INT_MASK	0x2
+
 enum hdmi_pll_pwr {
 	HDMI_PLLPWRCMD_ALLOFF = 0,
 	HDMI_PLLPWRCMD_PLLONLY = 1,
@@ -250,6 +276,12 @@ struct hdmi_phy_data {
 	u8 lane_polarity[4];
 };
 
+struct hdmi_cec_data {
+	void __iomem *base;
+	struct cec_adapter *adap;
+	u16 phys_addr;
+};
+
 struct hdmi_core_data {
 	void __iomem *base;
 };
@@ -319,6 +351,33 @@ void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s);
 int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy);
 int hdmi_phy_parse_lanes(struct hdmi_phy_data *phy, const u32 *lanes);
 
+/* HDMI CEC funcs */
+#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
+void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa);
+void hdmi_cec_irq(struct hdmi_cec_data *cec);
+int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec);
+void hdmi_cec_uninit(struct hdmi_cec_data *cec);
+#else
+static inline void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
+{
+}
+
+static inline void hdmi_cec_irq(struct hdmi_cec_data *cec)
+{
+}
+
+static inline int hdmi_cec_init(struct platform_device *pdev,
+				struct hdmi_cec_data *cec)
+{
+	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
+	return 0;
+}
+
+static inline void hdmi_cec_uninit(struct hdmi_cec_data *cec)
+{
+}
+#endif
+
 /* HDMI common funcs */
 int hdmi_parse_lanes_of(struct platform_device *pdev, struct device_node *ep,
 	struct hdmi_phy_data *phy);
@@ -344,6 +403,7 @@ struct omap_hdmi {
 	struct hdmi_wp_data	wp;
 	struct hdmi_pll_data	pll;
 	struct hdmi_phy_data	phy;
+	struct hdmi_cec_data	cec;
 	struct hdmi_core_data	core;
 
 	struct hdmi_config cfg;
@@ -361,7 +421,7 @@ struct omap_hdmi {
 	bool audio_configured;
 	struct omap_dss_audio audio_config;
 
-	/* This lock should be taken when booleans bellow are touched. */
+	/* This lock should be taken when booleans below are touched. */
 	spinlock_t audio_playing_lock;
 	bool audio_playing;
 	bool display_enabled;
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index f892ae15..47a60bf 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -34,6 +34,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/component.h>
 #include <video/omapdss.h>
+#include <media/cec-edid.h>
 #include <sound/omap-hdmi-audio.h>
 
 #include "hdmi4_core.h"
@@ -69,14 +70,15 @@ static void hdmi_runtime_put(void)
 
 static irqreturn_t hdmi_irq_handler(int irq, void *data)
 {
-	struct hdmi_wp_data *wp = data;
+	struct omap_hdmi *hdmi = data;
+	struct hdmi_wp_data *wp = &hdmi->wp;
 	u32 irqstatus;
 
 	irqstatus = hdmi_wp_get_irqstatus(wp);
 	hdmi_wp_set_irqstatus(wp, irqstatus);
 
 	if ((irqstatus & HDMI_IRQ_LINK_CONNECT) &&
-			irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
+	    (irqstatus & HDMI_IRQ_LINK_DISCONNECT)) {
 		/*
 		 * If we get both connect and disconnect interrupts at the same
 		 * time, turn off the PHY, clear interrupts, and restart, which
@@ -94,6 +96,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)
+			hdmi_cec_irq(&hdmi->cec);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -213,6 +222,12 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 
 	hdmi4_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
 
+	/* Initialize CEC clock divider */
+	/* CEC needs 2MHz clock hence set the devider to 24 to get
+	   48/24=2MHz clock */
+	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_CLK, 0x18, 5, 0);
+	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
+
 	/* bypass TV gamma table */
 	dispc_enable_gamma_table(0);
 
@@ -228,7 +243,11 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
 		goto err_vid_enable;
 
 	hdmi_wp_set_irqenable(wp,
-		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
+		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT |
+		HDMI_IRQ_CORE);
+
+	/* Unmask CEC interrupt */
+	REG_FLD_MOD(hdmi.core.base, HDMI_CORE_SYS_INTR_UNMASK4, 0x1, 3, 3);
 
 	return 0;
 
@@ -250,6 +269,8 @@ static void hdmi_power_off_full(struct omap_dss_device *dssdev)
 	enum omap_channel channel = dssdev->dispc_channel;
 
 	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
+	hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
+	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
 
 	hdmi_wp_video_stop(&hdmi.wp);
 
@@ -488,6 +509,10 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev,
 	}
 
 	r = read_edid(edid, len);
+	if (r >= 256)
+		hdmi.cec.phys_addr = cec_get_edid_phys_addr(edid, r, NULL);
+	else
+		hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
 
 	if (need_enable)
 		hdmi_core_disable(dssdev);
@@ -724,6 +749,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		goto err;
 
+	r = hdmi_cec_init(pdev, &hdmi.cec);
+	if (r)
+		goto err;
+
 	irq = platform_get_irq(pdev, 0);
 	if (irq < 0) {
 		DSSERR("platform_get_irq failed\n");
@@ -733,7 +762,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 
 	r = devm_request_threaded_irq(&pdev->dev, irq,
 			NULL, hdmi_irq_handler,
-			IRQF_ONESHOT, "OMAP HDMI", &hdmi.wp);
+			IRQF_ONESHOT, "OMAP HDMI", &hdmi);
 	if (r) {
 		DSSERR("HDMI IRQ request failed\n");
 		goto err;
@@ -768,6 +797,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
 
 	hdmi_uninit_output(pdev);
 
+	hdmi_cec_uninit(&hdmi.cec);
+
 	hdmi_pll_uninit(&hdmi.pll);
 
 	pm_runtime_disable(&pdev->dev);
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
new file mode 100644
index 0000000..d4309df
--- /dev/null
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
@@ -0,0 +1,329 @@
+/*
+ * HDMI CEC
+ *
+ * Based on the CEC code from hdmi_ti_4xxx_ip.c from Android.
+ *
+ * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
+ * Authors: Yong Zhi
+ *	Mythri pk <mythripk@ti.com>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ */
+
+#include <linux/kernel.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+#include <video/omapdss.h>
+
+#include "dss.h"
+#include "hdmi.h"
+
+#define HDMI_CORE_CEC_RETRY    200
+
+void hdmi_cec_transmit_fifo_empty(struct hdmi_cec_data *cec, u32 stat1)
+{
+	if (stat1 & 2) {
+		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_NACK | CEC_TX_STATUS_MAX_RETRIES,
+				  0, (dbg3 >> 4) & 7, 0, 0);
+	} else if (stat1 & 1) {
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_ARB_LOST | CEC_TX_STATUS_MAX_RETRIES,
+				  0, 0, 0, 0);
+	} else if (stat1 == 0) {
+		cec_transmit_done(cec->adap, CEC_TX_STATUS_OK,
+				  0, 0, 0, 0);
+	}
+}
+
+void hdmi_cec_received_msg(struct hdmi_cec_data *cec)
+{
+	u32 cnt = hdmi_read_reg(cec->base, HDMI_CEC_RX_COUNT) & 0xff;
+
+	/* While there are CEC frames in the FIFO */
+	while (cnt & 0x70) {
+		/* and the frame doesn't have an error */
+		if (!(cnt & 0x80)) {
+			struct cec_msg msg = {};
+			unsigned int i;
+
+			/* then read the message */
+			msg.len = cnt & 0xf;
+			msg.msg[0] = hdmi_read_reg(cec->base, HDMI_CEC_RX_CMD_HEADER) & 0xff;
+			msg.msg[1] = hdmi_read_reg(cec->base, HDMI_CEC_RX_COMMAND) & 0xff;
+			for (i = 0; i < msg.len; i++)
+				msg.msg[2 + i] =
+					hdmi_read_reg(cec->base, HDMI_CEC_RX_OPERAND + i * 4) & 0xff;
+			msg.len += 2;
+			cec_received_msg(cec->adap, &msg);
+		}
+		/* Clear the current frame from the FIFO */
+		hdmi_write_reg(cec->base, HDMI_CEC_RX_CONTROL, 1);
+		/* Wait until the current frame is cleared */
+		while (hdmi_read_reg(cec->base, HDMI_CEC_RX_CONTROL) & 1)
+			udelay(1);
+		/*
+		 * Re-read the count register and loop to see if there are
+		 * more messages in the FIFO.
+		 */
+		cnt = hdmi_read_reg(cec->base, HDMI_CEC_RX_COUNT) & 0xff;
+	}
+}
+
+void hdmi_cec_irq(struct hdmi_cec_data *cec)
+{
+	u32 stat0 = hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_0);
+	u32 stat1 = hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_1);
+
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_0, stat0);
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1, stat1);
+
+	if (stat0 & 0x02)
+		hdmi_cec_received_msg(cec);
+	if (stat0 & 0x40)
+		REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
+	else if (stat0 & 0x24)
+		hdmi_cec_transmit_fifo_empty(cec, stat1);
+	if (stat1 & 2) {
+		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_NACK | CEC_TX_STATUS_MAX_RETRIES,
+				  0, (dbg3 >> 4) & 7, 0, 0);
+	} else if (stat1 & 1) {
+		cec_transmit_done(cec->adap,
+				  CEC_TX_STATUS_ARB_LOST | CEC_TX_STATUS_MAX_RETRIES,
+				  0, 0, 0, 0);
+	}
+	if (stat1 & 0x3)
+		REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
+}
+
+static int hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+	struct hdmi_cec_data *cec = adap->priv;
+	int retry = HDMI_CORE_CEC_RETRY;
+	int temp;
+
+	/* Clear TX FIFO */
+	REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 0x1, 7, 7);
+	while (retry) {
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+		if (FLD_GET(temp, 7, 7) == 0)
+			break;
+		retry--;
+	}
+	if (retry == 0x0) {
+		pr_err("Could not clear TX FIFO");
+		return -EBUSY;
+	}
+
+	/* Clear RX FIFO */
+	hdmi_write_reg(cec->base, HDMI_CEC_RX_CONTROL, 0x3);
+	retry = HDMI_CORE_CEC_RETRY;
+	while (retry) {
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_RX_CONTROL);
+		if (FLD_GET(temp, 1, 0) == 0)
+			break;
+		retry--;
+	}
+	if (retry == 0x0) {
+		pr_err("Could not clear RX FIFO");
+		return -EBUSY;
+	}
+
+	/* Clear CEC interrupts */
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1,
+		hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_1));
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_0,
+		hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_0));
+
+	/* Enable HDMI core interrupts */
+	if (enable) {
+		/* Enable CEC interrupts */
+		/* Transmit FIFO full/empty event */
+		/* Receiver FIFO not empty event */
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_0, 0x26);
+		/* Frame retransmit count exceeded event */
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_1, 0x0f);
+	} else {
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_0, 0);
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_ENABLE_1, 0);
+		return 0;
+	}
+
+	/* Remove BYpass mode */
+
+	/* cec calibration enable (self clearing) */
+	hdmi_write_reg(cec->base, HDMI_CEC_SETUP, 0x03);
+	msleep(10);
+	hdmi_write_reg(cec->base, HDMI_CEC_SETUP, 0x04);
+
+	temp = hdmi_read_reg(cec->base, HDMI_CEC_SETUP);
+	if (FLD_GET(temp, 4, 4) != 0) {
+		temp = FLD_MOD(temp, 0, 4, 4);
+		hdmi_write_reg(cec->base, HDMI_CEC_SETUP, temp);
+
+		/*
+		 * If we enabled CEC in middle of a CEC messages on CEC n/w,
+		 * we could have start bit irregularity and/or short
+		 * pulse event. Clear them now.
+		 */
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_INT_STATUS_1);
+		temp = FLD_MOD(0x0, 0x5, 2, 0);
+		hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1, temp);
+	}
+	return 0;
+}
+
+static int hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
+{
+	struct hdmi_cec_data *cec = adap->priv;
+	u32 v;
+
+	if (log_addr == CEC_LOG_ADDR_INVALID) {
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_7_0, 0);
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_15_8, 0);
+		return 0;
+	}
+	if (log_addr <= 7) {
+		v = hdmi_read_reg(cec->base, HDMI_CEC_CA_7_0);
+		v |= 1 << log_addr;
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_7_0, v);
+	} else {
+		v = hdmi_read_reg(cec->base, HDMI_CEC_CA_15_8);
+		v |= 1 << (log_addr - 8);
+		hdmi_write_reg(cec->base, HDMI_CEC_CA_15_8, v);
+	}
+	return 0;
+}
+
+static int hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+				   u32 signal_free_time, struct cec_msg *msg)
+{
+	struct hdmi_cec_data *cec = adap->priv;
+	u32 retry = HDMI_CORE_CEC_RETRY;
+	u32 temp, i = 0;
+
+	/*
+	 * 1. Flush TX FIFO - required as change of initiator ID / destination
+	 *    ID while TX is in progress could result in corrupted message.
+	 * 2. Clear interrupt status registers for TX.
+	 * 3. Set initiator Address, set retry count
+	 * 4. Set Destination Address
+	 * 5. Clear TX interrupt flags - if required
+	 * 6. Set the command
+	 * 7. Transmit
+	 * 8. Check for NACK / ACK - report the same.
+	 */
+
+	/* Clear TX FIFO */
+	REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, 1, 7, 7);
+
+	while (retry) {
+		temp = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
+		if (FLD_GET(temp, 7, 7) == 0)
+			break;
+		udelay(10);
+		retry--;
+	}
+	if (retry == 0x0) {
+		pr_err("Could not clear TX FIFO");
+		pr_err("\n FIFO Reset - retry  : %d - was %d\n",
+			retry, HDMI_CORE_CEC_RETRY);
+		return -EIO;
+	}
+
+	/* Clear TX interrupts */
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_0,
+		       HDMI_CEC_TX_FIFO_INT_MASK);
+
+	hdmi_write_reg(cec->base, HDMI_CEC_INT_STATUS_1,
+		       HDMI_CEC_RETRANSMIT_CNT_INT_MASK);
+
+	/* Set the retry count */
+	REG_FLD_MOD(cec->base, HDMI_CEC_DBG_3, attempts - 1, 6, 4);
+
+	/* Set the initiator addresses */
+	hdmi_write_reg(cec->base, HDMI_CEC_TX_INIT, cec_msg_initiator(msg));
+
+	/* Set destination id */
+	temp = cec_msg_destination(msg);
+	if (msg->len == 1)
+		temp |= 0x80;
+	hdmi_write_reg(cec->base, HDMI_CEC_TX_DEST, temp);
+	if (msg->len == 1)
+		return 0;
+
+	/* Setup command and arguments for the command */
+	hdmi_write_reg(cec->base, HDMI_CEC_TX_COMMAND, msg->msg[1]);
+
+	for (i = 0; i < msg->len - 2; i++)
+		hdmi_write_reg(cec->base, HDMI_CEC_TX_OPERAND + i * 4,
+			       msg->msg[2 + i]);
+
+	/* Operand count */
+	hdmi_write_reg(cec->base, HDMI_CEC_TRANSMIT_DATA,
+		       (msg->len - 2) | 0x10);
+	return 0;
+}
+
+void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
+{
+	cec_s_phys_addr(cec->adap, pa, false);
+}
+
+const struct cec_adap_ops hdmi_cec_adap_ops = {
+	.adap_enable = hdmi_cec_adap_enable,
+	.adap_log_addr = hdmi_cec_adap_log_addr,
+	.adap_transmit = hdmi_cec_adap_transmit,
+};
+
+int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec)
+{
+	struct resource *res;
+	u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
+		CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
+	unsigned int ret;
+
+	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cec");
+	if (!res) {
+		DSSERR("can't get CEC mem resource\n");
+		return -EINVAL;
+	}
+
+	cec->base = devm_ioremap_resource(&pdev->dev, res);
+	if (IS_ERR(cec->base)) {
+		DSSERR("can't ioremap TX CEC\n");
+		return PTR_ERR(cec->base);
+	}
+
+	cec->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, cec,
+		"omap4", caps, CEC_MAX_LOG_ADDRS, &pdev->dev);
+	ret = PTR_ERR_OR_ZERO(cec->adap);
+	if (ret < 0)
+		return ret;
+	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
+	ret = cec_register_adapter(cec->adap);
+	if (ret < 0) {
+		cec_delete_adapter(cec->adap);
+		return ret;
+	}
+	return 0;
+}
+
+void hdmi_cec_uninit(struct hdmi_cec_data *cec)
+{
+	cec_unregister_adapter(cec->adap);
+}
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2016-04-29  9:39 ` Hans Verkuil
@ 2016-04-29  9:39   ` Hans Verkuil
  -1 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, dri-devel, Hans Verkuil

From: Hans Verkuil <hans.verkuil@cisco.com>

As long as there is a valid physical address in the EDID and the omap
CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
signal is passed through the tpd12s015.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
index 916a899..efbba23 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/gpio/consumer.h>
 
+#include <media/cec-edid.h>
 #include <video/omapdss.h>
 #include <video/omap-panel-data.h>
 
@@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
 		return;
 
 	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
+	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
 
 	dst->src = NULL;
 	dssdev->dst = NULL;
@@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
 {
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
 	struct omap_dss_device *in = ddata->in;
+	bool valid_phys_addr = 0;
 	int r;
 
 	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
@@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
 
 	r = in->ops.hdmi->read_edid(in, edid, len);
 
-	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
+#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
+	/*
+	 * In order to support CEC this pin should remain high
+	 * as long as the EDID has a valid physical address.
+	 */
+	valid_phys_addr =
+		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
+#endif
+	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
 
 	return r;
 }
-- 
2.8.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2016-04-29  9:39   ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2016-04-29  9:39 UTC (permalink / raw)
  To: linux-media; +Cc: tomi.valkeinen, Hans Verkuil, dri-devel

From: Hans Verkuil <hans.verkuil@cisco.com>

As long as there is a valid physical address in the EDID and the omap
CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
signal is passed through the tpd12s015.

Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
---
 drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
index 916a899..efbba23 100644
--- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
+++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
@@ -16,6 +16,7 @@
 #include <linux/platform_device.h>
 #include <linux/gpio/consumer.h>
 
+#include <media/cec-edid.h>
 #include <video/omapdss.h>
 #include <video/omap-panel-data.h>
 
@@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
 		return;
 
 	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
+	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
 
 	dst->src = NULL;
 	dssdev->dst = NULL;
@@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
 {
 	struct panel_drv_data *ddata = to_panel_data(dssdev);
 	struct omap_dss_device *in = ddata->in;
+	bool valid_phys_addr = 0;
 	int r;
 
 	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
@@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
 
 	r = in->ops.hdmi->read_edid(in, edid, len);
 
-	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
+#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
+	/*
+	 * In order to support CEC this pin should remain high
+	 * as long as the EDID has a valid physical address.
+	 */
+	valid_phys_addr =
+		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
+#endif
+	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
 
 	return r;
 }
-- 
2.8.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2016-04-29  9:39   ` Hans Verkuil
@ 2016-05-10 11:36     ` Tomi Valkeinen
  -1 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2016-05-10 11:36 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> As long as there is a valid physical address in the EDID and the omap
> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
> signal is passed through the tpd12s015.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> index 916a899..efbba23 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -16,6 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/gpio/consumer.h>
>  
> +#include <media/cec-edid.h>
>  #include <video/omapdss.h>
>  #include <video/omap-panel-data.h>
>  
> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
>  		return;
>  
>  	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>  
>  	dst->src = NULL;
>  	dssdev->dst = NULL;
> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *in = ddata->in;
> +	bool valid_phys_addr = 0;
>  	int r;
>  
>  	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>  
>  	r = in->ops.hdmi->read_edid(in, edid, len);
>  
> -	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
> +	/*
> +	 * In order to support CEC this pin should remain high
> +	 * as long as the EDID has a valid physical address.
> +	 */
> +	valid_phys_addr =
> +		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
> +#endif
> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
>  
>  	return r;
>  }

I think this works, but... Maybe it would be cleaner to have the LS_OE
enabled if a cable is connected. That's actually what we had earlier,
but I removed that due to a race issue:

a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015:
Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE
enabled even after reading the EDID, so I think it's better to go back
to the old model (after fixing the race issue, of course =).

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2016-05-10 11:36     ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2016-05-10 11:36 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 2627 bytes --]

Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> As long as there is a valid physical address in the EDID and the omap
> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
> signal is passed through the tpd12s015.
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
> ---
>  drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> index 916a899..efbba23 100644
> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
> @@ -16,6 +16,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/gpio/consumer.h>
>  
> +#include <media/cec-edid.h>
>  #include <video/omapdss.h>
>  #include <video/omap-panel-data.h>
>  
> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
>  		return;
>  
>  	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>  
>  	dst->src = NULL;
>  	dssdev->dst = NULL;
> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>  {
>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>  	struct omap_dss_device *in = ddata->in;
> +	bool valid_phys_addr = 0;
>  	int r;
>  
>  	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>  
>  	r = in->ops.hdmi->read_edid(in, edid, len);
>  
> -	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
> +	/*
> +	 * In order to support CEC this pin should remain high
> +	 * as long as the EDID has a valid physical address.
> +	 */
> +	valid_phys_addr =
> +		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
> +#endif
> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
>  
>  	return r;
>  }

I think this works, but... Maybe it would be cleaner to have the LS_OE
enabled if a cable is connected. That's actually what we had earlier,
but I removed that due to a race issue:

a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015:
Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE
enabled even after reading the EDID, so I think it's better to go back
to the old model (after fixing the race issue, of course =).

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 2/3] omap4: add CEC support
  2016-04-29  9:39   ` Hans Verkuil
@ 2016-05-10 12:01     ` Tomi Valkeinen
  -1 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2016-05-10 12:01 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
>  arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
>  arch/arm/boot/dts/omap4.dtsi           |   5 +-
>  drivers/gpu/drm/omapdrm/dss/Kconfig    |   8 +
>  drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
>  drivers/gpu/drm/omapdrm/dss/hdmi.h     |  62 ++++++-
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c    |  39 +++-
>  drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 +++++++++++++++++++++++++++++++++
>  8 files changed, 441 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

First, thanks for working on this! It's really nice if we get CEC working.

Are you planning to continue working on this patch, or is this a
proof-of-concept, and you want to move on to other things? I'm fine with
both, the patch looks quite good and I'm sure I can continue from here
if you want.

Also, what's the status of the general CEC support, will these patches
work on v4.7?

> diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts b/arch/arm/boot/dts/omap4-panda-a4.dts
> index 78d3631..f0c1020 100644
> --- a/arch/arm/boot/dts/omap4-panda-a4.dts
> +++ b/arch/arm/boot/dts/omap4-panda-a4.dts
> @@ -13,7 +13,7 @@
>  /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
>  &dss_hdmi_pins {
>  	pinctrl-single,pins = <
> -		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
> +		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)	/* hdmi_cec.hdmi_cec */

Looks fine as we discussed, but these need to be split to separate patch
(with explanation, of course =).

>  		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
>  		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
>  		>;
> diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts
> index 119f8e6..940fe4f 100644
> --- a/arch/arm/boot/dts/omap4-panda-es.dts
> +++ b/arch/arm/boot/dts/omap4-panda-es.dts
> @@ -34,7 +34,7 @@
>  /* PandaboardES has external pullups on SCL & SDA */
>  &dss_hdmi_pins {
>  	pinctrl-single,pins = <
> -		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
> +		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)		/* hdmi_cec.hdmi_cec */
>  		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
>  		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
>  		>;
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 2bd9c83..1bb490f 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -1006,8 +1006,9 @@
>  				reg = <0x58006000 0x200>,
>  				      <0x58006200 0x100>,
>  				      <0x58006300 0x100>,
> -				      <0x58006400 0x1000>;
> -				reg-names = "wp", "pll", "phy", "core";
> +				      <0x58006400 0x900>,
> +				      <0x58006D00 0x100>;
> +				reg-names = "wp", "pll", "phy", "core", "cec";

"core" contains four blocks, all of which are currently included there
in the "core" space. I'm not sure why they weren't split up properly
when the driver was written, but I think we should either keep the core
as one big block, or split it up to those four sections, instead of just
separating the CEC block.

>  				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>  				status = "disabled";
>  				ti,hwmods = "dss_hdmi";
> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
> index d1fa730..69638e9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>  	bool "HDMI support for OMAP4"
>          default y
>  	select OMAP2_DSS_HDMI_COMMON
> +	select MEDIA_CEC_EDID

Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?

>  	help
>  	  HDMI support for OMAP4 based SoCs.
>  
> +config OMAP2_DSS_HDMI_CEC

This should probably be OMAP2_DSS_HDMI4_CEC or such, as it's only for
OMAP4 HDMI. Or, following the omap4 hdmi's config name,
"OMAP4_DSS_HDMI_CEC".

> +	bool "Enable HDMI CEC support for OMAP4"
> +	depends on OMAP4_DSS_HDMI && MEDIA_CEC
> +	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 b651ec9..37eb597 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
> @@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>  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
> +ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
> +  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
> +endif

The file should be hdmi4_cec.o, as it's for omap4. And why the ifeq?
Isn't just

omapdss-$(OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o

enough?

>  omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.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/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
> index 53616b0..7cf8a91 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
> @@ -24,6 +24,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/hdmi.h>
>  #include <video/omapdss.h>
> +#include <media/cec.h>
>  
>  #include "dss.h"
>  
> @@ -83,6 +84,31 @@
>  #define HDMI_TXPHY_PAD_CFG_CTRL			0xC
>  #define HDMI_TXPHY_BIST_CONTROL			0x1C
>  
> +/* HDMI CEC */
> +#define HDMI_CEC_DEV_ID                         0x0
> +#define HDMI_CEC_SPEC                           0x4
> +#define HDMI_CEC_DBG_3                          0x1C
> +#define HDMI_CEC_TX_INIT                        0x20
> +#define HDMI_CEC_TX_DEST                        0x24
> +#define HDMI_CEC_SETUP                          0x38
> +#define HDMI_CEC_TX_COMMAND                     0x3C
> +#define HDMI_CEC_TX_OPERAND                     0x40
> +#define HDMI_CEC_TRANSMIT_DATA                  0x7C
> +#define HDMI_CEC_CA_7_0                         0x88
> +#define HDMI_CEC_CA_15_8                        0x8C
> +#define HDMI_CEC_INT_STATUS_0                   0x98
> +#define HDMI_CEC_INT_STATUS_1                   0x9C
> +#define HDMI_CEC_INT_ENABLE_0                   0x90
> +#define HDMI_CEC_INT_ENABLE_1                   0x94
> +#define HDMI_CEC_RX_CONTROL                     0xB0
> +#define HDMI_CEC_RX_COUNT                       0xB4
> +#define HDMI_CEC_RX_CMD_HEADER                  0xB8
> +#define HDMI_CEC_RX_COMMAND                     0xBC
> +#define HDMI_CEC_RX_OPERAND                     0xC0
> +
> +#define HDMI_CEC_TX_FIFO_INT_MASK		0x64
> +#define HDMI_CEC_RETRANSMIT_CNT_INT_MASK	0x2

hdmi.h is a common header for OMAP5 and OMAP4 hdmi. OMAP4 and 5 have
totally different HDMI IP. However, they have the same wrapper (WP), PHY
and PLL. That's why there are these common files, with common register
offsets. But the CEC is part of OMAP4 HDMI IP, so it's not common.

> +
>  enum hdmi_pll_pwr {
>  	HDMI_PLLPWRCMD_ALLOFF = 0,
>  	HDMI_PLLPWRCMD_PLLONLY = 1,
> @@ -250,6 +276,12 @@ struct hdmi_phy_data {
>  	u8 lane_polarity[4];
>  };
>  
> +struct hdmi_cec_data {
> +	void __iomem *base;
> +	struct cec_adapter *adap;
> +	u16 phys_addr;
> +};
> +
>  struct hdmi_core_data {
>  	void __iomem *base;
>  };
> @@ -319,6 +351,33 @@ void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s);
>  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy);
>  int hdmi_phy_parse_lanes(struct hdmi_phy_data *phy, const u32 *lanes);
>  
> +/* HDMI CEC funcs */
> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
> +void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa);
> +void hdmi_cec_irq(struct hdmi_cec_data *cec);
> +int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec);
> +void hdmi_cec_uninit(struct hdmi_cec_data *cec);
> +#else
> +static inline void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
> +{
> +}
> +
> +static inline void hdmi_cec_irq(struct hdmi_cec_data *cec)
> +{
> +}
> +
> +static inline int hdmi_cec_init(struct platform_device *pdev,
> +				struct hdmi_cec_data *cec)
> +{
> +	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
> +	return 0;
> +}
> +
> +static inline void hdmi_cec_uninit(struct hdmi_cec_data *cec)
> +{
> +}
> +#endif
> +
>  /* HDMI common funcs */
>  int hdmi_parse_lanes_of(struct platform_device *pdev, struct device_node *ep,
>  	struct hdmi_phy_data *phy);
> @@ -344,6 +403,7 @@ struct omap_hdmi {
>  	struct hdmi_wp_data	wp;
>  	struct hdmi_pll_data	pll;
>  	struct hdmi_phy_data	phy;
> +	struct hdmi_cec_data	cec;
>  	struct hdmi_core_data	core;
>  
>  	struct hdmi_config cfg;
> @@ -361,7 +421,7 @@ struct omap_hdmi {
>  	bool audio_configured;
>  	struct omap_dss_audio audio_config;
>  
> -	/* This lock should be taken when booleans bellow are touched. */
> +	/* This lock should be taken when booleans below are touched. */
>  	spinlock_t audio_playing_lock;
>  	bool audio_playing;
>  	bool display_enabled;
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> index f892ae15..47a60bf 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -34,6 +34,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/component.h>
>  #include <video/omapdss.h>
> +#include <media/cec-edid.h>
>  #include <sound/omap-hdmi-audio.h>
>  
>  #include "hdmi4_core.h"
> @@ -69,14 +70,15 @@ static void hdmi_runtime_put(void)
>  
>  static irqreturn_t hdmi_irq_handler(int irq, void *data)
>  {
> -	struct hdmi_wp_data *wp = data;
> +	struct omap_hdmi *hdmi = data;
> +	struct hdmi_wp_data *wp = &hdmi->wp;
>  	u32 irqstatus;
>  
>  	irqstatus = hdmi_wp_get_irqstatus(wp);
>  	hdmi_wp_set_irqstatus(wp, irqstatus);
>  
>  	if ((irqstatus & HDMI_IRQ_LINK_CONNECT) &&
> -			irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
> +	    (irqstatus & HDMI_IRQ_LINK_DISCONNECT)) {
>  		/*
>  		 * If we get both connect and disconnect interrupts at the same
>  		 * time, turn off the PHY, clear interrupts, and restart, which
> @@ -94,6 +96,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)
> +			hdmi_cec_irq(&hdmi->cec);
> +	}
>  
>  	return IRQ_HANDLED;
>  }
> @@ -213,6 +222,12 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
>  
>  	hdmi4_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
>  
> +	/* Initialize CEC clock divider */
> +	/* CEC needs 2MHz clock hence set the devider to 24 to get
> +	   48/24=2MHz clock */
> +	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_CLK, 0x18, 5, 0);
> +	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
> +
>  	/* bypass TV gamma table */
>  	dispc_enable_gamma_table(0);
>  
> @@ -228,7 +243,11 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
>  		goto err_vid_enable;
>  
>  	hdmi_wp_set_irqenable(wp,
> -		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
> +		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT |
> +		HDMI_IRQ_CORE);
> +
> +	/* Unmask CEC interrupt */
> +	REG_FLD_MOD(hdmi.core.base, HDMI_CORE_SYS_INTR_UNMASK4, 0x1, 3, 3);
>  
>  	return 0;
>  
> @@ -250,6 +269,8 @@ static void hdmi_power_off_full(struct omap_dss_device *dssdev)
>  	enum omap_channel channel = dssdev->dispc_channel;
>  
>  	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
> +	hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
> +	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
>  
>  	hdmi_wp_video_stop(&hdmi.wp);
>  
> @@ -488,6 +509,10 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev,
>  	}
>  
>  	r = read_edid(edid, len);
> +	if (r >= 256)
> +		hdmi.cec.phys_addr = cec_get_edid_phys_addr(edid, r, NULL);
> +	else
> +		hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
>  
>  	if (need_enable)
>  		hdmi_core_disable(dssdev);
> @@ -724,6 +749,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  	if (r)
>  		goto err;
>  
> +	r = hdmi_cec_init(pdev, &hdmi.cec);
> +	if (r)
> +		goto err;
> +
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		DSSERR("platform_get_irq failed\n");
> @@ -733,7 +762,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  
>  	r = devm_request_threaded_irq(&pdev->dev, irq,
>  			NULL, hdmi_irq_handler,
> -			IRQF_ONESHOT, "OMAP HDMI", &hdmi.wp);
> +			IRQF_ONESHOT, "OMAP HDMI", &hdmi);
>  	if (r) {
>  		DSSERR("HDMI IRQ request failed\n");
>  		goto err;
> @@ -768,6 +797,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
>  
>  	hdmi_uninit_output(pdev);
>  
> +	hdmi_cec_uninit(&hdmi.cec);
> +
>  	hdmi_pll_uninit(&hdmi.pll);
>  
>  	pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
> new file mode 100644
> index 0000000..d4309df
> --- /dev/null
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
> @@ -0,0 +1,329 @@
> +/*
> + * HDMI CEC
> + *
> + * Based on the CEC code from hdmi_ti_4xxx_ip.c from Android.
> + *
> + * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
> + * Authors: Yong Zhi
> + *	Mythri pk <mythripk@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <video/omapdss.h>
> +
> +#include "dss.h"
> +#include "hdmi.h"
> +
> +#define HDMI_CORE_CEC_RETRY    200
> +
> +void hdmi_cec_transmit_fifo_empty(struct hdmi_cec_data *cec, u32 stat1)
> +{
> +	if (stat1 & 2) {
> +		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);

This is a debug register. I haven't looked at how CEC is to be used, but
using a debug register looks a bit suspicious =).

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 2/3] omap4: add CEC support
@ 2016-05-10 12:01     ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2016-05-10 12:01 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 15082 bytes --]

Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil <hans.verkuil@cisco.com>
> 
> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
> ---
>  arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
>  arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
>  arch/arm/boot/dts/omap4.dtsi           |   5 +-
>  drivers/gpu/drm/omapdrm/dss/Kconfig    |   8 +
>  drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
>  drivers/gpu/drm/omapdrm/dss/hdmi.h     |  62 ++++++-
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c    |  39 +++-
>  drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 +++++++++++++++++++++++++++++++++
>  8 files changed, 441 insertions(+), 9 deletions(-)
>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c

First, thanks for working on this! It's really nice if we get CEC working.

Are you planning to continue working on this patch, or is this a
proof-of-concept, and you want to move on to other things? I'm fine with
both, the patch looks quite good and I'm sure I can continue from here
if you want.

Also, what's the status of the general CEC support, will these patches
work on v4.7?

> diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts b/arch/arm/boot/dts/omap4-panda-a4.dts
> index 78d3631..f0c1020 100644
> --- a/arch/arm/boot/dts/omap4-panda-a4.dts
> +++ b/arch/arm/boot/dts/omap4-panda-a4.dts
> @@ -13,7 +13,7 @@
>  /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
>  &dss_hdmi_pins {
>  	pinctrl-single,pins = <
> -		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
> +		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)	/* hdmi_cec.hdmi_cec */

Looks fine as we discussed, but these need to be split to separate patch
(with explanation, of course =).

>  		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
>  		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
>  		>;
> diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts
> index 119f8e6..940fe4f 100644
> --- a/arch/arm/boot/dts/omap4-panda-es.dts
> +++ b/arch/arm/boot/dts/omap4-panda-es.dts
> @@ -34,7 +34,7 @@
>  /* PandaboardES has external pullups on SCL & SDA */
>  &dss_hdmi_pins {
>  	pinctrl-single,pins = <
> -		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
> +		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)		/* hdmi_cec.hdmi_cec */
>  		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
>  		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
>  		>;
> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
> index 2bd9c83..1bb490f 100644
> --- a/arch/arm/boot/dts/omap4.dtsi
> +++ b/arch/arm/boot/dts/omap4.dtsi
> @@ -1006,8 +1006,9 @@
>  				reg = <0x58006000 0x200>,
>  				      <0x58006200 0x100>,
>  				      <0x58006300 0x100>,
> -				      <0x58006400 0x1000>;
> -				reg-names = "wp", "pll", "phy", "core";
> +				      <0x58006400 0x900>,
> +				      <0x58006D00 0x100>;
> +				reg-names = "wp", "pll", "phy", "core", "cec";

"core" contains four blocks, all of which are currently included there
in the "core" space. I'm not sure why they weren't split up properly
when the driver was written, but I think we should either keep the core
as one big block, or split it up to those four sections, instead of just
separating the CEC block.

>  				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>  				status = "disabled";
>  				ti,hwmods = "dss_hdmi";
> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
> index d1fa730..69638e9 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>  	bool "HDMI support for OMAP4"
>          default y
>  	select OMAP2_DSS_HDMI_COMMON
> +	select MEDIA_CEC_EDID

Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?

>  	help
>  	  HDMI support for OMAP4 based SoCs.
>  
> +config OMAP2_DSS_HDMI_CEC

This should probably be OMAP2_DSS_HDMI4_CEC or such, as it's only for
OMAP4 HDMI. Or, following the omap4 hdmi's config name,
"OMAP4_DSS_HDMI_CEC".

> +	bool "Enable HDMI CEC support for OMAP4"
> +	depends on OMAP4_DSS_HDMI && MEDIA_CEC
> +	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 b651ec9..37eb597 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
> @@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>  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
> +ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
> +  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
> +endif

The file should be hdmi4_cec.o, as it's for omap4. And why the ifeq?
Isn't just

omapdss-$(OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o

enough?

>  omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.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/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
> index 53616b0..7cf8a91 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
> @@ -24,6 +24,7 @@
>  #include <linux/platform_device.h>
>  #include <linux/hdmi.h>
>  #include <video/omapdss.h>
> +#include <media/cec.h>
>  
>  #include "dss.h"
>  
> @@ -83,6 +84,31 @@
>  #define HDMI_TXPHY_PAD_CFG_CTRL			0xC
>  #define HDMI_TXPHY_BIST_CONTROL			0x1C
>  
> +/* HDMI CEC */
> +#define HDMI_CEC_DEV_ID                         0x0
> +#define HDMI_CEC_SPEC                           0x4
> +#define HDMI_CEC_DBG_3                          0x1C
> +#define HDMI_CEC_TX_INIT                        0x20
> +#define HDMI_CEC_TX_DEST                        0x24
> +#define HDMI_CEC_SETUP                          0x38
> +#define HDMI_CEC_TX_COMMAND                     0x3C
> +#define HDMI_CEC_TX_OPERAND                     0x40
> +#define HDMI_CEC_TRANSMIT_DATA                  0x7C
> +#define HDMI_CEC_CA_7_0                         0x88
> +#define HDMI_CEC_CA_15_8                        0x8C
> +#define HDMI_CEC_INT_STATUS_0                   0x98
> +#define HDMI_CEC_INT_STATUS_1                   0x9C
> +#define HDMI_CEC_INT_ENABLE_0                   0x90
> +#define HDMI_CEC_INT_ENABLE_1                   0x94
> +#define HDMI_CEC_RX_CONTROL                     0xB0
> +#define HDMI_CEC_RX_COUNT                       0xB4
> +#define HDMI_CEC_RX_CMD_HEADER                  0xB8
> +#define HDMI_CEC_RX_COMMAND                     0xBC
> +#define HDMI_CEC_RX_OPERAND                     0xC0
> +
> +#define HDMI_CEC_TX_FIFO_INT_MASK		0x64
> +#define HDMI_CEC_RETRANSMIT_CNT_INT_MASK	0x2

hdmi.h is a common header for OMAP5 and OMAP4 hdmi. OMAP4 and 5 have
totally different HDMI IP. However, they have the same wrapper (WP), PHY
and PLL. That's why there are these common files, with common register
offsets. But the CEC is part of OMAP4 HDMI IP, so it's not common.

> +
>  enum hdmi_pll_pwr {
>  	HDMI_PLLPWRCMD_ALLOFF = 0,
>  	HDMI_PLLPWRCMD_PLLONLY = 1,
> @@ -250,6 +276,12 @@ struct hdmi_phy_data {
>  	u8 lane_polarity[4];
>  };
>  
> +struct hdmi_cec_data {
> +	void __iomem *base;
> +	struct cec_adapter *adap;
> +	u16 phys_addr;
> +};
> +
>  struct hdmi_core_data {
>  	void __iomem *base;
>  };
> @@ -319,6 +351,33 @@ void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s);
>  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy);
>  int hdmi_phy_parse_lanes(struct hdmi_phy_data *phy, const u32 *lanes);
>  
> +/* HDMI CEC funcs */
> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
> +void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa);
> +void hdmi_cec_irq(struct hdmi_cec_data *cec);
> +int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec);
> +void hdmi_cec_uninit(struct hdmi_cec_data *cec);
> +#else
> +static inline void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
> +{
> +}
> +
> +static inline void hdmi_cec_irq(struct hdmi_cec_data *cec)
> +{
> +}
> +
> +static inline int hdmi_cec_init(struct platform_device *pdev,
> +				struct hdmi_cec_data *cec)
> +{
> +	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
> +	return 0;
> +}
> +
> +static inline void hdmi_cec_uninit(struct hdmi_cec_data *cec)
> +{
> +}
> +#endif
> +
>  /* HDMI common funcs */
>  int hdmi_parse_lanes_of(struct platform_device *pdev, struct device_node *ep,
>  	struct hdmi_phy_data *phy);
> @@ -344,6 +403,7 @@ struct omap_hdmi {
>  	struct hdmi_wp_data	wp;
>  	struct hdmi_pll_data	pll;
>  	struct hdmi_phy_data	phy;
> +	struct hdmi_cec_data	cec;
>  	struct hdmi_core_data	core;
>  
>  	struct hdmi_config cfg;
> @@ -361,7 +421,7 @@ struct omap_hdmi {
>  	bool audio_configured;
>  	struct omap_dss_audio audio_config;
>  
> -	/* This lock should be taken when booleans bellow are touched. */
> +	/* This lock should be taken when booleans below are touched. */
>  	spinlock_t audio_playing_lock;
>  	bool audio_playing;
>  	bool display_enabled;
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> index f892ae15..47a60bf 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -34,6 +34,7 @@
>  #include <linux/regulator/consumer.h>
>  #include <linux/component.h>
>  #include <video/omapdss.h>
> +#include <media/cec-edid.h>
>  #include <sound/omap-hdmi-audio.h>
>  
>  #include "hdmi4_core.h"
> @@ -69,14 +70,15 @@ static void hdmi_runtime_put(void)
>  
>  static irqreturn_t hdmi_irq_handler(int irq, void *data)
>  {
> -	struct hdmi_wp_data *wp = data;
> +	struct omap_hdmi *hdmi = data;
> +	struct hdmi_wp_data *wp = &hdmi->wp;
>  	u32 irqstatus;
>  
>  	irqstatus = hdmi_wp_get_irqstatus(wp);
>  	hdmi_wp_set_irqstatus(wp, irqstatus);
>  
>  	if ((irqstatus & HDMI_IRQ_LINK_CONNECT) &&
> -			irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
> +	    (irqstatus & HDMI_IRQ_LINK_DISCONNECT)) {
>  		/*
>  		 * If we get both connect and disconnect interrupts at the same
>  		 * time, turn off the PHY, clear interrupts, and restart, which
> @@ -94,6 +96,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)
> +			hdmi_cec_irq(&hdmi->cec);
> +	}
>  
>  	return IRQ_HANDLED;
>  }
> @@ -213,6 +222,12 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
>  
>  	hdmi4_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
>  
> +	/* Initialize CEC clock divider */
> +	/* CEC needs 2MHz clock hence set the devider to 24 to get
> +	   48/24=2MHz clock */
> +	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_CLK, 0x18, 5, 0);
> +	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
> +
>  	/* bypass TV gamma table */
>  	dispc_enable_gamma_table(0);
>  
> @@ -228,7 +243,11 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
>  		goto err_vid_enable;
>  
>  	hdmi_wp_set_irqenable(wp,
> -		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
> +		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT |
> +		HDMI_IRQ_CORE);
> +
> +	/* Unmask CEC interrupt */
> +	REG_FLD_MOD(hdmi.core.base, HDMI_CORE_SYS_INTR_UNMASK4, 0x1, 3, 3);
>  
>  	return 0;
>  
> @@ -250,6 +269,8 @@ static void hdmi_power_off_full(struct omap_dss_device *dssdev)
>  	enum omap_channel channel = dssdev->dispc_channel;
>  
>  	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
> +	hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
> +	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
>  
>  	hdmi_wp_video_stop(&hdmi.wp);
>  
> @@ -488,6 +509,10 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev,
>  	}
>  
>  	r = read_edid(edid, len);
> +	if (r >= 256)
> +		hdmi.cec.phys_addr = cec_get_edid_phys_addr(edid, r, NULL);
> +	else
> +		hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
>  
>  	if (need_enable)
>  		hdmi_core_disable(dssdev);
> @@ -724,6 +749,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  	if (r)
>  		goto err;
>  
> +	r = hdmi_cec_init(pdev, &hdmi.cec);
> +	if (r)
> +		goto err;
> +
>  	irq = platform_get_irq(pdev, 0);
>  	if (irq < 0) {
>  		DSSERR("platform_get_irq failed\n");
> @@ -733,7 +762,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  
>  	r = devm_request_threaded_irq(&pdev->dev, irq,
>  			NULL, hdmi_irq_handler,
> -			IRQF_ONESHOT, "OMAP HDMI", &hdmi.wp);
> +			IRQF_ONESHOT, "OMAP HDMI", &hdmi);
>  	if (r) {
>  		DSSERR("HDMI IRQ request failed\n");
>  		goto err;
> @@ -768,6 +797,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
>  
>  	hdmi_uninit_output(pdev);
>  
> +	hdmi_cec_uninit(&hdmi.cec);
> +
>  	hdmi_pll_uninit(&hdmi.pll);
>  
>  	pm_runtime_disable(&pdev->dev);
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
> new file mode 100644
> index 0000000..d4309df
> --- /dev/null
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
> @@ -0,0 +1,329 @@
> +/*
> + * HDMI CEC
> + *
> + * Based on the CEC code from hdmi_ti_4xxx_ip.c from Android.
> + *
> + * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
> + * Authors: Yong Zhi
> + *	Mythri pk <mythripk@ti.com>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2 as published by
> + * the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
> + * more details.
> + */
> +
> +#include <linux/kernel.h>
> +#include <linux/err.h>
> +#include <linux/io.h>
> +#include <linux/platform_device.h>
> +#include <linux/slab.h>
> +#include <video/omapdss.h>
> +
> +#include "dss.h"
> +#include "hdmi.h"
> +
> +#define HDMI_CORE_CEC_RETRY    200
> +
> +void hdmi_cec_transmit_fifo_empty(struct hdmi_cec_data *cec, u32 stat1)
> +{
> +	if (stat1 & 2) {
> +		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);

This is a debug register. I haven't looked at how CEC is to be used, but
using a debug register looks a bit suspicious =).

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 2/3] omap4: add CEC support
  2016-05-10 12:01     ` Tomi Valkeinen
  (?)
@ 2016-05-10 12:26     ` Hans Verkuil
  2016-05-10 12:41         ` Tomi Valkeinen
  -1 siblings, 1 reply; 31+ messages in thread
From: Hans Verkuil @ 2016-05-10 12:26 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

Hi Tomi,

On 05/10/16 14:01, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 29/04/16 12:39, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> ---
>>  arch/arm/boot/dts/omap4-panda-a4.dts   |   2 +-
>>  arch/arm/boot/dts/omap4-panda-es.dts   |   2 +-
>>  arch/arm/boot/dts/omap4.dtsi           |   5 +-
>>  drivers/gpu/drm/omapdrm/dss/Kconfig    |   8 +
>>  drivers/gpu/drm/omapdrm/dss/Makefile   |   3 +
>>  drivers/gpu/drm/omapdrm/dss/hdmi.h     |  62 ++++++-
>>  drivers/gpu/drm/omapdrm/dss/hdmi4.c    |  39 +++-
>>  drivers/gpu/drm/omapdrm/dss/hdmi_cec.c | 329 +++++++++++++++++++++++++++++++++
>>  8 files changed, 441 insertions(+), 9 deletions(-)
>>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
> 
> First, thanks for working on this! It's really nice if we get CEC working.
> 
> Are you planning to continue working on this patch, or is this a
> proof-of-concept, and you want to move on to other things? I'm fine with
> both, the patch looks quite good and I'm sure I can continue from here
> if you want.

I am planning to continue work on this, but...

> Also, what's the status of the general CEC support, will these patches
> work on v4.7?

... I am waiting for the CEC framework to get merged. Possibly for 4.7, but more
likely for 4.8.

It will be staging initially so I have some more time to make API changes if
necessary.

These patches should work for 4.7.

> 
>> diff --git a/arch/arm/boot/dts/omap4-panda-a4.dts b/arch/arm/boot/dts/omap4-panda-a4.dts
>> index 78d3631..f0c1020 100644
>> --- a/arch/arm/boot/dts/omap4-panda-a4.dts
>> +++ b/arch/arm/boot/dts/omap4-panda-a4.dts
>> @@ -13,7 +13,7 @@
>>  /* Pandaboard Rev A4+ have external pullups on SCL & SDA */
>>  &dss_hdmi_pins {
>>  	pinctrl-single,pins = <
>> -		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
>> +		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
> 
> Looks fine as we discussed, but these need to be split to separate patch
> (with explanation, of course =).
> 
>>  		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
>>  		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
>>  		>;
>> diff --git a/arch/arm/boot/dts/omap4-panda-es.dts b/arch/arm/boot/dts/omap4-panda-es.dts
>> index 119f8e6..940fe4f 100644
>> --- a/arch/arm/boot/dts/omap4-panda-es.dts
>> +++ b/arch/arm/boot/dts/omap4-panda-es.dts
>> @@ -34,7 +34,7 @@
>>  /* PandaboardES has external pullups on SCL & SDA */
>>  &dss_hdmi_pins {
>>  	pinctrl-single,pins = <
>> -		OMAP4_IOPAD(0x09a, PIN_INPUT_PULLUP | MUX_MODE0)	/* hdmi_cec.hdmi_cec */
>> +		OMAP4_IOPAD(0x09a, PIN_INPUT | MUX_MODE0)		/* hdmi_cec.hdmi_cec */
>>  		OMAP4_IOPAD(0x09c, PIN_INPUT | MUX_MODE0)		/* hdmi_scl.hdmi_scl */
>>  		OMAP4_IOPAD(0x09e, PIN_INPUT | MUX_MODE0)		/* hdmi_sda.hdmi_sda */
>>  		>;
>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>> index 2bd9c83..1bb490f 100644
>> --- a/arch/arm/boot/dts/omap4.dtsi
>> +++ b/arch/arm/boot/dts/omap4.dtsi
>> @@ -1006,8 +1006,9 @@
>>  				reg = <0x58006000 0x200>,
>>  				      <0x58006200 0x100>,
>>  				      <0x58006300 0x100>,
>> -				      <0x58006400 0x1000>;
>> -				reg-names = "wp", "pll", "phy", "core";
>> +				      <0x58006400 0x900>,
>> +				      <0x58006D00 0x100>;
>> +				reg-names = "wp", "pll", "phy", "core", "cec";
> 
> "core" contains four blocks, all of which are currently included there
> in the "core" space. I'm not sure why they weren't split up properly
> when the driver was written, but I think we should either keep the core
> as one big block, or split it up to those four sections, instead of just
> separating the CEC block.

I don't entirely agree with that, partially because it would mean extra work for
me :-) and partially because CEC is different from the other blocks in that it
is an optional HDMI feature.

> 
>>  				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>>  				status = "disabled";
>>  				ti,hwmods = "dss_hdmi";
>> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
>> index d1fa730..69638e9 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
>> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
>> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>>  	bool "HDMI support for OMAP4"
>>          default y
>>  	select OMAP2_DSS_HDMI_COMMON
>> +	select MEDIA_CEC_EDID
> 
> Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?

Helper functions that manipulate the physical address in an EDID. CEC may be
optional, but the EDID isn't. These functions were just too big to make them
static inlines, so instead it's a simple module.

> 
>>  	help
>>  	  HDMI support for OMAP4 based SoCs.
>>  
>> +config OMAP2_DSS_HDMI_CEC
> 
> This should probably be OMAP2_DSS_HDMI4_CEC or such, as it's only for
> OMAP4 HDMI. Or, following the omap4 hdmi's config name,
> "OMAP4_DSS_HDMI_CEC".
> 
>> +	bool "Enable HDMI CEC support for OMAP4"
>> +	depends on OMAP4_DSS_HDMI && MEDIA_CEC
>> +	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 b651ec9..37eb597 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
>> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
>> @@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>>  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
>> +ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
>> +  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
>> +endif
> 
> The file should be hdmi4_cec.o, as it's for omap4. And why the ifeq?
> Isn't just
> 
> omapdss-$(OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o

OMAP4_DSS_HDMI_CEC is a bool, not a tristate.

> 
> enough?
> 
>>  omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.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/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
>> index 53616b0..7cf8a91 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
>> @@ -24,6 +24,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/hdmi.h>
>>  #include <video/omapdss.h>
>> +#include <media/cec.h>
>>  
>>  #include "dss.h"
>>  
>> @@ -83,6 +84,31 @@
>>  #define HDMI_TXPHY_PAD_CFG_CTRL			0xC
>>  #define HDMI_TXPHY_BIST_CONTROL			0x1C
>>  
>> +/* HDMI CEC */
>> +#define HDMI_CEC_DEV_ID                         0x0
>> +#define HDMI_CEC_SPEC                           0x4
>> +#define HDMI_CEC_DBG_3                          0x1C
>> +#define HDMI_CEC_TX_INIT                        0x20
>> +#define HDMI_CEC_TX_DEST                        0x24
>> +#define HDMI_CEC_SETUP                          0x38
>> +#define HDMI_CEC_TX_COMMAND                     0x3C
>> +#define HDMI_CEC_TX_OPERAND                     0x40
>> +#define HDMI_CEC_TRANSMIT_DATA                  0x7C
>> +#define HDMI_CEC_CA_7_0                         0x88
>> +#define HDMI_CEC_CA_15_8                        0x8C
>> +#define HDMI_CEC_INT_STATUS_0                   0x98
>> +#define HDMI_CEC_INT_STATUS_1                   0x9C
>> +#define HDMI_CEC_INT_ENABLE_0                   0x90
>> +#define HDMI_CEC_INT_ENABLE_1                   0x94
>> +#define HDMI_CEC_RX_CONTROL                     0xB0
>> +#define HDMI_CEC_RX_COUNT                       0xB4
>> +#define HDMI_CEC_RX_CMD_HEADER                  0xB8
>> +#define HDMI_CEC_RX_COMMAND                     0xBC
>> +#define HDMI_CEC_RX_OPERAND                     0xC0
>> +
>> +#define HDMI_CEC_TX_FIFO_INT_MASK		0x64
>> +#define HDMI_CEC_RETRANSMIT_CNT_INT_MASK	0x2
> 
> hdmi.h is a common header for OMAP5 and OMAP4 hdmi. OMAP4 and 5 have
> totally different HDMI IP. However, they have the same wrapper (WP), PHY
> and PLL. That's why there are these common files, with common register
> offsets. But the CEC is part of OMAP4 HDMI IP, so it's not common.
> 
>> +
>>  enum hdmi_pll_pwr {
>>  	HDMI_PLLPWRCMD_ALLOFF = 0,
>>  	HDMI_PLLPWRCMD_PLLONLY = 1,
>> @@ -250,6 +276,12 @@ struct hdmi_phy_data {
>>  	u8 lane_polarity[4];
>>  };
>>  
>> +struct hdmi_cec_data {
>> +	void __iomem *base;
>> +	struct cec_adapter *adap;
>> +	u16 phys_addr;
>> +};
>> +
>>  struct hdmi_core_data {
>>  	void __iomem *base;
>>  };
>> @@ -319,6 +351,33 @@ void hdmi_phy_dump(struct hdmi_phy_data *phy, struct seq_file *s);
>>  int hdmi_phy_init(struct platform_device *pdev, struct hdmi_phy_data *phy);
>>  int hdmi_phy_parse_lanes(struct hdmi_phy_data *phy, const u32 *lanes);
>>  
>> +/* HDMI CEC funcs */
>> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
>> +void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa);
>> +void hdmi_cec_irq(struct hdmi_cec_data *cec);
>> +int hdmi_cec_init(struct platform_device *pdev, struct hdmi_cec_data *cec);
>> +void hdmi_cec_uninit(struct hdmi_cec_data *cec);
>> +#else
>> +static inline void hdmi_cec_set_phys_addr(struct hdmi_cec_data *cec, u16 pa)
>> +{
>> +}
>> +
>> +static inline void hdmi_cec_irq(struct hdmi_cec_data *cec)
>> +{
>> +}
>> +
>> +static inline int hdmi_cec_init(struct platform_device *pdev,
>> +				struct hdmi_cec_data *cec)
>> +{
>> +	cec->phys_addr = CEC_PHYS_ADDR_INVALID;
>> +	return 0;
>> +}
>> +
>> +static inline void hdmi_cec_uninit(struct hdmi_cec_data *cec)
>> +{
>> +}
>> +#endif
>> +
>>  /* HDMI common funcs */
>>  int hdmi_parse_lanes_of(struct platform_device *pdev, struct device_node *ep,
>>  	struct hdmi_phy_data *phy);
>> @@ -344,6 +403,7 @@ struct omap_hdmi {
>>  	struct hdmi_wp_data	wp;
>>  	struct hdmi_pll_data	pll;
>>  	struct hdmi_phy_data	phy;
>> +	struct hdmi_cec_data	cec;
>>  	struct hdmi_core_data	core;
>>  
>>  	struct hdmi_config cfg;
>> @@ -361,7 +421,7 @@ struct omap_hdmi {
>>  	bool audio_configured;
>>  	struct omap_dss_audio audio_config;
>>  
>> -	/* This lock should be taken when booleans bellow are touched. */
>> +	/* This lock should be taken when booleans below are touched. */
>>  	spinlock_t audio_playing_lock;
>>  	bool audio_playing;
>>  	bool display_enabled;
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> index f892ae15..47a60bf 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> @@ -34,6 +34,7 @@
>>  #include <linux/regulator/consumer.h>
>>  #include <linux/component.h>
>>  #include <video/omapdss.h>
>> +#include <media/cec-edid.h>
>>  #include <sound/omap-hdmi-audio.h>
>>  
>>  #include "hdmi4_core.h"
>> @@ -69,14 +70,15 @@ static void hdmi_runtime_put(void)
>>  
>>  static irqreturn_t hdmi_irq_handler(int irq, void *data)
>>  {
>> -	struct hdmi_wp_data *wp = data;
>> +	struct omap_hdmi *hdmi = data;
>> +	struct hdmi_wp_data *wp = &hdmi->wp;
>>  	u32 irqstatus;
>>  
>>  	irqstatus = hdmi_wp_get_irqstatus(wp);
>>  	hdmi_wp_set_irqstatus(wp, irqstatus);
>>  
>>  	if ((irqstatus & HDMI_IRQ_LINK_CONNECT) &&
>> -			irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
>> +	    (irqstatus & HDMI_IRQ_LINK_DISCONNECT)) {
>>  		/*
>>  		 * If we get both connect and disconnect interrupts at the same
>>  		 * time, turn off the PHY, clear interrupts, and restart, which
>> @@ -94,6 +96,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)
>> +			hdmi_cec_irq(&hdmi->cec);
>> +	}
>>  
>>  	return IRQ_HANDLED;
>>  }
>> @@ -213,6 +222,12 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
>>  
>>  	hdmi4_configure(&hdmi.core, &hdmi.wp, &hdmi.cfg);
>>  
>> +	/* Initialize CEC clock divider */
>> +	/* CEC needs 2MHz clock hence set the devider to 24 to get
>> +	   48/24=2MHz clock */
>> +	REG_FLD_MOD(hdmi.wp.base, HDMI_WP_CLK, 0x18, 5, 0);
>> +	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
>> +
>>  	/* bypass TV gamma table */
>>  	dispc_enable_gamma_table(0);
>>  
>> @@ -228,7 +243,11 @@ static int hdmi_power_on_full(struct omap_dss_device *dssdev)
>>  		goto err_vid_enable;
>>  
>>  	hdmi_wp_set_irqenable(wp,
>> -		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT);
>> +		HDMI_IRQ_LINK_CONNECT | HDMI_IRQ_LINK_DISCONNECT |
>> +		HDMI_IRQ_CORE);
>> +
>> +	/* Unmask CEC interrupt */
>> +	REG_FLD_MOD(hdmi.core.base, HDMI_CORE_SYS_INTR_UNMASK4, 0x1, 3, 3);
>>  
>>  	return 0;
>>  
>> @@ -250,6 +269,8 @@ static void hdmi_power_off_full(struct omap_dss_device *dssdev)
>>  	enum omap_channel channel = dssdev->dispc_channel;
>>  
>>  	hdmi_wp_clear_irqenable(&hdmi.wp, 0xffffffff);
>> +	hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
>> +	hdmi_cec_set_phys_addr(&hdmi.cec, hdmi.cec.phys_addr);
>>  
>>  	hdmi_wp_video_stop(&hdmi.wp);
>>  
>> @@ -488,6 +509,10 @@ static int hdmi_read_edid(struct omap_dss_device *dssdev,
>>  	}
>>  
>>  	r = read_edid(edid, len);
>> +	if (r >= 256)
>> +		hdmi.cec.phys_addr = cec_get_edid_phys_addr(edid, r, NULL);
>> +	else
>> +		hdmi.cec.phys_addr = CEC_PHYS_ADDR_INVALID;
>>  
>>  	if (need_enable)
>>  		hdmi_core_disable(dssdev);
>> @@ -724,6 +749,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>>  	if (r)
>>  		goto err;
>>  
>> +	r = hdmi_cec_init(pdev, &hdmi.cec);
>> +	if (r)
>> +		goto err;
>> +
>>  	irq = platform_get_irq(pdev, 0);
>>  	if (irq < 0) {
>>  		DSSERR("platform_get_irq failed\n");
>> @@ -733,7 +762,7 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>>  
>>  	r = devm_request_threaded_irq(&pdev->dev, irq,
>>  			NULL, hdmi_irq_handler,
>> -			IRQF_ONESHOT, "OMAP HDMI", &hdmi.wp);
>> +			IRQF_ONESHOT, "OMAP HDMI", &hdmi);
>>  	if (r) {
>>  		DSSERR("HDMI IRQ request failed\n");
>>  		goto err;
>> @@ -768,6 +797,8 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
>>  
>>  	hdmi_uninit_output(pdev);
>>  
>> +	hdmi_cec_uninit(&hdmi.cec);
>> +
>>  	hdmi_pll_uninit(&hdmi.pll);
>>  
>>  	pm_runtime_disable(&pdev->dev);
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
>> new file mode 100644
>> index 0000000..d4309df
>> --- /dev/null
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi_cec.c
>> @@ -0,0 +1,329 @@
>> +/*
>> + * HDMI CEC
>> + *
>> + * Based on the CEC code from hdmi_ti_4xxx_ip.c from Android.
>> + *
>> + * Copyright (C) 2010-2011 Texas Instruments Incorporated - http://www.ti.com/
>> + * Authors: Yong Zhi
>> + *	Mythri pk <mythripk@ti.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2 as published by
>> + * the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/err.h>
>> +#include <linux/io.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <video/omapdss.h>
>> +
>> +#include "dss.h"
>> +#include "hdmi.h"
>> +
>> +#define HDMI_CORE_CEC_RETRY    200
>> +
>> +void hdmi_cec_transmit_fifo_empty(struct hdmi_cec_data *cec, u32 stat1)
>> +{
>> +	if (stat1 & 2) {
>> +		u32 dbg3 = hdmi_read_reg(cec->base, HDMI_CEC_DBG_3);
> 
> This is a debug register. I haven't looked at how CEC is to be used, but
> using a debug register looks a bit suspicious =).

There is some functionality in those 'debug' registers that doesn't really
belong there as it has nothing to do with debugging.

Regards,

	Hans

> 
>  Tomi
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 2/3] omap4: add CEC support
  2016-05-10 12:26     ` Hans Verkuil
@ 2016-05-10 12:41         ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2016-05-10 12:41 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

On 10/05/16 15:26, Hans Verkuil wrote:

>>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>>> index 2bd9c83..1bb490f 100644
>>> --- a/arch/arm/boot/dts/omap4.dtsi
>>> +++ b/arch/arm/boot/dts/omap4.dtsi
>>> @@ -1006,8 +1006,9 @@
>>>  				reg = <0x58006000 0x200>,
>>>  				      <0x58006200 0x100>,
>>>  				      <0x58006300 0x100>,
>>> -				      <0x58006400 0x1000>;
>>> -				reg-names = "wp", "pll", "phy", "core";
>>> +				      <0x58006400 0x900>,
>>> +				      <0x58006D00 0x100>;
>>> +				reg-names = "wp", "pll", "phy", "core", "cec";
>>
>> "core" contains four blocks, all of which are currently included there
>> in the "core" space. I'm not sure why they weren't split up properly
>> when the driver was written, but I think we should either keep the core
>> as one big block, or split it up to those four sections, instead of just
>> separating the CEC block.
> 
> I don't entirely agree with that, partially because it would mean extra work for
> me :-) and partially because CEC is different from the other blocks in that it
> is an optional HDMI feature.

I don't think it matters in this context if it's an optional HDMI
feature or not. This is about representing the HW memory areas, and I'd
like to keep it consistent, so either one big block, or if we want to
split it, split it up properly as shown in the TRM.

I'm fine with keeping one big "core" memory area there, that should work
fine for CEC, shouldn't it? And it would be the easiest option for you ;).

> 
>>
>>>  				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>>>  				status = "disabled";
>>>  				ti,hwmods = "dss_hdmi";
>>> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> index d1fa730..69638e9 100644
>>> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>>>  	bool "HDMI support for OMAP4"
>>>          default y
>>>  	select OMAP2_DSS_HDMI_COMMON
>>> +	select MEDIA_CEC_EDID
>>
>> Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?
> 
> Helper functions that manipulate the physical address in an EDID. CEC may be
> optional, but the EDID isn't. These functions were just too big to make them
> static inlines, so instead it's a simple module.

Oh, I see, even if OMAP4's HDMI CEC is disabled, you use cec edid
functions in hdmi4.c.

>>> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
>>> index b651ec9..37eb597 100644
>>> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
>>> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
>>> @@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>>>  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
>>> +ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
>>> +  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
>>> +endif
>>
>> The file should be hdmi4_cec.o, as it's for omap4. And why the ifeq?
>> Isn't just
>>
>> omapdss-$(OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o
> 
> OMAP4_DSS_HDMI_CEC is a bool, not a tristate.

Yes, and that's fine. You're not compiling hdmi4_cec.o as a module,
you're compiling it into omapdss.o. Objects are added with "omapdss-y +=
xyz" to omapdss.o.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 2/3] omap4: add CEC support
@ 2016-05-10 12:41         ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2016-05-10 12:41 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 3389 bytes --]

On 10/05/16 15:26, Hans Verkuil wrote:

>>> diff --git a/arch/arm/boot/dts/omap4.dtsi b/arch/arm/boot/dts/omap4.dtsi
>>> index 2bd9c83..1bb490f 100644
>>> --- a/arch/arm/boot/dts/omap4.dtsi
>>> +++ b/arch/arm/boot/dts/omap4.dtsi
>>> @@ -1006,8 +1006,9 @@
>>>  				reg = <0x58006000 0x200>,
>>>  				      <0x58006200 0x100>,
>>>  				      <0x58006300 0x100>,
>>> -				      <0x58006400 0x1000>;
>>> -				reg-names = "wp", "pll", "phy", "core";
>>> +				      <0x58006400 0x900>,
>>> +				      <0x58006D00 0x100>;
>>> +				reg-names = "wp", "pll", "phy", "core", "cec";
>>
>> "core" contains four blocks, all of which are currently included there
>> in the "core" space. I'm not sure why they weren't split up properly
>> when the driver was written, but I think we should either keep the core
>> as one big block, or split it up to those four sections, instead of just
>> separating the CEC block.
> 
> I don't entirely agree with that, partially because it would mean extra work for
> me :-) and partially because CEC is different from the other blocks in that it
> is an optional HDMI feature.

I don't think it matters in this context if it's an optional HDMI
feature or not. This is about representing the HW memory areas, and I'd
like to keep it consistent, so either one big block, or if we want to
split it, split it up properly as shown in the TRM.

I'm fine with keeping one big "core" memory area there, that should work
fine for CEC, shouldn't it? And it would be the easiest option for you ;).

> 
>>
>>>  				interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>>>  				status = "disabled";
>>>  				ti,hwmods = "dss_hdmi";
>>> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> index d1fa730..69638e9 100644
>>> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
>>> @@ -71,9 +71,17 @@ config OMAP4_DSS_HDMI
>>>  	bool "HDMI support for OMAP4"
>>>          default y
>>>  	select OMAP2_DSS_HDMI_COMMON
>>> +	select MEDIA_CEC_EDID
>>
>> Hmm, what's in MEDIA_CEC_EDID, why does OMAP4 HDMI need to select that?
> 
> Helper functions that manipulate the physical address in an EDID. CEC may be
> optional, but the EDID isn't. These functions were just too big to make them
> static inlines, so instead it's a simple module.

Oh, I see, even if OMAP4's HDMI CEC is disabled, you use cec edid
functions in hdmi4.c.

>>> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
>>> index b651ec9..37eb597 100644
>>> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
>>> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
>>> @@ -10,6 +10,9 @@ omapdss-$(CONFIG_OMAP2_DSS_SDI) += sdi.o
>>>  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
>>> +ifeq ($(CONFIG_OMAP2_DSS_HDMI_CEC),y)
>>> +  omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_cec.o
>>> +endif
>>
>> The file should be hdmi4_cec.o, as it's for omap4. And why the ifeq?
>> Isn't just
>>
>> omapdss-$(OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o
> 
> OMAP4_DSS_HDMI_CEC is a bool, not a tristate.

Yes, and that's fine. You're not compiling hdmi4_cec.o as a module,
you're compiling it into omapdss.o. Objects are added with "omapdss-y +=
xyz" to omapdss.o.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2016-05-10 11:36     ` Tomi Valkeinen
  (?)
@ 2017-04-08 10:11     ` Hans Verkuil
  2017-04-08 10:57         ` Hans Verkuil
  2017-04-10 11:59         ` Tomi Valkeinen
  -1 siblings, 2 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-08 10:11 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

Hi Tomi,

On 05/10/2016 01:36 PM, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 29/04/16 12:39, Hans Verkuil wrote:
>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>
>> As long as there is a valid physical address in the EDID and the omap
>> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
>> signal is passed through the tpd12s015.
>>
>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>> ---
>>  drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>> index 916a899..efbba23 100644
>> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>> @@ -16,6 +16,7 @@
>>  #include <linux/platform_device.h>
>>  #include <linux/gpio/consumer.h>
>>  
>> +#include <media/cec-edid.h>
>>  #include <video/omapdss.h>
>>  #include <video/omap-panel-data.h>
>>  
>> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
>>  		return;
>>  
>>  	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
>> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>>  
>>  	dst->src = NULL;
>>  	dssdev->dst = NULL;
>> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>>  {
>>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>>  	struct omap_dss_device *in = ddata->in;
>> +	bool valid_phys_addr = 0;
>>  	int r;
>>  
>>  	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
>> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>>  
>>  	r = in->ops.hdmi->read_edid(in, edid, len);
>>  
>> -	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
>> +	/*
>> +	 * In order to support CEC this pin should remain high
>> +	 * as long as the EDID has a valid physical address.
>> +	 */
>> +	valid_phys_addr =
>> +		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
>> +#endif
>> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
>>  
>>  	return r;
>>  }
> 
> I think this works, but... Maybe it would be cleaner to have the LS_OE
> enabled if a cable is connected. That's actually what we had earlier,
> but I removed that due to a race issue:
> 
> a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015:
> Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE
> enabled even after reading the EDID, so I think it's better to go back
> to the old model (after fixing the race issue, of course =).

So, this is a bit of a blast from the past since the omap4 CEC development
has been on hold for almost a year. But I am about to resume my work on this
now that the CEC framework was merged.

The latest code is here, if you are interested:

https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec

It's pretty much unchanged from the version I posted a year ago, just rebased.

But before I continue with this I have one question for you. First some
background:

There is a special corner case (and I wasn't aware of that a year ago!) where
it is allowed to send a CEC message when there is *no HPD*.

The reason is that some displays turn off the hotplug detect pin when they go
into standby or when another input is active. The only way to communicate with
such displays is via CEC.

The problem is that without a HPD there is no EDID and basically no way for an
HDMI transmitter to detect that something is connected at all, unless you are
using CEC.

What this means is that if we want to implement this on the omap4 the CEC support
has to be on all the time.

We have seen modern displays that behave like this, so this is a real issue. And
this corner case is specifically allowed by the CEC specification: the Poll,
Image/Text View On and the Active Source messages can be sent to a TV even when
there is no HPD in order to turn on the display if it was in standby and to make
us the active input.

The CEC framework in the kernel supports this starting with 4.12 (this code is
in the panda-cec branch above).

If this *can't* be supported by the omap4, then I will likely have to add a CEC
capability to signal to the application that this specific corner case is not
supported.

I just did some tests with omap4 and I my impression is that this can't be
supported: when the HPD goes away it seems like most/all of the HDMI blocks are
all powered off and any attempt to even access the CEC registers will fail.

Changing this looks to be non-trivial if not impossible.

Can you confirm that that isn't possible? If you think this can be done, then
I'd appreciate if you can give me a few pointers.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-08 10:11     ` Hans Verkuil
@ 2017-04-08 10:57         ` Hans Verkuil
  2017-04-10 11:59         ` Tomi Valkeinen
  1 sibling, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-08 10:57 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

On 04/08/2017 12:11 PM, Hans Verkuil wrote:
> Hi Tomi,
> 
> On 05/10/2016 01:36 PM, Tomi Valkeinen wrote:
>> Hi Hans,
>>
>> On 29/04/16 12:39, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> As long as there is a valid physical address in the EDID and the omap
>>> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
>>> signal is passed through the tpd12s015.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>  drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>>> index 916a899..efbba23 100644
>>> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>>> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>>> @@ -16,6 +16,7 @@
>>>  #include <linux/platform_device.h>
>>>  #include <linux/gpio/consumer.h>
>>>  
>>> +#include <media/cec-edid.h>
>>>  #include <video/omapdss.h>
>>>  #include <video/omap-panel-data.h>
>>>  
>>> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
>>>  		return;
>>>  
>>>  	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
>>> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>>>  
>>>  	dst->src = NULL;
>>>  	dssdev->dst = NULL;
>>> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>>>  {
>>>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>>>  	struct omap_dss_device *in = ddata->in;
>>> +	bool valid_phys_addr = 0;
>>>  	int r;
>>>  
>>>  	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
>>> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>>>  
>>>  	r = in->ops.hdmi->read_edid(in, edid, len);
>>>  
>>> -	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>>> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
>>> +	/*
>>> +	 * In order to support CEC this pin should remain high
>>> +	 * as long as the EDID has a valid physical address.
>>> +	 */
>>> +	valid_phys_addr =
>>> +		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
>>> +#endif
>>> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
>>>  
>>>  	return r;
>>>  }
>>
>> I think this works, but... Maybe it would be cleaner to have the LS_OE
>> enabled if a cable is connected. That's actually what we had earlier,
>> but I removed that due to a race issue:
>>
>> a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015:
>> Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE
>> enabled even after reading the EDID, so I think it's better to go back
>> to the old model (after fixing the race issue, of course =).
> 
> So, this is a bit of a blast from the past since the omap4 CEC development
> has been on hold for almost a year. But I am about to resume my work on this
> now that the CEC framework was merged.
> 
> The latest code is here, if you are interested:
> 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec
> 
> It's pretty much unchanged from the version I posted a year ago, just rebased.
> 
> But before I continue with this I have one question for you. First some
> background:
> 
> There is a special corner case (and I wasn't aware of that a year ago!) where
> it is allowed to send a CEC message when there is *no HPD*.
> 
> The reason is that some displays turn off the hotplug detect pin when they go
> into standby or when another input is active. The only way to communicate with
> such displays is via CEC.
> 
> The problem is that without a HPD there is no EDID and basically no way for an
> HDMI transmitter to detect that something is connected at all, unless you are
> using CEC.
> 
> What this means is that if we want to implement this on the omap4 the CEC support
> has to be on all the time.
> 
> We have seen modern displays that behave like this, so this is a real issue. And
> this corner case is specifically allowed by the CEC specification: the Poll,
> Image/Text View On and the Active Source messages can be sent to a TV even when
> there is no HPD in order to turn on the display if it was in standby and to make
> us the active input.
> 
> The CEC framework in the kernel supports this starting with 4.12 (this code is
> in the panda-cec branch above).
> 
> If this *can't* be supported by the omap4, then I will likely have to add a CEC
> capability to signal to the application that this specific corner case is not
> supported.

FYI: I've just added support for this to the panda-cec branch. CEC on the omap4
now works again, but you can't send CEC messages as long as there is no valid
physical address.

Regards,

	Hans

> 
> I just did some tests with omap4 and I my impression is that this can't be
> supported: when the HPD goes away it seems like most/all of the HDMI blocks are
> all powered off and any attempt to even access the CEC registers will fail.
> 
> Changing this looks to be non-trivial if not impossible.
> 
> Can you confirm that that isn't possible? If you think this can be done, then
> I'd appreciate if you can give me a few pointers.
> 
> Regards,
> 
> 	Hans
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2017-04-08 10:57         ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-08 10:57 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: Hans Verkuil, dri-devel

On 04/08/2017 12:11 PM, Hans Verkuil wrote:
> Hi Tomi,
> 
> On 05/10/2016 01:36 PM, Tomi Valkeinen wrote:
>> Hi Hans,
>>
>> On 29/04/16 12:39, Hans Verkuil wrote:
>>> From: Hans Verkuil <hans.verkuil@cisco.com>
>>>
>>> As long as there is a valid physical address in the EDID and the omap
>>> CEC support is enabled, then we keep ls_oe_gpio on to ensure the CEC
>>> signal is passed through the tpd12s015.
>>>
>>> Signed-off-by: Hans Verkuil <hans.verkuil@cisco.com>
>>> Suggested-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
>>> ---
>>>  drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c | 13 ++++++++++++-
>>>  1 file changed, 12 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>>> index 916a899..efbba23 100644
>>> --- a/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>>> +++ b/drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c
>>> @@ -16,6 +16,7 @@
>>>  #include <linux/platform_device.h>
>>>  #include <linux/gpio/consumer.h>
>>>  
>>> +#include <media/cec-edid.h>
>>>  #include <video/omapdss.h>
>>>  #include <video/omap-panel-data.h>
>>>  
>>> @@ -65,6 +66,7 @@ static void tpd_disconnect(struct omap_dss_device *dssdev,
>>>  		return;
>>>  
>>>  	gpiod_set_value_cansleep(ddata->ct_cp_hpd_gpio, 0);
>>> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>>>  
>>>  	dst->src = NULL;
>>>  	dssdev->dst = NULL;
>>> @@ -142,6 +144,7 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>>>  {
>>>  	struct panel_drv_data *ddata = to_panel_data(dssdev);
>>>  	struct omap_dss_device *in = ddata->in;
>>> +	bool valid_phys_addr = 0;
>>>  	int r;
>>>  
>>>  	if (!gpiod_get_value_cansleep(ddata->hpd_gpio))
>>> @@ -151,7 +154,15 @@ static int tpd_read_edid(struct omap_dss_device *dssdev,
>>>  
>>>  	r = in->ops.hdmi->read_edid(in, edid, len);
>>>  
>>> -	gpiod_set_value_cansleep(ddata->ls_oe_gpio, 0);
>>> +#ifdef CONFIG_OMAP2_DSS_HDMI_CEC
>>> +	/*
>>> +	 * In order to support CEC this pin should remain high
>>> +	 * as long as the EDID has a valid physical address.
>>> +	 */
>>> +	valid_phys_addr =
>>> +		cec_get_edid_phys_addr(edid, r, NULL) != CEC_PHYS_ADDR_INVALID;
>>> +#endif
>>> +	gpiod_set_value_cansleep(ddata->ls_oe_gpio, valid_phys_addr);
>>>  
>>>  	return r;
>>>  }
>>
>> I think this works, but... Maybe it would be cleaner to have the LS_OE
>> enabled if a cable is connected. That's actually what we had earlier,
>> but I removed that due to a race issue:
>>
>> a87a6d6b09de3118e5679c2057b99b7791b7673b ("OMAPDSS: encoder-tpd12s015:
>> Fix race issue with LS_OE"). Now, with CEC, there's need to have LS_OE
>> enabled even after reading the EDID, so I think it's better to go back
>> to the old model (after fixing the race issue, of course =).
> 
> So, this is a bit of a blast from the past since the omap4 CEC development
> has been on hold for almost a year. But I am about to resume my work on this
> now that the CEC framework was merged.
> 
> The latest code is here, if you are interested:
> 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec
> 
> It's pretty much unchanged from the version I posted a year ago, just rebased.
> 
> But before I continue with this I have one question for you. First some
> background:
> 
> There is a special corner case (and I wasn't aware of that a year ago!) where
> it is allowed to send a CEC message when there is *no HPD*.
> 
> The reason is that some displays turn off the hotplug detect pin when they go
> into standby or when another input is active. The only way to communicate with
> such displays is via CEC.
> 
> The problem is that without a HPD there is no EDID and basically no way for an
> HDMI transmitter to detect that something is connected at all, unless you are
> using CEC.
> 
> What this means is that if we want to implement this on the omap4 the CEC support
> has to be on all the time.
> 
> We have seen modern displays that behave like this, so this is a real issue. And
> this corner case is specifically allowed by the CEC specification: the Poll,
> Image/Text View On and the Active Source messages can be sent to a TV even when
> there is no HPD in order to turn on the display if it was in standby and to make
> us the active input.
> 
> The CEC framework in the kernel supports this starting with 4.12 (this code is
> in the panda-cec branch above).
> 
> If this *can't* be supported by the omap4, then I will likely have to add a CEC
> capability to signal to the application that this specific corner case is not
> supported.

FYI: I've just added support for this to the panda-cec branch. CEC on the omap4
now works again, but you can't send CEC messages as long as there is no valid
physical address.

Regards,

	Hans

> 
> I just did some tests with omap4 and I my impression is that this can't be
> supported: when the HPD goes away it seems like most/all of the HDMI blocks are
> all powered off and any attempt to even access the CEC registers will fail.
> 
> Changing this looks to be non-trivial if not impossible.
> 
> Can you confirm that that isn't possible? If you think this can be done, then
> I'd appreciate if you can give me a few pointers.
> 
> Regards,
> 
> 	Hans
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-08 10:11     ` Hans Verkuil
@ 2017-04-10 11:59         ` Tomi Valkeinen
  2017-04-10 11:59         ` Tomi Valkeinen
  1 sibling, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-10 11:59 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

On 08/04/17 13:11, Hans Verkuil wrote:

> So, this is a bit of a blast from the past since the omap4 CEC development
> has been on hold for almost a year. But I am about to resume my work on this
> now that the CEC framework was merged.
> 
> The latest code is here, if you are interested:
> 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec
> 
> It's pretty much unchanged from the version I posted a year ago, just rebased.
> 
> But before I continue with this I have one question for you. First some
> background:
> 
> There is a special corner case (and I wasn't aware of that a year ago!) where
> it is allowed to send a CEC message when there is *no HPD*.
> 
> The reason is that some displays turn off the hotplug detect pin when they go
> into standby or when another input is active. The only way to communicate with
> such displays is via CEC.
> 
> The problem is that without a HPD there is no EDID and basically no way for an
> HDMI transmitter to detect that something is connected at all, unless you are
> using CEC.
> 
> What this means is that if we want to implement this on the omap4 the CEC support
> has to be on all the time.
> 
> We have seen modern displays that behave like this, so this is a real issue. And
> this corner case is specifically allowed by the CEC specification: the Poll,
> Image/Text View On and the Active Source messages can be sent to a TV even when
> there is no HPD in order to turn on the display if it was in standby and to make
> us the active input.
> 
> The CEC framework in the kernel supports this starting with 4.12 (this code is
> in the panda-cec branch above).
> 
> If this *can't* be supported by the omap4, then I will likely have to add a CEC
> capability to signal to the application that this specific corner case is not
> supported.
> 
> I just did some tests with omap4 and I my impression is that this can't be
> supported: when the HPD goes away it seems like most/all of the HDMI blocks are
> all powered off and any attempt to even access the CEC registers will fail.
> 
> Changing this looks to be non-trivial if not impossible.
> 
> Can you confirm that that isn't possible? If you think this can be done, then
> I'd appreciate if you can give me a few pointers.

HPD doesn't control the HW, it's all in the SW. So while I don't know
much at all about CEC and haven't looked at this particular use case, I
believe it's doable. HPD going off will make the DRM connector to be in
"disconnected" state, which on omapdrm will cause everything about HDMI
to be turned off.

Does it work on some other DRM driver? I'm wondering if there's
something in the DRM framework side that also has to be changed, in
addition to omapdrm changes.

It could require larger SW redesigns, though... Which makes me think
that the work shouldn't be done until we have changed the omapdrm's
driver model to DRM's common bridge driver model, and then all this
could possibly be done in a more generic manner.

Well, then again, I think the hdmi driver's internal power state
handling could be improved even before that. Currently it's not very
versatile. We should have ways to partially enable the IP for just the
required parts.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2017-04-10 11:59         ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-10 11:59 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 3276 bytes --]

On 08/04/17 13:11, Hans Verkuil wrote:

> So, this is a bit of a blast from the past since the omap4 CEC development
> has been on hold for almost a year. But I am about to resume my work on this
> now that the CEC framework was merged.
> 
> The latest code is here, if you are interested:
> 
> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec
> 
> It's pretty much unchanged from the version I posted a year ago, just rebased.
> 
> But before I continue with this I have one question for you. First some
> background:
> 
> There is a special corner case (and I wasn't aware of that a year ago!) where
> it is allowed to send a CEC message when there is *no HPD*.
> 
> The reason is that some displays turn off the hotplug detect pin when they go
> into standby or when another input is active. The only way to communicate with
> such displays is via CEC.
> 
> The problem is that without a HPD there is no EDID and basically no way for an
> HDMI transmitter to detect that something is connected at all, unless you are
> using CEC.
> 
> What this means is that if we want to implement this on the omap4 the CEC support
> has to be on all the time.
> 
> We have seen modern displays that behave like this, so this is a real issue. And
> this corner case is specifically allowed by the CEC specification: the Poll,
> Image/Text View On and the Active Source messages can be sent to a TV even when
> there is no HPD in order to turn on the display if it was in standby and to make
> us the active input.
> 
> The CEC framework in the kernel supports this starting with 4.12 (this code is
> in the panda-cec branch above).
> 
> If this *can't* be supported by the omap4, then I will likely have to add a CEC
> capability to signal to the application that this specific corner case is not
> supported.
> 
> I just did some tests with omap4 and I my impression is that this can't be
> supported: when the HPD goes away it seems like most/all of the HDMI blocks are
> all powered off and any attempt to even access the CEC registers will fail.
> 
> Changing this looks to be non-trivial if not impossible.
> 
> Can you confirm that that isn't possible? If you think this can be done, then
> I'd appreciate if you can give me a few pointers.

HPD doesn't control the HW, it's all in the SW. So while I don't know
much at all about CEC and haven't looked at this particular use case, I
believe it's doable. HPD going off will make the DRM connector to be in
"disconnected" state, which on omapdrm will cause everything about HDMI
to be turned off.

Does it work on some other DRM driver? I'm wondering if there's
something in the DRM framework side that also has to be changed, in
addition to omapdrm changes.

It could require larger SW redesigns, though... Which makes me think
that the work shouldn't be done until we have changed the omapdrm's
driver model to DRM's common bridge driver model, and then all this
could possibly be done in a more generic manner.

Well, then again, I think the hdmi driver's internal power state
handling could be improved even before that. Currently it's not very
versatile. We should have ways to partially enable the IP for just the
required parts.

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-10 11:59         ` Tomi Valkeinen
  (?)
@ 2017-04-12 13:03         ` Hans Verkuil
  2017-04-12 13:10           ` Hans Verkuil
  2017-04-12 13:21             ` Tomi Valkeinen
  -1 siblings, 2 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-12 13:03 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

Hi Tomi,

On 04/10/2017 01:59 PM, Tomi Valkeinen wrote:
> On 08/04/17 13:11, Hans Verkuil wrote:
> 
>> So, this is a bit of a blast from the past since the omap4 CEC development
>> has been on hold for almost a year. But I am about to resume my work on this
>> now that the CEC framework was merged.
>>
>> The latest code is here, if you are interested:
>>
>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec
>>
>> It's pretty much unchanged from the version I posted a year ago, just rebased.
>>
>> But before I continue with this I have one question for you. First some
>> background:
>>
>> There is a special corner case (and I wasn't aware of that a year ago!) where
>> it is allowed to send a CEC message when there is *no HPD*.
>>
>> The reason is that some displays turn off the hotplug detect pin when they go
>> into standby or when another input is active. The only way to communicate with
>> such displays is via CEC.
>>
>> The problem is that without a HPD there is no EDID and basically no way for an
>> HDMI transmitter to detect that something is connected at all, unless you are
>> using CEC.
>>
>> What this means is that if we want to implement this on the omap4 the CEC support
>> has to be on all the time.
>>
>> We have seen modern displays that behave like this, so this is a real issue. And
>> this corner case is specifically allowed by the CEC specification: the Poll,
>> Image/Text View On and the Active Source messages can be sent to a TV even when
>> there is no HPD in order to turn on the display if it was in standby and to make
>> us the active input.
>>
>> The CEC framework in the kernel supports this starting with 4.12 (this code is
>> in the panda-cec branch above).
>>
>> If this *can't* be supported by the omap4, then I will likely have to add a CEC
>> capability to signal to the application that this specific corner case is not
>> supported.
>>
>> I just did some tests with omap4 and I my impression is that this can't be
>> supported: when the HPD goes away it seems like most/all of the HDMI blocks are
>> all powered off and any attempt to even access the CEC registers will fail.
>>
>> Changing this looks to be non-trivial if not impossible.
>>
>> Can you confirm that that isn't possible? If you think this can be done, then
>> I'd appreciate if you can give me a few pointers.
> 
> HPD doesn't control the HW, it's all in the SW. So while I don't know
> much at all about CEC and haven't looked at this particular use case, I
> believe it's doable. HPD going off will make the DRM connector to be in
> "disconnected" state, which on omapdrm will cause everything about HDMI
> to be turned off.
> 
> Does it work on some other DRM driver? I'm wondering if there's
> something in the DRM framework side that also has to be changed, in
> addition to omapdrm changes.
> 
> It could require larger SW redesigns, though... Which makes me think
> that the work shouldn't be done until we have changed the omapdrm's
> driver model to DRM's common bridge driver model, and then all this
> could possibly be done in a more generic manner.
> 
> Well, then again, I think the hdmi driver's internal power state
> handling could be improved even before that. Currently it's not very
> versatile. We should have ways to partially enable the IP for just the
> required parts.

I noticed while experimenting with this that tpd_disconnect() in
drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when
I disconnect the HDMI cable. Is that a bug somewhere?

I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable
and tpd_disable calls are properly balanced and I see the tpd_disable when I
disconnect the HDMI cable.

The key to keeping CEC up and running, even when there is no HPD is to keep
the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be
on, otherwise I won't get any CEC interrupts.

So if the omap4 CEC support is enabled in the kernel config, then always enable
this regulator and irq, and otherwise keep the current code.

Does that look sensible to you?

Regards,

	Hans

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-12 13:03         ` Hans Verkuil
@ 2017-04-12 13:10           ` Hans Verkuil
  2017-04-12 13:21             ` Tomi Valkeinen
  1 sibling, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-12 13:10 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

On 04/12/2017 03:03 PM, Hans Verkuil wrote:
> Hi Tomi,
> 
> On 04/10/2017 01:59 PM, Tomi Valkeinen wrote:
>> On 08/04/17 13:11, Hans Verkuil wrote:
>>
>>> So, this is a bit of a blast from the past since the omap4 CEC development
>>> has been on hold for almost a year. But I am about to resume my work on this
>>> now that the CEC framework was merged.
>>>
>>> The latest code is here, if you are interested:
>>>
>>> https://git.linuxtv.org/hverkuil/media_tree.git/log/?h=panda-cec
>>>
>>> It's pretty much unchanged from the version I posted a year ago, just rebased.
>>>
>>> But before I continue with this I have one question for you. First some
>>> background:
>>>
>>> There is a special corner case (and I wasn't aware of that a year ago!) where
>>> it is allowed to send a CEC message when there is *no HPD*.
>>>
>>> The reason is that some displays turn off the hotplug detect pin when they go
>>> into standby or when another input is active. The only way to communicate with
>>> such displays is via CEC.
>>>
>>> The problem is that without a HPD there is no EDID and basically no way for an
>>> HDMI transmitter to detect that something is connected at all, unless you are
>>> using CEC.
>>>
>>> What this means is that if we want to implement this on the omap4 the CEC support
>>> has to be on all the time.
>>>
>>> We have seen modern displays that behave like this, so this is a real issue. And
>>> this corner case is specifically allowed by the CEC specification: the Poll,
>>> Image/Text View On and the Active Source messages can be sent to a TV even when
>>> there is no HPD in order to turn on the display if it was in standby and to make
>>> us the active input.
>>>
>>> The CEC framework in the kernel supports this starting with 4.12 (this code is
>>> in the panda-cec branch above).
>>>
>>> If this *can't* be supported by the omap4, then I will likely have to add a CEC
>>> capability to signal to the application that this specific corner case is not
>>> supported.
>>>
>>> I just did some tests with omap4 and I my impression is that this can't be
>>> supported: when the HPD goes away it seems like most/all of the HDMI blocks are
>>> all powered off and any attempt to even access the CEC registers will fail.
>>>
>>> Changing this looks to be non-trivial if not impossible.
>>>
>>> Can you confirm that that isn't possible? If you think this can be done, then
>>> I'd appreciate if you can give me a few pointers.
>>
>> HPD doesn't control the HW, it's all in the SW. So while I don't know
>> much at all about CEC and haven't looked at this particular use case, I
>> believe it's doable. HPD going off will make the DRM connector to be in
>> "disconnected" state, which on omapdrm will cause everything about HDMI
>> to be turned off.
>>
>> Does it work on some other DRM driver? I'm wondering if there's
>> something in the DRM framework side that also has to be changed, in
>> addition to omapdrm changes.
>>
>> It could require larger SW redesigns, though... Which makes me think
>> that the work shouldn't be done until we have changed the omapdrm's
>> driver model to DRM's common bridge driver model, and then all this
>> could possibly be done in a more generic manner.
>>
>> Well, then again, I think the hdmi driver's internal power state
>> handling could be improved even before that. Currently it's not very
>> versatile. We should have ways to partially enable the IP for just the
>> required parts.
> 
> I noticed while experimenting with this that tpd_disconnect() in
> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when
> I disconnect the HDMI cable. Is that a bug somewhere?
> 
> I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable
> and tpd_disable calls are properly balanced and I see the tpd_disable when I
> disconnect the HDMI cable.
> 
> The key to keeping CEC up and running, even when there is no HPD is to keep
> the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be
> on, otherwise I won't get any CEC interrupts.
> 
> So if the omap4 CEC support is enabled in the kernel config, then always enable
> this regulator and irq, and otherwise keep the current code.

And of course the ls_oe_gpio in tpd12s015.c should always be high as well, otherwise
the CEC signal would never get through. This too can depend on the kernel config.

	Hans

> 
> Does that look sensible to you?
> 
> Regards,
> 
> 	Hans
> 

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-12 13:03         ` Hans Verkuil
@ 2017-04-12 13:21             ` Tomi Valkeinen
  2017-04-12 13:21             ` Tomi Valkeinen
  1 sibling, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-12 13:21 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

On 12/04/17 16:03, Hans Verkuil wrote:

> I noticed while experimenting with this that tpd_disconnect() in
> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when
> I disconnect the HDMI cable. Is that a bug somewhere?
> 
> I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable
> and tpd_disable calls are properly balanced and I see the tpd_disable when I
> disconnect the HDMI cable.

The connect/disconnect naming there is legacy... It's not about cable
connect, it's about the initial "connect" of the drivers in the video
pipeline. It's done just once when starting omapdrm.

> The key to keeping CEC up and running, even when there is no HPD is to keep
> the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be
> on, otherwise I won't get any CEC interrupts.

At the moment there's no way to enable the pipeline without enabling the
video.

> So if the omap4 CEC support is enabled in the kernel config, then always enable
> this regulator and irq, and otherwise keep the current code.

Well, I have no clue about how CEC is used, but don't we have some
userspace components using it? I presume there's an open() or something
similar that signals that the userspace is interested in CEC. That
should be the trigger to enable the HW required for CEC.

So is some other driver supporting this already? Or is the omap4 the
first platform you're trying this on?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2017-04-12 13:21             ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-12 13:21 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1470 bytes --]

On 12/04/17 16:03, Hans Verkuil wrote:

> I noticed while experimenting with this that tpd_disconnect() in
> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when
> I disconnect the HDMI cable. Is that a bug somewhere?
> 
> I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable
> and tpd_disable calls are properly balanced and I see the tpd_disable when I
> disconnect the HDMI cable.

The connect/disconnect naming there is legacy... It's not about cable
connect, it's about the initial "connect" of the drivers in the video
pipeline. It's done just once when starting omapdrm.

> The key to keeping CEC up and running, even when there is no HPD is to keep
> the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be
> on, otherwise I won't get any CEC interrupts.

At the moment there's no way to enable the pipeline without enabling the
video.

> So if the omap4 CEC support is enabled in the kernel config, then always enable
> this regulator and irq, and otherwise keep the current code.

Well, I have no clue about how CEC is used, but don't we have some
userspace components using it? I presume there's an open() or something
similar that signals that the userspace is interested in CEC. That
should be the trigger to enable the HW required for CEC.

So is some other driver supporting this already? Or is the omap4 the
first platform you're trying this on?

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-12 13:21             ` Tomi Valkeinen
  (?)
@ 2017-04-12 14:04             ` Hans Verkuil
  2017-04-13  8:43                 ` Tomi Valkeinen
  -1 siblings, 1 reply; 31+ messages in thread
From: Hans Verkuil @ 2017-04-12 14:04 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

On 04/12/2017 03:21 PM, Tomi Valkeinen wrote:
> On 12/04/17 16:03, Hans Verkuil wrote:
> 
>> I noticed while experimenting with this that tpd_disconnect() in
>> drivers/gpu/drm/omapdrm/displays/encoder-tpd12s015.c isn't called when
>> I disconnect the HDMI cable. Is that a bug somewhere?
>>
>> I would expect that tpd_connect and tpd_disconnect are balanced. The tpd_enable
>> and tpd_disable calls are properly balanced and I see the tpd_disable when I
>> disconnect the HDMI cable.
> 
> The connect/disconnect naming there is legacy... It's not about cable
> connect, it's about the initial "connect" of the drivers in the video
> pipeline. It's done just once when starting omapdrm.

Ah, good to know.

>> The key to keeping CEC up and running, even when there is no HPD is to keep
>> the hdmi.vdda_reg regulator enabled. Also the HDMI_IRQ_CORE should always be
>> on, otherwise I won't get any CEC interrupts.
> 
> At the moment there's no way to enable the pipeline without enabling the
> video.
> 
>> So if the omap4 CEC support is enabled in the kernel config, then always enable
>> this regulator and irq, and otherwise keep the current code.
> 
> Well, I have no clue about how CEC is used, but don't we have some
> userspace components using it? I presume there's an open() or something
> similar that signals that the userspace is interested in CEC. That
> should be the trigger to enable the HW required for CEC.

Why didn't I think of that. I did a quick implementation to test this and it
works.

> So is some other driver supporting this already? Or is the omap4 the
> first platform you're trying this on?

No, there are quite a few CEC drivers by now, but typically the CEC block is
a totally independent IP block with its own power, irq, etc. The omap4 is by far
the most complex one to set up with various GPIO pins, interrupts, regulators,
etc. to deal with.

Normally it takes about 2 days to make a new CEC driver, but the omap4 is much
more work :-(

Regards,

	Hans

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-12 14:04             ` Hans Verkuil
@ 2017-04-13  8:43                 ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-13  8:43 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

On 12/04/17 17:04, Hans Verkuil wrote:

>> So is some other driver supporting this already? Or is the omap4 the
>> first platform you're trying this on?
> 
> No, there are quite a few CEC drivers by now, but typically the CEC block is
> a totally independent IP block with its own power, irq, etc. The omap4 is by far
> the most complex one to set up with various GPIO pins, interrupts, regulators,
> etc. to deal with.
> 
> Normally it takes about 2 days to make a new CEC driver, but the omap4 is much
> more work :-(

Ok.

I mentioned the omapdrm restructuring that we've planned to do, I think
after that this will be easier to implement in a nice way.

For now, I think more or less what you have now is an acceptable
solution. We can hack the tpd12s015 to keep the level shifter always
enabled, and, afaics, everything else can be handled inside the hdmi4
driver, right?

Generally speaking, what are the "dependencies" for CEC? It needs to
access EDID? Does CEC care about HPD? Does it care if the cable is
connected or not? For Panda, the level shifter of tpd12s015 is obviously
one hard dendency.

Is there anything else CEC needs to access or control (besides the CEC
IP itself)?

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2017-04-13  8:43                 ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-13  8:43 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: Hans Verkuil, dri-devel


[-- Attachment #1.1.1: Type: text/plain, Size: 1231 bytes --]

On 12/04/17 17:04, Hans Verkuil wrote:

>> So is some other driver supporting this already? Or is the omap4 the
>> first platform you're trying this on?
> 
> No, there are quite a few CEC drivers by now, but typically the CEC block is
> a totally independent IP block with its own power, irq, etc. The omap4 is by far
> the most complex one to set up with various GPIO pins, interrupts, regulators,
> etc. to deal with.
> 
> Normally it takes about 2 days to make a new CEC driver, but the omap4 is much
> more work :-(

Ok.

I mentioned the omapdrm restructuring that we've planned to do, I think
after that this will be easier to implement in a nice way.

For now, I think more or less what you have now is an acceptable
solution. We can hack the tpd12s015 to keep the level shifter always
enabled, and, afaics, everything else can be handled inside the hdmi4
driver, right?

Generally speaking, what are the "dependencies" for CEC? It needs to
access EDID? Does CEC care about HPD? Does it care if the cable is
connected or not? For Panda, the level shifter of tpd12s015 is obviously
one hard dendency.

Is there anything else CEC needs to access or control (besides the CEC
IP itself)?

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

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

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-13  8:43                 ` Tomi Valkeinen
@ 2017-04-13  9:12                   ` Hans Verkuil
  -1 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-13  9:12 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: dri-devel, Hans Verkuil

On 04/13/2017 10:43 AM, Tomi Valkeinen wrote:
> On 12/04/17 17:04, Hans Verkuil wrote:
> 
>>> So is some other driver supporting this already? Or is the omap4 the
>>> first platform you're trying this on?
>>
>> No, there are quite a few CEC drivers by now, but typically the CEC block is
>> a totally independent IP block with its own power, irq, etc. The omap4 is by far
>> the most complex one to set up with various GPIO pins, interrupts, regulators,
>> etc. to deal with.
>>
>> Normally it takes about 2 days to make a new CEC driver, but the omap4 is much
>> more work :-(
> 
> Ok.
> 
> I mentioned the omapdrm restructuring that we've planned to do, I think
> after that this will be easier to implement in a nice way.
> 
> For now, I think more or less what you have now is an acceptable
> solution. We can hack the tpd12s015 to keep the level shifter always
> enabled, and, afaics, everything else can be handled inside the hdmi4
> driver, right?

Right.

> Generally speaking, what are the "dependencies" for CEC? It needs to
> access EDID? Does CEC care about HPD? Does it care if the cable is
> connected or not? For Panda, the level shifter of tpd12s015 is obviously
> one hard dendency.
> 
> Is there anything else CEC needs to access or control (besides the CEC
> IP itself)?

The CEC framework needs to be informed about the physical address contained
in the EDID (part of the CEA-861 block). And when the HPD goes down it needs
to be informed as well (same call, but with CEC_PHYS_ADDR_INVALID as argument).

And it needs to stay powered up even if the HPD is down.

That's all.

Regards,

	Hans

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2017-04-13  9:12                   ` Hans Verkuil
  0 siblings, 0 replies; 31+ messages in thread
From: Hans Verkuil @ 2017-04-13  9:12 UTC (permalink / raw)
  To: Tomi Valkeinen, linux-media; +Cc: Hans Verkuil, dri-devel

On 04/13/2017 10:43 AM, Tomi Valkeinen wrote:
> On 12/04/17 17:04, Hans Verkuil wrote:
> 
>>> So is some other driver supporting this already? Or is the omap4 the
>>> first platform you're trying this on?
>>
>> No, there are quite a few CEC drivers by now, but typically the CEC block is
>> a totally independent IP block with its own power, irq, etc. The omap4 is by far
>> the most complex one to set up with various GPIO pins, interrupts, regulators,
>> etc. to deal with.
>>
>> Normally it takes about 2 days to make a new CEC driver, but the omap4 is much
>> more work :-(
> 
> Ok.
> 
> I mentioned the omapdrm restructuring that we've planned to do, I think
> after that this will be easier to implement in a nice way.
> 
> For now, I think more or less what you have now is an acceptable
> solution. We can hack the tpd12s015 to keep the level shifter always
> enabled, and, afaics, everything else can be handled inside the hdmi4
> driver, right?

Right.

> Generally speaking, what are the "dependencies" for CEC? It needs to
> access EDID? Does CEC care about HPD? Does it care if the cable is
> connected or not? For Panda, the level shifter of tpd12s015 is obviously
> one hard dendency.
> 
> Is there anything else CEC needs to access or control (besides the CEC
> IP itself)?

The CEC framework needs to be informed about the physical address contained
in the EDID (part of the CEA-861 block). And when the HPD goes down it needs
to be informed as well (same call, but with CEC_PHYS_ADDR_INVALID as argument).

And it needs to stay powered up even if the HPD is down.

That's all.

Regards,

	Hans
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
  2017-04-13  9:12                   ` Hans Verkuil
@ 2017-04-13  9:26                     ` Tomi Valkeinen
  -1 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-13  9:26 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

On 13/04/17 12:12, Hans Verkuil wrote:

>> Is there anything else CEC needs to access or control (besides the CEC
>> IP itself)?
> 
> The CEC framework needs to be informed about the physical address contained
> in the EDID (part of the CEA-861 block). And when the HPD goes down it needs
> to be informed as well (same call, but with CEC_PHYS_ADDR_INVALID as argument).

Ah, hmm... And currently that's (kind of) handled in
hdmi_power_off_full() by setting CEC_PHYS_ADDR_INVALID? That's not the
same thing as HPD off, though, but maybe it's enough (for now).

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid
@ 2017-04-13  9:26                     ` Tomi Valkeinen
  0 siblings, 0 replies; 31+ messages in thread
From: Tomi Valkeinen @ 2017-04-13  9:26 UTC (permalink / raw)
  To: Hans Verkuil, linux-media; +Cc: dri-devel, Hans Verkuil


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

On 13/04/17 12:12, Hans Verkuil wrote:

>> Is there anything else CEC needs to access or control (besides the CEC
>> IP itself)?
> 
> The CEC framework needs to be informed about the physical address contained
> in the EDID (part of the CEA-861 block). And when the HPD goes down it needs
> to be informed as well (same call, but with CEC_PHYS_ADDR_INVALID as argument).

Ah, hmm... And currently that's (kind of) handled in
hdmi_power_off_full() by setting CEC_PHYS_ADDR_INVALID? That's not the
same thing as HPD off, though, but maybe it's enough (for now).

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2017-04-13  9:26 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-04-29  9:39 [RFC PATCH 0/3] OMAP4 HDMI CEC support Hans Verkuil
2016-04-29  9:39 ` Hans Verkuil
2016-04-29  9:39 ` [RFC PATCH 1/3] drm/omap: fix OMAP4 hdmi_core_powerdown_disable() Hans Verkuil
2016-04-29  9:39   ` Hans Verkuil
2016-04-29  9:39 ` [RFC PATCH 2/3] omap4: add CEC support Hans Verkuil
2016-04-29  9:39   ` Hans Verkuil
2016-05-10 12:01   ` Tomi Valkeinen
2016-05-10 12:01     ` Tomi Valkeinen
2016-05-10 12:26     ` Hans Verkuil
2016-05-10 12:41       ` Tomi Valkeinen
2016-05-10 12:41         ` Tomi Valkeinen
2016-04-29  9:39 ` [RFC PATCH 3/3] encoder-tpd12s015: keep the ls_oe_gpio on while the phys_addr is valid Hans Verkuil
2016-04-29  9:39   ` Hans Verkuil
2016-05-10 11:36   ` Tomi Valkeinen
2016-05-10 11:36     ` Tomi Valkeinen
2017-04-08 10:11     ` Hans Verkuil
2017-04-08 10:57       ` Hans Verkuil
2017-04-08 10:57         ` Hans Verkuil
2017-04-10 11:59       ` Tomi Valkeinen
2017-04-10 11:59         ` Tomi Valkeinen
2017-04-12 13:03         ` Hans Verkuil
2017-04-12 13:10           ` Hans Verkuil
2017-04-12 13:21           ` Tomi Valkeinen
2017-04-12 13:21             ` Tomi Valkeinen
2017-04-12 14:04             ` Hans Verkuil
2017-04-13  8:43               ` Tomi Valkeinen
2017-04-13  8:43                 ` Tomi Valkeinen
2017-04-13  9:12                 ` Hans Verkuil
2017-04-13  9:12                   ` Hans Verkuil
2017-04-13  9:26                   ` Tomi Valkeinen
2017-04-13  9:26                     ` Tomi Valkeinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.