[alsa-devel] [PATCH v3 0/2] ALSA controls management using index/device/sub-devices fields
V3: I changed "RFC" prefix to "PATCH", hoping that patchset is enough mature in term of code and discussions, to be considered as a potential patch...
Previous discussions summary: - topic of the patchset: Patch concerns ASoc DAI drivers. Aim is to be able to instantiate PCM control using device field according to the associated PCM char device. For this, a relationship between DAIs PCM control and PCM char device needs to be establish. => Proposal is to add field in DAI driver struct to declare PCM controls that need to be linked to PCM character device on DAI link probing.
- Limitation of the patchset: This patchset seems only a first step as 2 concerns are been highlighted: - Patch is limited to "static" DAI-link. This not responds to backend (no_pcm dai links) and some none-DAI codecs, that could register PCm controls. In theses cases it is not possible to establish relationship during probe. => No solution proposed for time beeing.
- This patchset does not fix conflict of 2 "identical" PCM controls declared by 2 drivers. For instance, a conflict can exist between a PCM control created in DAI driver and the same control declared in a codec driver. As index field is forced to 0, prefix should be used in codec. => To discuss solution in a separate thread as issue already present without this patchset several approaches seem possible like using index field ( control auto indexation in ASoC) or prefix in Naming.
Changes: - [PATCH v3 1/2] ASoC: core: allow DAI PCM controls bound to PCM device Code optimization based on Takashi Sakamoto suggestion - [RFC V2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi patch suppressed as it was send as example in previous version. Patch is valid but can have impact on existing drivers as control device field value is updated - [PATCH v3 2/2] ASoC: sti: bind pcm controls to pcm device. No change vs V2
V2: http://www.spinics.net/lists/alsa-devel/msg57045.html
Aim of this version is to continue discussion on DAI PCM control focused on ASoC drivers. In this V2 implementation in Soc-core is simplified to limit impact on existing code.
Update of the RFC V1 based on discussions: - [RFC 4/4] iecset: allow to select control with device and sub-device numbers no more part of the RFC V2, will be discussed in a separate thread - [RFC 2/4] ALSA: control: increment index field for duplicated control. no more part of the RFC V2, no more need as RFC subject is PCM controls
- [RFC V2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device Patch reworked from V1 to simplify implementation - Binding is not done for Dai links tagged with no_pcm (DPCM). - no more possibility to add the controls after the DAI link probing.
- [RFC V2 2/3] ASoC: sti: bind PCM controls to PCM device. - [RFC V2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi Example of implementation in STI DAI driver and HDMI-codec drivers
V1: http://www.spinics.net/lists/alsa-devel/msg56479.html
1) Alsa-utils patch
- iecset: allow to select control with device and sub-device numbers This patch allows to access to 2 iec controls differentiated by device/sub-devices numbers => For me, this patch is mandatory to be able to address the ASoC IEC controls, in case of no fix is implemented to allows index field update in ASoC.
2) Alsa driver patches - ASoC: core: allow PCM control binding to PCM device Add relationship between DAIs PCM controls and PCM device.
- ALSA: control: increment index field for duplicated control. Generic implementation of the patch proposed in HDA driver (http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=ea9b4...)
- ASoC: sti: use bind_pcm_ctl implementation of bind_pcm_ctl for sti driver.
Arnaud Pouliquen (2): ASoC: core: allow DAI PCM controls bound to PCM device ASoC: sti: bind pcm controls to pcm device.
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 28 ++++++++++++++++++++++++++++ sound/soc/sti/sti_uniperif.c | 33 ++++----------------------------- 3 files changed, 36 insertions(+), 29 deletions(-)
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com --- include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 964b7de..93624c9 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -247,6 +247,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order; + + /* Optional PCM controls to bind to PCM device on DAIs link*/ + const struct snd_kcontrol_new *pcm_controls; + int num_pcm_controls; };
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..ace83c9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1552,6 +1552,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais, + struct snd_soc_pcm_runtime *rtd) +{ + struct snd_kcontrol_new kctl; + int i, j, err; + + for (i = 0; i < num_dais; ++i) { + for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) { + kctl = dais[i]->driver->pcm_controls[j]; + if (!rtd->dai_link->no_pcm) + kctl.device = rtd->pcm->device; + if (snd_soc_add_dai_controls(dais[i], &kctl, 1)) + return err; + } + } + + return 0; +} + static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1663,6 +1682,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; } + + /* Bind DAIs pcm controls to the PCM device */ + ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd); + if (ret < 0) + return ret; + ret = soc_link_dai_pcm_controls(rtd->codec_dais, + rtd->num_codecs, rtd); + if (ret < 0) + return ret; } else { INIT_DELAYED_WORK(&rtd->delayed_work, codec2codec_close_delayed_work);
On Nov 28 2016 18:33, Arnaud Pouliquen wrote:
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 964b7de..93624c9 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -247,6 +247,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order;
- /* Optional PCM controls to bind to PCM device on DAIs link*/
- const struct snd_kcontrol_new *pcm_controls;
- int num_pcm_controls;
};
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..ace83c9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1552,6 +1552,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
struct snd_soc_pcm_runtime *rtd)+{
- struct snd_kcontrol_new kctl;
- int i, j, err;
- for (i = 0; i < num_dais; ++i) {
for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {kctl = dais[i]->driver->pcm_controls[j];if (!rtd->dai_link->no_pcm)kctl.device = rtd->pcm->device;
For the above codes, please request some comment to the other developers working for ALSA SoC part. I'm not so experienced developer for this part, sorry.
if (snd_soc_add_dai_controls(dais[i], &kctl, 1))return err;
Return value from snd_soc_add_dai_controls() should be assigned to the 'err' variable, I think.
}- }
- return 0;
+}
static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1663,6 +1682,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; }
/* Bind DAIs pcm controls to the PCM device */ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd);if (ret < 0)return ret;ret = soc_link_dai_pcm_controls(rtd->codec_dais,rtd->num_codecs, rtd);if (ret < 0)return ret;
In the other part of this subsystem, the first parameter of helper functions typically represents a 'subject'. In this context, the subject is PCM runtime specialized for ALSA SoC part, which get some new control element sets for the PCM runtime. Therefore, it's better to move the 'rtd' parameter to the first argument. (This is not so strong demand, and somewhat depends on developers' taste. Furthermore, I have no self-confidence to tell my intension to you correctly in English...)
Regards
Takashi Sakamoto
On 11/28/2016 02:18 PM, Takashi Sakamoto wrote:
On Nov 28 2016 18:33, Arnaud Pouliquen wrote:
In case of several instances of the same PCM control (e.g IEC controls). Application should be able to address the control using the device field number, according to the PCM character device. This patch allows to link DAI PCM controls to the PCM device. During DAI_link probe, PCM controls are added after device field is forced to the PCM device number.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 28 ++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+)
diff --git a/include/sound/soc-dai.h b/include/sound/soc-dai.h index 964b7de..93624c9 100644 --- a/include/sound/soc-dai.h +++ b/include/sound/soc-dai.h @@ -247,6 +247,10 @@ struct snd_soc_dai_driver { /* probe ordering - for components with runtime dependencies */ int probe_order; int remove_order;
- /* Optional PCM controls to bind to PCM device on DAIs link*/
- const struct snd_kcontrol_new *pcm_controls;
- int num_pcm_controls;
};
/* diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..ace83c9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1552,6 +1552,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
struct snd_soc_pcm_runtime *rtd)+{
- struct snd_kcontrol_new kctl;
- int i, j, err;
- for (i = 0; i < num_dais; ++i) {
for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {kctl = dais[i]->driver->pcm_controls[j];if (!rtd->dai_link->no_pcm)kctl.device = rtd->pcm->device;For the above codes, please request some comment to the other developers working for ALSA SoC part. I'm not so experienced developer for this part, sorry.
I think something is missing here, to return an error if following condition is not respected: iface == SNDRV_CTL_ELEM_IFACE_PCM I'm going to add a test.
if (snd_soc_add_dai_controls(dais[i], &kctl, 1))return err;Return value from snd_soc_add_dai_controls() should be assigned to the 'err' variable, I think.
Oops, stupid mistake... thanks! i will correct it in my next version
}- }
- return 0;
+}
static int soc_link_dai_widgets(struct snd_soc_card *card, struct snd_soc_dai_link *dai_link, struct snd_soc_pcm_runtime *rtd) @@ -1663,6 +1682,15 @@ static int soc_probe_link_dais(struct snd_soc_card *card, dai_link->stream_name, ret); return ret; }
/* Bind DAIs pcm controls to the PCM device */ret = soc_link_dai_pcm_controls(&cpu_dai, 1, rtd);if (ret < 0)return ret;ret = soc_link_dai_pcm_controls(rtd->codec_dais,rtd->num_codecs, rtd);if (ret < 0)return ret;In the other part of this subsystem, the first parameter of helper functions typically represents a 'subject'. In this context, the subject is PCM runtime specialized for ALSA SoC part, which get some new control element sets for the PCM runtime. Therefore, it's better to move the 'rtd' parameter to the first argument. (This is not so strong demand, and somewhat depends on developers' taste. Furthermore, I have no self-confidence to tell my intension to you correctly in English...)
I understand your point. Nevertheless, i would not see the PCM runtime as subject, but the DAI. In this way, I was inspired by the soc_link_dai_widgets helper that is also in the subsytem... if rtd is the subject, i think soc_link_dai_pcm_controls should also be renamed snd_soc_runtime_link_dai_ctls (or something like that)
But i'm not expert in ASoC semantic, so based on my answer, please confirm me you point of view, i will integrate it in my V4 with other fixes.
Regards
Arnaud
Hi,
On Nov 29 2016 00:18, Arnaud Pouliquen wrote:
diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c index 4afa8db..ace83c9 100644 --- a/sound/soc/soc-core.c +++ b/sound/soc/soc-core.c @@ -1552,6 +1552,25 @@ static int soc_probe_dai(struct snd_soc_dai *dai, int order) return 0; }
+static int soc_link_dai_pcm_controls(struct snd_soc_dai **dais, int num_dais,
struct snd_soc_pcm_runtime *rtd)+{
- struct snd_kcontrol_new kctl;
- int i, j, err;
- for (i = 0; i < num_dais; ++i) {
for (j = 0; j < dais[i]->driver->num_pcm_controls; j++) {kctl = dais[i]->driver->pcm_controls[j];if (!rtd->dai_link->no_pcm)kctl.device = rtd->pcm->device;For the above codes, please request some comment to the other developers working for ALSA SoC part. I'm not so experienced developer for this part, sorry.
I think something is missing here, to return an error if following condition is not respected: iface == SNDRV_CTL_ELEM_IFACE_PCM I'm going to add a test.
When you have unclear points, it's better to ask them to the person who knows it. Furthermore, you attempt to change core codes of ALSA SoC part. Enough deliberation is preferable.
Liam Girdwood committed to the 'no_pcm' feature in below patch, and he is still active in this subsystem (I met him this month). You could request him to review. https://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=01d7...
if (snd_soc_add_dai_controls(dais[i], &kctl, 1))return err;Return value from snd_soc_add_dai_controls() should be assigned to the 'err' variable, I think.
Oops, stupid mistake... thanks! i will correct it in my next version
OK.
In the other part of this subsystem, the first parameter of helper functions typically represents a 'subject'. In this context, the subject is PCM runtime specialized for ALSA SoC part, which get some new control element sets for the PCM runtime. Therefore, it's better to move the 'rtd' parameter to the first argument. (This is not so strong demand, and somewhat depends on developers' taste. Furthermore, I have no self-confidence to tell my intension to you correctly in English...)
I understand your point. Nevertheless, i would not see the PCM runtime as subject, but the DAI. In this way, I was inspired by the soc_link_dai_widgets helper that is also in the subsytem... if rtd is the subject, i think soc_link_dai_pcm_controls should also be renamed snd_soc_runtime_link_dai_ctls (or something like that)
Fair enough. I might had a confusion due to complexity of ALSA SoC part against ALSA control core, sorry.
But i'm not expert in ASoC semantic, so based on my answer, please confirm me you point of view, i will integrate it in my V4 with other fixes.
Go a head.
Regards
Takashi Sakamoto
Hello,
I'm trying to understand how to implement the speaker allocation for HDMI output.
input: - HDMI sink provides the speaker presence in ELD structure - audio source can also contain channels position.
output: Audio info frame (IF) should contain audio channel allocation ( hdmi_audio_infoframe structure).
So if i well understand the concept, application should be in charge of aligning channels with HDMI sink speakers...
Regarding HDA driver, audio IF channel allocation is computing based on "Playback Channel Map"control . Application set the speaker information of the audio IF through this control.
On ASOC side, in hdmi-codec, -'ELD" control exists to export speaker information to application - info frame channel allocation is hardware coded in hdmi_codec_hw_params. The configuration is set by default depending on the number of channels. Drawback is that application is not aware of the configuration selected by the driver.
So it seems that something is missing to do the link between EDID information and audio source channel mapping on one side and audio IF configuration on other side.
What should be the best strategy to support feature in ASoC? - No update in hdmi-codec, consider that application should know the configuration available for 2,4,6 and 8 channels => seems not very flexible...
- Update hdmi-codec to implement "Playback Channel Map" in read-only, to provide channel mapping information to application. => impact is light, but application can get the correct mapping only after the update of the number of channels (so after hw_params ops call)
- Update hdmi-codec to implement "Playback Channel Map" in read and write access. => HDA implementation. Seems the more flexible solution from my windows...
- Implement control in drm drivers => seems not reasonable.
- Other?
Thanks in advance for your answers.
Regards Arnaud
On 11/30/16 11:54 AM, Arnaud Pouliquen wrote:
Hello,
I'm trying to understand how to implement the speaker allocation for HDMI output.
input:
- HDMI sink provides the speaker presence in ELD structure
- audio source can also contain channels position.
output: Audio info frame (IF) should contain audio channel allocation ( hdmi_audio_infoframe structure).
So if i well understand the concept, application should be in charge of aligning channels with HDMI sink speakers...
Yes. However some HDMI IPs provide hardware means to swap the channels - mainly because the order of channels varies between OSes and encoding schemes. The notion of 'aligning' might be limited in those cases to setting the relevant ALSA controls, not necessarily an active software channel swap and change of the channel map handled by the application.
Regarding HDA driver, audio IF channel allocation is computing based on "Playback Channel Map"control . Application set the speaker information of the audio IF through this control.
On ASOC side, in hdmi-codec, -'ELD" control exists to export speaker information to application
- info frame channel allocation is hardware coded in
hdmi_codec_hw_params. The configuration is set by default depending on the number of channels. Drawback is that application is not aware of the configuration selected by the driver.
So it seems that something is missing to do the link between EDID information and audio source channel mapping on one side and audio IF configuration on other side.
What should be the best strategy to support feature in ASoC?
- No update in hdmi-codec, consider that application should know
the configuration available for 2,4,6 and 8 channels => seems not very flexible...
- Update hdmi-codec to implement "Playback Channel Map" in read-only, to
provide channel mapping information to application. => impact is light, but application can get the correct mapping only after the update of the number of channels (so after hw_params ops call)
- Update hdmi-codec to implement "Playback Channel Map" in read and
write access. => HDA implementation. Seems the more flexible solution from my windows...
Implement control in drm drivers => seems not reasonable.
Other?
Thanks in advance for your answers.
Regards Arnaud
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
On Wed, 30 Nov 2016 23:27:22 +0100, Pierre-Louis Bossart wrote:
On 11/30/16 11:54 AM, Arnaud Pouliquen wrote:
Hello,
I'm trying to understand how to implement the speaker allocation for HDMI output.
input:
- HDMI sink provides the speaker presence in ELD structure
- audio source can also contain channels position.
output: Audio info frame (IF) should contain audio channel allocation ( hdmi_audio_infoframe structure).
So if i well understand the concept, application should be in charge of aligning channels with HDMI sink speakers...
Yes. However some HDMI IPs provide hardware means to swap the channels
mainly because the order of channels varies between OSes and encoding schemes. The notion of 'aligning' might be limited in those cases to setting the relevant ALSA controls, not necessarily an active software channel swap and change of the channel map handled by the application.
The application may just leave the channel mapping as the driver default, too. It's a freedom for user-space whether to accept the default channel mapping as is, or choose the own preferred one.
For some IPs that can't handle the channel maps fully, the limitation can be implemented in the driver side, I suppose. It just needs to pass the limited set of channel-map array the hardware may accept, instead of the arrays that are created by parsing the ELD.
Regarding HDA driver, audio IF channel allocation is computing based on "Playback Channel Map"control . Application set the speaker information of the audio IF through this control.
On ASOC side, in hdmi-codec, -'ELD" control exists to export speaker information to application
- info frame channel allocation is hardware coded in
hdmi_codec_hw_params. The configuration is set by default depending on the number of channels. Drawback is that application is not aware of the configuration selected by the driver.
So it seems that something is missing to do the link between EDID information and audio source channel mapping on one side and audio IF configuration on other side.
What should be the best strategy to support feature in ASoC?
- No update in hdmi-codec, consider that application should know
the configuration available for 2,4,6 and 8 channels => seems not very flexible...
- Update hdmi-codec to implement "Playback Channel Map" in read-only, to
provide channel mapping information to application. => impact is light, but application can get the correct mapping only after the update of the number of channels (so after hw_params ops call)
- Update hdmi-codec to implement "Playback Channel Map" in read and
write access. => HDA implementation. Seems the more flexible solution from my windows...
Implement control in drm drivers => seems not reasonable.
Other?
We can start from the read-only chmap ctls, with a plan to enhance to the read/write later on. I guess only few applications require the channel map change, and the hardest part is the proper integration in ASoC, not the write support itself.
thanks,
Takashi
On 12/01/2016 11:07 AM, Takashi Iwai wrote:
On Wed, 30 Nov 2016 23:27:22 +0100, Pierre-Louis Bossart wrote:
On 11/30/16 11:54 AM, Arnaud Pouliquen wrote:
Hello,
I'm trying to understand how to implement the speaker allocation for HDMI output.
input:
- HDMI sink provides the speaker presence in ELD structure
- audio source can also contain channels position.
output: Audio info frame (IF) should contain audio channel allocation ( hdmi_audio_infoframe structure).
So if i well understand the concept, application should be in charge of aligning channels with HDMI sink speakers...
Yes. However some HDMI IPs provide hardware means to swap the channels
mainly because the order of channels varies between OSes and encoding schemes. The notion of 'aligning' might be limited in those cases to setting the relevant ALSA controls, not necessarily an active software channel swap and change of the channel map handled by the application.
The application may just leave the channel mapping as the driver default, too. It's a freedom for user-space whether to accept the default channel mapping as is, or choose the own preferred one.
For some IPs that can't handle the channel maps fully, the limitation can be implemented in the driver side, I suppose. It just needs to pass the limited set of channel-map array the hardware may accept, instead of the arrays that are created by parsing the ELD.
Regarding HDA driver, audio IF channel allocation is computing based on "Playback Channel Map"control . Application set the speaker information of the audio IF through this control.
On ASOC side, in hdmi-codec, -'ELD" control exists to export speaker information to application
- info frame channel allocation is hardware coded in
hdmi_codec_hw_params. The configuration is set by default depending on the number of channels. Drawback is that application is not aware of the configuration selected by the driver.
Erratum: Channel configuration is not updated but always set to default value 0: stereo FL+FR, even for more than 2 channels playback.
So it seems that something is missing to do the link between EDID information and audio source channel mapping on one side and audio IF configuration on other side.
What should be the best strategy to support feature in ASoC?
- No update in hdmi-codec, consider that application should know
the configuration available for 2,4,6 and 8 channels => seems not very flexible...
- Update hdmi-codec to implement "Playback Channel Map" in read-only, to
provide channel mapping information to application. => impact is light, but application can get the correct mapping only after the update of the number of channels (so after hw_params ops call)
- Update hdmi-codec to implement "Playback Channel Map" in read and
write access. => HDA implementation. Seems the more flexible solution from my windows...
Implement control in drm drivers => seems not reasonable.
Other?
We can start from the read-only chmap ctls, with a plan to enhance to the read/write later on. I guess only few applications require the channel map change, and the hardest part is the proper integration in ASoC, not the write support itself.
That seems reasonable. If OK, as a first step I will try to propose chmap control in read-only with some predefined configurations in HDMI-codec.c.
Concerning channel allocation in HDMI_CODEC, Proposal is to align channels location according to HDA default ones: (hdac_cea_channel_speaker_allocation struct)
static struct hdac_cea_channel_speaker_allocation channel_allocations[] = { /* channel: 7 6 5 4 3 2 1 0 */ /*mono or stereo */ { .ca_index = 0x00, .speakers = { 0, 0, 0, 0, 0, 0, FR, FL } }, /* 2.1 */ { .ca_index = 0x01, .speakers = { 0, 0, 0, 0, 0, LFE, FR, FL } }, /* Dolby Surround */ { .ca_index = 0x02, .speakers = { 0, 0, 0, 0, FC, 0, FR, FL } }, /* surround40 */ { .ca_index = 0x08, .speakers = { 0, 0, RR, RL, 0, 0, FR, FL } }, /* surround41 */ { .ca_index = 0x09, .speakers = { 0, 0, RR, RL, 0, LFE, FR, FL } }, /* surround50 */ { .ca_index = 0x0a, .speakers = { 0, 0, RR, RL, FC, 0, FR, FL } }, /* surround51 */ { .ca_index = 0x0b, .speakers = { 0, 0, RR, RL, FC, LFE, FR, FL } }, /* 6.1 */ { .ca_index = 0x0f, .speakers = { 0, RC, RR, RL, FC, LFE, FR, FL } }, /* surround71 */ { .ca_index = 0x13, .speakers = { RRC, RLC, RR, RL, FC, LFE, FR, FL } },
Thanks and Regards,
Arnaud
Hi,
This is a nitpicking.
In-Reply-To: b6cf1216-7408-584f-6966-2d2a7cb20df3@sakamocchi.jp
It's preferable to start one mail thread for one patchset and one issue. It's better to add URL references into your message instead of adding 'in-reply-to' header, when you post new patchset.
Regards
Takashi Sakamoto
Move PCM control list in DAI driver struct, to bind PCM control to PCM device created during DAI linking.
Signed-off-by: Arnaud Pouliquen arnaud.pouliquen@st.com --- sound/soc/sti/sti_uniperif.c | 33 ++++----------------------------- 1 file changed, 4 insertions(+), 29 deletions(-)
diff --git a/sound/soc/sti/sti_uniperif.c b/sound/soc/sti/sti_uniperif.c index 549fac3..db8bab5 100644 --- a/sound/soc/sti/sti_uniperif.c +++ b/sound/soc/sti/sti_uniperif.c @@ -225,34 +225,6 @@ int sti_uniperiph_get_tdm_word_pos(struct uniperif *uni, }
/* - * sti_uniperiph_dai_create_ctrl - * This function is used to create Ctrl associated to DAI but also pcm device. - * Request is done by front end to associate ctrl with pcm device id - */ -static int sti_uniperiph_dai_create_ctrl(struct snd_soc_dai *dai) -{ - struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai); - struct uniperif *uni = priv->dai_data.uni; - struct snd_kcontrol_new *ctrl; - int i; - - if (!uni->num_ctrls) - return 0; - - for (i = 0; i < uni->num_ctrls; i++) { - /* - * Several Control can have same name. Controls are indexed on - * Uniperipheral instance ID - */ - ctrl = &uni->snd_ctrls[i]; - ctrl->index = uni->id; - ctrl->device = uni->id; - } - - return snd_soc_add_dai_controls(dai, uni->snd_ctrls, uni->num_ctrls); -} - -/* * DAI */ int sti_uniperiph_dai_hw_params(struct snd_pcm_substream *substream, @@ -342,7 +314,7 @@ static int sti_uniperiph_dai_probe(struct snd_soc_dai *dai) dai_data->dma_data.addr = dai_data->uni->fifo_phys_address; dai_data->dma_data.addr_width = DMA_SLAVE_BUSWIDTH_4_BYTES;
- return sti_uniperiph_dai_create_ctrl(dai); + return 0; }
static const struct snd_soc_dai_driver sti_uniperiph_dai_template = { @@ -460,6 +432,9 @@ static int sti_uniperiph_probe(struct platform_device *pdev)
ret = sti_uniperiph_cpu_dai_of(node, priv);
+ priv->dai->pcm_controls = priv->dai_data.uni->snd_ctrls; + priv->dai->num_pcm_controls = priv->dai_data.uni->num_ctrls; + dev_set_drvdata(&pdev->dev, priv);
ret = devm_snd_soc_register_component(&pdev->dev,
On Nov 28 2016 18:33, Arnaud Pouliquen wrote:
V3: I changed "RFC" prefix to "PATCH", hoping that patchset is enough mature in term of code and discussions, to be considered as a potential patch...
Previous discussions summary:
topic of the patchset: Patch concerns ASoc DAI drivers. Aim is to be able to instantiate PCM control using device field according to the associated PCM char device. For this, a relationship between DAIs PCM control and PCM char device needs to be establish. => Proposal is to add field in DAI driver struct to declare PCM controls that need to be linked to PCM character device on DAI link probing.
Limitation of the patchset:
This patchset seems only a first step as 2 concerns are been highlighted: - Patch is limited to "static" DAI-link. This not responds to backend (no_pcm dai links) and some none-DAI codecs, that could register PCm controls. In theses cases it is not possible to establish relationship during probe. => No solution proposed for time beeing.
- This patchset does not fix conflict of 2 "identical" PCM controls declared by 2 drivers. For instance, a conflict can exist between a PCM control created in DAI driver and the same control declared in a codec driver. As index field is forced to 0, prefix should be used in codec. => To discuss solution in a separate thread as issue already present without this patchset several approaches seem possible like using index field ( control auto indexation in ASoC) or prefix in Naming.
Enough information for reviewers, very nice ;)
Changes:
- [PATCH v3 1/2] ASoC: core: allow DAI PCM controls bound to PCM device Code optimization based on Takashi Sakamoto suggestion
- [RFC V2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi patch suppressed as it was send as example in previous version. Patch is valid but can have impact on existing drivers as control device field value is updated
- [PATCH v3 2/2] ASoC: sti: bind pcm controls to pcm device.
No change vs V2
V2: http://www.spinics.net/lists/alsa-devel/msg57045.html
Aim of this version is to continue discussion on DAI PCM control focused on ASoC drivers. In this V2 implementation in Soc-core is simplified to limit impact on existing code.
Update of the RFC V1 based on discussions:
[RFC 4/4] iecset: allow to select control with device and sub-device numbers no more part of the RFC V2, will be discussed in a separate thread
[RFC 2/4] ALSA: control: increment index field for duplicated control. no more part of the RFC V2, no more need as RFC subject is PCM controls
[RFC V2 1/3] ASoC: core: allow DAI PCM controls bound to PCM device Patch reworked from V1 to simplify implementation - Binding is not done for Dai links tagged with no_pcm (DPCM). - no more possibility to add the controls after the DAI link probing.
[RFC V2 2/3] ASoC: sti: bind PCM controls to PCM device.
[RFC V2 3/3] ASoC: hdmi-codec: Example of PCM control bound to PCM device for multi Example of implementation in STI DAI driver and HDMI-codec drivers
V1: http://www.spinics.net/lists/alsa-devel/msg56479.html
- Alsa-utils patch
- iecset: allow to select control with device and sub-device numbers This patch allows to access to 2 iec controls differentiated by device/sub-devices numbers
=> For me, this patch is mandatory to be able to address the ASoC IEC controls, in case of no fix is implemented to allows index field update in ASoC.
- Alsa driver patches
- ASoC: core: allow PCM control binding to PCM device
Add relationship between DAIs PCM controls and PCM device.
ALSA: control: increment index field for duplicated control. Generic implementation of the patch proposed in HDA driver (http://git.kernel.org/cgit/linux/kernel/git/tiwai/sound.git/commit/?id=ea9b4...)
ASoC: sti: use bind_pcm_ctl
implementation of bind_pcm_ctl for sti driver.
Arnaud Pouliquen (2): ASoC: core: allow DAI PCM controls bound to PCM device ASoC: sti: bind pcm controls to pcm device.
include/sound/soc-dai.h | 4 ++++ sound/soc/soc-core.c | 28 ++++++++++++++++++++++++++++ sound/soc/sti/sti_uniperif.c | 33 ++++----------------------------- 3 files changed, 36 insertions(+), 29 deletions(-)
Regards
Takashi Sakamoto
participants (4)
-
Arnaud Pouliquen -
Pierre-Louis Bossart -
Takashi Iwai -
Takashi Sakamoto