Repository navigation
Conversation
Currently you can't set bespoke info callback for controls in topology, because the info type is used to deduce the kcontrol type. So make a new op to snd_soc_tplg_ops for asking the bespoke type from lower level and embed this call to new function soc_tplg_get_kcontrol_type, which also cleans up the redundant control type parsing. Signed-off-by: Jaska Uimonen <jaska.uimonen@linux.intel.com>
Add control type ops for sof driver so that asoc can ask the control type defined by bespoke id in topology. Add also info callback for volume and switch. This enables the definition of multichannel controls in topology, so that they will be usable from user space software such as alsamixer. Signed-off-by: Jaska Uimonen <jaska.uimonen@linux.intel.com>
|
This worked perfectly in my test with four DMICs, all exercised individual volume and mute commands worked. Note: The prepared topology file (needs SOF patch) does not load with vanilla topic/sof-dev kernel. |
| return 0; | ||
| } | ||
|
|
||
| int snd_sof_control_type(type) |
There was a problem hiding this comment.
how does this compile? you dont have a datatype for type?
There was a problem hiding this comment.
besides I think the argument should be called info and the return value should be called type?
There was a problem hiding this comment.
whoops...it is compiling as you see from CI results also... but yes don't know how and clearly a mistake, will fix.
There was a problem hiding this comment.
wow, maybe it compiles because the prototype has a type? funny...
| int snd_sof_volume_put(struct snd_kcontrol *kcontrol, | ||
| struct snd_ctl_elem_value *ucontrol); | ||
| int snd_sof_volume_info(struct snd_kcontrol *kcontrol, | ||
| struct snd_ctl_elem_info *uinfo); |
There was a problem hiding this comment.
should we put these to one line as the other ones surrounding are not? it just looks ugly...
|
@juimonen I still think this change is quite restrictive in terms of allowing bespoke kcontrols. After checking for the type, you're still assuming that it is one of mixer,bytes or enum right? I thought you were exploring the other option of expanding the channels to support 8 instead of 2 in the core? |
Right, this feels like 'semi-custom' info. And I don't get if this is in addition to PR #2469 or as an alternative. |
|
@plbossart the difference here to #2469 is that we can choose (and show to the user) from topology that we are using custom info callback and not just setting it behind the curtains. |
@ranj063 sorry I didn't understand this comment? The controls are always mixer, bytes or enum right? Anyway this PR is problematic in that sense, that if I make this custom info change to some topology, and someone uses that new topology with old kernel, the whole topology loading fails... So even I think this is a good thing to do, the "force override" might be just better in this sense. |
thats what I was wondering. are they? wha does bespoke mean then? |
So I see it this way: currently we have a list of 10~ enum definitions for info, like volsw, volsw_sx, range, enum, bytes etc. These are types for 3 different items: mixer, bytes and enum. At least that's how they are categorized in asoc. so for example volsw and range are mixers, just different types. I see info of being bespoke subtype, which is falling into some of these 3 different types/categories. |
| ret = SND_SOC_TPLG_TYPE_ENUM; | ||
| break; | ||
| default: | ||
| ret = -EINVAL; |
There was a problem hiding this comment.
how many lines of code you'd save, if you just did return SND_SOC_TPLG_TYPE_x; and removed all the breaks and the ret variable ;-)
There was a problem hiding this comment.
yeah, sure can do :) just have some vague remembrance in my head for someone complaining about these "excessive" returns... also not sure if needed to add some other logic later, this would be then better. But it can be changed then.
|
Closing this as other solution has been considered better.. |
This is an attempt to add the possibility of bespoke info definition in topology.