On Tue, 01 Aug 2023 13:38:54 +0200, Marek Vasut wrote: > > On 8/1/23 08:11, Takashi Iwai wrote: > > On Tue, 01 Aug 2023 04:56:09 +0200, > > Jeff LaBundy wrote: > >> > >> Hi all, > >> > >> On Mon, Jul 31, 2023 at 07:49:50PM +0200, Marek Vasut wrote: > >>> On 7/31/23 18:24, Dmitry Torokhov wrote: > >>>> On Mon, Jul 31, 2023 at 04:36:01PM +0200, Marek Vasut wrote: > >>>>> On 7/31/23 16:20, Takashi Iwai wrote: > >>>>> > >>>>> [...] > >>>>> > >>>>>>>>> Uh, I don't need a full sound device to emit beeps, that's not even > >>>>>>>>> possible with this hardware. > >>>>>>>> > >>>>>>>> Heh, I also don't recommend that route, either :) > >>>>>>>> (Though, it must be possible to create a sound device with that beep > >>>>>>>> control in theory) > >>>>>>> > >>>>>>> I mean, I can imagine one could possibly use PCM DMA to cook samples > >>>>>>> to feed some of the PWM devices so they could possibly be used to > >>>>>>> generate low quality audio, as a weird limited DAC, but ... that's not > >>>>>>> really generic, and not what I want. > >>>>>> > >>>>>> Oh I see how the misunderstanding came; I didn't mean the PCM > >>>>>> implementation like pcsp driver. The pcsp driver is a real hack and > >>>>>> it's there just for fun, not for any real practical use. > >>>>> > >>>>> Ah :) > >>>>> > >>>>>> What I meant was rather that you can create a sound device containing > >>>>>> a mixer volume control that serves exactly like the sysfs or whatever > >>>>>> other interface, without any PCM stream or other interface. > >>>>> > >>>>> Ahhh, hum, I still feel like this might be a bit overkill here. > >>>>> > >>>>>>>>> I only need to control loudness of the > >>>>>>>>> beeper that is controlled by PWM output. That's why I am trying to > >>>>>>>>> extend the pwm-beeper driver, which seems the best fit for such a > >>>>>>>>> device, it is only missing this one feature (loudness control). > >>>>>>>> > >>>>>>>> So the question is what's expected from user-space POV. If a more > >>>>>>>> generic control of beep volume is required, e.g. for desktop-like > >>>>>>>> usages, an implementation of sound driver wouldn't be too bad. > >>>>>>>> OTOH, for other specific use-cases, it doesn't matter much in which > >>>>>>>> interface it's implemented, and sysfs could be an easy choice. > >>>>>>> > >>>>>>> The whole discussion above has been exactly about this. Basically the > >>>>>>> thing is, we can either have: > >>>>>>> - SND_TONE (via some /dev/input/eventX) + sysfs volume control > >>>>>>> -> This is simple, but sounds racy between input and sysfs accesses > >>>>>> > >>>>>> Hmm, how can it be racy if you do proper locking? > >>>>> > >>>>> I can imagine two applications can each grab one of the controls and that > >>>>> makes the interface a bit not nice. That would require extra synchronization > >>>>> in userspace and so on. > >>>>> > >>>>>>> - SND_TONE + SND_TONE_SET_VOLUME > >>>>>>> -> User needs to do two ioctls, hum > >>>>>>> - some new SND_TONE_WITH_VOLUME > >>>>>>> -> Probably the best option, user sets both tone frequency and volume > >>>>>>> in one go, and it also only extends the IOCTL interface, so older > >>>>>>> userspace won't have issues > >>>>>> > >>>>>> Those are "extensions" I have mentioned, and I'm not a big fan for > >>>>>> that, honestly speaking. > >>>>>> > >>>>>> The fact that the beep *output* stuff is provided by the *input* > >>>>>> device is already confusing > >>>>> > >>>>> I agree, this confused me as well. > >>>> > >>>> This comes from the times when keyboards themselves were capable of > >>>> emitting bells (SUN, DEC, etc). In hindsight it was not the best way of > >>>> structuring things, same with the keyboard LEDs (that are now plugged > >>>> into the LED subsystem, but still allow be driven through input). > >>>> > >>>> And in the same vein I wonder if we should bite the bullet and pay with > >>>> a bit of complexity but move sound-related things to sound subsystem. > >>> > >>> I am not sure that's the right approach here, since the device cannot do PCM > >>> playback, just bleeps. > >>> > >>>>>> (it was so just because of historical > >>>>>> reason), and yet you start implementing more full-featured mixer > >>>>>> control. I'd rather keep fingers away. > >>>>>> > >>>>>> Again, if user-space requires the compatible behavior like the > >>>>>> existing desktop usages > >>>>> > >>>>> It does not. These pwm-beeper devices keep showing up in various embedded > >>>>> devices these days. > >>>>> > >>>>>> , it can be implemented in a similar way like > >>>>>> the existing ones; i.e. provide a mixer control with a proper sound > >>>>>> device. The sound device doesn't need to provide a PCM interface but > >>>>>> just with a mixer interface. > >>>>>> > >>>>>> Or, if the purpose of your target device is a special usage, you don't > >>>>>> need to consider too much about the existing interface, and try to > >>>>>> keep the change as minimal as possible without too intrusive API > >>>>>> changes. > >>>>> > >>>>> My use case is almost perfectly matched by the current input pwm-beeper > >>>>> driver, the only missing bit is the ability to control the loudness at > >>>>> runtime. I think adding the SND_TONE_WITH_VOLUME parameter would cover it, > >>>>> with least intrusive API changes. > >>>>> > >>>>> The SND_TONE already supports configuring tone frequency in Hz as its > >>>>> parameter. Since anything above 64 kHz is certainly not hearable by humans, > >>>>> I would say the SND_TONE_WITH_VOLUME could use 16 LSbits for frequency (so > >>>>> up to 65535 Hz , 0 is OFF), and 16 MSbits for volume . > >>>>> > >>>>> I'm hesitant to overcomplicate something which can currently be controlled > >>>>> via single ioctl by pulling in sound subsystem into the picture. > >>>> > >>>> Can you tell a bit more about your use case? What needs to control the > >>>> volume of beeps? Is this the only source of sounds on the system? > >>> > >>> Custom user space application. The entire userspace is custom built in this > >>> case. > >>> > >>> In this case, it is a single-use device (think e.g. the kind of thermometer > >>> you stick in your ear when you're ill, to find out how warm you are). > >>> > >>> The beeper there is used to do just that, bleep (with different frequencies > >>> to indicate different stuff), and that works. What I need in addition to > >>> that is control the volume of the bleeps from the application, so it isn't > >>> too noisy. And that needs to be user-controllable at runtime, so not > >>> something that goes in DT. > >>> > >>> Right now there is just the bleeper , yes. > >> > >> It sounds like we essentially need an option within pcsp to drive PWM > >> instead of PCM, but input already has pwm-beeper; it seems harmless to > >> gently extend the latter for this use-case as opposed to reworking the > >> former. > > > > Nah, please forget pcsp driver. As mentioned earlier, it's a driver > > that is present just for fun. > > > > I believe what we need is a simple sound card instance providing a > > mixer control for the beep volume, something like a patch like below > > (totally untested!) > > Do we really want to add dependency on the entire sound subsystem > (which is currently not needed on the device I care about) only to > configure one single tunable of the PWM beeper ? It seems to add too > much bloat to me. That really depends on the use case. If the driver is supposed to be used generically as seen in the desktop scenes, it's worth to have a support of the standard interface like others. OTOH, if the driver is for limited situations and better to be as slim as possible, a tailored interface like sysfs would be the way to go. My proposal was under assumption of the former -- a generic usage. If the latter scene is expected, a sysfs implementation can be the right way, IMO. OTOH, we really need to be careful about the blind extension of API. Although adding a new input event type sounds easy, the influence could be much more than seen there... Takashi
