[PATCHv9 07/15] cec: add HDMI CEC framework

[Hans Verkuil]

On 10/15/2015 07:34 PM, Russell King - ARM Linux wrote:
> On Wed, Oct 14, 2015 at 08:29:44AM +0200, Hans Verkuil wrote:
>> On 10/14/2015 12:51 AM, Russell King - ARM Linux wrote:
>>> On Mon, Oct 12, 2015 at 01:35:54PM +0200, Hans Verkuil wrote:
 On 10/06/2015 07:06 PM, Russell King - ARM Linux wrote:
> Surely you aren't proposing that drivers should write directly to
> adap->phys_addr without calling some notification function that the
> physical address has changed?

 Userspace is informed through CEC_EVENT_STATE_CHANGE when the adapter is
 enabled/disabled. When the adapter is enabled and CEC_CAP_PHYS_ADDR is
 not set (i.e. the kernel takes care of this), then calling 
 CEC_ADAP_G_PHYS_ADDR
 returns the new physical address.
>>>
>>> Okay, so when I see the EDID arrive, I should be doing:
>>>
>>> phys = parse_hdmi_addr(block->edid);
>>> cec->adap->phys_addr = phys;
>>> cec_enable(cec->adap, true);
>>>
>>> IOW, you _are_ expecting adap->phys_addr to be written, but only while
>>> the adapter is disabled?
>>
>> Right.
>>
>> And when the hotplug goes down you should call cec_enable(cec->adap, false).
>> While the adapter is disabled, CEC_ADAP_G_PHYS_ADDR will always return
>> CEC_PHYS_ADDR_INVALID regardless of the cec->adap->phys_addr value.
> 
> There seems to be a few bugs.  Is there a way to monitor (in a similar
> way to tcpdump) the activity on the bus?

To monitor the bus use:

cec-ctl -m

This will monitor any CEC messages sent by the CEC adapter, any CEC broadcast
messages received and any CEC messages directed to the configured logical
address(es) of the CEC adapter.

I have yet to see hardware that can see CEC messages directed to other
devices. If your HW can do that, then it would be nice to add support for that
to the CEC framework.

> What I'm seeing is that if the TV is switched to the appropriate AV
> input, and then I do:
> 
>   cec-ctl --playback
> 
> to use the kernel to pick up a playback logical address, I then can't
> use the remote control media playback keys until I switch away from
> the AV input and back to it.
> 

I've found cec-ctl -m very useful for debugging, it's hard to see what's
going on otherwise.

Regards,

Hans

[PATCHv9 07/15] cec: add HDMI CEC framework

[Russell King - ARM Linux]

On Wed, Oct 14, 2015 at 08:29:44AM +0200, Hans Verkuil wrote:
> On 10/14/2015 12:51 AM, Russell King - ARM Linux wrote:
> > On Mon, Oct 12, 2015 at 01:35:54PM +0200, Hans Verkuil wrote:
> >> On 10/06/2015 07:06 PM, Russell King - ARM Linux wrote:
> >>> Surely you aren't proposing that drivers should write directly to
> >>> adap->phys_addr without calling some notification function that the
> >>> physical address has changed?
> >>
> >> Userspace is informed through CEC_EVENT_STATE_CHANGE when the adapter is
> >> enabled/disabled. When the adapter is enabled and CEC_CAP_PHYS_ADDR is
> >> not set (i.e. the kernel takes care of this), then calling 
> >> CEC_ADAP_G_PHYS_ADDR
> >> returns the new physical address.
> > 
> > Okay, so when I see the EDID arrive, I should be doing:
> > 
> > phys = parse_hdmi_addr(block->edid);
> > cec->adap->phys_addr = phys;
> > cec_enable(cec->adap, true);
> > 
> > IOW, you _are_ expecting adap->phys_addr to be written, but only while
> > the adapter is disabled?
> 
> Right.
> 
> And when the hotplug goes down you should call cec_enable(cec->adap, false).
> While the adapter is disabled, CEC_ADAP_G_PHYS_ADDR will always return
> CEC_PHYS_ADDR_INVALID regardless of the cec->adap->phys_addr value.

There seems to be a few bugs.  Is there a way to monitor (in a similar
way to tcpdump) the activity on the bus?

What I'm seeing is that if the TV is switched to the appropriate AV
input, and then I do:

cec-ctl --playback

to use the kernel to pick up a playback logical address, I then can't
use the remote control media playback keys until I switch away from
the AV input and back to it.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCHv9 07/15] cec: add HDMI CEC framework

[Hans Verkuil]

On 10/14/2015 12:51 AM, Russell King - ARM Linux wrote:
> On Mon, Oct 12, 2015 at 01:35:54PM +0200, Hans Verkuil wrote:
>> On 10/06/2015 07:06 PM, Russell King - ARM Linux wrote:
>>> Surely you aren't proposing that drivers should write directly to
>>> adap->phys_addr without calling some notification function that the
>>> physical address has changed?
>>
>> Userspace is informed through CEC_EVENT_STATE_CHANGE when the adapter is
>> enabled/disabled. When the adapter is enabled and CEC_CAP_PHYS_ADDR is
>> not set (i.e. the kernel takes care of this), then calling 
>> CEC_ADAP_G_PHYS_ADDR
>> returns the new physical address.
> 
> Okay, so when I see the EDID arrive, I should be doing:
> 
> phys = parse_hdmi_addr(block->edid);
> cec->adap->phys_addr = phys;
> cec_enable(cec->adap, true);
> 
> IOW, you _are_ expecting adap->phys_addr to be written, but only while
> the adapter is disabled?

Right.

And when the hotplug goes down you should call cec_enable(cec->adap, false).
While the adapter is disabled, CEC_ADAP_G_PHYS_ADDR will always return
CEC_PHYS_ADDR_INVALID regardless of the cec->adap->phys_addr value.

Regards,

Hans

[PATCHv9 07/15] cec: add HDMI CEC framework

[Russell King - ARM Linux]

On Mon, Oct 12, 2015 at 01:35:54PM +0200, Hans Verkuil wrote:
> On 10/06/2015 07:06 PM, Russell King - ARM Linux wrote:
> > Surely you aren't proposing that drivers should write directly to
> > adap->phys_addr without calling some notification function that the
> > physical address has changed?
> 
> Userspace is informed through CEC_EVENT_STATE_CHANGE when the adapter is
> enabled/disabled. When the adapter is enabled and CEC_CAP_PHYS_ADDR is
> not set (i.e. the kernel takes care of this), then calling 
> CEC_ADAP_G_PHYS_ADDR
> returns the new physical address.

Okay, so when I see the EDID arrive, I should be doing:

phys = parse_hdmi_addr(block->edid);
cec->adap->phys_addr = phys;
cec_enable(cec->adap, true);

IOW, you _are_ expecting adap->phys_addr to be written, but only while
the adapter is disabled?

Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCHv9 07/15] cec: add HDMI CEC framework

[Hans Verkuil]

On 10/06/2015 07:06 PM, Russell King - ARM Linux wrote:
> On Mon, Sep 07, 2015 at 03:44:36PM +0200, Hans Verkuil wrote:
>> From: Hans Verkuil 
>>
>> The added HDMI CEC framework provides a generic kernel interface for
>> HDMI CEC devices.
>>
>> Signed-off-by: Hans Verkuil 
>> [k.debski at samsung.com: Merged CEC Updates commit by Hans Verkuil]
>> [k.debski at samsung.com: Merged Update author commit by Hans Verkuil]
>> [k.debski at samsung.com: change kthread handling when setting logical
>> address]
>> [k.debski at samsung.com: code cleanup and fixes]
>> [k.debski at samsung.com: add missing CEC commands to match spec]
>> [k.debski at samsung.com: add RC framework support]
>> [k.debski at samsung.com: move and edit documentation]
>> [k.debski at samsung.com: add vendor id reporting]
>> [k.debski at samsung.com: add possibility to clear assigned logical
>> addresses]
>> [k.debski at samsung.com: documentation fixes, clenaup and expansion]
>> [k.debski at samsung.com: reorder of API structs and add reserved fields]
>> [k.debski at samsung.com: fix handling of events and fix 32/64bit timespec
>> problem]
>> [k.debski at samsung.com: add cec.h to include/uapi/linux/Kbuild]
>> [k.debski at samsung.com: add sequence number handling]
>> [k.debski at samsung.com: add passthrough mode]
>> [k.debski at samsung.com: fix CEC defines, add missing CEC 2.0 commands]
>> minor additions]
>> Signed-off-by: Kamil Debski 
>> Signed-off-by: Hans Verkuil 
> 
> I don't see much in the way of support for source devices in this:
> how do we handle hotplug of the sink, and how to do we configure the
> physical address?

The source device driver should call cec_enable(false) if the hotplug
goes down and cec_enable(true) when the EDID has been read and the physical
address has been retrieved and configured in the cec adapter.

This however assumes that the source driver is the one controlling the
CEC hardware. This is the case for the cobalt driver, but not apparently
for the exynos hardware. In that case userspace will have to handle this:
disable the CEC adapter when the hotplug disappears, set the physical address
and enable the CEC adapter when a new EDID is read.

Such drivers have the CEC_CAP_STATE and CEC_CAP_PHYS_ADDR caps set. I.e.
they expect that userspace does this.

This is also something that USB CEC dongles will do, although I don't have
a driver for that.

> Surely you aren't proposing that drivers should write directly to
> adap->phys_addr without calling some notification function that the
> physical address has changed?

Userspace is informed through CEC_EVENT_STATE_CHANGE when the adapter is
enabled/disabled. When the adapter is enabled and CEC_CAP_PHYS_ADDR is
not set (i.e. the kernel takes care of this), then calling CEC_ADAP_G_PHYS_ADDR
returns the new physical address.

> Please can you give some guidance on how a HDMI source bridge driver
> should deal with these issues.  Thanks.
> 

The cec.txt file will give more information about the kernel internals.
All I need is time (ha!) to update that file since the current version
is completely outdated (as mentioned in the cover letter).

Regards,

Hans

[PATCHv9 07/15] cec: add HDMI CEC framework

[Russell King - ARM Linux]

On Mon, Sep 07, 2015 at 03:44:36PM +0200, Hans Verkuil wrote:
> From: Hans Verkuil 
> 
> The added HDMI CEC framework provides a generic kernel interface for
> HDMI CEC devices.
> 
> Signed-off-by: Hans Verkuil 
> [k.debski at samsung.com: Merged CEC Updates commit by Hans Verkuil]
> [k.debski at samsung.com: Merged Update author commit by Hans Verkuil]
> [k.debski at samsung.com: change kthread handling when setting logical
> address]
> [k.debski at samsung.com: code cleanup and fixes]
> [k.debski at samsung.com: add missing CEC commands to match spec]
> [k.debski at samsung.com: add RC framework support]
> [k.debski at samsung.com: move and edit documentation]
> [k.debski at samsung.com: add vendor id reporting]
> [k.debski at samsung.com: add possibility to clear assigned logical
> addresses]
> [k.debski at samsung.com: documentation fixes, clenaup and expansion]
> [k.debski at samsung.com: reorder of API structs and add reserved fields]
> [k.debski at samsung.com: fix handling of events and fix 32/64bit timespec
> problem]
> [k.debski at samsung.com: add cec.h to include/uapi/linux/Kbuild]
> [k.debski at samsung.com: add sequence number handling]
> [k.debski at samsung.com: add passthrough mode]
> [k.debski at samsung.com: fix CEC defines, add missing CEC 2.0 commands]
> minor additions]
> Signed-off-by: Kamil Debski 
> Signed-off-by: Hans Verkuil 

I don't see much in the way of support for source devices in this:
how do we handle hotplug of the sink, and how to do we configure the
physical address?

Surely you aren't proposing that drivers should write directly to
adap->phys_addr without calling some notification function that the
physical address has changed?

Please can you give some guidance on how a HDMI source bridge driver
should deal with these issues.  Thanks.

-- 
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.

[PATCHv9 07/15] cec: add HDMI CEC framework

[Hans Verkuil]

From: Hans Verkuil 

The added HDMI CEC framework provides a generic kernel interface for
HDMI CEC devices.

Signed-off-by: Hans Verkuil 
[k.debski at samsung.com: Merged CEC Updates commit by Hans Verkuil]
[k.debski at samsung.com: Merged Update author commit by Hans Verkuil]
[k.debski at samsung.com: change kthread handling when setting logical
address]
[k.debski at samsung.com: code cleanup and fixes]
[k.debski at samsung.com: add missing CEC commands to match spec]
[k.debski at samsung.com: add RC framework support]
[k.debski at samsung.com: move and edit documentation]
[k.debski at samsung.com: add vendor id reporting]
[k.debski at samsung.com: add possibility to clear assigned logical
addresses]
[k.debski at samsung.com: documentation fixes, clenaup and expansion]
[k.debski at samsung.com: reorder of API structs and add reserved fields]
[k.debski at samsung.com: fix handling of events and fix 32/64bit timespec
problem]
[k.debski at samsung.com: add cec.h to include/uapi/linux/Kbuild]
[k.debski at samsung.com: add sequence number handling]
[k.debski at samsung.com: add passthrough mode]
[k.debski at samsung.com: fix CEC defines, add missing CEC 2.0 commands]
minor additions]
Signed-off-by: Kamil Debski 
Signed-off-by: Hans Verkuil 
---
 MAINTAINERS|   12 +
 drivers/media/Kconfig  |6 +
 drivers/media/Makefile |2 +
 drivers/media/cec.c| 1921 
 include/media/cec.h|  175 
 include/uapi/linux/Kbuild  |2 +
 include/uapi/linux/cec-funcs.h | 1771 
 include/uapi/linux/cec.h   |  781 
 8 files changed, 4670 insertions(+)
 create mode 100644 drivers/media/cec.c
 create mode 100644 include/media/cec.h
 create mode 100644 include/uapi/linux/cec-funcs.h
 create mode 100644 include/uapi/linux/cec.h

diff --git a/MAINTAINERS b/MAINTAINERS
index e0946a0..2904a66 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2553,6 +2553,18 @@ F:   drivers/net/ieee802154/cc2520.c
 F: include/linux/spi/cc2520.h
 F: Documentation/devicetree/bindings/net/ieee802154/cc2520.txt

+CEC DRIVER
+M: Hans Verkuil 
+L: linux-media at vger.kernel.org
+T: git git://linuxtv.org/media_tree.git
+W: http://linuxtv.org
+S: Supported
+F: drivers/media/cec.c
+F: drivers/media/rc/keymaps/rc-cec.c
+F: include/media/cec.h
+F: include/uapi/linux/cec.h
+F: include/uapi/linux/cec-funcs.h
+
 CELL BROADBAND ENGINE ARCHITECTURE
 M: Arnd Bergmann 
 L: linuxppc-dev at lists.ozlabs.org
diff --git a/drivers/media/Kconfig b/drivers/media/Kconfig
index 3ef3d6c..48e44f5 100644
--- a/drivers/media/Kconfig
+++ b/drivers/media/Kconfig
@@ -15,6 +15,12 @@ if MEDIA_SUPPORT

 comment "Multimedia core support"

+config CEC
+   tristate "CEC API (EXPERIMENTAL)"
+   select RC_CORE
+   ---help---
+ Enable the CEC API.
+
 #
 # Multimedia support - automatically enable V4L2 and DVB core
 #
diff --git a/drivers/media/Makefile b/drivers/media/Makefile
index e608bbc..db66014 100644
--- a/drivers/media/Makefile
+++ b/drivers/media/Makefile
@@ -2,6 +2,8 @@
 # Makefile for the kernel multimedia device drivers.
 #

+obj-$(CONFIG_CEC) += cec.o
+
 media-objs := media-device.o media-devnode.o media-entity.o

 #
diff --git a/drivers/media/cec.c b/drivers/media/cec.c
new file mode 100644
index 000..7084d2c
--- /dev/null
+++ b/drivers/media/cec.c
@@ -0,0 +1,1921 @@
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define CEC_NUM_DEVICES256
+#define CEC_NAME   "cec"
+
+static int debug;
+module_param(debug, int, 0644);
+MODULE_PARM_DESC(debug, "debug level (0-2)");
+
+#define dprintk(lvl, fmt, arg...)  \
+   do {\
+   if (lvl <= debug)   \
+   pr_info("cec-%s: " fmt, adap->name, ## arg);\
+   } while (0)
+
+static dev_t cec_dev_t;
+
+/* Active devices */
+static DEFINE_MUTEX(cec_devnode_lock);
+static DECLARE_BITMAP(cec_devnode_nums, CEC_NUM_DEVICES);
+
+/* dev to cec_devnode */
+#define to_cec_devnode(cd) container_of(cd, struct cec_devnode, dev)
+
+static inline struct cec_devnode *cec_devnode_data(struct file *filp)
+{
+   struct cec_fh *fh = filp->private_data;
+
+   return >adap->devnode;
+}
+
+static bool cec_pa_are_adjacent(const struct cec_adapter *adap, u16 pa1, u16 
pa2)
+{
+   u16 mask = 0xf000;
+   int i;
+
+   if (pa1 == CEC_PHYS_ADDR_INVALID || pa2 == CEC_PHYS_ADDR_INVALID)
+   return false;
+   for (i = 0; i < 3; i++) {
+   if ((pa1 & mask) != (pa2 & mask))
+   break;
+   mask = (mask >> 4) | 0xf000;
+   }
+   if ((pa1 & ~mask) || (pa2 &