Skip to contentSkip to conversation

topology: complete PCM parsing and code refactoring

PATCH Superseded [email protected] 4 participants 12 messages Apr 28, 2016 – May 8, 2016 Sound

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover topology: complete PCM parsing and code refactoring

From: Mengdong Lin <[email protected]>

There is no ABI change in this series.

This series fixes bugs and missing info of PCM (Front-end DAI & DAI link) defined by the text conf file.

There is also some code refactoring, as a preparation for Back-end DAI support and ABI update which is under internal test and review atm.

Mengdong Lin (6):
topology: Set manifest size for ABI
topology: Refactor functions to parse and build streams
topology: Use generic pointer to realloc buffer for private data
topology: Fix pcm ID & name parsing
topology: Parse front-end DAI name and ID for the PCM
topology: Update PCM configurations in Broadwell text conf file

 include/sound/asoc.h                       |   2 +-
 include/topology.h                         |   4 +
 src/conf/topology/broadwell/broadwell.conf |  34 ++++++---
 src/topology/data.c                        |  24 ++----
 src/topology/parser.c                      |   4 +-
 src/topology/pcm.c                         | 119 ++++++++++++++++++++++-------
 src/topology/tplg_local.h                  |   2 +-
 7 files changed, 131 insertions(+), 58 deletions(-)
Signature · 1 line
2.5.0
author
#1 lore raw

PATCH 1/6 topology: Set manifest size for ABI

From: Mengdong Lin <[email protected]>

The topology kernel driver will check the size of manifest struct, and will stop loading topology info if size mismatch is detected.

Signed-off-by Mengdong Lin <[email protected]>
src/topology/parser.c +2 -0
index 30d91f9..84117c3 100644
@@ -414,6 +414,8 @@ snd_tplg_t *snd_tplg_new(void)
 	if (!tplg)
 		return NULL;
 
+	tplg->manifest.size = sizeof(struct snd_soc_tplg_manifest);
+
 	INIT_LIST_HEAD(&tplg->tlv_list);
 	INIT_LIST_HEAD(&tplg->widget_list);
 	INIT_LIST_HEAD(&tplg->pcm_list);
Signature · 1 line
2.5.0
author
#2 lore raw

PATCH 2/6 topology: Refactor functions to parse and build streams

In reply to [email protected]

From: Mengdong Lin <[email protected]>

Previously these functions are only used by pcm elements (front-end DAI & DAI link) to parse stream capablities. Now refactor them to be reused by back-end DAI elements later.

Signed-off-by Mengdong Lin <[email protected]>
src/topology/parser.c +1 -1
index 84117c3..f6fc944 100644
@@ -119,7 +119,7 @@ static int tplg_parse_config(snd_tplg_t *tplg, snd_config_t *cfg)
 
 		if (strcmp(id, "SectionPCMCapabilities") == 0) {
 			err = tplg_parse_compound(tplg, n,
-				tplg_parse_pcm_caps, NULL);
+				tplg_parse_stream_caps, NULL);
 			if (err < 0)
 				return err;
 			continue;
src/topology/pcm.c +34 -26

133 lines not on this page · raw message

src/topology/tplg_local.h +1 -1
index 4c601d4..9239aef 100644
@@ -210,7 +210,7 @@ int tplg_parse_dapm_graph(snd_tplg_t *tplg, snd_config_t *cfg,
 int tplg_parse_dapm_widget(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
-int tplg_parse_pcm_caps(snd_tplg_t *tplg,
+int tplg_parse_stream_caps(snd_tplg_t *tplg,
 	snd_config_t *cfg, void *private ATTRIBUTE_UNUSED);
 
 int tplg_parse_pcm(snd_tplg_t *tplg,
Signature · 1 line
2.5.0
author
#3 lore raw

PATCH 3/6 topology: Use generic pointer to realloc buffer for private data

In reply to [email protected]

From: Mengdong Lin <[email protected]>

Many element types have private data. So use the generic obj pointer instead of the type-specific pointer when reallocating the object to accommodate the private data.

Empty private data will be overlooked.

Signed-off-by Mengdong Lin <[email protected]>
src/topology/data.c +7 -17
index 19c31bf..6606ca9 100644
@@ -827,39 +827,29 @@ int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref)
 		return -EINVAL;
 
 	tplg_dbg("Data '%s' used by '%s'\n", ref->id, elem->id);
+	if (!ref->data || !ref->data->size) /* overlook empty private data */
+		return 0;
+
 	priv_data_size = ref->data->size;
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;
 
 	switch (elem->type) {
 	case SND_TPLG_TYPE_MIXER:
-		elem->mixer_ctrl = realloc(elem->mixer_ctrl,
-			elem->size + priv_data_size);
-		if (!elem->mixer_ctrl)
-			return -ENOMEM;
 		priv = &elem->mixer_ctrl->priv;
 		break;
 
 	case SND_TPLG_TYPE_ENUM:
-		elem->enum_ctrl = realloc(elem->enum_ctrl,
-			elem->size + priv_data_size);
-		if (!elem->enum_ctrl)
-			return -ENOMEM;
 		priv = &elem->enum_ctrl->priv;
 		break;
 
 	case SND_TPLG_TYPE_BYTES:
-		elem->bytes_ext = realloc(elem->bytes_ext,
-			elem->size + priv_data_size);
-		if (!elem->bytes_ext)
-			return -ENOMEM;
 		priv = &elem->bytes_ext->priv;
 		break;
 
-
 	case SND_TPLG_TYPE_DAPM_WIDGET:
-		elem->widget = realloc(elem->widget,
-			elem->size + priv_data_size);
-		if (!elem->widget)
-			return -ENOMEM;
 		priv = &elem->widget->priv;
 		break;
Signature · 1 line
2.5.0
author
#9 lore raw

PATCH 4/6 topology: Fix pcm ID & name parsing

In reply to [email protected]

From: Mengdong Lin <[email protected]>

The name and ID of SectionPCM should be set to pcm_name and pcm_id, for a front-end DAI link in the kernel, not for the front-end DAI of the link.

Signed-off-by Mengdong Lin <[email protected]>
include/sound/asoc.h +1 -1
index 920c9e0..abe49c5 100644
@@ -414,7 +414,7 @@ struct snd_soc_tplg_pcm {
 	__le32 size;		/* in bytes of this structure */
 	char pcm_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
 	char dai_name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
-	__le32 pcm_id;		/* unique ID - used to match */
+	__le32 pcm_id;		/* unique ID - used to match with DAI link */
 	__le32 dai_id;		/* unique ID - used to match */
 	__le32 playback;	/* supports playback mode */
 	__le32 capture;		/* supports capture mode */
src/topology/pcm.c +3 -3
index 1df4f54..1661821 100644
@@ -337,7 +337,7 @@ int tplg_parse_pcm(snd_tplg_t *tplg,
 
 	pcm = elem->pcm;
 	pcm->size = elem->size;
-	elem_copy_text(pcm->dai_name, elem->id, SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
+	elem_copy_text(pcm->pcm_name, elem->id, SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
 
 	tplg_dbg(" PCM: %s\n", elem->id);
 

@@ -366,8 +366,8 @@ int tplg_parse_pcm(snd_tplg_t *tplg,
 			if (snd_config_get_string(n, &val) < 0)
 				return -EINVAL;
 
-			pcm->dai_id = atoi(val);
-			tplg_dbg("\t%s: %d\n", id, pcm->dai_id);
+			pcm->pcm_id = atoi(val);
+			tplg_dbg("\t%s: %d\n", id, pcm->pcm_id);
 			continue;
 		}
Signature · 1 line
2.5.0
author
#10 lore raw

PATCH 5/6 topology: Parse front-end DAI name and ID for the PCM

In reply to [email protected]

From: Mengdong Lin <[email protected]>

These two fields are necessary to create the front-end DAIs in kernel but the support is missing in text conf previously.

Signed-off-by Mengdong Lin <[email protected]>
include/topology.h +4 -0
index b47f422..9d57ce3 100644
@@ -533,6 +533,10 @@ extern "C" {
  *
  *	id "0"				# used for binding to the PCM
  *
+ *	dai."name of front-end DAI" {
+ *		id "0"		# used for binding to the front-end DAI
+ *	}
+ *
  *	pcm."playback" {
  *		capabilities "capabilities1"	# capabilities for playback
  *
src/topology/pcm.c +53 -0

65 lines not on this page · raw message

Signature · 1 line
2.5.0
author
#11 lore raw

PATCH 6/6 topology: Update PCM configurations in Broadwell text conf file

In reply to [email protected]

From: Mengdong Lin <[email protected]>

To make this conf file a better example, update the name & ID of PCMs (front-end DAI link) and their cpu DAI (front-end DAI), same as those defined by Broadwell upstream driver.

Signed-off-by Mengdong Lin <[email protected]>
src/conf/topology/broadwell/broadwell.conf +25 -9
index 05b3889..eb89377 100644
@@ -272,18 +272,22 @@ SectionPCMCapabilities."Offload0 Playback" {
 SectionPCMCapabilities."Offload1 Playback" {
 	formats "S24_LE,S16_LE"
 	rate_min "8000"
-	rate_max "48000"
+	rate_max "192000"
 	channels_min "2"
 	channels_max "2"
 }
 
 # PCM devices exported by Firmware
-SectionPCM."System Pin" {
+SectionPCM."System Playback/Capture" {
 
 	index "1"
 
 	# used for binding to the PCM
-	ID "0"
+	id "0"
+
+	dai."System Pin" {
+		id "0"
+	}
 
 	pcm."playback" {
 

@@ -307,12 +311,16 @@ SectionPCM."System Pin" {
 	}
 }
 
-SectionPCM."Offload0 Pin" {
+SectionPCM."Offload0 Playback" {
 
 	index "1"
 
 	# used for binding to the PCM
-	ID "1"
+	id "1"
+
+	dai."Offload0 Pin" {
+		id "1"
+	}
 
 	pcm."playback" {
 

@@ -325,12 +333,16 @@ SectionPCM."Offload0 Pin" {
 	}
 }
 
-SectionPCM."Offload1 Pin" {
+SectionPCM."Offload1 Playback" {
 
 	index "1"
 
 	# used for binding to the PCM
-	ID "2"
+	id "2"
+
+	dai."Offload1 Pin" {
+		id "2"
+	}
 
 	pcm."playback" {
 

@@ -343,12 +355,16 @@ SectionPCM."Offload1 Pin" {
 	}
 }
 
-SectionPCM."Loopback Pin" {
+SectionPCM."Loopback PCM" {
 
 	index "1"
 
 	# used for binding to the PCM
-	ID "3"
+	id "3"
+
+	dai."Loopback Pin" {
+		id "3"
+	}
 
 	pcm."capture" {
Signature · 1 line
2.5.0
maintainer
#4 lore raw

Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

In reply to [email protected]

Dne 28.4.2016 v 10:41 [email protected] napsal(a):

src/topology/data.c int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) · line 837

Quoted 5 lines
src/topology/data.c +4 -0
 	priv_data_size = ref->data->size;
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;

This causes a memory leak when realloc fails. You should free the original pointer when realloc() fails.

Jaroslav

Signature · 2 lines
Jaroslav Kysela <[email protected]>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.
#5 lore raw

Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 10 lines · Jaroslav Kysela

Dne 28.4.2016 v 10:41 [email protected] napsal(a):

src/topology/data.c int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) · line 837

Quoted 5 lines
src/topology/data.c +4 -0
 	priv_data_size = ref->data->size;
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;

This causes a memory leak when realloc fails. You should free the original pointer when realloc() fails.

Right, and the bug (the leak) has been already present before the patch...

thanks,

Takashi

#6 lore raw

Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 23 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, April 28, 2016 9:55 PM
To: Jaroslav Kysela
Cc: [email protected]; [email protected]; [email protected]; Girdwood, Liam R; Lin, Mengdong; Shah, Hardik T; Singh, Guneshwor O
Subject: Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 10 lines · Jaroslav Kysela

Dne 28.4.2016 v 10:41 [email protected] napsal(a):

src/topology/data.c int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) · line 837

Quoted 5 lines
src/topology/data.c +4 -0
 	priv_data_size = ref->data->size;
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;

This causes a memory leak when realloc fails. You should free the original pointer when realloc() fails.

Quoted 2 lines

Right, and the bug (the leak) has been already present before the patch...

Okay, we'll fix this. Thanks for pointing out this.

Regards
Mengdong

author
#7 lore raw

Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 23 lines · Mengdong
Quoted 23 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, April 28, 2016 9:55 PM
To: Jaroslav Kysela
Cc: [email protected]; [email protected]; [email protected]; Girdwood, Liam R; Lin, Mengdong; Shah, Hardik T; Singh, Guneshwor O
Subject: Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 10 lines · Jaroslav Kysela

Dne 28.4.2016 v 10:41 [email protected] napsal(a):

src/topology/data.c int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) · line 837

Quoted 5 lines
  	priv_data_size = ref->data->size;
src/topology/data.c +4 -0
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;

This causes a memory leak when realloc fails. You should free the original pointer when realloc() fails.

Quoted 5 lines
Quoted 2 lines

Right, and the bug (the leak) has been already present before the patch...

Okay, we'll fix this. Thanks for pointing out this.

I fixed this issue in v2 series. Would you please have a review?

Thanks again
Mengdong

#8 lore raw

Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 27 lines · Mengdong Lin
Quoted 23 lines · Mengdong
Quoted 23 lines

-----Original Message-----
From: Takashi Iwai [mailto:[email protected]]
Sent: Thursday, April 28, 2016 9:55 PM
To: Jaroslav Kysela
Cc: [email protected]; [email protected]; [email protected]; Girdwood, Liam R; Lin, Mengdong; Shah, Hardik T; Singh, Guneshwor O
Subject: Re: [PATCH 3/6] topology: Use generic pointer to realloc buffer for private data

Quoted 10 lines · Jaroslav Kysela

Dne 28.4.2016 v 10:41 [email protected] napsal(a):

src/topology/data.c int tplg_copy_data(struct tplg_elem *elem, struct tplg_elem *ref) · line 837

Quoted 5 lines
  	priv_data_size = ref->data->size;
src/topology/data.c +4 -0
+	elem->obj = realloc(elem->obj,
+			elem->size + priv_data_size);
+	if (!elem->obj)
+		return -ENOMEM;

This causes a memory leak when realloc fails. You should free the original pointer when realloc() fails.

Quoted 7 lines
Quoted 5 lines
Quoted 2 lines

Right, and the bug (the leak) has been already present before the patch...

Okay, we'll fix this. Thanks for pointing out this.

I fixed this issue in v2 series. Would you please have a review?

Now I'm back to online. Will start reviewing the series in tomorrow or so.

thanks,

Takashi