public inbox for linux-media@vger.kernel.org
 help / color / mirror / Atom feed
* [linux-dvb] Ticlkess Mantis remote control implementation
@ 2008-06-24 19:50 Marko Ristola
  2008-06-24 20:36 ` Roland Scheidegger
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Marko Ristola @ 2008-06-24 19:50 UTC (permalink / raw)
  To: linux-dvb


Hi,

I have still my own version of Manu's jusst.de/mantis driver that is 
based on v4l-dvb-linuxtv main branch,
mainly because I use so new Linux kernels.
I have done the following improvement lately:

I implemented a remote control patch, that doesn't poll the remote 
control all the time.
It polls the remote control only if you press the button (a tickless 
version, you know).
It surprised me, that the actual implementation was really small, it 
took very few lines of code.


The idea is, that the remote control informs (thrue irq) 
mantis_query_rc() to be active.
mantis_query_rc takes care that it will reactivate itself after 250ms, 
until remote control
hasn't sent any "user is still pressing a button" messages.

POLL_FREQ (HZ/4) time (250ms) must be more than the remote control 
message interval (230ms).
This way we can ensure that the remote control sends at least one 
message per POLL_FREQ tick.
If on the last POLL_FREQ tick no button is pressed (-1), 
mantis_query_rc() doesn't activate the POLL_FREQ tick
anymore, just informs via ir_input_nokey() that the remote control user 
doesn't press any buttons.

So in this way the remote control works with Twinhan 2033 better than 
ever (the hand experience is good also).
+ Initial key press is delivered instantly.
+ No CPU consumption while the remote control isn't used.
Could possibly be improved with a tasklet, if the instant response 
experience would be that important.
Can be used from 1 to 4 key repeats per second (230ms remote control 
limit is the lower bound, we must be safely above).

Interrupt handler on mantis_pci.c:
        if (stat & MANTIS_INT_IRQ1) {
                mantis->ir.ir_last_code = mmread(0xe8);
                dprintk(verbose, MANTIS_DEBUG, 0, "* INT IRQ-1 *");
                schedule_delayed_work(&mantis->ir.rc_query_work, 0);
        }
 


mantis_rc.c:
#define POLL_FREQ (HZ/4)

void mantis_query_rc(struct work_struct *work)
{
        struct mantis_pci *mantis =
                container_of(work, struct mantis_pci, 
ir.rc_query_work.work);
        struct ir_input_state *ir = &mantis->ir.ir;

        u32 lastkey = mantis->ir.ir_last_code;

        ir_input_nokey(mantis->ir.rc_dev, ir);

        if (lastkey != -1) {
                ir_input_keydown(mantis->ir.rc_dev, ir, lastkey, 0);
                mantis->ir.ir_last_code = -1;
                schedule_delayed_work(&mantis->ir.rc_query_work, POLL_FREQ);
        }
}

int mantis_rc_init(struct mantis_pci *mantis):
        mantis->ir.ir_last_code = -1; /* key presses disabled here. */
        INIT_DELAYED_WORK(&mir->rc_query_work, mantis_query_rc);

int mantis_rc_exit(struct mantis_pci *mantis):

        mmwrite(mmread(MANTIS_INT_MASK) & (~MANTIS_INT_IRQ1), 
MANTIS_INT_MASK);
        mantis->ir.ir_last_code = -1; /* key presses disabled here. */
        cancel_delayed_work_sync(&mantis->ir.rc_query_work); /* not my 
idea */


_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] Ticlkess Mantis remote control implementation
  2008-06-24 19:50 [linux-dvb] Ticlkess Mantis remote control implementation Marko Ristola
@ 2008-06-24 20:36 ` Roland Scheidegger
  2008-06-25 12:08 ` Markus Rechberger
       [not found] ` <48615A7E.2030600@tungstengraphics.com>
  2 siblings, 0 replies; 6+ messages in thread
From: Roland Scheidegger @ 2008-06-24 20:36 UTC (permalink / raw)
  To: Marko Ristola; +Cc: linux-dvb

On 24.06.2008 21:50, Marko Ristola wrote:
> Hi,
> 
> I have still my own version of Manu's jusst.de/mantis driver that is 
> based on v4l-dvb-linuxtv main branch,
> mainly because I use so new Linux kernels.
> I have done the following improvement lately:
> 
> I implemented a remote control patch, that doesn't poll the remote 
> control all the time.
> It polls the remote control only if you press the button (a tickless 
> version, you know).
> It surprised me, that the actual implementation was really small, it 
> took very few lines of code.
> 
You're not the first to think that the constant polling is not
necessary, too bad these things always get lost because they aren't
integrated in the official driver...

http://www.linuxtv.org/pipermail/linux-dvb/2008-May/026102.html

Roland


_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] Ticlkess Mantis remote control implementation
  2008-06-24 19:50 [linux-dvb] Ticlkess Mantis remote control implementation Marko Ristola
  2008-06-24 20:36 ` Roland Scheidegger
@ 2008-06-25 12:08 ` Markus Rechberger
  2008-06-25 20:35   ` Marko Ristola
       [not found] ` <48615A7E.2030600@tungstengraphics.com>
  2 siblings, 1 reply; 6+ messages in thread
From: Markus Rechberger @ 2008-06-25 12:08 UTC (permalink / raw)
  To: Marko Ristola; +Cc: linux-dvb

Hi Marko,

On 6/24/08, Marko Ristola <marko.ristola@kolumbus.fi> wrote:
>
> Hi,
>
> I have still my own version of Manu's jusst.de/mantis driver that is
> based on v4l-dvb-linuxtv main branch,
> mainly because I use so new Linux kernels.
> I have done the following improvement lately:
>
> I implemented a remote control patch, that doesn't poll the remote
> control all the time.
> It polls the remote control only if you press the button (a tickless
> version, you know).
> It surprised me, that the actual implementation was really small, it
> took very few lines of code.
>
>
> The idea is, that the remote control informs (thrue irq)
> mantis_query_rc() to be active.
> mantis_query_rc takes care that it will reactivate itself after 250ms,
> until remote control
> hasn't sent any "user is still pressing a button" messages.
>
> POLL_FREQ (HZ/4) time (250ms) must be more than the remote control
> message interval (230ms).
> This way we can ensure that the remote control sends at least one
> message per POLL_FREQ tick.
> If on the last POLL_FREQ tick no button is pressed (-1),
> mantis_query_rc() doesn't activate the POLL_FREQ tick
> anymore, just informs via ir_input_nokey() that the remote control user
> doesn't press any buttons.
>
> So in this way the remote control works with Twinhan 2033 better than
> ever (the hand experience is good also).
> + Initial key press is delivered instantly.
> + No CPU consumption while the remote control isn't used.
> Could possibly be improved with a tasklet, if the instant response
> experience would be that important.
> Can be used from 1 to 4 key repeats per second (230ms remote control
> limit is the lower bound, we must be safely above).
>
> Interrupt handler on mantis_pci.c:
>        if (stat & MANTIS_INT_IRQ1) {
>                mantis->ir.ir_last_code = mmread(0xe8);
>                dprintk(verbose, MANTIS_DEBUG, 0, "* INT IRQ-1 *");
>                schedule_delayed_work(&mantis->ir.rc_query_work, 0);
>        }
>
>
>
> mantis_rc.c:
> #define POLL_FREQ (HZ/4)
>

maybe msecs_to_jiffies(250) would be easier to understand here?

Markus

> void mantis_query_rc(struct work_struct *work)
> {
>        struct mantis_pci *mantis =
>                container_of(work, struct mantis_pci,
> ir.rc_query_work.work);
>        struct ir_input_state *ir = &mantis->ir.ir;
>
>        u32 lastkey = mantis->ir.ir_last_code;
>
>        ir_input_nokey(mantis->ir.rc_dev, ir);
>
>        if (lastkey != -1) {
>                ir_input_keydown(mantis->ir.rc_dev, ir, lastkey, 0);
>                mantis->ir.ir_last_code = -1;
>                schedule_delayed_work(&mantis->ir.rc_query_work, POLL_FREQ);
>        }
> }
>
> int mantis_rc_init(struct mantis_pci *mantis):
>        mantis->ir.ir_last_code = -1; /* key presses disabled here. */
>        INIT_DELAYED_WORK(&mir->rc_query_work, mantis_query_rc);
>
> int mantis_rc_exit(struct mantis_pci *mantis):
>
>        mmwrite(mmread(MANTIS_INT_MASK) & (~MANTIS_INT_IRQ1),
> MANTIS_INT_MASK);
>        mantis->ir.ir_last_code = -1; /* key presses disabled here. */
>        cancel_delayed_work_sync(&mantis->ir.rc_query_work); /* not my
> idea */
>
>
> _______________________________________________
> linux-dvb mailing list
> linux-dvb@linuxtv.org
> http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
>

_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] Ticlkess Mantis remote control implementation
       [not found] ` <48615A7E.2030600@tungstengraphics.com>
@ 2008-06-25 20:22   ` Marko Ristola
  2008-06-25 22:05     ` Roland Scheidegger
  0 siblings, 1 reply; 6+ messages in thread
From: Marko Ristola @ 2008-06-25 20:22 UTC (permalink / raw)
  To: Roland Scheidegger; +Cc: linux-dvb

Roland Scheidegger wrote:
> On 24.06.2008 21:50, Marko Ristola wrote:
>   
>> Hi,
>>
>> I have still my own version of Manu's jusst.de/mantis driver that is 
>> based on v4l-dvb-linuxtv main branch,
>> mainly because I use so new Linux kernels.
>> I have done the following improvement lately:
>>
>> I implemented a remote control patch, that doesn't poll the remote 
>> control all the time.
>> It polls the remote control only if you press the button (a tickless 
>> version, you know).
>> It surprised me, that the actual implementation was really small, it 
>> took very few lines of code.
>>
>>     
> You're not the first to think that the constant polling is not
> necessary, too bad these things always get lost because they aren't
> integrated in the official driver...
>
> http://www.linuxtv.org/pipermail/linux-dvb/2008-May/026102.html
>   
Okay, I understood from your patch that you just deliver all key repeats 
as initial key presses.
You have disabled the polling altogether, but just deliver the key 
presses as initial key presses
and deliver the "key unpressed" instantly after the key press.
What is the feeling? I think that it might work for my remote control 
equally.
Also I noticed that you mentioned that you remote control's initial key 
repeat comes after 270ms.
So polling on my code should be done with 300ms intervals for example.
Maybe it would make my own remote control also more robust.

My version is better only in the sense that some application would be 
able to
catch the fact that the user is pressing a button and holding it down a 
long time. Maybe some programs need that for robustness.

Your version is better in the sense that you repeat 270ms and 220ms 
rerepeats correctly, as the remote control sends them.

Can we implement a version that has both advantages and is still 
acceptable to the kernel?

+ no work cancellation needs to be done during normal operation.
+ no current time calculation is done.

With both features working, it could be done in the following way:
Set a work with a timeout of 300ms.
Deliver all key presses and repeats instantly.
On key press delivery, cancel the assumed ealier delayed check and setup 
a new checking work with 300ms delay.
If a 300ms checking delay will be ever active, it will just inform "no 
key pressed" and continue.

On my opinion this "final" implementation might be a generic 
implementation which could be used in more places than on Mantis driver.

One question is, that are these Mantis remote controls at all like RC5 
remote controls mentioned in ir-common.c/h.
If yes, the correct way might be to use it's implementation.

Personally I haven't seen a "key unpressed" IRQ with my remote.
That event seems to be needed, if RC5 code is going to be used.

It might though be, that the interrupt line for "key unpressed" isn't 
the same as with "key pressed".
That might be rather easy to investigate and the implementation for "key 
unpressed" would be then trivial.

So currently using RC5 implementation on ir-common.c doesn't seem to be 
easy if at all possible.

Marko
> Roland
>
>   


_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] Ticlkess Mantis remote control implementation
  2008-06-25 12:08 ` Markus Rechberger
@ 2008-06-25 20:35   ` Marko Ristola
  0 siblings, 0 replies; 6+ messages in thread
From: Marko Ristola @ 2008-06-25 20:35 UTC (permalink / raw)
  To: Markus Rechberger; +Cc: linux-dvb

Markus Rechberger wrote:
> Hi Marko,
>
> On 6/24/08, Marko Ristola <marko.ristola@kolumbus.fi> wrote:
>   
>> Hi,
>>
>> I have still my own version of Manu's jusst.de/mantis driver that is
>> based on v4l-dvb-linuxtv main branch,
>> mainly because I use so new Linux kernels.
>> I have done the following improvement lately:
>>
>> I implemented a remote control patch, that doesn't poll the remote
>> control all the time.
>> It polls the remote control only if you press the button (a tickless
>> version, you know).
>> It surprised me, that the actual implementation was really small, it
>> took very few lines of code.
>>     

>>
>>
>> mantis_rc.c:
>> #define POLL_FREQ (HZ/4)
>>
>>     
>
> maybe msecs_to_jiffies(250) would be easier to understand here?
>   
Sounds good,
maybe something like

#define RC_REPEAT_DELAY msecs_to_jiffies(300)

So the increase into 300 is because the initial remote control delay 
might be about 270ms.
Further delays might be 220ms.
This way the delay is safely big enough.

Roland has done another tickless implementation that obeys 270ms and 
220ms delays exactly.
It might be even better now for VDR style applications than my 
implementation.

It is on http://www.linuxtv.org/pipermail/linux-dvb/2008-May/026102.html.

Marko
> Markus
>
>   
>> void mantis_query_rc(struct work_struct *work)
>> {
>>        struct mantis_pci *mantis =
>>                container_of(work, struct mantis_pci,
>> ir.rc_query_work.work);
>>        struct ir_input_state *ir = &mantis->ir.ir;
>>
>>        u32 lastkey = mantis->ir.ir_last_code;
>>
>>        ir_input_nokey(mantis->ir.rc_dev, ir);
>>
>>        if (lastkey != -1) {
>>                ir_input_keydown(mantis->ir.rc_dev, ir, lastkey, 0);
>>                mantis->ir.ir_last_code = -1;
>>                schedule_delayed_work(&mantis->ir.rc_query_work, POLL_FREQ);
>>        }
>> }
>>
>> int mantis_rc_init(struct mantis_pci *mantis):
>>        mantis->ir.ir_last_code = -1; /* key presses disabled here. */
>>        INIT_DELAYED_WORK(&mir->rc_query_work, mantis_query_rc);
>>
>> int mantis_rc_exit(struct mantis_pci *mantis):
>>
>>        mmwrite(mmread(MANTIS_INT_MASK) & (~MANTIS_INT_IRQ1),
>> MANTIS_INT_MASK);
>>        mantis->ir.ir_last_code = -1; /* key presses disabled here. */
>>        cancel_delayed_work_sync(&mantis->ir.rc_query_work); /* not my
>> idea */
>>
>>
>> _______________________________________________
>> linux-dvb mailing list
>> linux-dvb@linuxtv.org
>> http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb
>>
>>     
>
>   


_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [linux-dvb] Ticlkess Mantis remote control implementation
  2008-06-25 20:22   ` Marko Ristola
@ 2008-06-25 22:05     ` Roland Scheidegger
  0 siblings, 0 replies; 6+ messages in thread
From: Roland Scheidegger @ 2008-06-25 22:05 UTC (permalink / raw)
  To: Marko Ristola; +Cc: linux-dvb

On 25.06.2008 22:22, Marko Ristola wrote:
> Roland Scheidegger wrote:
>> On 24.06.2008 21:50, Marko Ristola wrote:
>>   
>>> Hi,
>>>
>>> I have still my own version of Manu's jusst.de/mantis driver that is 
>>> based on v4l-dvb-linuxtv main branch,
>>> mainly because I use so new Linux kernels.
>>> I have done the following improvement lately:
>>>
>>> I implemented a remote control patch, that doesn't poll the remote 
>>> control all the time.
>>> It polls the remote control only if you press the button (a tickless 
>>> version, you know).
>>> It surprised me, that the actual implementation was really small, it 
>>> took very few lines of code.
>>>
>>>     
>> You're not the first to think that the constant polling is not
>> necessary, too bad these things always get lost because they aren't
>> integrated in the official driver...
>>
>> http://www.linuxtv.org/pipermail/linux-dvb/2008-May/026102.html
>>   
> Okay, I understood from your patch that you just deliver all key repeats 
> as initial key presses.
> You have disabled the polling altogether, but just deliver the key 
> presses as initial key presses
> and deliver the "key unpressed" instantly after the key press.
Yes, I guess though this is probably somewhat of an abuse of the API and
maybe there could be an easier way by just delivering one key press
without having to send an unpress event. I dunno though, I'm not
familiar enough with the ir / input code.

> What is the feeling? I think that it might work for my remote control 
> equally.
> Also I noticed that you mentioned that you remote control's initial key 
> repeat comes after 270ms.
> So polling on my code should be done with 300ms intervals for example.
> Maybe it would make my own remote control also more robust.
> 
> My version is better only in the sense that some application would be 
> able to
> catch the fact that the user is pressing a button and holding it down a 
> long time. Maybe some programs need that for robustness.
That's the theory, but with the delays from the remote it didn't happen
to work in practice here consistently. Maybe if you'd change the default
initial repeat / frequency of the input layer code (which I failed to
do), since increasing the poll frequency just above 300ms is a bad idea
(larger than the input layer code initial repeat - hence it is
impossible to get single key presses).
Also, I'm not sure there's any program out there which actually cares if
you press a key multiple times or hold it down? And ultimately, you
can't reliably detect this from the remote itself anyway (well you could
try based on the timing intervals), since apparently you just get the
same keycode again if you hold it down.

> 
> Your version is better in the sense that you repeat 270ms and 220ms 
> rerepeats correctly, as the remote control sends them.
> 
> Can we implement a version that has both advantages and is still 
> acceptable to the kernel?
> 
> + no work cancellation needs to be done during normal operation.
> + no current time calculation is done.
> 
> With both features working, it could be done in the following way:
> Set a work with a timeout of 300ms.
> Deliver all key presses and repeats instantly.
> On key press delivery, cancel the assumed ealier delayed check and setup 
> a new checking work with 300ms delay.
> If a 300ms checking delay will be ever active, it will just inform "no 
> key pressed" and continue.
It sounds good in theory (and I tried something along these lines), but
finally decided I didn't want to waste any more time - it got too
complicated (mostly because I failed to change the initial repeat /
frequency of the input layer code, looking at it I'm not sure it can be
done after initialization, and if you do it before you will need to
duplicate all the logic for key repeats which is in the input code
yourself). Just didn't seem worth the trouble.

> 
> On my opinion this "final" implementation might be a generic 
> implementation which could be used in more places than on Mantis driver.
> 
> One question is, that are these Mantis remote controls at all like RC5 
> remote controls mentioned in ir-common.c/h.
> If yes, the correct way might be to use it's implementation.
> 
> Personally I haven't seen a "key unpressed" IRQ with my remote.
> That event seems to be needed, if RC5 code is going to be used.
> 
> It might though be, that the interrupt line for "key unpressed" isn't 
> the same as with "key pressed".
> That might be rather easy to investigate and the implementation for "key 
> unpressed" would be then trivial.
I don't think you'd ever get something for key unpressed. Seems quite
logical, the uart just generates an interrupt for any key it receives,
and if you "unpress" well there simply won't be any more keys.

So all in all I figured not using any kind of auto-repeat works just as
well, and the only real improvement possible (e.g. the timing based
recognition of hold-down keys) is something likely noone cares about
anyway but requires 10 times as complicated code.

Roland

_______________________________________________
linux-dvb mailing list
linux-dvb@linuxtv.org
http://www.linuxtv.org/cgi-bin/mailman/listinfo/linux-dvb

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2008-06-25 22:06 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-06-24 19:50 [linux-dvb] Ticlkess Mantis remote control implementation Marko Ristola
2008-06-24 20:36 ` Roland Scheidegger
2008-06-25 12:08 ` Markus Rechberger
2008-06-25 20:35   ` Marko Ristola
     [not found] ` <48615A7E.2030600@tungstengraphics.com>
2008-06-25 20:22   ` Marko Ristola
2008-06-25 22:05     ` Roland Scheidegger

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox