Re: [RFC PATCH 2/3] omap4: add CEC support

2016-05-10 Thread Tomi Valkeinen
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 = ;
>>> 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



signature.asc
Description: OpenPGP digital signature


Re: [RFC PATCH 2/3] omap4: add CEC support

2016-05-10 Thread 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 
>>
>> Signed-off-by: Hans Verkuil 
>> ---
>>  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 */
>>  _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 */
>>  _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 = ;
>>  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 

Re: [RFC PATCH 2/3] omap4: add CEC support

2016-05-10 Thread Tomi Valkeinen
Hi Hans,

On 29/04/16 12:39, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> Signed-off-by: Hans Verkuil 
> ---
>  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 */
>  _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 */
>  _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 = ;
>   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 

[RFC PATCH 2/3] omap4: add CEC support

2016-04-29 Thread Hans Verkuil
From: Hans Verkuil 

Signed-off-by: Hans Verkuil 
---
 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 */
 _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 */
 _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 = ;
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 
 #include 
 #include 
+#include 
 
 #include "dss.h"
 
@@ -83,6 +84,31 @@
 #define HDMI_TXPHY_PAD_CFG_CTRL0xC
 #define HDMI_TXPHY_BIST_CONTROL0x1C
 
+/* HDMI CEC */
+#define HDMI_CEC_DEV_ID 0x0
+#define HDMI_CEC_SPEC   0x4
+#define HDMI_CEC_DBG_3  0x1C
+#define HDMI_CEC_TX_INIT