cvs commit: src/sys/amd64/include clock.h timerreg.h src/sys/amd64/isa clock.c src/sys/dev/speaker spkr.c src/sys/dev/syscons syscons.c src/sys/i386/i386 trap.c src/sys/i386/include clock.h timerreg.h src/sys/i386/isa clock.c ...

[Bruce Evans]

On Sat, 5 Apr 2008, Takahashi Yoshihiro wrote:

> In article <200803262009.m2QK9MAo082269 at repoman.freebsd.org>
> Poul-Henning Kamp <phk at FreeBSD.org> writes:
>
>>   In the other function, sc_bell() it seems to get the period from
>>   the KDMKTONE ioctl in terms if 1/1193182th second, so we hardcode
>>   the 1193182 and leave it at that.  It's probably not important.
>
> PC98 uses the different frequency.  It's 1996800 or 2457600.
> Please see i8254_init().

The period of 1/1193182th seconds is part of the (broken as designed or
implemented) KDMKTONE ioctl and/or the syscons bell escape sequence.
Conversion to these units is broken almost everywhere.  I use the following
fix for this in kbdcontrol (except I actually use visual bell since I can't
hear either bell without hearing aids.  With hearing aids, I can hear the
non-broken bell at 800 Hz but not the broken bell at 1193182/800 = 1491 Hz).

IIRC, the default is a broken bell, with sc->bell_pitch = BELL_PITCH
= 800.  800 is correct for the pitch, but it is passed without
conversion to sysbeep(pitch, period), and sysbeep()'s pitch arg is
actually the period in IBM-PC-i8254 cycles, while sysbeep()'s period
arg actually is a period, but it is the period of the beep and is named
better elsewhere as the duration of the beep.  This for the i386
sysbeep().  IIRC, alpha and ia64 sysbeep()'s actually expected the
pitch arg to be a pitch -- they inverted it and used to panic for
division by 0 when the pitch was 0.  The pcvt passed a "pitch" of 1493
IBM-PC-i8254 cycles to sysbeep(), so its default was correct.  Magic
numbers near 800 and 1491 (but rarely ones that actually give exactly
800 Hz) appeared in a few other places.  E.g., i386/machdep.c had the
the slightly different pitch 880 and scaled it with the slightly
wrong scale TIMER_FREQ.

The follow change converts to the syscons ABI in kbdcontrol, so that
kbdcontrol actually sets the pitch in hertz as documented if the pitch
is specified explicitly, but it preserves the broken default of a period
of 800 cycles otherwise.  (The units for these cycles are supposed to be
1/1193182th seconds, but they are in fact very machine-dependent, with
alpha and ia64 inverting them and PC98 having a different timer frequency.
2457600/800 = 3072 Hz would be an even more unusual and harder to hear
default beep frequency than 1491 Hz.)

% Index: kbdcontrol.c
% ===================================================================
% RCS file: /home/ncvs/src/usr.sbin/kbdcontrol/kbdcontrol.c,v
% retrieving revision 1.47
% diff -u -2 -r1.47 kbdcontrol.c
% --- kbdcontrol.c	3 May 2003 21:06:37 -0000	1.47
% +++ kbdcontrol.c	4 May 2003 03:14:51 -0000
% @@ -903,5 +903,5 @@
%  set_bell_values(char *opt)
%  {
% -	int bell, duration, pitch;
% +	int bell, duration, period, pitch;
% 
%  	bell = 0;
% @@ -910,10 +910,15 @@
%  		opt += 6;
%  	}
% -	if (!strcmp(opt, "visual"))
% +	if (!strcmp(opt, "visual")) {
%  		bell |= 1;
% -	else if (!strcmp(opt, "normal"))
% -		duration = 5, pitch = 800;
% +		duration = 0, period = 0;	/* XXX */
% +	} else if (!strcmp(opt, "normal"))
% +		/*
% +		 * XXX the historical normal "pitch" of 800 Hz is actually
% +		 * a period of 800 IBM-PC-i8254 cycles or 670 usec.
% +		 */
% +		duration = 5, period = 800;
%  	else if (!strcmp(opt, "off"))
% -		duration = 0, pitch = 0;
% +		duration = 0, period = 0;	/* XXX */
%  	else {
%  		char		*v1;
% @@ -931,5 +936,17 @@
%  		}
%  		if (pitch != 0)
% -			pitch = 1193182 / pitch;	/* in Hz */
% +			/*
% +			 * XXX the boundary cases are broken.  The kernel
% +			 * silently truncates large periods modulo 0x10000
% +			 * (18.2 msec).  I'm not sure what a period of 0
% +			 * does.
% +			 */
% +			period = 1193182 / pitch;	/* in i8254 cycles */
% +		else
% +			/*
% +			 * A pitch of 0 should give a period of "infinity"
% +			 * (actually 65535).  But be bug for bug compatible.
% +			 */
% +			period = 0;
%  		duration /= 10;	/* in 10 m sec */
%  	}
% @@ -937,5 +954,5 @@
%  	ioctl(0, CONS_BELLTYPE, &bell);
%  	if ((bell & ~2) == 0)
% -		fprintf(stderr, "[=%d;%dB", pitch, duration);
% +		fprintf(stderr, "[=%d;%dB", period, duration);
%  }
%

Bruce