| Message ID | 20220331000449.41062-1-povik+lin@cutebit.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Apple Macs machine-level ASoC driver | expand |
On Thu, Mar 31, 2022 at 02:04:44AM +0200, Martin Povišer wrote: > I put together a machine-level ASoC driver for recent Apple Macs (the > ones with ARM64 SoCs) and want to gauge opinions. This would be a bit easier to review with a description of the hardware. > Commit 2 adds a new ASoC card method (filter_controls) to let the card > prevent some codec kcontrols from being visible to userspace. For example > the TAS2770 speaker amp driver would be happy to expose TDM slot selection > and ISENSE/VSENSE enables which is ridiculous. I am all ears on how to > make the patch acceptable to upstream. The broad issue here is that what you consider ridiculous someone else might have some bright ideas for configuring dynamically - if things are being exposed for dynamic configuration it's probably because someone wanted them, if the control is genuinely useless then it should just be removed. Rather than getting in the way of people's policy arguments about how to set things we expose them to userspace and let userspace worry about it, usually with the help of UCM files. The general userspace model is that people interact with their sound server more than the hardware card. This is also helpful for people developing use cases, it means they're not having to get the kernel rebuilt to tune things. The TDM swap thing you're mentioning looks like it's a left/right selection which people do use sometimes as a way of doing mono mixes and reorientation. The ISENSE/VSENSE is less obvious, though it's possible there's issues with not having enough slots on a heavily used TDM bus or sometimes disabling the speaker protection processing for whatever reason.
On 31/03/2022 21.34, Mark Brown wrote: > On Thu, Mar 31, 2022 at 02:04:44AM +0200, Martin Povišer wrote: > >> I put together a machine-level ASoC driver for recent Apple Macs (the >> ones with ARM64 SoCs) and want to gauge opinions. > > This would be a bit easier to review with a description of the hardware. > >> Commit 2 adds a new ASoC card method (filter_controls) to let the card >> prevent some codec kcontrols from being visible to userspace. For example >> the TAS2770 speaker amp driver would be happy to expose TDM slot selection >> and ISENSE/VSENSE enables which is ridiculous. I am all ears on how to >> make the patch acceptable to upstream. > > The broad issue here is that what you consider ridiculous someone else > might have some bright ideas for configuring dynamically - if things are > being exposed for dynamic configuration it's probably because someone > wanted them, if the control is genuinely useless then it should just be > removed. Rather than getting in the way of people's policy arguments > about how to set things we expose them to userspace and let userspace > worry about it, usually with the help of UCM files. The general > userspace model is that people interact with their sound server more > than the hardware card. This is also helpful for people developing use > cases, it means they're not having to get the kernel rebuilt to tune > things. The problem with this model is that, in particular in the case of speaker amps, incorrect settings can cause your speakers to blow up. This has been a longstanding problem with ASoC platforms (I should know, I *melted* the speakers in a Chromebook by toggling the wrong alsamixer control once, it even warped the external case, all without making any audible noise). It's the kernel's job to ensure that broadly exposed user controls are safe and cannot be used to cause hardware damage; if that is possible, then that's a kernel security vulnerability worthy of a CVE, in my opinion. I think this idea of exposing what is effectively raw codec chip registers as ALSA controls that is so popular these days was a terrible idea from the start, and only makes some sense within the world of highly integrated vendor-controlled embedded platforms running kiosk-style software with no user control. It is completely unsuitable for a desktop Linux system, since it means users *will* destroy their hardware accidentally. So, some way or another, whatever is exposed has to be sanitized so that it can't go outside the envelope of what is safe for the hardware design. That cannot be known at the level of codec chips and speaker amp chips; it requires platform integration knowledge. That knowledge is what is (intended to be) encoded in the macaudio driver. It's supposed to know how to drive the underlying codec chips and disable access to things that don't make any sense on the platform, and expose controls to the user that are reasonable for what a user would want to do on that specific hardware platform, and no more.
On Thu, Mar 31, 2022 at 03:28:12PM +0200, Martin Povišer wrote: > > On 31. 3. 2022, at 14:34, Mark Brown <broonie@kernel.org> wrote: > > The broad issue here is that what you consider ridiculous someone else > > might have some bright ideas for configuring dynamically - if things are > > being exposed for dynamic configuration it's probably because someone > > wanted them, if the control is genuinely useless then it should just be > Well but these are codec drivers reused on different systems, it can both > be 'not genuinely useless’ on some system and ridiculous to leave open on > the systems I am trying to write drivers for. It wouldn't be the first time that we've had someone turn up with a new idea for how to configure an already existing bit of hardware, part of the reason for this approach is that people do get surprised by user creativity with their systems. > > The TDM swap thing you're mentioning looks like it's a left/right > > selection which people do use sometimes as a way of doing mono mixes and > > reorientation. The ISENSE/VSENSE is less obvious, though it's possible > > there's issues with not having enough slots on a heavily used TDM bus or > > sometimes disabling the speaker protection processing for whatever > > reason. > Not only that. On TAS2770 the default value for ‘ASI1 Sel’ is ‘I2C offset’ > meaning the speaker amp driver ignores my set_tdm_slot calls. If you tell > me it’s okay to change that behaviour and it won’t be considered backwards > compatibility breaking, that would be part of the solution I am seeking > here. Having the default state be muted or not routed is quite common, UCM files or equivalent are typically required for embedded style hardware like this. > But even then, what for example if the system has a single speaker (as it > does on the Mac mini to be covered by this driver) and the I2S bus is left > undriven for the duration of unused TDM slots? That may genuinely pose > a risk of people blowing their speakers by switching something in alsamixer. Right, so that's a more sensible and valid use case. We do have the platform_max feature available for precisely this reason - that's probably more appropriate here since if there's a danger of people blowing their speaker with a floating input they could also blow their speaker with just a very loud audio signal so limiting the volume people can set on the speaker driver seems sensible and would also cover them for misrouting. Whatever the device might pick up from noise on an undriven bus could also be played as audio down the bus. This does become a little fun with speaker protection as we'd want to raise the kernel limit so that userspace can dynamically manage the volume to contorl power (though that might be done with software control), but it's easy enoguh to raise limits later. On the other hand it seems like userspace might reasonably choose to do a mono mix for this output entirely in software, in which case telling the speaker amp to pick up one channel would make sense, or to just play out a stereo signal over I2S and have the amplifier do a mono mix and I'm not seeing why we'd force one or the other in the machine driver. > The ISENSE/VSENSE controls are also actually useless on these systems as we > are not doing anything to pick up the measured values (which are sent back > over the I2S lines). I don’t know if there can be driver conflict between Presumably someone might want to work on figuring that out though, and from a hardware safety point of view it would be better if they did. > two speaker amps trying to drive the I2S lines at the same time should > the user happen to enable SENSE facilities on more than one of them. > Now I can grudgingly study that and rule it out but I would rather hide > the controls altogether. Yes, having two devices driving the bus at the same time wouldn't be great. How is the TDM slot selection for the signals done in the hardware, I'm not seeing anything immediately obvious in the driver? I'd have thought that things would be implemented such that you could implement speaker protection on all speakers simultaneously but perhaps not. > That’s the reasoning anyway. To reiterate, seems to me the controls > are useless/confusing at best and dangerous at worst. I'm just not seeing an issue for the slot selection.
On Thu, Mar 31, 2022 at 10:28:56PM +0900, Hector Martin wrote: > The problem with this model is that, in particular in the case of > speaker amps, incorrect settings can cause your speakers to blow up. > This has been a longstanding problem with ASoC platforms (I should know, > I *melted* the speakers in a Chromebook by toggling the wrong alsamixer > control once, it even warped the external case, all without making any > audible noise). Yes, that's why we have platform_max - it was added for use with Chromebooks originally, someone else had the same idea you did. It's used less often than I'd like since most embedded systems and even things like Chromebooks have a software model where the actual sound card isn't accessible to normal users but that's not the case once you try to run a general purpose distro on there. > kiosk-style software with no user control. It is completely unsuitable > for a desktop Linux system, since it means users *will* destroy their > hardware accidentally. So, some way or another, whatever is exposed has > to be sanitized so that it can't go outside the envelope of what is safe > for the hardware design. That cannot be known at the level of codec > chips and speaker amp chips; it requires platform integration knowledge. Yes, we should be trying to exclude configurations that could be physically destructive but that's not what had been articulated and like I said in reply to his last mail it's really not clear to me that what's being proposed would actually accomplish the intended goal. Targeted restrictions that protect the system are fine and good, random "why would anyone want this?" or "this is how you accomplish use case X" ones are not since we do get users turning up with new ideas. This is one reason why it's important to articulate what the intended goal of changes is, what you've written above is perfectly fine and reasonable but there was nothing about this in the original changelogs, just statements about how silly it would be to configure these controls.
On Thu, Mar 31, 2022 at 05:04:32PM +0200, Martin Povišer wrote: > > On 31. 3. 2022, at 16:18, Mark Brown <broonie@kernel.org> wrote: > > Yes, having two devices driving the bus at the same time wouldn't be > > great. How is the TDM slot selection for the signals done in the > > hardware, I'm not seeing anything immediately obvious in the driver? > > I'd have thought that things would be implemented such that you could > > implement speaker protection on all speakers simultaneously but perhaps > > not. > I don’t know. I would have to go study the details of this. Should I see > if I can find a combination of ‘ASI1 Sel’ ‘VSENSE’ ‘ISENSE’ settings > that would lead to driver conflict on one of the models, or is there > a chance we could hide those controls just on the basis of ‘it doesn’t > do anything usable and is possibly dangerous’? If ISENSE and VSENSE output are controlled by the same mux as routing then we should lock one of the controls out for at least stereo devices (it might be a good idea to check if the output is actually high Z when ISENSE and VSENSE are off rather than just driving zeros, if not it definitely has to be the routing control). My instinct is that it's better to preserve the ability to implement speaker protection in future since that is something that'd be broadly useful, especially if someone comes up with a generic speaker protection implementation in which case there should be an awful lot of systems out there which could benefit. > >> That’s the reasoning anyway. To reiterate, seems to me the controls > >> are useless/confusing at best and dangerous at worst. > > I'm just not seeing an issue for the slot selection. > Yeah, agreed there’s no (damage) issue as we should to proper volume > caps anyway. Though see above about how ISENSE/VSENSE output slot is controlled I guess :/
On Thu, 31 Mar 2022 02:04:44 +0200, Martin Povišer wrote: > I put together a machine-level ASoC driver for recent Apple Macs (the > ones with ARM64 SoCs) and want to gauge opinions. > > Commit 1 is the binding. It is some subset of simple-audio-card with > the extra distinction of allowing multiple CPU/CODEC DAIs per a DAI > link. I want to draw special attention to the issue of describing > speaker topologies. The way it now works is that the driver expects > the speakers to be declared in a fixed order in the sound-dai= list. > This populates a topology the driver expects on a particular machine > model. Mark (in CC) has made the suggestion of keeping the topology > descriptions with the codec nodes themselves in some generic manner, > akin to how sound-name-prefix= already helps identify codecs to the > user. > > [...] Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [4/5] ASoC: Introduce snd_soc_of_get_dai_link_cpus commit: 900dedd7e47cc3f8d93dfa0ae6ac6cf49eda0c97 All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted. You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed. If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced. Please add any relevant lists and maintainers to the CCs when replying to this mail. Thanks, Mark
> On 31. 3. 2022, at 17:36, Mark Brown <broonie@kernel.org> wrote: > > On Thu, Mar 31, 2022 at 05:04:32PM +0200, Martin Povišer wrote: >>> On 31. 3. 2022, at 16:18, Mark Brown <broonie@kernel.org> wrote: > >>> Yes, having two devices driving the bus at the same time wouldn't be >>> great. How is the TDM slot selection for the signals done in the >>> hardware, I'm not seeing anything immediately obvious in the driver? >>> I'd have thought that things would be implemented such that you could >>> implement speaker protection on all speakers simultaneously but perhaps >>> not. > >> I don’t know. I would have to go study the details of this. Should I see >> if I can find a combination of ‘ASI1 Sel’ ‘VSENSE’ ‘ISENSE’ settings >> that would lead to driver conflict on one of the models, or is there >> a chance we could hide those controls just on the basis of ‘it doesn’t >> do anything usable and is possibly dangerous’? > > If ISENSE and VSENSE output are controlled by the same mux as routing > then we should lock one of the controls out for at least stereo devices > (it might be a good idea to check if the output is actually high Z when > ISENSE and VSENSE are off rather than just driving zeros, if not it > definitely has to be the routing control). My instinct is that it's > better to preserve the ability to implement speaker protection in future > since that is something that'd be broadly useful, especially if someone > comes up with a generic speaker protection implementation in which case > there should be an awful lot of systems out there which could benefit. Sorry for having put this on hold for a while. I looked in the TAS2770 and TAS2764 drivers/datasheets, and to answer the questions we had: * VSENSE/ISENSE output slots are configured independently of audio samples routing. Kernel drivers configure the slots based on the 'ti,imon-slot-no' and 'ti,vmon-slot-no' properties of devicetree. * By default codecs transmit Hi-Z for duration of unused slots. So once we supply the devicetree props it should be electrically sound under any configuration of userspace knobs. One final thought on the playback routing controls: On systems with >2 speakers, the codecs need to be assigned slots through set_tdm_slot. The macaudio driver RFCed here assigns a single slot to each speaker, making the effect of each speaker's routing control this: 'I2C offset' -- uses a random slot 'Left' 'Right' 'LeftRight' -- uses the single slot we configured I suppose I better assign two slots to speakers in each left-right pair of the same kind (e.g. woofer 1, woofer 2, tweeter). This way the routing control will mimic its behavior from simple stereo systems but replicated within each left-right pair. (I would prefer to hide the controls altogether, but as I learned that hiding things unless proven dangerous is an ASoC non-goal, this way I can make the controls do something interesting.) Martin
On Fri, Apr 22, 2022 at 12:43:30PM +0200, Martin Povišer wrote: > I looked in the TAS2770 and TAS2764 drivers/datasheets, and to answer > the questions we had: > * VSENSE/ISENSE output slots are configured independently of audio samples > routing. Kernel drivers configure the slots based on the 'ti,imon-slot-no' > and 'ti,vmon-slot-no' properties of devicetree. > * By default codecs transmit Hi-Z for duration of unused slots. > So once we supply the devicetree props it should be electrically sound > under any configuration of userspace knobs. Great, that's a relief. > One final thought on the playback routing controls: On systems with >2 > speakers, the codecs need to be assigned slots through set_tdm_slot. > The macaudio driver RFCed here assigns a single slot to each speaker, > making the effect of each speaker's routing control this: > 'I2C offset' -- uses a random slot > 'Left' 'Right' 'LeftRight' -- uses the single slot we configured > I suppose I better assign two slots to speakers in each left-right pair > of the same kind (e.g. woofer 1, woofer 2, tweeter). This way the > routing control will mimic its behavior from simple stereo systems but > replicated within each left-right pair. (I would prefer to hide the > controls altogether, but as I learned that hiding things unless proven > dangerous is an ASoC non-goal, this way I can make the controls do > something interesting.) I don't quite grasp the difference between the arrangement you're proposing and assigning a single slot to each speaker? Possibly it's just a reordering of the slots?
> On 22. 4. 2022, at 13:19, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Apr 22, 2022 at 12:43:30PM +0200, Martin Povišer wrote: > >> I looked in the TAS2770 and TAS2764 drivers/datasheets, and to answer >> the questions we had: > >> * VSENSE/ISENSE output slots are configured independently of audio samples >> routing. Kernel drivers configure the slots based on the 'ti,imon-slot-no' >> and 'ti,vmon-slot-no' properties of devicetree. > >> * By default codecs transmit Hi-Z for duration of unused slots. > >> So once we supply the devicetree props it should be electrically sound >> under any configuration of userspace knobs. > > Great, that's a relief. > >> One final thought on the playback routing controls: On systems with >2 >> speakers, the codecs need to be assigned slots through set_tdm_slot. >> The macaudio driver RFCed here assigns a single slot to each speaker, >> making the effect of each speaker's routing control this: > >> 'I2C offset' -- uses a random slot > >> 'Left' 'Right' 'LeftRight' -- uses the single slot we configured > >> I suppose I better assign two slots to speakers in each left-right pair >> of the same kind (e.g. woofer 1, woofer 2, tweeter). This way the >> routing control will mimic its behavior from simple stereo systems but >> replicated within each left-right pair. (I would prefer to hide the >> controls altogether, but as I learned that hiding things unless proven >> dangerous is an ASoC non-goal, this way I can make the controls do >> something interesting.) > > I don't quite grasp the difference between the arrangement you're > proposing and assigning a single slot to each speaker? Possibly it's > just a reordering of the slots? Ah, maybe what’s missing is the fact that the way the speaker amp drivers are written, if they are assigned two slots with a call to set_tdm_slot, the first slot is considered 'left' and the second is 'right'. So in the arrangement I am proposing the 'Left', 'Right' and 'LeftRight' values of the routing control have the nominal effect (within the left-right speaker pair), while in the other arrangement it is as I described above.
On Fri, Apr 22, 2022 at 01:28:20PM +0200, Martin Povišer wrote: > > On 22. 4. 2022, at 13:19, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Apr 22, 2022 at 12:43:30PM +0200, Martin Povišer wrote: > >> One final thought on the playback routing controls: On systems with >2 > >> speakers, the codecs need to be assigned slots through set_tdm_slot. > >> The macaudio driver RFCed here assigns a single slot to each speaker, > >> making the effect of each speaker's routing control this: ... > > I don't quite grasp the difference between the arrangement you're > > proposing and assigning a single slot to each speaker? Possibly it's > > just a reordering of the slots? > Ah, maybe what’s missing is the fact that the way the speaker amp drivers > are written, if they are assigned two slots with a call to set_tdm_slot, > the first slot is considered 'left' and the second is 'right'. > So in the arrangement I am proposing the 'Left', 'Right' and 'LeftRight' > values of the routing control have the nominal effect (within the left-right > speaker pair), while in the other arrangement it is as I described above. So previously each speaker would get two slots but now it just gets one?
> On 22. 4. 2022, at 13:33, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Apr 22, 2022 at 01:28:20PM +0200, Martin Povišer wrote: >>> On 22. 4. 2022, at 13:19, Mark Brown <broonie@kernel.org> wrote: >>> On Fri, Apr 22, 2022 at 12:43:30PM +0200, Martin Povišer wrote: > >>>> One final thought on the playback routing controls: On systems with >2 >>>> speakers, the codecs need to be assigned slots through set_tdm_slot. >>>> The macaudio driver RFCed here assigns a single slot to each speaker, >>>> making the effect of each speaker's routing control this: > > ... > >>> I don't quite grasp the difference between the arrangement you're >>> proposing and assigning a single slot to each speaker? Possibly it's >>> just a reordering of the slots? > >> Ah, maybe what’s missing is the fact that the way the speaker amp drivers >> are written, if they are assigned two slots with a call to set_tdm_slot, >> the first slot is considered 'left' and the second is 'right'. > >> So in the arrangement I am proposing the 'Left', 'Right' and 'LeftRight' >> values of the routing control have the nominal effect (within the left-right >> speaker pair), while in the other arrangement it is as I described above. > > So previously each speaker would get two slots but now it just gets one? No the other way around. Previously (with the driver as it is RFCed), each speaker gets a single slot, and 'Left', 'Right' and ‘LeftRight' values of the routing control don't do anything different from each other (well except maybe 'LeftRight' lessens the volume due to how the chip handles the edge case of mixing down two channels from the same slot). With the new arrangement I am proposing, the two speakers in a left-right pair get both the same two slots, meaning they get to choose one of the two slots based on the 'Left' 'Right' value of their routing control.
On Fri, Apr 22, 2022 at 01:44:06PM +0200, Martin Povišer wrote: > > So previously each speaker would get two slots but now it just gets one? > No the other way around. Previously (with the driver as it is RFCed), > each speaker gets a single slot, and 'Left', 'Right' and ‘LeftRight' > values of the routing control don't do anything different from each > other (well except maybe 'LeftRight' lessens the volume due to how > the chip handles the edge case of mixing down two channels from the > same slot). > With the new arrangement I am proposing, the two speakers in a left-right > pair get both the same two slots, meaning they get to choose one of the > two slots based on the 'Left' 'Right' value of their routing control. Ah, I think the confusion here is that I'm using slot and channel interchangably whereas you're saying that previously the driver would allocate two channels to each speaker with duplicate data?
> On 22. 4. 2022, at 14:22, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Apr 22, 2022 at 01:44:06PM +0200, Martin Povišer wrote: > >>> So previously each speaker would get two slots but now it just gets one? > >> No the other way around. Previously (with the driver as it is RFCed), >> each speaker gets a single slot, and 'Left', 'Right' and ‘LeftRight' >> values of the routing control don't do anything different from each >> other (well except maybe 'LeftRight' lessens the volume due to how >> the chip handles the edge case of mixing down two channels from the >> same slot). > >> With the new arrangement I am proposing, the two speakers in a left-right >> pair get both the same two slots, meaning they get to choose one of the >> two slots based on the 'Left' 'Right' value of their routing control. > > Ah, I think the confusion here is that I'm using slot and channel > interchangably whereas you're saying that previously the driver would > allocate two channels to each speaker with duplicate data? I guess you could say that. Not that there’s duplicate data on the I2S bus, but the speaker amp would previously be configured to look for the left and right channel in the same TDM slot (see e.g. set_tdm_slot of tas2770 [0]). (Each speaker amp drives a single speaker, but it still has a notion of left and right channel.) [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/sound/soc/codecs/tas2770.c#n416
On Fri, Apr 22, 2022 at 02:36:03PM +0200, Martin Povišer wrote: > > Ah, I think the confusion here is that I'm using slot and channel > > interchangably whereas you're saying that previously the driver would > > allocate two channels to each speaker with duplicate data? > I guess you could say that. Not that there’s duplicate data on the I2S > bus, but the speaker amp would previously be configured to look for the > left and right channel in the same TDM slot (see e.g. set_tdm_slot of > tas2770 [0]). (Each speaker amp drives a single speaker, but it still > has a notion of left and right channel.) Oh, I see - the speaker actually allows configuration of the slots independently. Usually the left/right thing on mono devices only does something for I2S where the bus clocking enforces that there be both left and right channels. Either configuration is fine by me TBH, if you can do that then you could just keep them mapped to the same channel then mark the control as disabled since it should have no effect.
> On 22. 4. 2022, at 14:44, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Apr 22, 2022 at 02:36:03PM +0200, Martin Povišer wrote: > >>> Ah, I think the confusion here is that I'm using slot and channel >>> interchangably whereas you're saying that previously the driver would >>> allocate two channels to each speaker with duplicate data? > >> I guess you could say that. Not that there’s duplicate data on the I2S >> bus, but the speaker amp would previously be configured to look for the >> left and right channel in the same TDM slot (see e.g. set_tdm_slot of >> tas2770 [0]). (Each speaker amp drives a single speaker, but it still >> has a notion of left and right channel.) > > Oh, I see - the speaker actually allows configuration of the slots > independently. Usually the left/right thing on mono devices only does > something for I2S where the bus clocking enforces that there be both > left and right channels. Either configuration is fine by me TBH, if you > can do that then you could just keep them mapped to the same channel > then mark the control as disabled since it should have no effect. Well but is there some established way to mark a control as disabled? Another issue here is that if I disable it I can’t leave the routing control in it’s default value, which is ‘I2C Offset’ and makes the speaker amp ignore the slot mapping.
On Fri, Apr 22, 2022 at 02:53:54PM +0200, Martin Povišer wrote: > > Oh, I see - the speaker actually allows configuration of the slots > > independently. Usually the left/right thing on mono devices only does > > something for I2S where the bus clocking enforces that there be both > > left and right channels. Either configuration is fine by me TBH, if you > > can do that then you could just keep them mapped to the same channel > > then mark the control as disabled since it should have no effect. > Well but is there some established way to mark a control as disabled? snd_ctl_activate_id(). > Another issue here is that if I disable it I can’t leave the routing > control in it’s default value, which is ‘I2C Offset’ and makes the speaker > amp ignore the slot mapping. Sure, that's fine - if a control genuinely has no effect it's fine to hide it from userspace. The issue is where it's just that you don't see the use, if the control demonstrably does nothing then that's fine.
> On 22. 4. 2022, at 15:06, Mark Brown <broonie@kernel.org> wrote: > > On Fri, Apr 22, 2022 at 02:53:54PM +0200, Martin Povišer wrote: > >>> Oh, I see - the speaker actually allows configuration of the slots >>> independently. Usually the left/right thing on mono devices only does >>> something for I2S where the bus clocking enforces that there be both >>> left and right channels. Either configuration is fine by me TBH, if you >>> can do that then you could just keep them mapped to the same channel >>> then mark the control as disabled since it should have no effect. > >> Well but is there some established way to mark a control as disabled? > > snd_ctl_activate_id(). Ha! Great. >> Another issue here is that if I disable it I can’t leave the routing >> control in it’s default value, which is ‘I2C Offset’ and makes the speaker >> amp ignore the slot mapping. > > Sure, that's fine - if a control genuinely has no effect it's fine to > hide it from userspace. The issue is where it's just that you don't see > the use, if the control demonstrably does nothing then that's fine. So I assume I can set the control from the machine driver then disable it. Anyway, good, this is what I meant earlier when I said the controls I want to hide are 'useless/confusing at best’. I must walk back that they are ‘dangerous at worst’, but I am glad we can hide them anyway. (Not all of them of course, ISENSE/VSENSE will not be hidden, neither the routing control on systems with single mono speaker.)