[v3,9/9] ASoC: sti: Add IEC control

Commit Message

Arnaud POULIQUEN June 22, 2015, 2:31 p.m. UTC

Add control to configure IEC60958 settings.

Signed-off-by: Arnaud Pouliquen <[e-mail]>
---
 sound/soc/sti/uniperif_player.c | 77 ++++++++++++++++++++++++++++++++++++++---
 1 file changed, 73 insertions(+), 4 deletions(-)

Comments

Mark Brown July 10, 2015, 6:06 p.m. UTC | #1

On Mon, Jun 22, 2015 at 04:31:13PM +0200, Arnaud Pouliquen wrote:
> Add control to configure IEC60958 settings.

I had one question on the rest of the series but basically everything
else looks reasonable - I've applied some of the earlier patches, where
there were comments can you please send incremental fixes for them?
With this one there's no technical problem exactly but it seems like
something we should have a standard userspace interface for so I'd like
to check what's going on there and propose something standard if there
isn't anything.  It should at most affect the control naming.

Arnaud POULIQUEN July 15, 2015, 9:59 a.m. UTC | #2

On 07/10/2015 08:06 PM, Mark Brown wrote:
> On Mon, Jun 22, 2015 at 04:31:13PM +0200, Arnaud Pouliquen wrote:
>> Add control to configure IEC60958 settings.
>
> I had one question on the rest of the series but basically everything
> else looks reasonable - I've applied some of the earlier patches, where
> there were comments can you please send incremental fixes for them?
Incremental fix means V4 or additional patches?
> With this one there's no technical problem exactly but it seems like
> something we should have a standard userspace interface for so I'd like
> to check what's going on there and propose something standard if there
> isn't anything.  It should at most affect the control naming.
>
i not quite sure i follow you, this IEC control is not already standard?
Userland applications like Gstreamer seem already use it...
The only limitation i saw is that i did not succeed in associated 
control to PCM device, only to CPU DAI. As i will have one additional 
IEC control for HDMI, i will need to manage control index, aligned to 
CPU DAI index, but not to PCM device index.

Mark Brown July 15, 2015, 11:03 a.m. UTC | #3

On Wed, Jul 15, 2015 at 11:59:46AM +0200, Arnaud Pouliquen wrote:
> On 07/10/2015 08:06 PM, Mark Brown wrote:
> >On Mon, Jun 22, 2015 at 04:31:13PM +0200, Arnaud Pouliquen wrote:
> >>Add control to configure IEC60958 settings.

> >I had one question on the rest of the series but basically everything
> >else looks reasonable - I've applied some of the earlier patches, where
> >there were comments can you please send incremental fixes for them?

> Incremental fix means V4 or additional patches?

It means any patches should apply against my current tree.

> >With this one there's no technical problem exactly but it seems like
> >something we should have a standard userspace interface for so I'd like
> >to check what's going on there and propose something standard if there
> >isn't anything.  It should at most affect the control naming.

> i not quite sure i follow you, this IEC control is not already standard?
> Userland applications like Gstreamer seem already use it...
> The only limitation i saw is that i did not succeed in associated control to
> PCM device, only to CPU DAI. As i will have one additional IEC control for
> HDMI, i will need to manage control index, aligned to CPU DAI index, but not
> to PCM device index.

Right, the PCM association bit is part of it.

Patch

diff --git a/sound/soc/sti/uniperif_player.c b/sound/soc/sti/uniperif_player.c
index a1b0135..69103e9 100644
--- a/sound/soc/sti/uniperif_player.c
+++ b/sound/soc/sti/uniperif_player.c
@@ -250,6 +250,7 @@  static void uni_player_set_channel_status(struct uniperif *player,
 	 * sampling frequency. If no sample rate is already specified, then
 	 * set one.
 	 */
+	mutex_lock(&player->ctrl_lock);
 	if (runtime && (player->stream_settings.iec958.status[3]
 					== IEC958_AES3_CON_FS_NOTID)) {
 		switch (runtime->rate) {
@@ -327,6 +328,7 @@  static void uni_player_set_channel_status(struct uniperif *player,
 		player->stream_settings.iec958.status[3 + (n * 4)] << 24;
 		SET_UNIPERIF_CHANNEL_STA_REGN(player, n, status);
 	}
+	mutex_unlock(&player->ctrl_lock);
 
 	/* Update the channel status */
 	if (player->ver < SND_ST_UNIPERIF_VERSION_UNI_PLR_TOP_1_0)
@@ -538,6 +540,63 @@  static int uni_player_prepare_pcm(struct uniperif *player,
 }
 
 /*
+ * ALSA uniperipheral iec958 controls
+ */
+static int  uni_player_ctl_iec958_info(struct snd_kcontrol *kcontrol,
+				       struct snd_ctl_elem_info *uinfo)
+{
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_IEC958;
+	uinfo->count = 1;
+
+	return 0;
+}
+
+static int uni_player_ctl_iec958_get(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
+	struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
+	struct uniperif *player = priv->dai_data.uni;
+	struct snd_aes_iec958 *iec958 = &player->stream_settings.iec958;
+
+	mutex_lock(&player->ctrl_lock);
+	ucontrol->value.iec958.status[0] = iec958->status[0];
+	ucontrol->value.iec958.status[1] = iec958->status[1];
+	ucontrol->value.iec958.status[2] = iec958->status[2];
+	ucontrol->value.iec958.status[3] = iec958->status[3];
+	mutex_unlock(&player->ctrl_lock);
+	return 0;
+}
+
+static int uni_player_ctl_iec958_put(struct snd_kcontrol *kcontrol,
+				     struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_dai *dai = snd_kcontrol_chip(kcontrol);
+	struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
+	struct uniperif *player = priv->dai_data.uni;
+	struct snd_aes_iec958 *iec958 =  &player->stream_settings.iec958;
+
+	mutex_lock(&player->ctrl_lock);
+	iec958->status[0] = ucontrol->value.iec958.status[0];
+	iec958->status[1] = ucontrol->value.iec958.status[1];
+	iec958->status[2] = ucontrol->value.iec958.status[2];
+	iec958->status[3] = ucontrol->value.iec958.status[3];
+	mutex_unlock(&player->ctrl_lock);
+
+	uni_player_set_channel_status(player, NULL);
+
+	return 0;
+}
+
+static struct snd_kcontrol_new uni_player_iec958_ctl = {
+	.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+	.name = SNDRV_CTL_NAME_IEC958("", PLAYBACK, DEFAULT),
+	.info = uni_player_ctl_iec958_info,
+	.get = uni_player_ctl_iec958_get,
+	.put = uni_player_ctl_iec958_put,
+};
+
+/*
  * uniperif rate adjustement control
  */
 static int snd_sti_clk_adjustment_info(struct snd_kcontrol *kcontrol,
@@ -559,7 +618,9 @@  static int snd_sti_clk_adjustment_get(struct snd_kcontrol *kcontrol,
 	struct sti_uniperiph_data *priv = snd_soc_dai_get_drvdata(dai);
 	struct uniperif *player = priv->dai_data.uni;
 
+	mutex_lock(&player->ctrl_lock);
 	ucontrol->value.integer.value[0] = player->clk_adj;
+	mutex_unlock(&player->ctrl_lock);
 
 	return 0;
 }
@@ -594,7 +655,12 @@  static struct snd_kcontrol_new uni_player_clk_adj_ctl = {
 	.put = snd_sti_clk_adjustment_put,
 };
 
-static struct snd_kcontrol_new *snd_sti_ctl[] = {
+static struct snd_kcontrol_new *snd_sti_pcm_ctl[] = {
+	&uni_player_clk_adj_ctl,
+};
+
+static struct snd_kcontrol_new *snd_sti_iec_ctl[] = {
+	&uni_player_iec958_ctl,
 	&uni_player_clk_adj_ctl,
 };
 
@@ -1030,10 +1096,13 @@  int uni_player_init(struct platform_device *pdev,
 		player->stream_settings.iec958.status[4] =
 					IEC958_AES4_CON_MAX_WORDLEN_24 |
 					IEC958_AES4_CON_WORDLEN_24_20;
-	}
 
-	player->num_ctrls = ARRAY_SIZE(snd_sti_ctl);
-	player->snd_ctrls = snd_sti_ctl[0];
+		player->num_ctrls = ARRAY_SIZE(snd_sti_iec_ctl);
+		player->snd_ctrls = snd_sti_iec_ctl[0];
+	} else {
+		player->num_ctrls = ARRAY_SIZE(snd_sti_pcm_ctl);
+		player->snd_ctrls = snd_sti_pcm_ctl[0];
+	}
 
 	return 0;
 }