Hi Mauro, From: Mauro Carvalho Chehab [mailto:mchehab@xxxxxxxxxxxxxxx] Sent: Sunday, March 08, 2015 3:21 PM > Em Thu, 22 Jan 2015 17:04:34 +0100 > Kamil Debski <k.debski@xxxxxxxxxxx> escreveu: > > (c/c linux-input ML) > > > Add cec protocol handling the RC framework. > > I added some comments, that reflects my understanding from what's there > at the keymap definitions found at: > http://xtreamerdev.googlecode.com/files/CEC_Specs.pdf Thank you very much for the review, Mauro. Your comments are very much appreciated. > > > > > Signed-off-by: Kamil Debski <k.debski@xxxxxxxxxxx> > > --- > > drivers/media/rc/keymaps/Makefile | 1 + > > drivers/media/rc/keymaps/rc-cec.c | 133 > +++++++++++++++++++++++++++++++++++++ > > drivers/media/rc/rc-main.c | 1 + > > include/media/rc-core.h | 1 + > > include/media/rc-map.h | 5 +- > > 5 files changed, 140 insertions(+), 1 deletion(-) create mode > 100644 > > drivers/media/rc/keymaps/rc-cec.c > > > > diff --git a/drivers/media/rc/keymaps/Makefile > > b/drivers/media/rc/keymaps/Makefile > > index abf6079..56f10d6 100644 > > --- a/drivers/media/rc/keymaps/Makefile > > +++ b/drivers/media/rc/keymaps/Makefile > > @@ -18,6 +18,7 @@ obj-$(CONFIG_RC_MAP) += rc-adstech-dvb-t-pci.o \ > > rc-behold.o \ > > rc-behold-columbus.o \ > > rc-budget-ci-old.o \ > > + rc-cec.o \ > > rc-cinergy-1400.o \ > > rc-cinergy.o \ > > rc-delock-61959.o \ > > diff --git a/drivers/media/rc/keymaps/rc-cec.c > > b/drivers/media/rc/keymaps/rc-cec.c > > new file mode 100644 > > index 0000000..f2826c5 > > --- /dev/null > > +++ b/drivers/media/rc/keymaps/rc-cec.c > > @@ -0,0 +1,133 @@ > > +/* Keytable for the CEC remote control > > + * > > + * Copyright (c) 2015 by Kamil Debski > > + * > > + * This program is free software; you can redistribute it and/or > > +modify > > + * it under the terms of the GNU General Public License as published > > +by > > + * the Free Software Foundation; either version 2 of the License, or > > + * (at your option) any later version. > > + */ > > + > > +#include <media/rc-map.h> > > +#include <linux/module.h> > > + > > +/* CEC Spec "High-Definition Multimedia Interface Specification" can > > +be obtained > > + * here: http://xtreamerdev.googlecode.com/files/CEC_Specs.pdf > > + * The list of control codes is listed in Table 27: User Control > > +Codes p. 95 */ > > + > > +static struct rc_map_table cec[] = { > > + { 0x00, KEY_SELECT }, /* XXX CEC Spec: Select, should it be > > +KEY_SELECT or KEY_OK? */ > > KEY_OK is better, IMHO. > > > + { 0x01, KEY_UP }, > > + { 0x02, KEY_DOWN }, > > + { 0x03, KEY_LEFT }, > > + { 0x04, KEY_RIGHT }, > > + /* XXX 0x05-0x08 CEC Spec: Right-Up, Right-Down, Left-Up, Left- > Down > > +*/ > > I think you need to send a patch to linux-input, in order to add those > keycodes. > > > + { 0x09, KEY_CONTEXT_MENU }, /* CEC Spec: Root Menu - see Note 2 > */ > > + /* Note 2: This is the initial display that a device shows. It is > > + * device-dependent and can be, for example, a contents menu, > setup > > + * menu, favorite menu or other menu. The actual menu displayed > > + * may also depend on the device’s current state. */ > > + { 0x0a, KEY_SETUP }, > > + { 0x0b, KEY_MENU }, /* CEC Spec: Contents Menu */ > > + { 0x0c, KEY_FAVORITES }, /* CEC Spec: Favorite Menu */ > > + { 0x0d, KEY_EXIT }, > > + /* 0x0e-0x1f: Reserved */ > > + /* 0x20-0x29: Keys 0 to 9 */ > > + { 0x20, KEY_0 }, > > + { 0x21, KEY_1 }, > > + { 0x22, KEY_2 }, > > + { 0x23, KEY_3 }, > > + { 0x24, KEY_4 }, > > + { 0x25, KEY_5 }, > > + { 0x26, KEY_6 }, > > + { 0x27, KEY_7 }, > > + { 0x28, KEY_8 }, > > + { 0x29, KEY_9 }, > > Better to use KEY_NUMERIC_* here, as this is not affected by the shift > state. > > > + { 0x2a, KEY_DOT }, > > + { 0x2b, KEY_ENTER }, > > + { 0x2c, KEY_CLEAR }, > > + /* 0x2d-0x2e: Reserved */ > > > + /* XXX 0x2f: CEC Spec: Next Favorite */ > Again another addition to Linux keystroke codes. > > > + { 0x30, KEY_CHANNELUP }, > > + { 0x31, KEY_CHANNELDOWN }, > > + { 0x32, KEY_PREVIOUS }, /* CEC Spec: Previous Channel */ > > + { 0x33, KEY_SOUND }, /* CEC Spec: Sound Select */ > > + /* XXX 0x34: CEC Spec: Input Select */ > > Another key to be added. Yet, other keymaps have a key to select the > input source. Most use KEY_VIDEO, to select the video input source, and > KEY_AUDIO, to select the audio input source. > > So, KEY_VIDEO is likely the best choice here. > > > + { 0x35, KEY_INFO }, /* CEC Spec: Display Information */ > > + { 0x36, KEY_HELP }, > > + { 0x37, KEY_PAGEUP }, > > + { 0x38, KEY_PAGEDOWN }, > > + /* 0x39-0x3f: Reserved */ > > + { 0x40, KEY_POWER }, > > + { 0x41, KEY_VOLUMEUP }, > > + { 0x42, KEY_VOLUMEDOWN }, > > + { 0x43, KEY_MUTE }, > > + { 0x44, KEY_PLAY }, > > + { 0x45, KEY_STOP }, /* XXX CEC Spec: Stop, what about KEY_STOPCD? > */ > > + { 0x46, KEY_PAUSE },/* XXX CEC Spec: Pause, what about > KEY_PAUSECD? > > +*/ > > The CD variants are to control the CD player on multimedia keyboards I > think. > only two IR maps use it. All the rest uses KEY_STOP/KEY_PAUSE. > > > + { 0x47, KEY_RECORD }, > > + { 0x48, KEY_REWIND }, > > + { 0x49, KEY_FASTFORWARD }, > > + { 0x4a, KEY_EJECTCD }, /* CEC Spec: Eject */ > > + { 0x4b, KEY_FORWARD }, > > > + { 0x4c, }, /* XXX */ > > Hmm.. I guess it would be KEY_BACK for the backward keycode. > > > + { 0x4d, KEY_STOP }, /* XXX CEC Spec: Stop-Record, what about > KEY_STOPCD? */ > > + { 0x4e, KEY_PAUSE }, /* XXX CEC Spec: Pause-Record, what about > > +KEY_PAUSECD? */ > > Probably, we'll need to define two new keycodes here. > > > + /* 0x4f: Reserved */ > > + { 0x50, KEY_ANGLE }, > > + { 0x51, KEY_SUBTITLE }, /* XXX CEC Spec: Sub picture, should it > be > > +KEY_SUBTITLE or something else? */ > > KEY_SUBTITLE is for subtitle OSG data. Perhaps KEY_TV2? > > Different maps do different things with the PIP key: > > $ git grep -i pip drivers/media/rc/keymaps/ > drivers/media/rc/keymaps/rc-alink-dtu-m.c: { 0x0805, KEY_NEW }, > /* symbol: PIP */ > drivers/media/rc/keymaps/rc-avermedia-cardbus.c: { 0x2b, > KEY_VIDEO }, /* PIP (Picture-in-picture) */ > drivers/media/rc/keymaps/rc-avermedia-m135a.c: { 0x022b, KEY_TV2 }, > /* TV2 or PIP */ > drivers/media/rc/keymaps/rc-avermedia-m135a.c: { 0x041a, KEY_TV2 }, > /* PIP */ > drivers/media/rc/keymaps/rc-avermedia-m733a-rm-k6.c: { 0x041a, > KEY_TV2 }, /* PIP */ > drivers/media/rc/keymaps/rc-azurewave-ad-tu700.c: { 0x0047, > KEY_NEW }, /* PIP */ > drivers/media/rc/keymaps/rc-dib0700-rc5.c: { 0x064c, > KEY_RESERVED }, /* PIP button*/ > drivers/media/rc/keymaps/rc-digitalnow-tinytwin.c: { 0x0053, > KEY_NEW }, /* [symbol PIP?] */ > drivers/media/rc/keymaps/rc-dntv-live-dvbt-pro.c: { 0x47, > KEY_TV2 }, /* pip */ > drivers/media/rc/keymaps/rc-dvbsky.c: { 0x000f, KEY_SUBTITLE }, > /*PIP*/ > drivers/media/rc/keymaps/rc-encore-enltv2.c: { 0x71, KEY_TV2 }, > /* PIP */ > drivers/media/rc/keymaps/rc-evga-indtube.c: { 0x16, KEY_TV2}, > /* PIP */ > drivers/media/rc/keymaps/rc-flydvb.c: { 0x1a, KEY_TV2 }, > /* PIP */ > drivers/media/rc/keymaps/rc-terratec-slim.c: { 0x02bd0b, KEY_NEW }, > /* symbol: PIP */ > drivers/media/rc/keymaps/rc-winfast-usbii-deluxe.c: { 0x3a, > KEY_NEW}, /* PIP */ > drivers/media/rc/keymaps/rc-winfast.c: { 0x2a, KEY_TV2 }, > /* PIP (Picture in picture */ > > > + { 0x52, KEY_VIDEO }, /* XXX CEC Spec: Video on Demand / input.h: > AL > > +Movie Browser, maybe KEY_DIRECTORY? */ > > I would add a KEY_VOD for that. > > > + { 0x53, KEY_EPG }, > > + { 0x54, KEY_TIME }, /* XXX CEC Spec: Timer */ > > + { 0x55, KEY_CONFIG }, > > + /* 0x56-0x5f: Reserved */ > > > + { 0x60, KEY_PLAY }, /* XXX CEC Spec: Play Function */ > > + { 0x61, KEY_PLAYPAUSE }, /* XXX CEC Spec: Pause-Play Function */ > > + { 0x62, KEY_RECORD }, /* XXX CEC Spec: Record Function */ > > + { 0x63, KEY_PAUSE }, /* XXX CEC Spec: Pause-Record Function */ > > + { 0x64, KEY_STOP }, /* XXX CEC Spec: Stop Function */ > > + { 0x65, KEY_MUTE }, /* XXX CEC Spec: Mute Function */ > > + /* 0x66: CEC Spec: Restore Volume Function */ > > + { 0x67, KEY_TUNER }, /* XXX CEC Spec: Tune Function */ > > + { 0x68, KEY_MEDIA }, /* CEC Spec: Select Media Function */ > > + { 0x69, KEY_SWITCHVIDEOMODE} /* XXX CEC Spec: Select A/V Input > Function */, > > + { 0x6a, KEY_AUDIO} /* CEC Spec: Select Audio Input Function */, > > + { 0x6b, KEY_POWER} /* CEC Spec: Power Toggle Function */, > > + { 0x6c, KEY_SLEEP} /* XXX CEC Spec: Power Off Function */, > > + { 0x6d, KEY_WAKEUP} /* XXX CEC Spec: Power On Function */, > > Those "function" keycodes look weird. What's the difference between > those and the pure non-function variants? > > The spec (CEC 13.13.3) says that: > > "Unlike the other codes, which just pass remote control presses > to the target (often with manufacturer-specific results), > the Functions are deterministic, ie they specify exactly the > state > after executing these commands. Several of these also have > further > operands, specifying the function in more detail, immediately > following the relevant [UI Command] operand." > > Some codes are actually compund ones. For example, 0x60 has a "play > mode" > operand. So, the actual mapping would be: > > 0x60 + 0x24 - "play forward" > 0x61 + 0x20 - "play reverse" > ... > (see CEC17 for operand descriptions) > > So, IMHO, the mapping should be > > { 0x6024, KEY_PLAY }, > { 0x6020, KEY_PLAY_REVERSE }, // to be created > ... > > > > + /* 0x6e-0x70: Reserved */ > > + { 0x71, KEY_BLUE }, /* XXX CEC Spec: F1 (Blue) */ > > + { 0x72, KEY_RED }, /* XXX CEC Spec: F2 (Red) */ > > + { 0x73, KEY_GREEN }, /* XXX CEC Spec: F3 (Green) */ > > + { 0x74, KEY_YELLOW }, /* XXX CEC Spec: F4 (Yellow) */ > > + { 0x75, KEY_F5 }, > > + { 0x76, KEY_CONNECT }, /* XXX CEC Spec: Data - see Note 3 */ > > + /* Note 3: This is used, for example, to enter or leave a digital > TV > > + * data broadcast application. */ > > perhaps a new keycode, like KEY_DVB? The spec is not actually too clear > about what that "Data" key means. > > > + /* 0x77-0xff: Reserved */ > > +}; > > + > > +static struct rc_map_list cec_map = { > > + .map = { > > + .scan = cec, > > + .size = ARRAY_SIZE(cec), > > + .rc_type = RC_TYPE_CEC, > > + .name = RC_MAP_CEC, > > + } > > +}; > > + > > +static int __init init_rc_map_cec(void) { > > + return rc_map_register(&cec_map); > > +} > > + > > +static void __exit exit_rc_map_cec(void) { > > + rc_map_unregister(&cec_map); > > +} > > + > > +module_init(init_rc_map_cec); > > +module_exit(exit_rc_map_cec); > > + > > +MODULE_LICENSE("GPL"); > > +MODULE_AUTHOR("Kamil Debski"); > > diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c > > index f8c5e47..37d1ce0 100644 > > --- a/drivers/media/rc/rc-main.c > > +++ b/drivers/media/rc/rc-main.c > > @@ -801,6 +801,7 @@ static struct { > > { RC_BIT_MCE_KBD, "mce_kbd" }, > > { RC_BIT_LIRC, "lirc" }, > > { RC_BIT_XMP, "xmp" }, > > + { RC_BIT_CEC, "cec" }, > > }; > > > > /** > > diff --git a/include/media/rc-core.h b/include/media/rc-core.h index > > 2c7fbca..7c9d15d 100644 > > --- a/include/media/rc-core.h > > +++ b/include/media/rc-core.h > > @@ -32,6 +32,7 @@ do { > \ > > enum rc_driver_type { > > RC_DRIVER_SCANCODE = 0, /* Driver or hardware generates a > scancode */ > > RC_DRIVER_IR_RAW, /* Needs a Infra-Red pulse/space decoder */ > > + RC_DRIVER_CEC, > > }; > > > > /** > > diff --git a/include/media/rc-map.h b/include/media/rc-map.h index > > e7a1514..2058a89 100644 > > --- a/include/media/rc-map.h > > +++ b/include/media/rc-map.h > > @@ -32,6 +32,7 @@ enum rc_type { > > RC_TYPE_RC6_MCE = 17, /* MCE (Philips RC6-6A-32 subtype) > protocol */ > > RC_TYPE_SHARP = 18, /* Sharp protocol */ > > RC_TYPE_XMP = 19, /* XMP protocol */ > > + RC_TYPE_CEC = 20, /* CEC protocol */ > > }; > > > > #define RC_BIT_NONE 0 > > @@ -55,6 +56,7 @@ enum rc_type { > > #define RC_BIT_RC6_MCE (1 << RC_TYPE_RC6_MCE) > > #define RC_BIT_SHARP (1 << RC_TYPE_SHARP) > > #define RC_BIT_XMP (1 << RC_TYPE_XMP) > > +#define RC_BIT_CEC (1 << RC_TYPE_CEC) > > > > #define RC_BIT_ALL (RC_BIT_UNKNOWN | RC_BIT_OTHER | RC_BIT_LIRC | > \ > > RC_BIT_RC5 | RC_BIT_RC5X | RC_BIT_RC5_SZ | \ @@ - > 63,7 +65,7 @@ > > enum rc_type { > > RC_BIT_NEC | RC_BIT_SANYO | RC_BIT_MCE_KBD | \ > > RC_BIT_RC6_0 | RC_BIT_RC6_6A_20 | RC_BIT_RC6_6A_24 | > \ > > RC_BIT_RC6_6A_32 | RC_BIT_RC6_MCE | RC_BIT_SHARP | \ > > - RC_BIT_XMP) > > + RC_BIT_XMP | RC_BIT_CEC) > > > > > > #define RC_SCANCODE_UNKNOWN(x) (x) > > @@ -125,6 +127,7 @@ void rc_map_init(void); > > #define RC_MAP_BEHOLD_COLUMBUS "rc-behold-columbus" > > #define RC_MAP_BEHOLD "rc-behold" > > #define RC_MAP_BUDGET_CI_OLD "rc-budget-ci-old" > > +#define RC_MAP_CEC "rc-cec" > > #define RC_MAP_CINERGY_1400 "rc-cinergy-1400" > > #define RC_MAP_CINERGY "rc-cinergy" > > #define RC_MAP_DELOCK_61959 "rc-delock-61959" Best wishes, -- Kamil Debski Samsung R&D Institute Poland _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
