[alsa-devel] [RFC][PATCH 0/3] ALSA: control: delegate TLV eventing to each driver
Hi,
This patchset is to avoid a trouble on ALSA control core, kernel API of ALSA control component and ALSA control interface/protocol to handle TLV operations from user space.
In a design of ALSA control core, a set of elements is represented by 'struct snd_kcontrol' to share common attributes. The set of elements shares TLV (Type-Length-Value) data, too.
On the other hand, in ALSA control interface/protocol for applications, a TLV operation is committed to an element. Totally, the operation can have sub-effect to the other elements in the set. For example, TLV_WRITE operation is expected to change TLV data, which returns to applications. Applications attempt to change the TLV data per element, but in the above design, they can effect to elements in the same set.
As a default, ALSA control core has no implementation except for TLV_READ operation. Thus, the above design looks to have no issue. However, in kernel APIs of ALSA control component, developers can program a handler for any request of the TLV operation. Therefore, for elements in a set which has the handler, applications can commit TLV_WRITE and TLV_COMMAND requests.
For the above scenario, ALSA control core assist notification. When the handler returns positive value, the core queueing an event for a requested element. However, this includes design defects that the event is not queued for the other element in a set. Actually, developers can program the handlers to keep per-element TLV data, but it depends on each driver.
In this patchset, I attempts to solve the above issue, by delegating the event notification to each driver of ALSA control component.
Additionally, this patchset includes a solution against an issue in below message thread:
[alsa-devel] snd_ctl_add_enumerated_elem_set and TLV http://mailman.alsa-project.org/pipermail/alsa-devel/2017-July/123364.html
Takashi Sakamoto (3): ALSA: control: delegate TLV eventing to each driver ALSA: control: queue TLV event for all of elements in an user-defined set ALSA: control: disable TLV data at initial state of user-defined element set
sound/core/control.c | 34 ++++++++++++++++++++++------------ 1 file changed, 22 insertions(+), 12 deletions(-)
In a design of ALSA control core, a set of elements is represented by 'struct snd_kcontrol' to share common attributes. The set of elements shares TLV (Type-Length-Value) data, too.
On the other hand, in ALSA control interface/protocol for applications, a TLV operation is committed to an element. Totally, the operation can have sub-effect to the other elements in the set. For example, TLV_WRITE operation is expected to change TLV data, which returns to applications. Applications attempt to change the TLV data per element, but in the above design, they can effect to elements in the same set.
As a default, ALSA control core has no implementation except for TLV_READ operation. Thus, the above design looks to have no issue. However, in kernel APIs of ALSA control component, developers can program a handler for any request of the TLV operation. Therefore, for elements in a set which has the handler, applications can commit TLV_WRITE and TLV_COMMAND requests.
For the above scenario, ALSA control core assist notification. When the handler returns positive value, the core queueing an event for a requested element. However, this includes design defects that the event is not queued for the other element in a set. Actually, developers can program the handlers to keep per-element TLV data, but it depends on each driver.
As of v4.13-rc6, there's no driver in tree to utilize the notification, except for user-defined element set. This commit delegates the notification into each driver to prevent developers from the design defects.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/control.c | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-)
diff --git a/sound/core/control.c b/sound/core/control.c index 249140c15d64..bbcf4833ad1c 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1138,6 +1138,8 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, ue->tlv_data = container; ue->tlv_data_size = size;
+ snd_ctl_notify(ue->card, SNDRV_CTL_EVENT_MASK_TLV, &kctl->id); + return change; }
@@ -1423,7 +1425,6 @@ static int call_tlv_handler(struct snd_ctl_file *file, int op_flag, }; struct snd_kcontrol_volatile *vd = &kctl->vd[snd_ctl_get_ioff(kctl, id)]; int i; - int err;
/* Check support of the request for this element. */ for (i = 0; i < ARRAY_SIZE(pairs); ++i) { @@ -1440,14 +1441,7 @@ static int call_tlv_handler(struct snd_ctl_file *file, int op_flag, if (vd->owner != NULL && vd->owner != file) return -EPERM;
- err = kctl->tlv.c(kctl, op_flag, size, buf); - if (err < 0) - return err; - - if (err > 0) - snd_ctl_notify(file->card, SNDRV_CTL_EVENT_MASK_TLV, id); - - return 0; + return kctl->tlv.c(kctl, op_flag, size, buf); }
static int read_tlv_buf(struct snd_kcontrol *kctl, struct snd_ctl_elem_id *id,
In a design of user-defined element set, applications allow to change TLV data on the set. This operation doesn't only affects to a target element, but also to elements in the set.
This commit generates TLV event for all of elements in the set when the TLV data is changed.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/control.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/sound/core/control.c b/sound/core/control.c index bbcf4833ad1c..b8d2fa465a9d 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1117,6 +1117,8 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, { struct user_element *ue = kctl->private_data; unsigned int *container; + struct snd_ctl_elem_id id; + int i; int change;
if (size > 1024 * 128) /* sane value */ @@ -1138,7 +1140,10 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, ue->tlv_data = container; ue->tlv_data_size = size;
- snd_ctl_notify(ue->card, SNDRV_CTL_EVENT_MASK_TLV, &kctl->id); + for (i = 0; i < kctl->count; ++i) { + snd_ctl_build_ioff(&id, kctl, i); + snd_ctl_notify(ue->card, SNDRV_CTL_EVENT_MASK_TLV, &id); + }
return change; }
For user-defined element set, in its initial state, TLV data is not registered. It's firstly available when any application register it by an additional operation. However, in current implementation, it's available in its initial state. As a result, applications get -ENXIO to read it.
This commit controls its readability to manage info flags properly. In an initial state, elements don't have SND_CTL_ELEM_ACCESS_TLV_READ flag. Once TLV write operation is executed, they get the flag.
Signed-off-by: Takashi Sakamoto o-takashi@sakamocchi.jp --- sound/core/control.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/sound/core/control.c b/sound/core/control.c index b8d2fa465a9d..f77b8ccbce34 100644 --- a/sound/core/control.c +++ b/sound/core/control.c @@ -1118,6 +1118,7 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, struct user_element *ue = kctl->private_data; unsigned int *container; struct snd_ctl_elem_id id; + unsigned int mask = 0; int i; int change;
@@ -1136,13 +1137,21 @@ static int replace_user_tlv(struct snd_kcontrol *kctl, unsigned int __user *buf, return 0; }
+ if (ue->tlv_data == NULL) { + /* Now TLV data is available. */ + for (i = 0; i < kctl->count; ++i) + kctl->vd[i].access |= SNDRV_CTL_ELEM_ACCESS_TLV_READ; + mask = SNDRV_CTL_EVENT_MASK_INFO; + } + kfree(ue->tlv_data); ue->tlv_data = container; ue->tlv_data_size = size;
+ mask |= SNDRV_CTL_EVENT_MASK_TLV; for (i = 0; i < kctl->count; ++i) { snd_ctl_build_ioff(&id, kctl, i); - snd_ctl_notify(ue->card, SNDRV_CTL_EVENT_MASK_TLV, &id); + snd_ctl_notify(ue->card, mask, &id); }
return change; @@ -1277,8 +1286,10 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, access = SNDRV_CTL_ELEM_ACCESS_READWRITE; access &= (SNDRV_CTL_ELEM_ACCESS_READWRITE | SNDRV_CTL_ELEM_ACCESS_INACTIVE | - SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE); - if (access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) + SNDRV_CTL_ELEM_ACCESS_TLV_WRITE); + + /* In initial state, nothing is available as TLV container. */ + if (access & SNDRV_CTL_ELEM_ACCESS_TLV_WRITE) access |= SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK; access |= SNDRV_CTL_ELEM_ACCESS_USER;
@@ -1341,7 +1352,7 @@ static int snd_ctl_elem_add(struct snd_ctl_file *file, kctl->get = snd_ctl_elem_user_get; if (access & SNDRV_CTL_ELEM_ACCESS_WRITE) kctl->put = snd_ctl_elem_user_put; - if (access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) + if (access & SNDRV_CTL_ELEM_ACCESS_TLV_WRITE) kctl->tlv.c = snd_ctl_elem_user_tlv;
/* This function manage to free the instance on failure. */
On Tue, 22 Aug 2017 01:42:19 +0200, Takashi Sakamoto wrote:
Hi,
This patchset is to avoid a trouble on ALSA control core, kernel API of ALSA control component and ALSA control interface/protocol to handle TLV operations from user space.
In a design of ALSA control core, a set of elements is represented by 'struct snd_kcontrol' to share common attributes. The set of elements shares TLV (Type-Length-Value) data, too.
On the other hand, in ALSA control interface/protocol for applications, a TLV operation is committed to an element. Totally, the operation can have sub-effect to the other elements in the set. For example, TLV_WRITE operation is expected to change TLV data, which returns to applications. Applications attempt to change the TLV data per element, but in the above design, they can effect to elements in the same set.
As a default, ALSA control core has no implementation except for TLV_READ operation. Thus, the above design looks to have no issue. However, in kernel APIs of ALSA control component, developers can program a handler for any request of the TLV operation. Therefore, for elements in a set which has the handler, applications can commit TLV_WRITE and TLV_COMMAND requests.
For the above scenario, ALSA control core assist notification. When the handler returns positive value, the core queueing an event for a requested element. However, this includes design defects that the event is not queued for the other element in a set. Actually, developers can program the handlers to keep per-element TLV data, but it depends on each driver.
In this patchset, I attempts to solve the above issue, by delegating the event notification to each driver of ALSA control component.
Additionally, this patchset includes a solution against an issue in below message thread:
[alsa-devel] snd_ctl_add_enumerated_elem_set and TLV http://mailman.alsa-project.org/pipermail/alsa-devel/2017-July/123364.html
Takashi Sakamoto (3): ALSA: control: delegate TLV eventing to each driver ALSA: control: queue TLV event for all of elements in an user-defined set ALSA: control: disable TLV data at initial state of user-defined element set
The patch set looks OK to me through a quick glance. If it's confirmed to work with your tests, feel free to resubmit as the patches for merge.
thanks,
Takashi
Hi,
On Aug 22 2017 15:27, Takashi Iwai wrote:
On Tue, 22 Aug 2017 01:42:19 +0200, Takashi Sakamoto wrote:
Hi,
This patchset is to avoid a trouble on ALSA control core, kernel API of ALSA control component and ALSA control interface/protocol to handle TLV operations from user space.
In a design of ALSA control core, a set of elements is represented by 'struct snd_kcontrol' to share common attributes. The set of elements shares TLV (Type-Length-Value) data, too.
On the other hand, in ALSA control interface/protocol for applications, a TLV operation is committed to an element. Totally, the operation can have sub-effect to the other elements in the set. For example, TLV_WRITE operation is expected to change TLV data, which returns to applications. Applications attempt to change the TLV data per element, but in the above design, they can effect to elements in the same set.
As a default, ALSA control core has no implementation except for TLV_READ operation. Thus, the above design looks to have no issue. However, in kernel APIs of ALSA control component, developers can program a handler for any request of the TLV operation. Therefore, for elements in a set which has the handler, applications can commit TLV_WRITE and TLV_COMMAND requests.
For the above scenario, ALSA control core assist notification. When the handler returns positive value, the core queueing an event for a requested element. However, this includes design defects that the event is not queued for the other element in a set. Actually, developers can program the handlers to keep per-element TLV data, but it depends on each driver.
In this patchset, I attempts to solve the above issue, by delegating the event notification to each driver of ALSA control component.
Additionally, this patchset includes a solution against an issue in below message thread:
[alsa-devel] snd_ctl_add_enumerated_elem_set and TLV http://mailman.alsa-project.org/pipermail/alsa-devel/2017-July/123364.html
Takashi Sakamoto (3): ALSA: control: delegate TLV eventing to each driver ALSA: control: queue TLV event for all of elements in an user-defined set ALSA: control: disable TLV data at initial state of user-defined element set
The patch set looks OK to me through a quick glance. If it's confirmed to work with your tests, feel free to resubmit as the patches for merge.
Thanks for your positive comment. After re-investigating again and I posted it with a patch for test program in alsa-lib.
Takashi Sakamoto
participants (2)
-
Takashi Iwai
-
Takashi Sakamoto