| Message ID | 20090828041459.67c1499a@pedra.chehab.org (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
Would like to add one more dimension to the discussion.
The situation of having multiple DVB type boards in one system.
Using one remote would be enough to control the system. So we should have a
mechanism/kernel config option, to enable/disable an IR device on a board.
For multiple boards of the same type, enable the first and disable any
subsequently detected boards.
Peter
>
--
To unsubscribe from this list: send the line "unsubscribe linux-media" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Mauro, On Fri, 28 Aug 2009, Mauro Carvalho Chehab wrote: > Em Fri, 28 Aug 2009 00:46:28 -0300 > Mauro Carvalho Chehab <mchehab@infradead.org> escreveu: > >> So, we need a sort of TODO list for IR changes. A start point (on a random >> order) would be something like: >> >> 2) Implement a v4l handler for EVIOCGKEYCODE/EVIOCSKEYCODE; >> 3) use a different arrangement for IR tables to not spend 16 K for IR table, >> yet allowing RC5 full table; >> 4) Use the common IR framework at the dvb drivers with their own iplementation; >> 5) Allow getkeycode/setkeycode to work with the dvb framework using the new >> methods; > > Ok, I have a code that handles the above for dvb-usb. Se enclosed. It turned to be > simpler than what I've expected originally ;) Yeah, this is due to the nature of modularity of dvb-usb. Saying that, all drivers which have (re)implemented IR-handling needs to ported as well. Or included in dvb-usb :P . > Tested with kernel 2.6.30.3 and a dibcom device. > > While this patch works fine, for now, it is just a proof of concept, since there are a few > details to be decided/solved for a version 2, as described bellow. This is the answer to the question I had several times in the past years. Very good job. It will solve the memory waste in the driver for key-tables filled up with keys for different remotes where the user of one board only has one. The code will also be smaller and easier to read. This also allows the user to use any remote with any (sensitive) ir-sensor in a USB device. Is there a feature in 'input' which allows to request a file (like request_firmware)? This would be ideal for a transition from in-kernel-keymaps to user-space-keymap-loading: By default it would request the file corresponding to the remote delivered with the device. Is that possible somehow? Patrick. -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Fri, 28 Aug 2009 12:50:27 +0200 (CEST) Patrick Boettcher <pboettcher@kernellabs.com> escreveu: > Hi Mauro, > > On Fri, 28 Aug 2009, Mauro Carvalho Chehab wrote: > > > Em Fri, 28 Aug 2009 00:46:28 -0300 > > Mauro Carvalho Chehab <mchehab@infradead.org> escreveu: > > > >> So, we need a sort of TODO list for IR changes. A start point (on a random > >> order) would be something like: > >> > >> 2) Implement a v4l handler for EVIOCGKEYCODE/EVIOCSKEYCODE; > >> 3) use a different arrangement for IR tables to not spend 16 K for IR table, > >> yet allowing RC5 full table; > >> 4) Use the common IR framework at the dvb drivers with their own iplementation; > >> 5) Allow getkeycode/setkeycode to work with the dvb framework using the new > >> methods; > > > > Ok, I have a code that handles the above for dvb-usb. Se enclosed. It turned to be > > simpler than what I've expected originally ;) > > Yeah, this is due to the nature of modularity of dvb-usb. Yes, but I was afraid that we would need to use a different struct for IR's. This feature is already available for years at V4L, but the tables needed to use scancode as their indexes to use the default handlers (I'm not sure, but afaik, in the past there weren't ways to override them). > Saying that, all > drivers which have (re)implemented IR-handling needs to ported as well. Or > included in dvb-usb :P . My suggestion is to port the current implementation to ir-common.ko. This module is not dependent of any other V4L or DVB specifics and has already some code to handle GPIO polling based and GPIO IRQ based IR's. It contains some IR tables for IR's that are used also on dvb-usb, like Hauppauge IR keycodes. Yet, we will need first to change ir-common.ko, since it is currently using the tables indexed by a 7bit scancode (due to size constraints, V4L code discards one RC5 byte), but this is already on our todo list (due to keycode standardization). > > Tested with kernel 2.6.30.3 and a dibcom device. > > > > While this patch works fine, for now, it is just a proof of concept, since there are a few > > details to be decided/solved for a version 2, as described bellow. > > This is the answer to the question I had several times in the past years. > > Very good job. It will solve the memory waste in the driver for > key-tables filled up with keys for different remotes where the user of > one board only has one. The code will also be smaller and easier to read. > > This also allows the user to use any remote with any (sensitive) > ir-sensor in a USB device. True. > Is there a feature in 'input' which allows to request a file (like > request_firmware)? This would be ideal for a transition from > in-kernel-keymaps to user-space-keymap-loading: By default it would > request the file corresponding to the remote delivered with the device. > > Is that possible somehow? There's nothing that I'm aware of, but I suspect that it shouldn't be hard to do it via udev. We'll need to do some work there, in order to be sure that each V4L and DVB device will have their own unique IR board ID's. We may then do an application based on v4l2-apps/util/keytable that will use the IR board ID to dynamically load the keytable, with a default config of loading the board's own IR, but allowing the user to replace it by a custom one. Currently, the Makefile at v4l2-apps/util creates a directory (keycodes) that contains lots of IR tables. It does it by parsing the existing IR tables at V4L side. After having all tables looking the same, it won't be hard to change it to parse all MCE tables, creating an updated repository of the existing scancode/keycode association. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Fri, 28 Aug 2009 11:13:33 +0100 Peter Brouwer <pb.maillists@googlemail.com> escreveu: > > Would like to add one more dimension to the discussion. > > The situation of having multiple DVB type boards in one system. > > Using one remote would be enough to control the system. So we should have a > mechanism/kernel config option, to enable/disable an IR device on a board. > For multiple boards of the same type, enable the first and disable any > subsequently detected boards. I agree that this feature would be useful, but we shouldn't assume that the user doesn't expect to have more than one IR working. With USB keyboard/mouse and computers supporting video cards with multiple heads, it is possible to use one server to handle several virtual machines with their own keyboard, mouse, video and IR (in fact, I've already seen similar scenarios on some expositions). Anyway, for this to happen with all drivers, the better way would be to use a common IR framework. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
2009/8/28 Peter Brouwer <pb.maillists@googlemail.com>: > > Would like to add one more dimension to the discussion. > > The situation of having multiple DVB type boards in one system. > > Using one remote would be enough to control the system. So we should have a > mechanism/kernel config option, to enable/disable an IR device on a board. > For multiple boards of the same type, enable the first and disable any > subsequently detected boards. Don't forget that completely different boards can have identical remotes. For example the RTL2831 driver has an almost identical copy of dvb-usb IR code in it. See here for in depth explanation and patch that makes it use dvb-usb instead: http://patchwork.kernel.org/patch/38794/ Now I'm not really familiar with frontends and tuners so it may be that the RTL driver should be reimplemented within dvb-usb instead, but if it can't be it would be nice if that IR code could be shared without pulling in all the rest of dvb-usb modules too. I'm told that the excessive code duplication is the reason this driver isn't in mainline yet - I've been using it with no problems for over two years now.
Em Fri, 28 Aug 2009 09:30:42 -0300 Mauro Carvalho Chehab <mchehab@infradead.org> escreveu: > Em Fri, 28 Aug 2009 12:50:27 +0200 (CEST) > Patrick Boettcher <pboettcher@kernellabs.com> escreveu: > > > Hi Mauro, > > > > On Fri, 28 Aug 2009, Mauro Carvalho Chehab wrote: > > > > > Em Fri, 28 Aug 2009 00:46:28 -0300 > > > Mauro Carvalho Chehab <mchehab@infradead.org> escreveu: > > > > > >> So, we need a sort of TODO list for IR changes. A start point (on a random > > >> order) would be something like: > > >> > > >> 2) Implement a v4l handler for EVIOCGKEYCODE/EVIOCSKEYCODE; > > >> 3) use a different arrangement for IR tables to not spend 16 K for IR table, > > >> yet allowing RC5 full table; > > >> 4) Use the common IR framework at the dvb drivers with their own iplementation; > > >> 5) Allow getkeycode/setkeycode to work with the dvb framework using the new > > >> methods; > > > > > > Ok, I have a code that handles the above for dvb-usb. Se enclosed. It turned to be > > > simpler than what I've expected originally ;) > > > > Yeah, this is due to the nature of modularity of dvb-usb. > > Yes, but I was afraid that we would need to use a different struct for IR's. > This feature is already available for years at V4L, but the tables needed to > use scancode as their indexes to use the default handlers (I'm not sure, but > afaik, in the past there weren't ways to override them). > > > Saying that, all > > drivers which have (re)implemented IR-handling needs to ported as well. Or > > included in dvb-usb :P . > > My suggestion is to port the current implementation to ir-common.ko. This > module is not dependent of any other V4L or DVB specifics and has already > some code to handle GPIO polling based and GPIO IRQ based IR's. It contains > some IR tables for IR's that are used also on dvb-usb, like Hauppauge IR > keycodes. > > Yet, we will need first to change ir-common.ko, since it is currently using the > tables indexed by a 7bit scancode (due to size constraints, V4L code discards > one RC5 byte), but this is already on our todo list (due to keycode > standardization). Ok, I've did several changes on both V4L and dvb-usb IR implementations. They scancode tables are now implemented at the same way, at: http://linuxtv.org/hg/~mchehab/IR I've also added there the current version of the getkeycode/setkeycode patch. Ah, the v4l2-apps/util/Makefile will now generate the scancode userspace tables for DVB IR's as well (currently, we have 85 IR layouts generated from source files). What's still missing: - For now, V4L is still using internally the old decoding process, based on the 7bit array of keycode. - I haven't yet added some extra empty data at dvb scancode tables; - although they're using the same structs, the code is still different. Please review. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Em Sat, 29 Aug 2009 15:45:28 -0300 Mauro Carvalho Chehab <mchehab@infradead.org> escreveu: > Ok, I've did several changes on both V4L and dvb-usb IR implementations. They > scancode tables are now implemented at the same way, at: > http://linuxtv.org/hg/~mchehab/IR Ok, I've also updated the V4L2 API spec with the default keyboard mapping on the above URL. If nobody complains, I'll update our development tree with the above changes likely today (Aug, 31) night, and prepare the changesets to be added at linux-next. Cheers, Mauro -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Mauro, On Mon, Aug 31, 2009 at 02:47:41AM -0300, Mauro Carvalho Chehab wrote: > Em Sat, 29 Aug 2009 15:45:28 -0300 > Mauro Carvalho Chehab <mchehab@infradead.org> escreveu: > > > Ok, I've did several changes on both V4L and dvb-usb IR implementations. They > > scancode tables are now implemented at the same way, at: > > http://linuxtv.org/hg/~mchehab/IR > > Ok, I've also updated the V4L2 API spec with the default keyboard mapping on > the above URL. If nobody complains, I'll update our development tree with the > above changes likely today (Aug, 31) night, and prepare the changesets to be > added at linux-next. > I see that you changed from KEY_KP* to KEY_*, unfortunately this change will break users who have digits in upper register. KEY_KP* were not perfect either since they are affected by NumLock state. I would recommend moving to KEY_NUMERIC_* which should be unaffected by either register, shift state or NumLock state. Of course there is an issue of them being absent from most keymaps; but nothing is perfect in thsi world ;)
diff -r ec87db9cb8db linux/drivers/media/dvb/dvb-usb/dvb-usb-remote.c --- a/drivers/media/dvb/dvb-usb/dvb-usb-remote.c Fri Aug 28 02:12:12 2009 -0300 +++ b/drivers/media/dvb/dvb-usb/dvb-usb-remote.c Fri Aug 28 03:46:35 2009 -0300 @@ -12,6 +12,65 @@ #include <linux/usb_input.h> #endif +static int dvb_usb_getkeycode(struct input_dev *dev, + int scancode, int *keycode) +{ + struct dvb_usb_device *d = input_get_drvdata(dev); + + struct dvb_usb_rc_key *keymap = d->props.rc_key_map; + int custom = (scancode >> 8) & 0xff; + int data = scancode & 0xff; + int i; + + /* See if we can match the raw key code. */ + for (i = 0; i < d->props.rc_key_map_size; i++) + if (keymap[i].custom == custom && + keymap[i].data == data) { + *keycode = keymap[i].event; + return 0; + } + return -EINVAL; +} + +static int dvb_usb_setkeycode(struct input_dev *dev, + int scancode, int keycode) +{ + struct dvb_usb_device *d = input_get_drvdata(dev); + + struct dvb_usb_rc_key *keymap = d->props.rc_key_map; + int custom = (scancode >> 8) & 0xff; + int data = scancode & 0xff; + int i; + + /* Search if it is replacing an existing keycode */ + for (i = 0; i < d->props.rc_key_map_size; i++) + if (keymap[i].custom == custom && + keymap[i].data == data) { + keymap[i].event = keycode; + return 0; + } + + /* Search if is there a clean entry. If so, use it */ + for (i = 0; i < d->props.rc_key_map_size; i++) + if (keymap[i].event == KEY_RESERVED || + keymap[i].event == KEY_UNKNOWN) { + keymap[i].custom = custom; + keymap[i].data = data; + keymap[i].event = keycode; + return 0; + } + + /* + * FIXME: Currently, it is not possible to increase the size of + * scancode table. For it to happen, one possibility + * would be to allocate a table with key_map_size + 1, + * copying data, appending the new key on it, and freeing + * the old one - or maybe just allocating some spare space + */ + + return -EINVAL; +} + /* Remote-control poll function - called every dib->rc_query_interval ms to see * whether the remote control has received anything. * @@ -127,6 +186,8 @@ #else input_dev->cdev.dev = &d->udev->dev; #endif + input_dev->getkeycode = dvb_usb_getkeycode; + input_dev->setkeycode = dvb_usb_setkeycode; /* set the bits for the keys */ deb_rc("key map size: %d\n", d->props.rc_key_map_size); @@ -144,6 +205,8 @@ input_dev->rep[REP_PERIOD] = d->props.rc_interval; input_dev->rep[REP_DELAY] = d->props.rc_interval + 150; + input_set_drvdata(input_dev, d); + err = input_register_device(input_dev); if (err) { input_free_device(input_dev);