PC Beep or PC Speaker or just Beep? and HDA Beep code...

PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

Hi,

 	I'm changing the behaviour of Beep / PC Beep controls for HDA 
drivers (comments?):

http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d

 	I would like to introduce uniform naming for these controls 
to not confuse users.

HDA driver: Beep, PC Beep
AC97: PC Speaker, Beep, PC Beep
etc..

 	I think that the original purpose of PC Speaker is a beep 
generator. Also, many SoC platforms use AC97 codecs, so removing 
abbreviation PC might make sense. It would be probably best to change all 
control names in drivers to 'Beep'. Comments?

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Thu, 22 Oct 2009 17:50:10 +0200 (CEST),
Jaroslav Kysela wrote:
> 
> Hi,
> 
>  	I'm changing the behaviour of Beep / PC Beep controls for HDA 
> drivers (comments?):
> 
> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d

Well, IMHO, the fundamental problem is that there is no logic to
select the beep device in the input layer.  A few line of code to
drivers/char/keyboard.c would do that.

Regarding your change: I'm afraid that too frequent attach/detach
calls aren't good.  Also, note that there are hardwares that still
beep even when the beep volume is set to zero.  So, you may get a 
situation that you can't disable beep tone.

>  	I would like to introduce uniform naming for these controls 
> to not confuse users.
> 
> HDA driver: Beep, PC Beep
> AC97: PC Speaker, Beep, PC Beep
> etc..
> 
>  	I think that the original purpose of PC Speaker is a beep 
> generator. Also, many SoC platforms use AC97 codecs, so removing 
> abbreviation PC might make sense. It would be probably best to change all 
> control names in drivers to 'Beep'. Comments?

Yes, I agree to rename all to Beep.  The word "PC Speaker" has been
always confusing.


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Fri, 30 Oct 2009, Takashi Iwai wrote:

> At Thu, 22 Oct 2009 17:50:10 +0200 (CEST),
> Jaroslav Kysela wrote:
>>
>> Hi,
>>
>>  	I'm changing the behaviour of Beep / PC Beep controls for HDA
>> drivers (comments?):
>>
>> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d
>
> Well, IMHO, the fundamental problem is that there is no logic to
> select the beep device in the input layer.  A few line of code to
> drivers/char/keyboard.c would do that.

Yes, it would be good to have possibility to configure which input device 
will receive beep events.

> Regarding your change: I'm afraid that too frequent attach/detach
> calls aren't good.

What's too frequent? The attach/detach is called only when Beep mute 
control is toggled. I don't think that user will toggle this control as 
more as once in a normal system setup. The default value is "muted" 
(no registration) anyway.

I would really like to have the HDA Beep registration to input layer 
configurable at runtime. The control layer seems good for this job.
I can eventually add one more control with name like "Beep Register 
Switch", but the "Beep Playback Switch" does this job well.

>  Also, note that there are hardwares that still
> beep even when the beep volume is set to zero.  So, you may get a
> situation that you can't disable beep tone.

??? If you don't set HDA codec frequency registers, then the HDA 
digital beep cannot be generated.

>>  	I would like to introduce uniform naming for these controls
>> to not confuse users.
>>
>> HDA driver: Beep, PC Beep
>> AC97: PC Speaker, Beep, PC Beep
>> etc..
>>
>>  	I think that the original purpose of PC Speaker is a beep
>> generator. Also, many SoC platforms use AC97 codecs, so removing
>> abbreviation PC might make sense. It would be probably best to change all
>> control names in drivers to 'Beep'. Comments?
>
> Yes, I agree to rename all to Beep.  The word "PC Speaker" has been
> always confusing.

I'll do this change.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Tue, 3 Nov 2009 12:01:57 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Fri, 30 Oct 2009, Takashi Iwai wrote:
> 
> > At Thu, 22 Oct 2009 17:50:10 +0200 (CEST),
> > Jaroslav Kysela wrote:
> >>
> >> Hi,
> >>
> >>  	I'm changing the behaviour of Beep / PC Beep controls for HDA
> >> drivers (comments?):
> >>
> >> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d
> >
> > Well, IMHO, the fundamental problem is that there is no logic to
> > select the beep device in the input layer.  A few line of code to
> > drivers/char/keyboard.c would do that.
> 
> Yes, it would be good to have possibility to configure which input device 
> will receive beep events.

I had even a patch somewhere... I need to dig down my archive.

> > Regarding your change: I'm afraid that too frequent attach/detach
> > calls aren't good.
> 
> What's too frequent? The attach/detach is called only when Beep mute 
> control is toggled. I don't think that user will toggle this control as 
> more as once in a normal system setup. The default value is "muted" 
> (no registration) anyway.

Heh, you can't expect how odd things users would do.
I can foresee this too frequent toggling can happen simply, e.g. key
auto-repeating on a mute button.

> I would really like to have the HDA Beep registration to input layer 
> configurable at runtime. The control layer seems good for this job.
> I can eventually add one more control with name like "Beep Register 
> Switch", but the "Beep Playback Switch" does this job well.

Well, the biggest problem in your patch is that its re-register the
input device.  When you watch the kernel message, you'll find that the
new input device is added at each time you toggle the beep switch,
  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input6
  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input7
  ...
With frequent toggles, you can reach to a very high number very
easily.

> >  Also, note that there are hardwares that still
> > beep even when the beep volume is set to zero.  So, you may get a
> > situation that you can't disable beep tone.
> 
> ??? If you don't set HDA codec frequency registers, then the HDA 
> digital beep cannot be generated.

It's a scenario like below:
  - "Beep Playback Switch" off -> pcspr is activated; no way to
    control the beep volume -> beep always heard
  - "Beep Playback Switch" on -> HD-audio beep active; but it's still
    audible even when "Beep Playback Volume" is zero -> beep always
    heard

So, in this case, you can't kill beep tones.


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Tue, 3 Nov 2009, Takashi Iwai wrote:

> At Tue, 3 Nov 2009 12:01:57 +0100 (CET),
> Jaroslav Kysela wrote:
>>
>> On Fri, 30 Oct 2009, Takashi Iwai wrote:
>>
>>> At Thu, 22 Oct 2009 17:50:10 +0200 (CEST),
>>> Jaroslav Kysela wrote:
>>>>
>>>> Hi,
>>>>
>>>>  	I'm changing the behaviour of Beep / PC Beep controls for HDA
>>>> drivers (comments?):
>>>>
>>>> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d
>>>
>>> Well, IMHO, the fundamental problem is that there is no logic to
>>> select the beep device in the input layer.  A few line of code to
>>> drivers/char/keyboard.c would do that.
>>
>> Yes, it would be good to have possibility to configure which input device
>> will receive beep events.
>
> I had even a patch somewhere... I need to dig down my archive.
>
>>> Regarding your change: I'm afraid that too frequent attach/detach
>>> calls aren't good.
>>
>> What's too frequent? The attach/detach is called only when Beep mute
>> control is toggled. I don't think that user will toggle this control as
>> more as once in a normal system setup. The default value is "muted"
>> (no registration) anyway.
>
> Heh, you can't expect how odd things users would do.
> I can foresee this too frequent toggling can happen simply, e.g. key
> auto-repeating on a mute button.
>
>> I would really like to have the HDA Beep registration to input layer
>> configurable at runtime. The control layer seems good for this job.
>> I can eventually add one more control with name like "Beep Register
>> Switch", but the "Beep Playback Switch" does this job well.
>
> Well, the biggest problem in your patch is that its re-register the
> input device.  When you watch the kernel message, you'll find that the
> new input device is added at each time you toggle the beep switch,
>  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input6
>  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input7
>  ...
> With frequent toggles, you can reach to a very high number very
> easily.

I don't see a problem. If you find the behaviour too risky, we can limit 
the register/unregister calls in time. Like one register call in one 
second.

>>>  Also, note that there are hardwares that still
>>> beep even when the beep volume is set to zero.  So, you may get a
>>> situation that you can't disable beep tone.
>>
>> ??? If you don't set HDA codec frequency registers, then the HDA
>> digital beep cannot be generated.
>
> It's a scenario like below:
>  - "Beep Playback Switch" off -> pcspr is activated; no way to
>    control the beep volume -> beep always heard

Users can still disable beeps using standard I/O layer (as described in 
patch comment):

The user can easy disable all beeps using 'setterm -blength 0' or 'xset b 
off' command.

>  - "Beep Playback Switch" on -> HD-audio beep active; but it's still
>    audible even when "Beep Playback Volume" is zero -> beep always
>    heard
>
> So, in this case, you can't kill beep tones.

You can.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Tue, 3 Nov 2009 12:43:17 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Tue, 3 Nov 2009, Takashi Iwai wrote:
> 
> > At Tue, 3 Nov 2009 12:01:57 +0100 (CET),
> > Jaroslav Kysela wrote:
> >>
> >> On Fri, 30 Oct 2009, Takashi Iwai wrote:
> >>
> >>> At Thu, 22 Oct 2009 17:50:10 +0200 (CEST),
> >>> Jaroslav Kysela wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>>  	I'm changing the behaviour of Beep / PC Beep controls for HDA
> >>>> drivers (comments?):
> >>>>
> >>>> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d
> >>>
> >>> Well, IMHO, the fundamental problem is that there is no logic to
> >>> select the beep device in the input layer.  A few line of code to
> >>> drivers/char/keyboard.c would do that.
> >>
> >> Yes, it would be good to have possibility to configure which input device
> >> will receive beep events.
> >
> > I had even a patch somewhere... I need to dig down my archive.
> >
> >>> Regarding your change: I'm afraid that too frequent attach/detach
> >>> calls aren't good.
> >>
> >> What's too frequent? The attach/detach is called only when Beep mute
> >> control is toggled. I don't think that user will toggle this control as
> >> more as once in a normal system setup. The default value is "muted"
> >> (no registration) anyway.
> >
> > Heh, you can't expect how odd things users would do.
> > I can foresee this too frequent toggling can happen simply, e.g. key
> > auto-repeating on a mute button.
> >
> >> I would really like to have the HDA Beep registration to input layer
> >> configurable at runtime. The control layer seems good for this job.
> >> I can eventually add one more control with name like "Beep Register
> >> Switch", but the "Beep Playback Switch" does this job well.
> >
> > Well, the biggest problem in your patch is that its re-register the
> > input device.  When you watch the kernel message, you'll find that the
> > new input device is added at each time you toggle the beep switch,
> >  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input6
> >  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input7
> >  ...
> > With frequent toggles, you can reach to a very high number very
> > easily.
> 
> I don't see a problem. If you find the behaviour too risky, we can limit 
> the register/unregister calls in time. Like one register call in one 
> second.

Oh no, that'd be very strange behavior as an mixer element.


> >>>  Also, note that there are hardwares that still
> >>> beep even when the beep volume is set to zero.  So, you may get a
> >>> situation that you can't disable beep tone.
> >>
> >> ??? If you don't set HDA codec frequency registers, then the HDA
> >> digital beep cannot be generated.
> >
> > It's a scenario like below:
> >  - "Beep Playback Switch" off -> pcspr is activated; no way to
> >    control the beep volume -> beep always heard
> 
> Users can still disable beeps using standard I/O layer (as described in 
> patch comment):
> 
> The user can easy disable all beeps using 'setterm -blength 0' or 'xset b 
> off' command.

OK, setterm seems working.

I thought there is an escape sequence to change the beep length, but
maybe not.  I remember the esc sequence to change the beep frequency,
though...


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Tue, 3 Nov 2009, Takashi Iwai wrote:

> At Tue, 3 Nov 2009 12:43:17 +0100 (CET),
> Jaroslav Kysela wrote:
>>
>> On Tue, 3 Nov 2009, Takashi Iwai wrote:
>>
>>> At Tue, 3 Nov 2009 12:01:57 +0100 (CET),
>>> Jaroslav Kysela wrote:
>>>>
>>>> On Fri, 30 Oct 2009, Takashi Iwai wrote:
>>>>
>>>>> At Thu, 22 Oct 2009 17:50:10 +0200 (CEST),
>>>>> Jaroslav Kysela wrote:
>>>>>>
>>>>>> Hi,
>>>>>>
>>>>>>  	I'm changing the behaviour of Beep / PC Beep controls for HDA
>>>>>> drivers (comments?):
>>>>>>
>>>>>> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d605dbeed1b102136513e09e95217e8dc19c800d
>>>>>
>>>>> Well, IMHO, the fundamental problem is that there is no logic to
>>>>> select the beep device in the input layer.  A few line of code to
>>>>> drivers/char/keyboard.c would do that.
>>>>
>>>> Yes, it would be good to have possibility to configure which input device
>>>> will receive beep events.
>>>
>>> I had even a patch somewhere... I need to dig down my archive.
>>>
>>>>> Regarding your change: I'm afraid that too frequent attach/detach
>>>>> calls aren't good.
>>>>
>>>> What's too frequent? The attach/detach is called only when Beep mute
>>>> control is toggled. I don't think that user will toggle this control as
>>>> more as once in a normal system setup. The default value is "muted"
>>>> (no registration) anyway.
>>>
>>> Heh, you can't expect how odd things users would do.
>>> I can foresee this too frequent toggling can happen simply, e.g. key
>>> auto-repeating on a mute button.
>>>
>>>> I would really like to have the HDA Beep registration to input layer
>>>> configurable at runtime. The control layer seems good for this job.
>>>> I can eventually add one more control with name like "Beep Register
>>>> Switch", but the "Beep Playback Switch" does this job well.
>>>
>>> Well, the biggest problem in your patch is that its re-register the
>>> input device.  When you watch the kernel message, you'll find that the
>>> new input device is added at each time you toggle the beep switch,
>>>  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input6
>>>  input: HDA Digital PCBeep as /devices/pci0000:00/0000:00:1b.0/input/input7
>>>  ...
>>> With frequent toggles, you can reach to a very high number very
>>> easily.
>>
>> I don't see a problem. If you find the behaviour too risky, we can limit
>> the register/unregister calls in time. Like one register call in one
>> second.
>
> Oh no, that'd be very strange behavior as an mixer element.

I recoded patch to delay the detach only to get consistent and abuse 
prone behaviour:

http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=ba9c08c61338f298df34445715287beeca94b024
http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=0633c8e977b7708ea122392d87cc15ca448fc5d5

The HDA beep output is muted immediately, of course.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Fri, 30 Oct 2009, Takashi Iwai wrote:

>>  	I think that the original purpose of PC Speaker is a beep
>> generator. Also, many SoC platforms use AC97 codecs, so removing
>> abbreviation PC might make sense. It would be probably best to change all
>> control names in drivers to 'Beep'. Comments?
>
> Yes, I agree to rename all to Beep.  The word "PC Speaker" has been
> always confusing.

Patch is here:

http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d2f1919956dbabd17afbc3a236b758f49f6c0658

I only don't know details about MAC (ppc) drivers. The "PC Speaker" 
controls are for beep generator or for full featured build-in internal 
speakers? I would propose to use "Internal Speaker" name for full featured 
outputs.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Tue, 3 Nov 2009 15:55:23 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Fri, 30 Oct 2009, Takashi Iwai wrote:
> 
> >>  	I think that the original purpose of PC Speaker is a beep
> >> generator. Also, many SoC platforms use AC97 codecs, so removing
> >> abbreviation PC might make sense. It would be probably best to change all
> >> control names in drivers to 'Beep'. Comments?
> >
> > Yes, I agree to rename all to Beep.  The word "PC Speaker" has been
> > always confusing.
> 
> Patch is here:
> 
> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=d2f1919956dbabd17afbc3a236b758f49f6c0658
> 
> I only don't know details about MAC (ppc) drivers. The "PC Speaker" 
> controls are for beep generator or for full featured build-in internal 
> speakers? I would propose to use "Internal Speaker" name for full featured 
> outputs.

So far, most of HD-audio driver uses "Speaker Playback Volume" since
"Internal Speaker" is too long and can't be shown well in alsamixer,
for example.


Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Tue, 3 Nov 2009, Jaroslav Kysela wrote:

>>> I don't see a problem. If you find the behaviour too risky, we can limit
>>> the register/unregister calls in time. Like one register call in one
>>> second.
>> 
>> Oh no, that'd be very strange behavior as an mixer element.
>
> I recoded patch to delay the detach only to get consistent and abuse prone 
> behaviour:
>
> http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=ba9c08c61338f298df34445715287beeca94b024
> http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=0633c8e977b7708ea122392d87cc15ca448fc5d5
>
> The HDA beep output is muted immediately, of course.

Any chance to get my HDA beep updates merged to linux-next tree?

[ALSA] hda_intel: Digital PC Beep - change behaviour for input layer
[ALSA] hda_intel: Digital PC Beep - delay input device unregistration
[ALSA] hda: beep - add missing cancel_delayed_work

Reasons for merge:

- it's tested
- it's compatible with all kernels (including ones without
   the future input layer modifications)
- the code is more structured which can help us to rebind
   "on request" input registration to another logic later
- Beep mute switch now disables the tone generator on the off request -
   it might be considered as bug in the previous beep code

 						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Thu, 12 Nov 2009 11:42:24 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Tue, 3 Nov 2009, Jaroslav Kysela wrote:
> 
> >>> I don't see a problem. If you find the behaviour too risky, we can limit
> >>> the register/unregister calls in time. Like one register call in one
> >>> second.
> >> 
> >> Oh no, that'd be very strange behavior as an mixer element.
> >
> > I recoded patch to delay the detach only to get consistent and abuse prone 
> > behaviour:
> >
> > http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=ba9c08c61338f298df34445715287beeca94b024
> > http://git.alsa-project.org/?p=alsa-kernel.git;a=commit;h=0633c8e977b7708ea122392d87cc15ca448fc5d5
> >
> > The HDA beep output is muted immediately, of course.
> 
> Any chance to get my HDA beep updates merged to linux-next tree?
> 
> [ALSA] hda_intel: Digital PC Beep - change behaviour for input layer
> [ALSA] hda_intel: Digital PC Beep - delay input device unregistration
> [ALSA] hda: beep - add missing cancel_delayed_work
> 
> Reasons for merge:
> 
> - it's tested
> - it's compatible with all kernels (including ones without
>    the future input layer modifications)
> - the code is more structured which can help us to rebind
>    "on request" input registration to another logic later
> - Beep mute switch now disables the tone generator on the off request -
>    it might be considered as bug in the previous beep code

Well, as mentioned, the only thing I'm really concerned is that the
registration/free is done so easily via a mixer switch.  A mixer
switch is very often and carelessly changed by a joe user,
intentionally or unintentionally.  Thus, doing registration/free that
can involve with the the other layer in that level is somewhat weird.

IOW, I'm fine with an additional implementation for the dynamic beep
on/off.  But, the mixer interface doesn't look like the best interface
to me.


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Thu, 12 Nov 2009, Takashi Iwai wrote:

> IOW, I'm fine with an additional implementation for the dynamic beep
> on/off.  But, the mixer interface doesn't look like the best interface
> to me.

I would prefer something which is comfortable for user. The module 
parameter or sysfs file is not a good way to explain users how they can 
change the beep behaviour.

We have basically four ways:

1) module parameter (difficult tracking of changes through sysfs)
2) sysfs file
3) current implementation
4) create new control with different name (like "HDA Beep Enable Switch")

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> 
> > IOW, I'm fine with an additional implementation for the dynamic beep
> > on/off.  But, the mixer interface doesn't look like the best interface
> > to me.
> 
> I would prefer something which is comfortable for user. The module 
> parameter or sysfs file is not a good way to explain users how they can 
> change the beep behaviour.

A module parameter would be easy enough for most users who really care
things like beep :)

> We have basically four ways:
> 
> 1) module parameter (difficult tracking of changes through sysfs)
> 2) sysfs file
> 3) current implementation
> 4) create new control with different name (like "HDA Beep Enable Switch")

3 and 4 have the same concern, so I'd vote for 1 or 2.


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Thu, 12 Nov 2009, Takashi Iwai wrote:

> At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
> Jaroslav Kysela wrote:
>>
>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
>>
>>> IOW, I'm fine with an additional implementation for the dynamic beep
>>> on/off.  But, the mixer interface doesn't look like the best interface
>>> to me.
>>
>> I would prefer something which is comfortable for user. The module
>> parameter or sysfs file is not a good way to explain users how they can
>> change the beep behaviour.
>
> A module parameter would be easy enough for most users who really care
> things like beep :)

Nope. You expect that all users knows how to change module parameters. I'm 
sorry, but I'm lazy to explain in bugzilla many times, how to change the 
kernel module parameter (it's enough to explain model= for HDA). But 'turn 
off "Beep" in alsamixer or any ALSA-aware mixer app' is quite shorther and 
intuitive.

>> We have basically four ways:
>>
>> 1) module parameter (difficult tracking of changes through sysfs)
>> 2) sysfs file
>> 3) current implementation
>> 4) create new control with different name (like "HDA Beep Enable Switch")
>
> 3 and 4 have the same concern, so I'd vote for 1 or 2.

For hackers, of course :-) But it might not be preferred way for all 
users.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Thu, 12 Nov 2009 12:24:27 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> 
> > At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
> > Jaroslav Kysela wrote:
> >>
> >> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> >>
> >>> IOW, I'm fine with an additional implementation for the dynamic beep
> >>> on/off.  But, the mixer interface doesn't look like the best interface
> >>> to me.
> >>
> >> I would prefer something which is comfortable for user. The module
> >> parameter or sysfs file is not a good way to explain users how they can
> >> change the beep behaviour.
> >
> > A module parameter would be easy enough for most users who really care
> > things like beep :)
> 
> Nope. You expect that all users knows how to change module parameters. I'm 
> sorry, but I'm lazy to explain in bugzilla many times, how to change the 
> kernel module parameter (it's enough to explain model= for HDA). But 'turn 
> off "Beep" in alsamixer or any ALSA-aware mixer app' is quite shorther and 
> intuitive.

But because of this "easy-to-use", the switch can be soo easily
changed unintentionally.  So, we are speaking roughly the same thing
of both good and bad sides.

And here I'm more concerned about the bad side than the good side.


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Thu, 12 Nov 2009, Takashi Iwai wrote:

> At Thu, 12 Nov 2009 12:24:27 +0100 (CET),
> Jaroslav Kysela wrote:
>>
>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
>>
>>> At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
>>> Jaroslav Kysela wrote:
>>>>
>>>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
>>>>
>>>>> IOW, I'm fine with an additional implementation for the dynamic beep
>>>>> on/off.  But, the mixer interface doesn't look like the best interface
>>>>> to me.
>>>>
>>>> I would prefer something which is comfortable for user. The module
>>>> parameter or sysfs file is not a good way to explain users how they can
>>>> change the beep behaviour.
>>>
>>> A module parameter would be easy enough for most users who really care
>>> things like beep :)
>>
>> Nope. You expect that all users knows how to change module parameters. I'm
>> sorry, but I'm lazy to explain in bugzilla many times, how to change the
>> kernel module parameter (it's enough to explain model= for HDA). But 'turn
>> off "Beep" in alsamixer or any ALSA-aware mixer app' is quite shorther and
>> intuitive.
>
> But because of this "easy-to-use", the switch can be soo easily
> changed unintentionally.  So, we are speaking roughly the same thing
> of both good and bad sides.
>
> And here I'm more concerned about the bad side than the good side.

Ok, take another look:

If you have an USB input device (mouse for example), how you can force the 
user to not plug in / plug out the device to USB port? With my latest 
change postponing the input device unregistration, the code is quite 
robust and even keyboard repeat should not make much hassle, because the 
input device will not be quickly unregistered in this case.

I'm sorry, but we cannot take responsibility about this. If our code does 
not crash and works as expected, everything is ok.

 						Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Thu, 12 Nov 2009 12:44:17 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> 
> > At Thu, 12 Nov 2009 12:24:27 +0100 (CET),
> > Jaroslav Kysela wrote:
> >>
> >> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> >>
> >>> At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
> >>> Jaroslav Kysela wrote:
> >>>>
> >>>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> >>>>
> >>>>> IOW, I'm fine with an additional implementation for the dynamic beep
> >>>>> on/off.  But, the mixer interface doesn't look like the best interface
> >>>>> to me.
> >>>>
> >>>> I would prefer something which is comfortable for user. The module
> >>>> parameter or sysfs file is not a good way to explain users how they can
> >>>> change the beep behaviour.
> >>>
> >>> A module parameter would be easy enough for most users who really care
> >>> things like beep :)
> >>
> >> Nope. You expect that all users knows how to change module parameters. I'm
> >> sorry, but I'm lazy to explain in bugzilla many times, how to change the
> >> kernel module parameter (it's enough to explain model= for HDA). But 'turn
> >> off "Beep" in alsamixer or any ALSA-aware mixer app' is quite shorther and
> >> intuitive.
> >
> > But because of this "easy-to-use", the switch can be soo easily
> > changed unintentionally.  So, we are speaking roughly the same thing
> > of both good and bad sides.
> >
> > And here I'm more concerned about the bad side than the good side.
> 
> Ok, take another look:
> 
> If you have an USB input device (mouse for example), how you can force the 
> user to not plug in / plug out the device to USB port?

This is a different issue.  It's the physical connection, and the system
explicitly allows the hotplugging.

Or, a different aspect: suppose you don't want to use USB mouse but
only PS/2 mouse.  How would you suggest?  You blame kernel or X that
it doesn't provide the one-click solution to suppress the USB mouse?

> With my latest 
> change postponing the input device unregistration, the code is quite 
> robust and even keyboard repeat should not make much hassle, because the 
> input device will not be quickly unregistered in this case.

I see it but still I'm not convinced by this dynamic registration.
The input layer registration is escalated to sysfs, which is parsed by
udev, whatever...  It's no more only the thing inside the kernel.


thanks,

Takashi

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Jaroslav Kysela]

On Thu, 12 Nov 2009, Takashi Iwai wrote:

> At Thu, 12 Nov 2009 12:44:17 +0100 (CET),
> Jaroslav Kysela wrote:
>>
>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
>>
>>> At Thu, 12 Nov 2009 12:24:27 +0100 (CET),
>>> Jaroslav Kysela wrote:
>>>>
>>>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
>>>>
>>>>> At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
>>>>> Jaroslav Kysela wrote:
>>>>>>
>>>>>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
>>>>>>
>>>>>>> IOW, I'm fine with an additional implementation for the dynamic beep
>>>>>>> on/off.  But, the mixer interface doesn't look like the best interface
>>>>>>> to me.
>>>>>>
>>>>>> I would prefer something which is comfortable for user. The module
>>>>>> parameter or sysfs file is not a good way to explain users how they can
>>>>>> change the beep behaviour.
>>>>>
>>>>> A module parameter would be easy enough for most users who really care
>>>>> things like beep :)
>>>>
>>>> Nope. You expect that all users knows how to change module parameters. I'm
>>>> sorry, but I'm lazy to explain in bugzilla many times, how to change the
>>>> kernel module parameter (it's enough to explain model= for HDA). But 'turn
>>>> off "Beep" in alsamixer or any ALSA-aware mixer app' is quite shorther and
>>>> intuitive.
>>>
>>> But because of this "easy-to-use", the switch can be soo easily
>>> changed unintentionally.  So, we are speaking roughly the same thing
>>> of both good and bad sides.
>>>
>>> And here I'm more concerned about the bad side than the good side.
>>
>> Ok, take another look:
>>
>> If you have an USB input device (mouse for example), how you can force the
>> user to not plug in / plug out the device to USB port?
>
> This is a different issue.  It's the physical connection, and the system
> explicitly allows the hotplugging.
>
> Or, a different aspect: suppose you don't want to use USB mouse but
> only PS/2 mouse.  How would you suggest?  You blame kernel or X that
> it doesn't provide the one-click solution to suppress the USB mouse?
>
>> With my latest
>> change postponing the input device unregistration, the code is quite
>> robust and even keyboard repeat should not make much hassle, because the
>> input device will not be quickly unregistered in this case.
>
> I see it but still I'm not convinced by this dynamic registration.
> The input layer registration is escalated to sysfs, which is parsed by
> udev, whatever...  It's no more only the thing inside the kernel.

Almost all users won't touch the control switch after initial setup.

I added module parameter beep_mode to snd-hda-intel which allows to 
specify all three modes (always unregistered, always registered, dynamic).
Default is "always registered" as in previous implementation.

ALSA: hda - add beep_mode module parameter
http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=cffb566ba294a2593f932b7b7b12fb386ac161f4

It's a compromise to allow selection of accepted behaviour from user.

other HDA beep patches:

[ALSA] hda_intel: Digital PC Beep - change behaviour for input layer
[ALSA] hda_intel: Digital PC Beep - delay input device unregistration
[ALSA] hda: beep - add missing cancel_delayed_work

Please, add these four patches to your tree.

 					Jaroslav

-----
Jaroslav Kysela <perex@perex.cz>
Linux Kernel Sound Maintainer
ALSA Project, Red Hat, Inc.

Re: PC Beep or PC Speaker or just Beep? and HDA Beep code...

[Takashi Iwai]

At Mon, 16 Nov 2009 10:20:08 +0100 (CET),
Jaroslav Kysela wrote:
> 
> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> 
> > At Thu, 12 Nov 2009 12:44:17 +0100 (CET),
> > Jaroslav Kysela wrote:
> >>
> >> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> >>
> >>> At Thu, 12 Nov 2009 12:24:27 +0100 (CET),
> >>> Jaroslav Kysela wrote:
> >>>>
> >>>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> >>>>
> >>>>> At Thu, 12 Nov 2009 12:06:57 +0100 (CET),
> >>>>> Jaroslav Kysela wrote:
> >>>>>>
> >>>>>> On Thu, 12 Nov 2009, Takashi Iwai wrote:
> >>>>>>
> >>>>>>> IOW, I'm fine with an additional implementation for the dynamic beep
> >>>>>>> on/off.  But, the mixer interface doesn't look like the best interface
> >>>>>>> to me.
> >>>>>>
> >>>>>> I would prefer something which is comfortable for user. The module
> >>>>>> parameter or sysfs file is not a good way to explain users how they can
> >>>>>> change the beep behaviour.
> >>>>>
> >>>>> A module parameter would be easy enough for most users who really care
> >>>>> things like beep :)
> >>>>
> >>>> Nope. You expect that all users knows how to change module parameters. I'm
> >>>> sorry, but I'm lazy to explain in bugzilla many times, how to change the
> >>>> kernel module parameter (it's enough to explain model= for HDA). But 'turn
> >>>> off "Beep" in alsamixer or any ALSA-aware mixer app' is quite shorther and
> >>>> intuitive.
> >>>
> >>> But because of this "easy-to-use", the switch can be soo easily
> >>> changed unintentionally.  So, we are speaking roughly the same thing
> >>> of both good and bad sides.
> >>>
> >>> And here I'm more concerned about the bad side than the good side.
> >>
> >> Ok, take another look:
> >>
> >> If you have an USB input device (mouse for example), how you can force the
> >> user to not plug in / plug out the device to USB port?
> >
> > This is a different issue.  It's the physical connection, and the system
> > explicitly allows the hotplugging.
> >
> > Or, a different aspect: suppose you don't want to use USB mouse but
> > only PS/2 mouse.  How would you suggest?  You blame kernel or X that
> > it doesn't provide the one-click solution to suppress the USB mouse?
> >
> >> With my latest
> >> change postponing the input device unregistration, the code is quite
> >> robust and even keyboard repeat should not make much hassle, because the
> >> input device will not be quickly unregistered in this case.
> >
> > I see it but still I'm not convinced by this dynamic registration.
> > The input layer registration is escalated to sysfs, which is parsed by
> > udev, whatever...  It's no more only the thing inside the kernel.
> 
> Almost all users won't touch the control switch after initial setup.

But, the problem is that it can happen even unintentionally so easily.
Nevertheless...

> I added module parameter beep_mode to snd-hda-intel which allows to 
> specify all three modes (always unregistered, always registered, dynamic).
> Default is "always registered" as in previous implementation.
> 
> ALSA: hda - add beep_mode module parameter
> http://git.alsa-project.org/?p=alsa-kernel.git;a=commitdiff;h=cffb566ba294a2593f932b7b7b12fb386ac161f4
> 
> It's a compromise to allow selection of accepted behaviour from user.
> 
> other HDA beep patches:
> 
> [ALSA] hda_intel: Digital PC Beep - change behaviour for input layer
> [ALSA] hda_intel: Digital PC Beep - delay input device unregistration
> [ALSA] hda: beep - add missing cancel_delayed_work
> 
> Please, add these four patches to your tree.

OK, looks like a good compromise.  I applied all patches, and added a
description to ALSA-Configuration.txt.

Thanks!


Takashi