| Message ID | 20210108112355.2053917-2-jaska.uimonen@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Enable using multiple kcontrol types in a widget | expand |
Dne 08. 01. 21 v 12:23 Jaska Uimonen napsal(a): > Current kcontrol structs don't have a member to describe the control > type. The type is present in the widget which contains the control. As > there can be many controls in one widget it is inherently presumed that > the control types are the same. > > Lately there has been use cases where different types of controls would > be needed for single widget. Thus enable this by adding the control type > to kcontrol and kcontrol_new structs. It looks like a SoC only extension. Use private_data to carry this information. It has no value for the toplevel code. Jaroslav
Hi, On Fri, Jan 08, 2021 at 12:40:28PM +0100, Jaroslav Kysela wrote: > Dne 08. 01. 21 v 12:23 Jaska Uimonen napsal(a): > > Current kcontrol structs don't have a member to describe the control > > type. The type is present in the widget which contains the control. As > > there can be many controls in one widget it is inherently presumed that > > the control types are the same. > > > > Lately there has been use cases where different types of controls would > > be needed for single widget. Thus enable this by adding the control type > > to kcontrol and kcontrol_new structs. > > It looks like a SoC only extension. Use private_data to carry this > information. It has no value for the toplevel code. > > Jaroslav In current design of ALSA control core, the type of control element is firstly determined by driver in callback of snd_kcontrol.info(). The callback is done when userspace applications call ioctl(2) with SNDRV_CTL_IOCTL_ELEM_INFO request. The patch doesn't touch to the above processing. It means that the type information is just for kernel-land implementation and is not exposed to userspace application. Essentially, driver is dominant to determine the type of control element in control element set which the driver adds. It's possible to achieve your intension without changing ALSA control core itself, in my opinion. As Jaroslav said, it's better to change core of ALSA SoC part according to your intention. If you'd like to change ALSA control core, I'd like to request for the check of mismatch between the value of added member in snd_kcontrol and the value of type of control element returned from driver, like: ``` diff --git a/sound/core/control.c b/sound/core/control.c index 809b0a62e..c3ae70574 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -973,6 +973,7 @@ static int __snd_ctl_elem_info(struct snd_card *card, result = kctl->info(kctl, info); if (result >= 0) { snd_BUG_ON(info->access); + snd_BUG_ON(info->type == kctl->kcontrol_type); index_offset = snd_ctl_get_ioff(kctl, &info->id); vd = &kctl->vd[index_offset]; snd_ctl_build_ioff(&info->id, kctl, index_offset); ``` Regards Takashi Sakamoto
On Fri, Jan 08, 2021 at 11:06:59PM +0900, Takashi Sakamoto wrote: > Hi, > > On Fri, Jan 08, 2021 at 12:40:28PM +0100, Jaroslav Kysela wrote: > > Dne 08. 01. 21 v 12:23 Jaska Uimonen napsal(a): > > > Current kcontrol structs don't have a member to describe the control > > > type. The type is present in the widget which contains the control. As > > > there can be many controls in one widget it is inherently presumed that > > > the control types are the same. > > > > > > Lately there has been use cases where different types of controls would > > > be needed for single widget. Thus enable this by adding the control type > > > to kcontrol and kcontrol_new structs. > > > > It looks like a SoC only extension. Use private_data to carry this > > information. It has no value for the toplevel code. > > > > Jaroslav > > In current design of ALSA control core, the type of control element is > firstly determined by driver in callback of snd_kcontrol.info(). The > callback is done when userspace applications call ioctl(2) with > SNDRV_CTL_IOCTL_ELEM_INFO request. > > The patch doesn't touch to the above processing. It means that the type > information is just for kernel-land implementation and is not exposed to > userspace application. > > Essentially, driver is dominant to determine the type of control element > in control element set which the driver adds. It's possible to achieve > your intension without changing ALSA control core itself, in my opinion. > > As Jaroslav said, it's better to change core of ALSA SoC part according > to your intention. If you'd like to change ALSA control core, I'd like > to request for the check of mismatch between the value of added member > in snd_kcontrol and the value of type of control element returned from > driver, like: > > ``` > diff --git a/sound/core/control.c b/sound/core/control.c > index 809b0a62e..c3ae70574 100644 > --- a/sound/core/control.c > +++ b/sound/core/control.c > @@ -973,6 +973,7 @@ static int __snd_ctl_elem_info(struct snd_card *card, > result = kctl->info(kctl, info); > if (result >= 0) { > snd_BUG_ON(info->access); > + snd_BUG_ON(info->type == kctl->kcontrol_type); > index_offset = snd_ctl_get_ioff(kctl, &info->id); > vd = &kctl->vd[index_offset]; > snd_ctl_build_ioff(&info->id, kctl, index_offset); > ``` > > > Regards > > Takashi Sakamoto Hi, Thanks for the comments, I tried to do the same thing now in asoc level, will send v2. br, Jaska
diff --git a/include/sound/control.h b/include/sound/control.h index 77d9fa10812d..3933823a606d 100644 --- a/include/sound/control.h +++ b/include/sound/control.h @@ -46,6 +46,7 @@ struct snd_kcontrol_new { unsigned int index; /* index of item */ unsigned int access; /* access rights */ unsigned int count; /* count of same elements */ + unsigned int kcontrol_type; snd_kcontrol_info_t *info; snd_kcontrol_get_t *get; snd_kcontrol_put_t *put; @@ -65,6 +66,7 @@ struct snd_kcontrol { struct list_head list; /* list of controls */ struct snd_ctl_elem_id id; unsigned int count; /* count of same elements */ + unsigned int kcontrol_type; snd_kcontrol_info_t *info; snd_kcontrol_get_t *get; snd_kcontrol_put_t *put; diff --git a/sound/core/control.c b/sound/core/control.c index 3b44378b9dec..f081ae4f839c 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -268,6 +268,8 @@ struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new *ncontrol, } kctl->id.index = ncontrol->index; + kctl->kcontrol_type = ncontrol->kcontrol_type; + kctl->info = ncontrol->info; kctl->get = ncontrol->get; kctl->put = ncontrol->put;
Current kcontrol structs don't have a member to describe the control type. The type is present in the widget which contains the control. As there can be many controls in one widget it is inherently presumed that the control types are the same. Lately there has been use cases where different types of controls would be needed for single widget. Thus enable this by adding the control type to kcontrol and kcontrol_new structs. Signed-off-by: Jaska Uimonen <jaska.uimonen@linux.intel.com> --- include/sound/control.h | 2 ++ sound/core/control.c | 2 ++ 2 files changed, 4 insertions(+)