Re: git: 9e007a88d65b - main - atkbd: Reduce polling rate from 10Hz to ~1Hz.

I saw scenarios with lost edge-triggered MSI interrupts on weird Marvell 
AHCIs.  So I can believe it may be weird for atkbd too, if it is 
edge-triggered aswell.  I haven't looked into its specs for very long 
time.  If you are sure it is not needed, then I have no problem to set 
it to 0 in separate commit, I've planned that possibility.

On 05.01.2022 12:26, Warner Losh wrote:
> Maybe we default hw.atkbd.hz to 0. That will give us more info if this 
> is even a thing still. And it would give users hit by this a no 
> recompile fix and give us feedback as to how often this happens.
> 
> We used to miss ISA interrupts in the early SMPNG days, and that's the 
> time this change was introduced. The PIC does a good job of latching the 
> state and we have no other drivers that have this workaround absent 
> issues with the device itself.
> 
> Warner
> 
> On Wed, Jan 5, 2022, 9:41 AM Alexander Motin <mav@freebsd.org 
> <mailto:mav@freebsd.org>> wrote:
> 
>     The branch main has been updated by mav:
> 
>     URL:
>     https://cgit.FreeBSD.org/src/commit/?id=9e007a88d65ba0d23e73c3c052d474a78260d503
>     <https://cgit.FreeBSD.org/src/commit/?id=9e007a88d65ba0d23e73c3c052d474a78260d503>
> 
>     commit 9e007a88d65ba0d23e73c3c052d474a78260d503
>     Author:     Alexander Motin <mav@FreeBSD.org>
>     AuthorDate: 2022-01-05 16:32:44 +0000
>     Commit:     Alexander Motin <mav@FreeBSD.org>
>     CommitDate: 2022-01-05 16:41:26 +0000
> 
>          atkbd: Reduce polling rate from 10Hz to ~1Hz.
> 
>          In my understanding this is only needed to workaround lost
>     interrupts.
>          I was thinking to remove it completely, but the comment about edge-
>          triggered interrupt may be true and needs deeper
>     investigation.  ~1Hz
>          should be often enough to handle the supposedly rare loss
>     cases, but
>          rare enough to not appear in top.  Add sysctl hw.atkbd.hz to
>     tune it.
> 
>          MFC after:      1 month
>     ---
>       sys/dev/atkbdc/atkbd.c | 15 +++++++++++++--
>       1 file changed, 13 insertions(+), 2 deletions(-)
> 
>     diff --git a/sys/dev/atkbdc/atkbd.c b/sys/dev/atkbdc/atkbd.c
>     index 40dd698984e3..cee1207df973 100644
>     --- a/sys/dev/atkbdc/atkbd.c
>     +++ b/sys/dev/atkbdc/atkbd.c
>     @@ -42,6 +42,7 @@ __FBSDID("$FreeBSD$");
>       #include <sys/proc.h>
>       #include <sys/limits.h>
>       #include <sys/malloc.h>
>     +#include <sys/sysctl.h>
> 
>       #include <machine/bus.h>
>       #include <machine/resource.h>
>     @@ -73,6 +74,13 @@ typedef struct atkbd_state {
>       #endif
>       } atkbd_state_t;
> 
>     +static SYSCTL_NODE(_hw, OID_AUTO, atkbd, CTLFLAG_RD |
>     CTLFLAG_MPSAFE, 0,
>     +    "AT keyboard");
>     +
>     +static int atkbdhz = 1;
>     +SYSCTL_INT(_hw_atkbd, OID_AUTO, hz, CTLFLAG_RWTUN, &atkbdhz, 0,
>     +    "Polling frequency (in hz)");
>     +
>       static void            atkbd_timeout(void *arg);
>       static int             atkbd_reset(KBDC kbdc, int flags, int c);
> 
>     @@ -198,8 +206,11 @@ atkbd_timeout(void *arg)
>                              kbdd_intr(kbd, NULL);
>              }
>              splx(s);
>     -       state = (atkbd_state_t *)kbd->kb_data;
>     -       callout_reset(&state->ks_timer, hz / 10, atkbd_timeout, arg);
>     +       if (atkbdhz > 0) {
>     +               state = (atkbd_state_t *)kbd->kb_data;
>     +               callout_reset_sbt(&state->ks_timer, SBT_1S / atkbdhz, 0,
>     +                   atkbd_timeout, arg, C_PREL(1));
>     +       }
>       }
> 
>       /* LOW-LEVEL */
> 

-- 
Alexander Motin