Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-05-06 Thread Arnd Bergmann
On Wednesday 06 May 2015 17:58:01 Hans Verkuil wrote:
 
 On 05/04/2015 12:14 PM, Arnd Bergmann wrote:
  On Monday 04 May 2015 09:42:36 Hans Verkuil wrote:
  Ping! (Added Arnd to the CC list)
  
  Hi Hans,
  
  sorry I missed this the first time
  
  On 04/27/2015 09:40 AM, Hans Verkuil wrote:
  Added the y2038 mailinglist since I would like to get their input for
  this API.
 
  Y2038 experts, can you take a look at my comment in the code below?
 
  Thanks!
 
  Arnd, I just saw your patch series adding struct __kernel_timespec to
  uapi/linux/time.h. I get the feeling that it might take a few kernel
  cycles before we have a timespec64 available in userspace. Based on that
  I think this CEC API should drop the timestamps for now and wait until
  timespec64 becomes available before adding it.
 
  The timestamps are a nice-to-have, but not critical. So adding it later
  shouldn't be a problem. What is your opinion?
  
  It will take a little while for the patches to make it in, I would guess
  4.3 at the earliest. Using your own struct works just as well and would
  be less ambiguous.
  
  However, for timestamps, I would recommend not using timespec anyway.
  Instead, just use a single 64-bit nanosecond value from ktime_get_ns()
  (or ktime_get_boot_ns() if you need a time that keeps ticking across
  suspend). This is more efficient to get and simpler to use as long
  as you don't need to convert from nanosecond to timespec.
 
 Possibly stupid follow-up question:
 
 is ktime_get_ns() just a different representation as ktime_get_ts64()?

Yes.

 Or is there some offset between the two? They seem to be identical based
 on a quick test, but I'd like to be certain that that's always the case.

 Users need to be able to relate this timestamp to a struct timespec as
 returned by V4L2 (and others).

* ktime_get_ns() uses the same timebase as ktime_get_ts64().
* ktime_get_boot_ns() uses the same timebase as ktime_get_boottime() or
  getboottime64(), which differs from the first after suspend
* ktime_get_real_ns() uses the same time as gettimeofday() in user space,
  which is always different from the other two.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-05-06 Thread Hans Verkuil
Hi Arnd,

On 05/04/2015 12:14 PM, Arnd Bergmann wrote:
 On Monday 04 May 2015 09:42:36 Hans Verkuil wrote:
 Ping! (Added Arnd to the CC list)
 
 Hi Hans,
 
 sorry I missed this the first time
 
 On 04/27/2015 09:40 AM, Hans Verkuil wrote:
 Added the y2038 mailinglist since I would like to get their input for
 this API.

 Y2038 experts, can you take a look at my comment in the code below?

 Thanks!

 Arnd, I just saw your patch series adding struct __kernel_timespec to
 uapi/linux/time.h. I get the feeling that it might take a few kernel
 cycles before we have a timespec64 available in userspace. Based on that
 I think this CEC API should drop the timestamps for now and wait until
 timespec64 becomes available before adding it.

 The timestamps are a nice-to-have, but not critical. So adding it later
 shouldn't be a problem. What is your opinion?
 
 It will take a little while for the patches to make it in, I would guess
 4.3 at the earliest. Using your own struct works just as well and would
 be less ambiguous.
 
 However, for timestamps, I would recommend not using timespec anyway.
 Instead, just use a single 64-bit nanosecond value from ktime_get_ns()
 (or ktime_get_boot_ns() if you need a time that keeps ticking across
 suspend). This is more efficient to get and simpler to use as long
 as you don't need to convert from nanosecond to timespec.

Possibly stupid follow-up question:

is ktime_get_ns() just a different representation as ktime_get_ts64()?

Or is there some offset between the two? They seem to be identical based
on a quick test, but I'd like to be certain that that's always the case.

Users need to be able to relate this timestamp to a struct timespec as
returned by V4L2 (and others).

Thanks!

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-05-04 Thread Hans Verkuil
Ping! (Added Arnd to the CC list)

On 04/27/2015 09:40 AM, Hans Verkuil wrote:
 Added the y2038 mailinglist since I would like to get their input for
 this API.
 
 Y2038 experts, can you take a look at my comment in the code below?
 
 Thanks!

Arnd, I just saw your patch series adding struct __kernel_timespec to
uapi/linux/time.h. I get the feeling that it might take a few kernel
cycles before we have a timespec64 available in userspace. Based on that
I think this CEC API should drop the timestamps for now and wait until
timespec64 becomes available before adding it.

The timestamps are a nice-to-have, but not critical. So adding it later
shouldn't be a problem. What is your opinion?

Hans

 
 On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com

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

 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h

 
 snip
 
 diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
 index 4842a98..5854cfd 100644
 --- a/include/uapi/linux/Kbuild
 +++ b/include/uapi/linux/Kbuild
 @@ -81,6 +81,7 @@ header-y += capi.h
  header-y += cciss_defs.h
  header-y += cciss_ioctl.h
  header-y += cdrom.h
 +header-y += cec.h
  header-y += cgroupstats.h
  header-y += chio.h
  header-y += cm4000_cs.h
 diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
 new file mode 100644
 index 000..bb6d66c
 --- /dev/null
 +++ b/include/uapi/linux/cec.h
 @@ -0,0 +1,303 @@
 +#ifndef _CEC_H
 +#define _CEC_H
 +
 +#include linux/types.h
 +
 +struct cec_time {
 +__u64 sec;
 +__u64 nsec;
 +};
 
 I don't like having to introduce a new struct for time here. Basically we are
 only doing this because there is still no public struct timespec64.
 
 When will that struct become available for use in a public API? If it is 4.2,
 then we can start using it. If it will take longer, then what alternative can
 we use to prevent having to change the API later?
 
 One alternative might be to drop it for now and just reserve space in the
 structs to add it later.
 
 Input from the y2038 experts will be welcome!
 
 Regards,
 
   Hans
 
 +
 +struct cec_msg {
 +struct cec_time ts;
 +__u32 len;
 +__u32 status;
 +__u32 timeout;
 +/* timeout (in ms) is used to timeout CEC_RECEIVE.
 +   Set to 0 if you want to wait forever. */
 +__u8  msg[16];
 +__u8  reply;
 +/* If non-zero, then wait for a reply with this opcode.
 +   If there was an error when sending the msg or FeatureAbort
 +   was returned, then reply is set to 0.
 +   If reply is non-zero upon return, then len/msg are set to
 +   the received message.
 +   If reply is zero upon return and status has the
 +   CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
 +   received feature abort message.
 +   If reply is zero upon return and status has the
 +   CEC_TX_STATUS_REPLY_TIMEOUT
 +   bit set, then no reply was seen at all.
 +   This field is ignored with CEC_RECEIVE.
 +   If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
 +   then -EINVAL is returned.
 +   if reply is non-zero, then timeout is set to 1000 (the required
 +   maximum response time).
 + */
 +__u8 reserved[31];
 +};
 +
 +static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
 +{
 +return msg-msg[0]  4;
 +}
 +
 +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
 +{
 +return msg-msg[0]  0xf;
 +}
 +
 +static inline bool 

Re: [Y2038] [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-05-04 Thread Arnd Bergmann
On Monday 04 May 2015 09:42:36 Hans Verkuil wrote:
 Ping! (Added Arnd to the CC list)

Hi Hans,

sorry I missed this the first time

 On 04/27/2015 09:40 AM, Hans Verkuil wrote:
  Added the y2038 mailinglist since I would like to get their input for
  this API.
  
  Y2038 experts, can you take a look at my comment in the code below?
  
  Thanks!
 
 Arnd, I just saw your patch series adding struct __kernel_timespec to
 uapi/linux/time.h. I get the feeling that it might take a few kernel
 cycles before we have a timespec64 available in userspace. Based on that
 I think this CEC API should drop the timestamps for now and wait until
 timespec64 becomes available before adding it.
 
 The timestamps are a nice-to-have, but not critical. So adding it later
 shouldn't be a problem. What is your opinion?

It will take a little while for the patches to make it in, I would guess
4.3 at the earliest. Using your own struct works just as well and would
be less ambiguous.

However, for timestamps, I would recommend not using timespec anyway.
Instead, just use a single 64-bit nanosecond value from ktime_get_ns()
(or ktime_get_boot_ns() if you need a time that keeps ticking across
suspend). This is more efficient to get and simpler to use as long
as you don't need to convert from nanosecond to timespec.

Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Kamil Debski
Hi Lars, 

Thank you for your comments.

From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Lars Op den Kamp
Sent: Friday, April 24, 2015 12:04 PM
 
 Hi Kamil, Hans,
 
 I'm the main developer of libCEC
 (https://github.com/Pulse-Eight/libcec). Sorry for the late time to
 jump
 in here, but I wasn't signed up to this mailing list and someone
 pointed
 me to this discussion.
 
 Unfortunately this approach will not work with half the TVs that are
 out
 there. I'll explain why:
 
 * because of how some (common) brands implemented CEC in their TVs,
 this
 implementation will not work, as the TV will just reject it. In libCEC,
 we've created work arounds for brands like this. Without these work
 arounds, your in-kernel implementation will be very vendor specific.
 e.g. this implementation will work for Samsung's TVs, but not for the
 TVs made by another big TV brand. All commands, including CEC_OP_ABORT,
 should be passed to userspace to make it work with all brands.

 * it should be made possible to not have the kernel send any CEC
 message, try to process any received CEC message, or ack to any logical
 address at all, to allow libraries like libCEC to fully handle all CEC
 traffic. Some brands only enable routing of some CEC keys when a
 specific device type is used. libCEC will allocate a logical address of
 the correct type for the brand that's used. If another address is first
 allocated by the kernel, and the TV communicates with it to find out
 it's name and things like that, and libCEC allocates another address a
 bit later when initialised, then you'll end up with multiple entries in
 the device list on the TV, and only one of them will work.

Adding a special mode in the CEC framework that disables parsing and
processing seems like a good idea for me. This way libCEC could be
completely
in charge of how the communication is handled. 

I discussed this with Hans and he is for this solution. This way there would
be two modes:
- One with handling of CEC messages enabled in the kernel, in idea behind
  this is to have processing adhere to the CEC spec as closely as possible.
  It should work with equipment that also follows the spec and has little
  vendor specific quirks.
- Second, the passthrough mode, in which the handling of CEC messages would
  be left to userspace application. Kernel would not be sending or
  receiving messages, unless specifically told to do so. Below you mentioned
  that allocating logical addresses and sending ACKs could be done in
kernel.

  The way I see it is following: If allocation of a logical address is made
  then ACKs will be handled by the framework. If no allocation is made then
  the userspace can still send and receive messages. However no filtering is
  done based on the logical address - all received messages are sent to the
  userspace.

 
 * CEC is *very* vendor specific. The main reason is, in my opinion, the
 use of the word should instead of shall in the spec. It's addressed
 in the new version, but it'll take years before all the non 2.x devices
 are gone. What works for vendor A will simply not work for vendor B.
 libCEC aims to address this, in a library that can be used on all major
 platforms and by all major programming languages. You could duplicate
 the work done there in the kernel to make make the implementation work
 with all brands, but I think that this does simply not belong in the
 kernel when it can be handled in userspace perfectly.

CEC being very vendor specific is a huge problem. I agree with you that
there is no need to duplicate the effort to mitigate all the vendor quirks.
Especially that a working implementation (libCEC) is already done.

 So I suggest that you limit the in-kernel implementation to handling
 raw
 traffic only, to have it do this (and nothing more):
 * allocate one or more logical addresses, and ack CEC traffic sent to
 those logical addresses
 * receive CEC traffic and forward it to userspace (traffic sent to all
 addresses is preferred, not just traffic sent to the logical address
 used by the device running this code)
 * transmit CEC traffic initiated by userspace

As mentioned above, I propose a passthorugh mode in which handling of
CEC messages by the kernel CEC framework will be very limited. I think
that the three functions listed above should be enough.

Any comments on this solution?

 
 thanks,
 
 Lars Op den Kamp

Best wishes,
-- 
Kamil Debski
Samsung RD Institute Poland

 
 
 On 23-04-15 15:03, Kamil Debski wrote:
  From: Hans Verkuil hansv...@cisco.com
 
  The added HDMI CEC framework provides a generic kernel interface for
  HDMI CEC devices.
 
  Signed-off-by: Hans Verkuil hansv...@cisco.com
  [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
  [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
  [k.deb...@samsung.com: change kthread handling when setting logical
  address]
  [k.deb...@samsung.com: code cleanup and fixes]
  

Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
On 04/27/2015 11:22 AM, Hans Verkuil wrote:
 Hi Kamil,
 
 Sorry for all the replies, but I'm writing the DocBook documentation, so
 whenever I find something missing I'll just reply to this patch.
 +/* The CEC version */
 
 Add support for version 1.3a here:
 
 #define CEC_VERSION_1_3A  4

To clarify: the core CEC framework will not support 1.3A, but of course
other devices can report it, so we do need the define.

Regards,

Hans

 
 +#define CEC_VERSION_1_4B5
 +#define CEC_VERSION_2_0 6
 
 Regards,
 
   Hans
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
Hi Kamil,

I've added some missing HDMI 2.0 commands:

On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com
 
 The added HDMI CEC framework provides a generic kernel interface for
 HDMI CEC devices.
 
 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h
 
 diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
 new file mode 100644
 index 000..bb6d66c
 --- /dev/null
 +++ b/include/uapi/linux/cec.h
 @@ -0,0 +1,303 @@
 +#ifndef _CEC_H
 +#define _CEC_H
 +
 +#include linux/types.h
 +
 +struct cec_time {
 + __u64 sec;
 + __u64 nsec;
 +};
 +
 +struct cec_msg {
 + struct cec_time ts;
 + __u32 len;
 + __u32 status;
 + __u32 timeout;
 + /* timeout (in ms) is used to timeout CEC_RECEIVE.
 +Set to 0 if you want to wait forever. */
 + __u8  msg[16];
 + __u8  reply;
 + /* If non-zero, then wait for a reply with this opcode.
 +If there was an error when sending the msg or FeatureAbort
 +was returned, then reply is set to 0.
 +If reply is non-zero upon return, then len/msg are set to
 +the received message.
 +If reply is zero upon return and status has the
 +CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
 +received feature abort message.
 +If reply is zero upon return and status has the
 +CEC_TX_STATUS_REPLY_TIMEOUT
 +bit set, then no reply was seen at all.
 +This field is ignored with CEC_RECEIVE.
 +If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
 +then -EINVAL is returned.
 +if reply is non-zero, then timeout is set to 1000 (the required
 +maximum response time).
 +  */
 + __u8 reserved[31];
 +};
 +
 +static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
 +{
 + return msg-msg[0]  4;
 +}
 +
 +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
 +{
 + return msg-msg[0]  0xf;
 +}
 +
 +static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
 +{
 + return (msg-msg[0]  0xf) == 0xf;
 +}
 +
 +/* cec status field */
 +#define CEC_TX_STATUS_OK(0)
 +#define CEC_TX_STATUS_ARB_LOST  (1  0)
 +#define CEC_TX_STATUS_RETRY_TIMEOUT (1  1)
 +#define CEC_TX_STATUS_FEATURE_ABORT (1  2)
 +#define CEC_TX_STATUS_REPLY_TIMEOUT (1  3)
 +#define CEC_RX_STATUS_READY (0)
 +
 +#define CEC_LOG_ADDR_INVALID 0xff
 +
 +/* The maximum number of logical addresses one device can be assigned to.
 + * The CEC 2.0 spec allows for only 2 logical addresses at the moment. The
 + * Analog Devices CEC hardware supports 3. So let's go wild and go for 4. */
 +#define CEC_MAX_LOG_ADDRS 4
 +
 +/* The Primary Device Type */
 +#define CEC_PRIM_DEVTYPE_TV  0
 +#define CEC_PRIM_DEVTYPE_RECORD  1
 +#define CEC_PRIM_DEVTYPE_TUNER   3
 +#define CEC_PRIM_DEVTYPE_PLAYBACK4
 +#define CEC_PRIM_DEVTYPE_AUDIOSYSTEM 5
 +#define CEC_PRIM_DEVTYPE_SWITCH  6
 +#define CEC_PRIM_DEVTYPE_VIDEOPROC   7
 +
 +/* The All Device Types flags (CEC 2.0) */
 +#define CEC_FL_ALL_DEVTYPE_TV(1  7)
 +#define CEC_FL_ALL_DEVTYPE_RECORD(1  6)
 +#define CEC_FL_ALL_DEVTYPE_TUNER (1  5)
 +#define CEC_FL_ALL_DEVTYPE_PLAYBACK  (1  4)
 +#define CEC_FL_ALL_DEVTYPE_AUDIOSYSTEM   (1  3)
 +#define CEC_FL_ALL_DEVTYPE_SWITCH(1  2)
 +/* And if you wondering what happened to VIDEOPROC devices: those should
 + * be mapped to a SWITCH. */
 +
 +/* The logical address types that the CEC device wants to 

Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com
 
 The added HDMI CEC framework provides a generic kernel interface for
 HDMI CEC devices.
 
 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h
 

 + case CEC_S_ADAP_LOG_ADDRS: {
 + struct cec_log_addrs log_addrs;
 +
 + if (!(adap-capabilities  CEC_CAP_LOG_ADDRS))
 + return -ENOTTY;
 + if (copy_from_user(log_addrs, parg, sizeof(log_addrs)))
 + return -EFAULT;
 + err = cec_claim_log_addrs(adap, log_addrs, true);

Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have 
CEC_EVENT_READY
I think it makes sense to just return in non-blocking mode and have 
cec_claim_log_addrs
generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS to 
discover
the result.

What do you think?

Regards,

Hans

 + if (err)
 + return err;
 +
 + if (copy_to_user(parg, log_addrs, sizeof(log_addrs)))
 + return -EFAULT;
 + break;
 + }

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
Hi Kamil,

Sorry for all the replies, but I'm writing the DocBook documentation, so
whenever I find something missing I'll just reply to this patch.
 +/* The CEC version */

Add support for version 1.3a here:

#define CEC_VERSION_1_3A4

 +#define CEC_VERSION_1_4B 5
 +#define CEC_VERSION_2_0  6

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com
 
 The added HDMI CEC framework provides a generic kernel interface for
 HDMI CEC devices.
 
 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h
 
 diff --git a/Documentation/cec.txt b/Documentation/cec.txt
 new file mode 100644
 index 000..2b6c08a
 --- /dev/null
 +++ b/Documentation/cec.txt
 @@ -0,0 +1,396 @@
 +- CEC_G_ADAP_LOG_ADDRS and CEC_S_ADAP_LOG_ADDRS
 +
 +These ioctl are used to configure the logical addresses of the CEC adapter.
 +
 +#define CEC_G_ADAP_LOG_ADDRS _IOR('a', 3, struct cec_log_addrs)
 +#define CEC_S_ADAP_LOG_ADDRS _IOWR('a', 4, struct cec_log_addrs)
 +
 +The struct cec_log_addrs is following:
 +
 +struct cec_log_addrs {
 + __u8 cec_version;
 + __u8 num_log_addrs;
 + __u8 primary_device_type[CEC_MAX_LOG_ADDRS];
 + __u8 log_addr_type[CEC_MAX_LOG_ADDRS];
 + __u8 log_addr[CEC_MAX_LOG_ADDRS];
 +
 + /* CEC 2.0 */
 + __u8 all_device_types;
 + __u8 features[CEC_MAX_LOG_ADDRS][12];
 +
 + __u8 reserved[9];
 +};
 +
 +The cec_version determines which CEC version should be used.
 +
 +/* The CEC version */
 +#define CEC_VERSION_1_4B 5
 +#define CEC_VERSION_2_0  6
 +
 +It will try to claim num_log_addrs devices. The log_addr_type array has
 +the logical address type that needs to be claimed for that device, and
 +the log_addr array will receive the actual logical address that was
 +claimed for that device or 0xff if no address could be claimed.
 +
 +The primary_device_type contains the primary device for each logical
 +address.
 +
 +For CEC 2.0 devices fill in the all_device_types parameter to use with the
 +Report Features command, and fill in the 'features' which contains the
 +remaining parameters (RC Profile and Device Features) to use in Report
 +Features.
 +
 +An error is returned if the adapter is disabled or if there
 +is no physical address assigned or if the cec_version is unknown.
 +
 +If no logical address of one or more of the given types could be claimed,
 +then log_addr will be set to CEC_LOG_ADDR_INVALID.

This does not appear to be the case looking at the cec_config_log_addrs 
function.
I don't see it being set to INVALID if it couldn't be claimed. I think that is
missing in the cec_config_log_addrs function.

Regards,

Hans

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
Hi Lars,

My thanks as well for your comments.

I'd like to add some background information as well as to why we move
the core CEC support into the kernel: the main reason for doing this
is to support the HEAC part of the CEC protocol. Specifically the ARC
support and handling the hotplug detect CEC/HEAC messages. This has to
be handled in the kernel and cannot be left to userspace. While the
current framework does not yet handle these messages support for this
will appear, probably later this year since I will have to work on ARC.

Out of curiosity: have you ever seen CEC adapters that implement the
ethernet part of HEAC? My understanding is that nobody uses that part
since wifi is the standard these days. But perhaps you know of examples
where it was actually implemented.

Regards,

Hans

On 04/27/2015 10:09 AM, Kamil Debski wrote:
 Hi Lars, 
 
 Thank you for your comments.
 
 From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
 ow...@vger.kernel.org] On Behalf Of Lars Op den Kamp
 Sent: Friday, April 24, 2015 12:04 PM
  
 Hi Kamil, Hans,

 I'm the main developer of libCEC
 (https://github.com/Pulse-Eight/libcec). Sorry for the late time to
 jump
 in here, but I wasn't signed up to this mailing list and someone
 pointed
 me to this discussion.

 Unfortunately this approach will not work with half the TVs that are
 out
 there. I'll explain why:

 * because of how some (common) brands implemented CEC in their TVs,
 this
 implementation will not work, as the TV will just reject it. In libCEC,
 we've created work arounds for brands like this. Without these work
 arounds, your in-kernel implementation will be very vendor specific.
 e.g. this implementation will work for Samsung's TVs, but not for the
 TVs made by another big TV brand. All commands, including CEC_OP_ABORT,
 should be passed to userspace to make it work with all brands.

 * it should be made possible to not have the kernel send any CEC
 message, try to process any received CEC message, or ack to any logical
 address at all, to allow libraries like libCEC to fully handle all CEC
 traffic. Some brands only enable routing of some CEC keys when a
 specific device type is used. libCEC will allocate a logical address of
 the correct type for the brand that's used. If another address is first
 allocated by the kernel, and the TV communicates with it to find out
 it's name and things like that, and libCEC allocates another address a
 bit later when initialised, then you'll end up with multiple entries in
 the device list on the TV, and only one of them will work.
 
 Adding a special mode in the CEC framework that disables parsing and
 processing seems like a good idea for me. This way libCEC could be
 completely
 in charge of how the communication is handled. 
 
 I discussed this with Hans and he is for this solution. This way there would
 be two modes:
 - One with handling of CEC messages enabled in the kernel, in idea behind
   this is to have processing adhere to the CEC spec as closely as possible.
   It should work with equipment that also follows the spec and has little
   vendor specific quirks.
 - Second, the passthrough mode, in which the handling of CEC messages would
   be left to userspace application. Kernel would not be sending or
   receiving messages, unless specifically told to do so. Below you mentioned
   that allocating logical addresses and sending ACKs could be done in
 kernel.
 
   The way I see it is following: If allocation of a logical address is made
   then ACKs will be handled by the framework. If no allocation is made then
   the userspace can still send and receive messages. However no filtering is
   done based on the logical address - all received messages are sent to the
   userspace.
 

 * CEC is *very* vendor specific. The main reason is, in my opinion, the
 use of the word should instead of shall in the spec. It's addressed
 in the new version, but it'll take years before all the non 2.x devices
 are gone. What works for vendor A will simply not work for vendor B.
 libCEC aims to address this, in a library that can be used on all major
 platforms and by all major programming languages. You could duplicate
 the work done there in the kernel to make make the implementation work
 with all brands, but I think that this does simply not belong in the
 kernel when it can be handled in userspace perfectly.
 
 CEC being very vendor specific is a huge problem. I agree with you that
 there is no need to duplicate the effort to mitigate all the vendor quirks.
 Especially that a working implementation (libCEC) is already done.
 
 So I suggest that you limit the in-kernel implementation to handling
 raw
 traffic only, to have it do this (and nothing more):
 * allocate one or more logical addresses, and ack CEC traffic sent to
 those logical addresses
 * receive CEC traffic and forward it to userspace (traffic sent to all
 addresses is preferred, not just traffic sent to the logical address
 used by the device 

Re: [PATCH v4 06/10] cec: add HDMI CEC framework: y2038 question

2015-04-27 Thread Hans Verkuil
Added the y2038 mailinglist since I would like to get their input for
this API.

Y2038 experts, can you take a look at my comments in the code below?

Thanks!

On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com
 
 The added HDMI CEC framework provides a generic kernel interface for
 HDMI CEC devices.
 
 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h
 

snip

 diff --git a/include/uapi/linux/Kbuild b/include/uapi/linux/Kbuild
 index 4842a98..5854cfd 100644
 --- a/include/uapi/linux/Kbuild
 +++ b/include/uapi/linux/Kbuild
 @@ -81,6 +81,7 @@ header-y += capi.h
  header-y += cciss_defs.h
  header-y += cciss_ioctl.h
  header-y += cdrom.h
 +header-y += cec.h
  header-y += cgroupstats.h
  header-y += chio.h
  header-y += cm4000_cs.h
 diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
 new file mode 100644
 index 000..bb6d66c
 --- /dev/null
 +++ b/include/uapi/linux/cec.h
 @@ -0,0 +1,303 @@
 +#ifndef _CEC_H
 +#define _CEC_H
 +
 +#include linux/types.h
 +
 +struct cec_time {
 + __u64 sec;
 + __u64 nsec;
 +};

I don't like having to introduce a new struct for time here. Basically we are
only doing this because there is still no public struct timespec64.

When will that struct become available for use in a public API? If it is 4.2,
then we can start using it. If it will take longer, then what alternative can
we use to prevent having to change the API later?

One alternative might be to drop it for now and just reserve space in the
structs to add it later.

Input from the y2038 experts will be welcome!

Regards,

Hans

 +
 +struct cec_msg {
 + struct cec_time ts;
 + __u32 len;
 + __u32 status;
 + __u32 timeout;
 + /* timeout (in ms) is used to timeout CEC_RECEIVE.
 +Set to 0 if you want to wait forever. */
 + __u8  msg[16];
 + __u8  reply;
 + /* If non-zero, then wait for a reply with this opcode.
 +If there was an error when sending the msg or FeatureAbort
 +was returned, then reply is set to 0.
 +If reply is non-zero upon return, then len/msg are set to
 +the received message.
 +If reply is zero upon return and status has the
 +CEC_TX_STATUS_FEATURE_ABORT bit set, then len/msg are set to the
 +received feature abort message.
 +If reply is zero upon return and status has the
 +CEC_TX_STATUS_REPLY_TIMEOUT
 +bit set, then no reply was seen at all.
 +This field is ignored with CEC_RECEIVE.
 +If reply is non-zero for CEC_TRANSMIT and the message is a broadcast,
 +then -EINVAL is returned.
 +if reply is non-zero, then timeout is set to 1000 (the required
 +maximum response time).
 +  */
 + __u8 reserved[31];
 +};
 +
 +static inline __u8 cec_msg_initiator(const struct cec_msg *msg)
 +{
 + return msg-msg[0]  4;
 +}
 +
 +static inline __u8 cec_msg_destination(const struct cec_msg *msg)
 +{
 + return msg-msg[0]  0xf;
 +}
 +
 +static inline bool cec_msg_is_broadcast(const struct cec_msg *msg)
 +{
 + return (msg-msg[0]  0xf) == 0xf;
 +}
 +
 +/* cec status field */
 +#define CEC_TX_STATUS_OK(0)
 +#define CEC_TX_STATUS_ARB_LOST  (1  0)
 +#define CEC_TX_STATUS_RETRY_TIMEOUT (1  1)
 +#define CEC_TX_STATUS_FEATURE_ABORT (1  2)
 +#define CEC_TX_STATUS_REPLY_TIMEOUT (1  3)
 +#define CEC_RX_STATUS_READY (0)
 +
 +#define CEC_LOG_ADDR_INVALID 0xff
 +
 +/* The maximum number of logical addresses one device can be assigned to.
 + * The CEC 2.0 spec allows 

Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
On 04/27/2015 11:22 AM, Hans Verkuil wrote:
 Hi Kamil,
 
 Sorry for all the replies, but I'm writing the DocBook documentation, so
 whenever I find something missing I'll just reply to this patch.
 +/* The CEC version */
 
 Add support for version 1.3a here:
 
 #define CEC_VERSION_1_3A  4
 
 +#define CEC_VERSION_1_4B5

Hmm, reading up on this I see that '5' is used for HDMI 1.4, 1.4a or 1.4b.

I think it should be renamed to VERSION_1_4 (i.e. drop the 'B').

Regards,

Hans

 +#define CEC_VERSION_2_0 6
 
 Regards,
 
   Hans
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


RE: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Kamil Debski
Hi Hans,

Thank you so much for all today's comments. I will consider them when
preparing the next version.

From: linux-media-ow...@vger.kernel.org [mailto:linux-media-
ow...@vger.kernel.org] On Behalf Of Hans Verkuil
Sent: Monday, April 27, 2015 1:27 PM
 
 On 04/27/2015 12:25 PM, Hans Verkuil wrote:
  On 04/23/2015 03:03 PM, Kamil Debski wrote:
  From: Hans Verkuil hansv...@cisco.com
 
  The added HDMI CEC framework provides a generic kernel interface for
  HDMI CEC devices.
 
  Signed-off-by: Hans Verkuil hansv...@cisco.com
  [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
  [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
  [k.deb...@samsung.com: change kthread handling when setting logical
  address]
  [k.deb...@samsung.com: code cleanup and fixes]
  [k.deb...@samsung.com: add missing CEC commands to match spec]
  [k.deb...@samsung.com: add RC framework support]
  [k.deb...@samsung.com: move and edit documentation]
  [k.deb...@samsung.com: add vendor id reporting]
  [k.deb...@samsung.com: add possibility to clear assigned logical
  addresses]
  [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
  [k.deb...@samsung.com: reorder of API structs and add reserved
  fields]
  [k.deb...@samsung.com: fix handling of events and fix 32/64bit
  timespec problem]
  [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
  Signed-off-by: Kamil Debski k.deb...@samsung.com
  ---
   Documentation/cec.txt |  396 
   drivers/media/Kconfig |6 +
   drivers/media/Makefile|2 +
   drivers/media/cec.c   | 1161
 +
   include/media/cec.h   |  140 ++
   include/uapi/linux/Kbuild |1 +
   include/uapi/linux/cec.h  |  303 
   7 files changed, 2009 insertions(+)
   create mode 100644 Documentation/cec.txt  create mode 100644
  drivers/media/cec.c  create mode 100644 include/media/cec.h  create
  mode 100644 include/uapi/linux/cec.h
 
 
  +  case CEC_S_ADAP_LOG_ADDRS: {
  +  struct cec_log_addrs log_addrs;
  +
  +  if (!(adap-capabilities  CEC_CAP_LOG_ADDRS))
  +  return -ENOTTY;
  +  if (copy_from_user(log_addrs, parg, sizeof(log_addrs)))
  +  return -EFAULT;
  +  err = cec_claim_log_addrs(adap, log_addrs, true);
 
  Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have
  CEC_EVENT_READY I think it makes sense to just return in non-blocking
  mode and have cec_claim_log_addrs generate CEC_EVENT_READY when done.
  Userspace can then call G_ADAP_LOG_ADDRS to discover the result.
 
  What do you think?
 

I am looking into this now. If I see this correctly this involves:
- adding cec_post_event(cla_int-adap, CEC_EVENT_READY); to
cec_config_thread_func
- adding O_NONBLOCK check in CEC_S_ADAP_LOG_ADDRS 
Right?
 
 On a related topic: non-blocking behavior for CEC_RECEIVE is well
 defined, but for CEC_TRA NSMIT it isn't. If reply == 0, then we need a
 way to inform userspace that the transmit finished (with a possible
 non-zero status code). An event would be suitable for that, but we
 would need a way to associate a transmit message with the event.
 
 One possibility might be to have the CEC framework assign a sequence
 number to a transmit message which is returned by CEC_TRANSMIT and used
 in the event.
 
 If reply != 0, then I think the received message should be queued up in
 the receive queue, but with a non-zero reply field and with the
 sequence number of the transmit message it is a reply of.

A sequence number is a good solution, I believe. To recap:
- a sequence number should be set by the framework and returned in the
CEC_TRANSMIT ioctl
- a new event should be added CEC_EVENT_TX_DONE and it should be posted on
each transmission
  finish 
- event struct has to include a sequence field as well
Is this ok?

 
 Regards,
 
   Hans

Best wishes,
-- 
Kamil Debski
Samsung RD Institute Poland

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-27 Thread Hans Verkuil
On 04/27/2015 12:25 PM, Hans Verkuil wrote:
 On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com

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

 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h

 
 +case CEC_S_ADAP_LOG_ADDRS: {
 +struct cec_log_addrs log_addrs;
 +
 +if (!(adap-capabilities  CEC_CAP_LOG_ADDRS))
 +return -ENOTTY;
 +if (copy_from_user(log_addrs, parg, sizeof(log_addrs)))
 +return -EFAULT;
 +err = cec_claim_log_addrs(adap, log_addrs, true);
 
 Currently CEC_S_ADAP_LOG_ADDRS is always blocking, but since we have 
 CEC_EVENT_READY
 I think it makes sense to just return in non-blocking mode and have 
 cec_claim_log_addrs
 generate CEC_EVENT_READY when done. Userspace can then call G_ADAP_LOG_ADDRS 
 to discover
 the result.
 
 What do you think?
 

On a related topic: non-blocking behavior for CEC_RECEIVE is well defined, but 
for
CEC_TRANSMIT it isn't. If reply == 0, then we need a way to inform userspace 
that
the transmit finished (with a possible non-zero status code). An event would be
suitable for that, but we would need a way to associate a transmit message with
the event.

One possibility might be to have the CEC framework assign a sequence number to
a transmit message which is returned by CEC_TRANSMIT and used in the event.

If reply != 0, then I think the received message should be queued up in the
receive queue, but with a non-zero reply field and with the sequence number of 
the
transmit message it is a reply of.

Regards,

Hans
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-24 Thread Lars Op den Kamp

Hi Kamil, Hans,

I'm the main developer of libCEC 
(https://github.com/Pulse-Eight/libcec). Sorry for the late time to jump 
in here, but I wasn't signed up to this mailing list and someone pointed 
me to this discussion.


Unfortunately this approach will not work with half the TVs that are out 
there. I'll explain why:


* because of how some (common) brands implemented CEC in their TVs, this 
implementation will not work, as the TV will just reject it. In libCEC, 
we've created work arounds for brands like this. Without these work 
arounds, your in-kernel implementation will be very vendor specific. 
e.g. this implementation will work for Samsung's TVs, but not for the 
TVs made by another big TV brand. All commands, including CEC_OP_ABORT, 
should be passed to userspace to make it work with all brands.


* it should be made possible to not have the kernel send any CEC 
message, try to process any received CEC message, or ack to any logical 
address at all, to allow libraries like libCEC to fully handle all CEC 
traffic. Some brands only enable routing of some CEC keys when a 
specific device type is used. libCEC will allocate a logical address of 
the correct type for the brand that's used. If another address is first 
allocated by the kernel, and the TV communicates with it to find out 
it's name and things like that, and libCEC allocates another address a 
bit later when initialised, then you'll end up with multiple entries in 
the device list on the TV, and only one of them will work.


* CEC is *very* vendor specific. The main reason is, in my opinion, the 
use of the word should instead of shall in the spec. It's addressed 
in the new version, but it'll take years before all the non 2.x devices 
are gone. What works for vendor A will simply not work for vendor B. 
libCEC aims to address this, in a library that can be used on all major 
platforms and by all major programming languages. You could duplicate 
the work done there in the kernel to make make the implementation work 
with all brands, but I think that this does simply not belong in the 
kernel when it can be handled in userspace perfectly.


So I suggest that you limit the in-kernel implementation to handling raw 
traffic only, to have it do this (and nothing more):
* allocate one or more logical addresses, and ack CEC traffic sent to 
those logical addresses
* receive CEC traffic and forward it to userspace (traffic sent to all 
addresses is preferred, not just traffic sent to the logical address 
used by the device running this code)

* transmit CEC traffic initiated by userspace

thanks,

Lars Op den Kamp


On 23-04-15 15:03, Kamil Debski wrote:

From: Hans Verkuil hansv...@cisco.com

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

Signed-off-by: Hans Verkuil hansv...@cisco.com
[k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
[k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
[k.deb...@samsung.com: change kthread handling when setting logical
address]
[k.deb...@samsung.com: code cleanup and fixes]
[k.deb...@samsung.com: add missing CEC commands to match spec]
[k.deb...@samsung.com: add RC framework support]
[k.deb...@samsung.com: move and edit documentation]
[k.deb...@samsung.com: add vendor id reporting]
[k.deb...@samsung.com: add possibility to clear assigned logical
addresses]
[k.deb...@samsung.com: documentation fixes, clenaup and expansion]
[k.deb...@samsung.com: reorder of API structs and add reserved fields]
[k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
problem]
[k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h

diff --git a/Documentation/cec.txt b/Documentation/cec.txt
new file mode 100644
index 000..2b6c08a
--- /dev/null
+++ b/Documentation/cec.txt
@@ -0,0 +1,396 @@
+CEC Kernel Support
+==
+
+The CEC framework provides a unified kernel interface for use with HDMI CEC
+hardware. It is designed to handle a multiple variants of hardware. Adding to
+the flexibility of the framework it enables to set which parts of the CEC
+protocol processing is handled by the hardware, by the driver and by the
+userspace application.
+
+
+The CEC Protocol
+
+
+The CEC protocol enables consumer electronic devices to communicate with each
+other through the HDMI connection. The protocol uses logical addresses in the

Re: [PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-23 Thread Hans Verkuil
On 04/23/2015 03:03 PM, Kamil Debski wrote:
 From: Hans Verkuil hansv...@cisco.com
 
 The added HDMI CEC framework provides a generic kernel interface for
 HDMI CEC devices.
 
 Signed-off-by: Hans Verkuil hansv...@cisco.com
 [k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
 [k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
 [k.deb...@samsung.com: change kthread handling when setting logical
 address]
 [k.deb...@samsung.com: code cleanup and fixes]
 [k.deb...@samsung.com: add missing CEC commands to match spec]
 [k.deb...@samsung.com: add RC framework support]
 [k.deb...@samsung.com: move and edit documentation]
 [k.deb...@samsung.com: add vendor id reporting]
 [k.deb...@samsung.com: add possibility to clear assigned logical
 addresses]
 [k.deb...@samsung.com: documentation fixes, clenaup and expansion]
 [k.deb...@samsung.com: reorder of API structs and add reserved fields]
 [k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
 problem]
 [k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
 Signed-off-by: Kamil Debski k.deb...@samsung.com
 ---
  Documentation/cec.txt |  396 
  drivers/media/Kconfig |6 +
  drivers/media/Makefile|2 +
  drivers/media/cec.c   | 1161 
 +
  include/media/cec.h   |  140 ++
  include/uapi/linux/Kbuild |1 +
  include/uapi/linux/cec.h  |  303 
  7 files changed, 2009 insertions(+)
  create mode 100644 Documentation/cec.txt
  create mode 100644 drivers/media/cec.c
  create mode 100644 include/media/cec.h
  create mode 100644 include/uapi/linux/cec.h
 
 diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
 new file mode 100644
 index 000..bb6d66c
 --- /dev/null
 +++ b/include/uapi/linux/cec.h
 @@ -0,0 +1,303 @@
 +
 +/* Userspace has to configure the adapter state (enable/disable) */
 +#define CEC_CAP_STATE(1  0)
 +/* Userspace has to configure the physical address */
 +#define CEC_CAP_PHYS_ADDR(1  1)
 +/* Userspace has to configure the logical addresses */
 +#define CEC_CAP_LOG_ADDRS(1  2)
 +/* Userspace can transmit messages */
 +#define CEC_CAP_TRANSMIT (1  3)
 +/* Userspace can receive messages */
 +#define CEC_CAP_RECEIVE  (1  4)
 +/* Userspace has to configure the vendor id */
 +#define CEC_CAP_VENDOR_ID(1  5)
 +/* The hardware has the possibility to work in the promiscuous mode */
 +#define CEC_CAP_PROMISCUOUS  (1  6)

Since promiscuous support has been dropped, this capability needs to be
dropped as well.

 +
 +struct cec_caps {
 + __u32 available_log_addrs;
 + __u32 capabilities;
 + __u32 vendor_id;
 + __u8  version;
 + __u8  reserved[11];

I'd increase this to 31.

 +};
 +
 +struct cec_log_addrs {
 + __u8 cec_version;
 + __u8 num_log_addrs;
 + __u8 primary_device_type[CEC_MAX_LOG_ADDRS];
 + __u8 log_addr_type[CEC_MAX_LOG_ADDRS];
 + __u8 log_addr[CEC_MAX_LOG_ADDRS];
 +
 + /* CEC 2.0 */
 + __u8 all_device_types;
 + __u8 features[CEC_MAX_LOG_ADDRS][12];
 +
 + __u8 reserved[9];

I'd increase this to 65 or so.

Regards,

Hans

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH v4 06/10] cec: add HDMI CEC framework

2015-04-23 Thread Kamil Debski
From: Hans Verkuil hansv...@cisco.com

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

Signed-off-by: Hans Verkuil hansv...@cisco.com
[k.deb...@samsung.com: Merged CEC Updates commit by Hans Verkuil]
[k.deb...@samsung.com: Merged Update author commit by Hans Verkuil]
[k.deb...@samsung.com: change kthread handling when setting logical
address]
[k.deb...@samsung.com: code cleanup and fixes]
[k.deb...@samsung.com: add missing CEC commands to match spec]
[k.deb...@samsung.com: add RC framework support]
[k.deb...@samsung.com: move and edit documentation]
[k.deb...@samsung.com: add vendor id reporting]
[k.deb...@samsung.com: add possibility to clear assigned logical
addresses]
[k.deb...@samsung.com: documentation fixes, clenaup and expansion]
[k.deb...@samsung.com: reorder of API structs and add reserved fields]
[k.deb...@samsung.com: fix handling of events and fix 32/64bit timespec
problem]
[k.deb...@samsung.com: add cec.h to include/uapi/linux/Kbuild]
Signed-off-by: Kamil Debski k.deb...@samsung.com
---
 Documentation/cec.txt |  396 
 drivers/media/Kconfig |6 +
 drivers/media/Makefile|2 +
 drivers/media/cec.c   | 1161 +
 include/media/cec.h   |  140 ++
 include/uapi/linux/Kbuild |1 +
 include/uapi/linux/cec.h  |  303 
 7 files changed, 2009 insertions(+)
 create mode 100644 Documentation/cec.txt
 create mode 100644 drivers/media/cec.c
 create mode 100644 include/media/cec.h
 create mode 100644 include/uapi/linux/cec.h

diff --git a/Documentation/cec.txt b/Documentation/cec.txt
new file mode 100644
index 000..2b6c08a
--- /dev/null
+++ b/Documentation/cec.txt
@@ -0,0 +1,396 @@
+CEC Kernel Support
+==
+
+The CEC framework provides a unified kernel interface for use with HDMI CEC
+hardware. It is designed to handle a multiple variants of hardware. Adding to
+the flexibility of the framework it enables to set which parts of the CEC
+protocol processing is handled by the hardware, by the driver and by the
+userspace application.
+
+
+The CEC Protocol
+
+
+The CEC protocol enables consumer electronic devices to communicate with each
+other through the HDMI connection. The protocol uses logical addresses in the
+communication. The logical address is strictly connected with the functionality
+provided by the device. The TV acting as the communication hub is always
+assigned address 0. The physical address is determined by the physical
+connection between devices.
+
+The protocol enables control of compatible devices with a single remote.
+Synchronous power on/standby, instant playback with changing the content source
+on the TV.
+
+The Kernel Interface
+
+
+CEC Adapter
+---
+
+#define CEC_LOG_ADDR_INVALID 0xff
+
+/* The maximum number of logical addresses one device can be assigned to.
+ * The CEC 2.0 spec allows for only 2 logical addresses at the moment. The
+ * Analog Devices CEC hardware supports 3. So let's go wild and go for 4. */
+#define CEC_MAX_LOG_ADDRS 4
+
+/* The Primary Device Type */
+#define CEC_PRIM_DEVTYPE_TV0
+#define CEC_PRIM_DEVTYPE_RECORD1
+#define CEC_PRIM_DEVTYPE_TUNER 3
+#define CEC_PRIM_DEVTYPE_PLAYBACK  4
+#define CEC_PRIM_DEVTYPE_AUDIOSYSTEM   5
+#define CEC_PRIM_DEVTYPE_SWITCH6
+#define CEC_PRIM_DEVTYPE_VIDEOPROC 7
+
+/* The All Device Types flags (CEC 2.0) */
+#define CEC_FL_ALL_DEVTYPE_TV  (1  7)
+#define CEC_FL_ALL_DEVTYPE_RECORD  (1  6)
+#define CEC_FL_ALL_DEVTYPE_TUNER   (1  5)
+#define CEC_FL_ALL_DEVTYPE_PLAYBACK(1  4)
+#define CEC_FL_ALL_DEVTYPE_AUDIOSYSTEM (1  3)
+#define CEC_FL_ALL_DEVTYPE_SWITCH  (1  2)
+/* And if you wondering what happened to VIDEOPROC devices: those should
+ * be mapped to a SWITCH. */
+
+/* The logical address types that the CEC device wants to claim */
+#define CEC_LOG_ADDR_TYPE_TV   0
+#define CEC_LOG_ADDR_TYPE_RECORD   1
+#define CEC_LOG_ADDR_TYPE_TUNER2
+#define CEC_LOG_ADDR_TYPE_PLAYBACK 3
+#define CEC_LOG_ADDR_TYPE_AUDIOSYSTEM  4
+#define CEC_LOG_ADDR_TYPE_SPECIFIC 5
+#define CEC_LOG_ADDR_TYPE_UNREGISTERED 6
+/* Switches should use UNREGISTERED.
+ * Video processors should use SPECIFIC. */
+
+/* The CEC version */
+#define CEC_VERSION_1_4B   5
+#define CEC_VERSION_2_06
+
+struct cec_adapter {
+   /* internal fields removed */
+
+   u16 phys_addr;
+   u32 capabilities;
+   u8 version;
+   u8 num_log_addrs;
+   u8 prim_device[CEC_MAX_LOG_ADDRS];
+   u8 log_addr_type[CEC_MAX_LOG_ADDRS];
+   u8 log_addr[CEC_MAX_LOG_ADDRS];
+
+   int (*adap_enable)(struct cec_adapter *adap, bool enable);
+   int (*adap_log_addr)(struct cec_adapter *adap, u8 logical_addr);
+   int (*adap_transmit)(struct cec_adapter *adap, struct cec_msg *msg);
+   void