[RFC PATCH 0/3] Add support for component devices

[RFC PATCH 0/3] Add support for component devices

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Sound cards are defined by machines. And off-soc codecs and DSPs embedded
in DSP can be taken as the components and resued by different machines/
sound cards. This series allows codec and SOC vendors to install UCM
configuration files for a specific codec or DSP, and sound card verb
files can just include these conf files of components, and trigger enable/
disable sequence defined by the component devices.

Component devices will not be exposed to applications for backward
compatibility. So audio servers like PulseAudio and CRAS still only see
the machine devices.

Mengdong Lin (3):
  ucm: Skip component directories when scanning sound card configuration
    files
  ucm: Parse sequence of component devices
  ucm: Execute sequence of component devices

 src/ucm/main.c      |  98 ++++++++++++++++++++++++++++++++++++-----
 src/ucm/parser.c    | 122 +++++++++++++++++++++++++++++++++++++++++++++++++++-
 src/ucm/ucm_local.h |  11 +++++
 src/ucm/utils.c     |   1 +
 4 files changed, 220 insertions(+), 12 deletions(-)

-- 
2.7.4

[RFC PATCH 1/3] ucm: Skip component directories when scanning sound card configuration files

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

Cards are defined by machines. DSPs embedded in SoC and off-soc codecs
can be taken as components for machines, and can be reused by different
machines/cards. Codec and SoC vendors can define their own UCM config
files. If a codec or DSP is used by a machine, the card configuration file
can include the conf file of the codec and DSP. Later patches will
complete support for this feature.

Two new directories will be used to store the UCM configuration files for
a specific codec or DSP firmware:
- /usr/share/alsa/ucm/dsps ... for DSP embedded in SoC
- /usr/share/alsa/ucm/codecs ... for off-soc codecs

These two directories will be skipped when UCM manager scans the card
directories under /usr/share/alsa/ucm.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 13f62d7..5fc98a1 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -1259,7 +1259,18 @@ static int filename_filter(const struct dirent *dirent)
 	return 0;
 }
 
-/* scan all cards and comments */
+/* scan all cards and comments
+ *
+ * Cards are defined by machines. Each card/machine installs its UCM
+ * configuration files in a subdirectory with the same name as the sound
+ * card under /usr/share/alsa/ucm. This function will scan all the card
+ * directories.
+ * Two direcotries, 'codecs' and 'dsps', are skipped in the scanning. These
+ * two directories are used to store UCM configurations file for off-soc
+ * codecs and DSPs embedded in SoC, which are components of machines.
+ * Their configuration files can be included by different machines/cards,
+ * and alsaconf will import the included files automatically.
+ */
 int uc_mgr_scan_master_configs(const char **_list[])
 {
 	char filename[MAX_FILE], dfl[MAX_FILE];
@@ -1309,6 +1320,13 @@ int uc_mgr_scan_master_configs(const char **_list[])
 	}
 
 	for (i = 0; i < cnt; i++) {
+
+		/* Skip the directories for component devices */
+		if (!strncmp(namelist[i]->d_name, "codecs", 6))
+			continue;
+		if (!strncmp(namelist[i]->d_name, "dsps", 4))
+			continue;
+
 		err = load_master_config(namelist[i]->d_name, &cfg);
 		if (err < 0)
 			goto __err;
-- 
2.7.4

[RFC PATCH 2/3] ucm: Parse sequence of component devices

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

A machine device's verb file (e.g.HiFi) can include configuration files
of component devices via alsaconf syntax <searchdir:xxx> and <file_name>.
Then the machine device's sequence can enable or disable a component
device by keyword 'enadev' and 'disdev' followed the name of the component
device.

UCM sequence parser will find the component device and mark if its enable
or disable sequence is needed by the parent, the machine device.

New element type and struct are defined for the sequence of a component
device. Component devices will be removed from the machine device list
'device_list' of a verb, since we don't want to expose them to audio
servers with original API to list devices for backward compatibility.
A new list 'cmpt_device_list' is used for the component devices of a verb.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/ucm/parser.c b/src/ucm/parser.c
index 5fc98a1..fe1dfbc 100644
--- a/src/ucm/parser.c
+++ b/src/ucm/parser.c
@@ -235,6 +235,81 @@ static int parse_device_list(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 	return 0;
 }
 
+/* find a component device by its name
+ *
+ * Component devices are defined by machine components (usually off-soc
+ * codes or DSP embeded in SoC). Since alsaconf imports their configuration
+ * files automatically, we don't know which devices are component devices
+ * until they are referenced by a machine device sequence. So here when we
+ * find a referenced device, we move it from the machine device list to the
+ * component device list. Component devices will not be exposed to applications
+ * by the original API to list devices for backward compatibility. So audio
+ * servers can only see the machine devices.
+ */
+struct use_case_device *find_component_dev(snd_use_case_mgr_t *uc_mgr,
+	const char *name)
+{
+	struct list_head *pos, *posdev, *_posdev;
+	struct use_case_verb *verb;
+	struct use_case_device *dev;
+
+	list_for_each(pos, &uc_mgr->verb_list) {
+		verb = list_entry(pos, struct use_case_verb, list);
+
+		/* search in the component device list */
+		list_for_each(posdev, &verb->cmpt_device_list) {
+			dev = list_entry(posdev, struct use_case_device, list);
+			if (!strcmp(dev->name, name))
+				return dev;
+		}
+
+		/* search the machine device list */
+		list_for_each_safe(posdev, _posdev, &verb->device_list) {
+			dev = list_entry(posdev, struct use_case_device, list);
+			if (!strcmp(dev->name, name)) {
+				/* find the component device, move it from the
+				 * machine device list to the component device
+				 * list.
+				 */
+				list_del(&dev->list);
+				list_add_tail(&dev->list,
+					      &verb->cmpt_device_list);
+				return dev;
+			}
+		}
+	}
+
+	return NULL;
+}
+
+/* parse sequence of a component device
+ *
+ * This function will find the component device and mark if its enable or
+ * disable sequence is needed by its parenet device.
+ */
+static int parse_component_seq(snd_use_case_mgr_t *uc_mgr,
+			  snd_config_t *n, int enable,
+			  struct component_sequence *cmpt_seq)
+{
+	const char *val;
+	int err;
+
+	err = snd_config_get_string(n, &val);
+	if (err < 0)
+		return err;
+
+	cmpt_seq->device = find_component_dev(uc_mgr, val);
+	if (!cmpt_seq->device) {
+		uc_error("error: Cannot find component device %s", val);
+		return -EINVAL;
+	}
+
+	/* Parent needs its enable or disable sequence */
+	cmpt_seq->enable = enable;
+
+	return 0;
+}
+
 /*
  * Parse sequences.
  *
@@ -249,7 +324,7 @@ static int parse_device_list(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
  *	cset "name='Master Playback Switch' 0,0"
  *      cset "iface=PCM,name='Disable HDMI',index=1 0"
  */
-static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
+static int parse_sequence(snd_use_case_mgr_t *uc_mgr,
 			  struct list_head *base,
 			  snd_config_t *cfg)
 {
@@ -306,6 +381,30 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
 			continue;
 		}
 
+		if (strcmp(cmd, "enadev") == 0) {
+			/* need to enable a component device */
+			curr->type = SEQUENCE_ELEMENT_TYPE_CMPT_SEQ;
+			err = parse_component_seq(uc_mgr, n, 1,
+						&curr->data.cmpt_seq);
+			if (err < 0) {
+				uc_error("error: enadev requires a valid device!");
+				return err;
+			}
+			continue;
+		}
+
+		if (strcmp(cmd, "disdev") == 0) {
+			/* need to disable a component device */
+			curr->type = SEQUENCE_ELEMENT_TYPE_CMPT_SEQ;
+			err = parse_component_seq(uc_mgr, n, 0,
+						&curr->data.cmpt_seq);
+			if (err < 0) {
+				uc_error("error: disdev requires a valid device!");
+				return err;
+			}
+			continue;
+		}
+
 		if (strcmp(cmd, "cset-bin-file") == 0) {
 			curr->type = SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE;
 			err = parse_string(n, &curr->data.cset);
@@ -938,6 +1037,7 @@ static int parse_verb_file(snd_use_case_mgr_t *uc_mgr,
 	INIT_LIST_HEAD(&verb->disable_list);
 	INIT_LIST_HEAD(&verb->transition_list);
 	INIT_LIST_HEAD(&verb->device_list);
+	INIT_LIST_HEAD(&verb->cmpt_device_list);
 	INIT_LIST_HEAD(&verb->modifier_list);
 	INIT_LIST_HEAD(&verb->value_list);
 	list_add_tail(&verb->list, &uc_mgr->verb_list);
diff --git a/src/ucm/ucm_local.h b/src/ucm/ucm_local.h
index b89de2a..3bfdd67 100644
--- a/src/ucm/ucm_local.h
+++ b/src/ucm/ucm_local.h
@@ -49,6 +49,7 @@
 #define SEQUENCE_ELEMENT_TYPE_EXEC	4
 #define SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE	5
 #define SEQUENCE_ELEMENT_TYPE_CSET_TLV	6
+#define SEQUENCE_ELEMENT_TYPE_CMPT_SEQ	7
 
 struct ucm_value {
         struct list_head list;
@@ -56,6 +57,12 @@ struct ucm_value {
         char *data;
 };
 
+/* sequence of a component device */
+struct component_sequence {
+	struct use_case_device *device; /* component device */
+	int enable; /* flag to choose enable or disable list of the device */
+};
+
 struct sequence_element {
 	struct list_head list;
 	unsigned int type;
@@ -64,6 +71,7 @@ struct sequence_element {
 		char *cdev;
 		char *cset;
 		char *exec;
+		struct component_sequence cmpt_seq; /* component sequence */
 	} data;
 };
 
@@ -167,6 +175,9 @@ struct use_case_verb {
 	/* hardware devices that can be used with this use case */
 	struct list_head device_list;
 
+	/* component device list */
+	struct list_head cmpt_device_list;
+
 	/* modifiers that can be used with this use case */
 	struct list_head modifier_list;
 
diff --git a/src/ucm/utils.c b/src/ucm/utils.c
index 45307b0..0fba85a 100644
--- a/src/ucm/utils.c
+++ b/src/ucm/utils.c
@@ -210,6 +210,7 @@ void uc_mgr_free_verb(snd_use_case_mgr_t *uc_mgr)
 		uc_mgr_free_transition(&verb->transition_list);
 		uc_mgr_free_value(&verb->value_list);
 		uc_mgr_free_device(&verb->device_list);
+		uc_mgr_free_device(&verb->cmpt_device_list);
 		uc_mgr_free_modifier(&verb->modifier_list);
 		list_del(&verb->list);
 		free(verb);
-- 
2.7.4

[RFC PATCH 3/3] ucm: Execute sequence of component devices

[mengdong.lin]

From: Mengdong Lin <mengdong.lin@linux.intel.com>

A machine device's sequence can enable or disable a component device. So
when executing a machine device's sequence, the enable or disable sequence
of its component devices will also be excecuted.

Add a parameter 'cdev_in' to function execute_sequence(). This function
is used to execute a sequence of either machine or component devices.
Since the sequence of a component device does not define the card device,
when executing its sequence, this parameter can pass the cdev defined by
the sequence of its parent, the machine device. When executing sequence of
machine devices, this parameter should be set to NULL.

Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>

diff --git a/src/ucm/main.c b/src/ucm/main.c
index 8cc9208..a852cf4 100644
--- a/src/ucm/main.c
+++ b/src/ucm/main.c
@@ -48,6 +48,13 @@ static int get_value3(char **value,
 		      struct list_head *value_list2,
 		      struct list_head *value_list3);
 
+static int execute_component_seq(snd_use_case_mgr_t *uc_mgr,
+				 struct component_sequence *cmpt_seq,
+				 struct list_head *value_list1,
+				 struct list_head *value_list2,
+				 struct list_head *value_list3,
+				 char *cdev_in);
+
 static int check_identifier(const char *identifier, const char *prefix)
 {
 	int len;
@@ -341,13 +348,22 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type)
  * \brief Execute the sequence
  * \param uc_mgr Use case manager
  * \param seq Sequence
+ * \param cdev_in input cdev,  parenet's cdev for a component device,
+ *				or NULL for a machine device.
  * \return zero on success, otherwise a negative error code
+ *
+ * A machine device's sequence can enable or disable a component device.
+ * But the enable/disable sequence of a component device does not define
+ * cdev, the card device. So when executing a component device's sequence,
+ * we need to pass the cdev defined by the sequence of its parent, the
+ * machine device. And for machine device, cdev should be set to NULL.
  */
 static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 			    struct list_head *seq,
 			    struct list_head *value_list1,
 			    struct list_head *value_list2,
-			    struct list_head *value_list3)
+			    struct list_head *value_list3,
+			    char *cdev_in)
 {
 	struct list_head *pos;
 	struct sequence_element *s;
@@ -366,7 +382,16 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 		case SEQUENCE_ELEMENT_TYPE_CSET:
 		case SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE:
 		case SEQUENCE_ELEMENT_TYPE_CSET_TLV:
-			if (cdev == NULL) {
+			if (cdev == NULL && cdev_in) {
+				/* Sequence of a component device, should use
+				 * the input cdev defined by sequence of its
+				 * parent, the machine device.
+				 */
+				cdev = strdup(cdev_in);
+				if (cdev == NULL)
+					goto __fail_nomem;
+
+			} else if (cdev == NULL) {
 				char *playback_ctl = NULL;
 				char *capture_ctl = NULL;
 
@@ -405,7 +430,9 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 					free(capture_ctl);
 				} else
 					cdev = capture_ctl;
+
 			}
+
 			if (ctl == NULL) {
 				err = open_ctl(uc_mgr, &ctl, cdev);
 				if (err < 0) {
@@ -427,6 +454,19 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 			if (err < 0)
 				goto __fail;
 			break;
+		case SEQUENCE_ELEMENT_TYPE_CMPT_SEQ:
+			/* Execute enable or disable sequence of a component
+			 * device. Pass the cdev defined by the machine device.
+			 */
+			err = execute_component_seq(uc_mgr,
+							&s->data.cmpt_seq,
+							value_list1,
+							value_list2,
+							value_list3,
+							cdev);
+			if (err < 0)
+				goto __fail;
+			break;
 		default:
 			uc_error("unknown sequence command %i", s->type);
 			break;
@@ -442,6 +482,39 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
 
 }
 
+/* Execute enable or disable sequence of a component device.
+ *
+ * For a component device (a codec or embedded DSP), its sequence doesn't
+ * specify the sound card device 'cdev', because a component can be reused
+ * by different sound cards (machines). So when executing its sequence, a
+ * parameter 'cdev_in' is used to pass cdev defined by the sequence of its
+ * parent, the machine device.
+ */
+static int execute_component_seq(snd_use_case_mgr_t *uc_mgr,
+				 struct component_sequence *cmpt_seq,
+				 struct list_head *value_list1,
+				 struct list_head *value_list2,
+				 struct list_head *value_list3,
+				 char *cdev_in)
+{
+	struct use_case_device *device = cmpt_seq->device;
+	struct list_head *seq;
+	int err;
+
+
+	if (cmpt_seq->enable)
+		seq = &device->enable_list;
+	else
+		seq = &device->disable_list;
+
+	err = execute_sequence(uc_mgr, seq,
+			       &device->value_list,
+			       &uc_mgr->active_verb->value_list,
+			       &uc_mgr->value_list,
+			       cdev_in);
+	return err;
+}
+
 /**
  * \brief Import master config and execute the default sequence
  * \param uc_mgr Use case manager
@@ -455,7 +528,7 @@ static int import_master_config(snd_use_case_mgr_t *uc_mgr)
 	if (err < 0)
 		return err;
 	err = execute_sequence(uc_mgr, &uc_mgr->default_list,
-			       &uc_mgr->value_list, NULL, NULL);
+			       &uc_mgr->value_list, NULL, NULL, NULL);
 	if (err < 0)
 		uc_error("Unable to execute default sequence");
 	return err;
@@ -761,7 +834,7 @@ static int set_verb(snd_use_case_mgr_t *uc_mgr,
 	err = execute_sequence(uc_mgr, seq,
 			       &verb->value_list,
 			       &uc_mgr->value_list,
-			       NULL);
+			       NULL, NULL);
 	if (enable && err >= 0)
 		uc_mgr->active_verb = verb;
 	return err;
@@ -792,7 +865,8 @@ static int set_modifier(snd_use_case_mgr_t *uc_mgr,
 	err = execute_sequence(uc_mgr, seq,
 			       &modifier->value_list,
 			       &uc_mgr->active_verb->value_list,
-			       &uc_mgr->value_list);
+			       &uc_mgr->value_list,
+			       NULL);
 	if (enable && err >= 0) {
 		list_add_tail(&modifier->active_list, &uc_mgr->active_modifiers);
 	} else if (!enable) {
@@ -826,7 +900,8 @@ static int set_device(snd_use_case_mgr_t *uc_mgr,
 	err = execute_sequence(uc_mgr, seq,
 			       &device->value_list,
 			       &uc_mgr->active_verb->value_list,
-			       &uc_mgr->value_list);
+			       &uc_mgr->value_list,
+			       NULL);
 	if (enable && err >= 0) {
 		list_add_tail(&device->active_list, &uc_mgr->active_devices);
 	} else if (!enable) {
@@ -953,7 +1028,7 @@ static int dismantle_use_case(snd_use_case_mgr_t *uc_mgr)
 	uc_mgr->active_verb = NULL;
 
 	err = execute_sequence(uc_mgr, &uc_mgr->default_list,
-			       &uc_mgr->value_list, NULL, NULL);
+			       &uc_mgr->value_list, NULL, NULL, NULL);
 	
 	return err;
 }
@@ -969,7 +1044,7 @@ int snd_use_case_mgr_reset(snd_use_case_mgr_t *uc_mgr)
 
 	pthread_mutex_lock(&uc_mgr->mutex);
 	err = execute_sequence(uc_mgr, &uc_mgr->default_list,
-			       &uc_mgr->value_list, NULL, NULL);
+			       &uc_mgr->value_list, NULL, NULL, NULL);
 	INIT_LIST_HEAD(&uc_mgr->active_modifiers);
 	INIT_LIST_HEAD(&uc_mgr->active_devices);
 	uc_mgr->active_verb = NULL;
@@ -1578,6 +1653,7 @@ static int handle_transition_verb(snd_use_case_mgr_t *uc_mgr,
                         err = execute_sequence(uc_mgr, &trans->transition_list,
 					       &uc_mgr->active_verb->value_list,
 					       &uc_mgr->value_list,
+					       NULL,
 					       NULL);
                         if (err >= 0)
                                 return 1;
@@ -1689,7 +1765,8 @@ static int switch_device(snd_use_case_mgr_t *uc_mgr,
                         err = execute_sequence(uc_mgr, &trans->transition_list,
 					       &xold->value_list,
 					       &uc_mgr->active_verb->value_list,
-					       &uc_mgr->value_list);
+					       &uc_mgr->value_list,
+					       NULL);
                         if (err >= 0) {
                                 list_del(&xold->active_list);
                                 list_add_tail(&xnew->active_list, &uc_mgr->active_devices);
@@ -1741,7 +1818,8 @@ static int switch_modifier(snd_use_case_mgr_t *uc_mgr,
                         err = execute_sequence(uc_mgr, &trans->transition_list,
 					       &xold->value_list,
 					       &uc_mgr->active_verb->value_list,
-					       &uc_mgr->value_list);
+					       &uc_mgr->value_list,
+					       NULL);
                         if (err >= 0) {
                                 list_del(&xold->active_list);
                                 list_add_tail(&xnew->active_list, &uc_mgr->active_modifiers);
-- 
2.7.4

Re: [RFC PATCH 1/3] ucm: Skip component directories when scanning sound card configuration files

[Liam Girdwood]

On Tue, 2016-11-15 at 16:02 +0800, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> Cards are defined by machines. DSPs embedded in SoC and off-soc codecs
> can be taken as components for machines, and can be reused by different
> machines/cards. Codec and SoC vendors can define their own UCM config
> files. If a codec or DSP is used by a machine, the card configuration file
> can include the conf file of the codec and DSP. Later patches will
> complete support for this feature.
> 
> Two new directories will be used to store the UCM configuration files for
> a specific codec or DSP firmware:
> - /usr/share/alsa/ucm/dsps ... for DSP embedded in SoC
> - /usr/share/alsa/ucm/codecs ... for off-soc codecs
> 
> These two directories will be skipped when UCM manager scans the card
> directories under /usr/share/alsa/ucm.
> 
> Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> diff --git a/src/ucm/parser.c b/src/ucm/parser.c
> index 13f62d7..5fc98a1 100644
> --- a/src/ucm/parser.c
> +++ b/src/ucm/parser.c
> @@ -1259,7 +1259,18 @@ static int filename_filter(const struct dirent *dirent)
>  	return 0;
>  }
>  
> -/* scan all cards and comments */
> +/* scan all cards and comments
> + *
> + * Cards are defined by machines. Each card/machine installs its UCM
> + * configuration files in a subdirectory with the same name as the sound
> + * card under /usr/share/alsa/ucm. This function will scan all the card
> + * directories.
> + * Two direcotries, 'codecs' and 'dsps', are skipped in the scanning. These
> + * two directories are used to store UCM configurations file for off-soc
> + * codecs and DSPs embedded in SoC, which are components of machines.
> + * Their configuration files can be included by different machines/cards,
> + * and alsaconf will import the included files automatically.
> + */
>  int uc_mgr_scan_master_configs(const char **_list[])
>  {
>  	char filename[MAX_FILE], dfl[MAX_FILE];
> @@ -1309,6 +1320,13 @@ int uc_mgr_scan_master_configs(const char **_list[])
>  	}
>  
>  	for (i = 0; i < cnt; i++) {
> +
> +		/* Skip the directories for component devices */
> +		if (!strncmp(namelist[i]->d_name, "codecs", 6))
> +			continue;
> +		if (!strncmp(namelist[i]->d_name, "dsps", 4))
> +			continue;

Maybe we should have an array of strings that denote directories that
contain non master components, that way we could easily add other
directories for components.

Liam 

Re: [RFC PATCH 2/3] ucm: Parse sequence of component devices

[Liam Girdwood]

On Tue, 2016-11-15 at 16:02 +0800, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> A machine device's verb file (e.g.HiFi) can include configuration files
> of component devices via alsaconf syntax <searchdir:xxx> and <file_name>.
> Then the machine device's sequence can enable or disable a component
> device by keyword 'enadev' and 'disdev' followed the name of the component
> device.
> 
> UCM sequence parser will find the component device and mark if its enable
> or disable sequence is needed by the parent, the machine device.
> 
> New element type and struct are defined for the sequence of a component
> device. Component devices will be removed from the machine device list
> 'device_list' of a verb, since we don't want to expose them to audio
> servers with original API to list devices for backward compatibility.
> A new list 'cmpt_device_list' is used for the component devices of a verb.
> 
> Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> 



> @@ -306,6 +381,30 @@ static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,
>  			continue;
>  		}
>  
> +		if (strcmp(cmd, "enadev") == 0) {
> +			/* need to enable a component device */
> +			curr->type = SEQUENCE_ELEMENT_TYPE_CMPT_SEQ;
> +			err = parse_component_seq(uc_mgr, n, 1,
> +						&curr->data.cmpt_seq);
> +			if (err < 0) {
> +				uc_error("error: enadev requires a valid device!");
> +				return err;
> +			}
> +			continue;
> +		}
> +
> +		if (strcmp(cmd, "disdev") == 0) {

Do we need to document enadev and disdev anywhere ?

Liam

Re: [RFC PATCH 3/3] ucm: Execute sequence of component devices

[Liam Girdwood]

On Tue, 2016-11-15 at 16:02 +0800, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> A machine device's sequence can enable or disable a component device. So
> when executing a machine device's sequence, the enable or disable sequence
> of its component devices will also be excecuted.
> 
> Add a parameter 'cdev_in' to function execute_sequence(). This function
> is used to execute a sequence of either machine or component devices.
> Since the sequence of a component device does not define the card device,
> when executing its sequence, this parameter can pass the cdev defined by
> the sequence of its parent, the machine device. When executing sequence of
> machine devices, this parameter should be set to NULL.
> 
> Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> diff --git a/src/ucm/main.c b/src/ucm/main.c
> index 8cc9208..a852cf4 100644
> --- a/src/ucm/main.c
> +++ b/src/ucm/main.c
> @@ -48,6 +48,13 @@ static int get_value3(char **value,
>  		      struct list_head *value_list2,
>  		      struct list_head *value_list3);
>  
> +static int execute_component_seq(snd_use_case_mgr_t *uc_mgr,
> +				 struct component_sequence *cmpt_seq,
> +				 struct list_head *value_list1,
> +				 struct list_head *value_list2,
> +				 struct list_head *value_list3,
> +				 char *cdev_in);
> +
>  static int check_identifier(const char *identifier, const char *prefix)
>  {
>  	int len;
> @@ -341,13 +348,22 @@ static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type)
>   * \brief Execute the sequence
>   * \param uc_mgr Use case manager
>   * \param seq Sequence
> + * \param cdev_in input cdev,  parenet's cdev for a component device,
> + *				or NULL for a machine device.
>   * \return zero on success, otherwise a negative error code
> + *
> + * A machine device's sequence can enable or disable a component device.
> + * But the enable/disable sequence of a component device does not define
> + * cdev, the card device. So when executing a component device's sequence,
> + * we need to pass the cdev defined by the sequence of its parent, the
> + * machine device. And for machine device, cdev should be set to NULL.
>   */
>  static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
>  			    struct list_head *seq,
>  			    struct list_head *value_list1,
>  			    struct list_head *value_list2,
> -			    struct list_head *value_list3)
> +			    struct list_head *value_list3,
> +			    char *cdev_in)

Could the current cdev be embedded in uc_mgr ? 

>  {
>  	struct list_head *pos;
>  	struct sequence_element *s;
> @@ -366,7 +382,16 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
>  		case SEQUENCE_ELEMENT_TYPE_CSET:
>  		case SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE:
>  		case SEQUENCE_ELEMENT_TYPE_CSET_TLV:
> -			if (cdev == NULL) {
> +			if (cdev == NULL && cdev_in) {
> +				/* Sequence of a component device, should use
> +				 * the input cdev defined by sequence of its
> +				 * parent, the machine device.
> +				 */
> +				cdev = strdup(cdev_in);
> +				if (cdev == NULL)
> +					goto __fail_nomem;
> +
> +			} else if (cdev == NULL) {
>  				char *playback_ctl = NULL;
>  				char *capture_ctl = NULL;
>  
> @@ -405,7 +430,9 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
>  					free(capture_ctl);
>  				} else
>  					cdev = capture_ctl;
> +
>  			}
> +

Looks like this formatting change was added by mistake ?

>  			if (ctl == NULL) {
>  				err = open_ctl(uc_mgr, &ctl, cdev);
>  				if (err < 0) {
> @@ -427,6 +454,19 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
>  			if (err < 0)
>  				goto __fail;
>  			break;
> +		case SEQUENCE_ELEMENT_TYPE_CMPT_SEQ:
> +			/* Execute enable or disable sequence of a component
> +			 * device. Pass the cdev defined by the machine device.
> +			 */
> +			err = execute_component_seq(uc_mgr,
> +							&s->data.cmpt_seq,
> +							value_list1,
> +							value_list2,
> +							value_list3,
> +							cdev);
> +			if (err < 0)
> +				goto __fail;
> +			break;
>  		default:
>  			uc_error("unknown sequence command %i", s->type);
>  			break;
> @@ -442,6 +482,39 @@ static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
>  
>  }
>  
> +/* Execute enable or disable sequence of a component device.
> + *
> + * For a component device (a codec or embedded DSP), its sequence doesn't
> + * specify the sound card device 'cdev', because a component can be reused
> + * by different sound cards (machines). So when executing its sequence, a
> + * parameter 'cdev_in' is used to pass cdev defined by the sequence of its
> + * parent, the machine device.
> + */
> +static int execute_component_seq(snd_use_case_mgr_t *uc_mgr,
> +				 struct component_sequence *cmpt_seq,
> +				 struct list_head *value_list1,
> +				 struct list_head *value_list2,
> +				 struct list_head *value_list3,
> +				 char *cdev_in)
> +{
> +	struct use_case_device *device = cmpt_seq->device;
> +	struct list_head *seq;
> +	int err;
> +
> +
> +	if (cmpt_seq->enable)
> +		seq = &device->enable_list;
> +	else
> +		seq = &device->disable_list;

What happens here if there is only an enable sequence and no disable
sequence ? i.e. will seq be NULL ?

> +
> +	err = execute_sequence(uc_mgr, seq,
> +			       &device->value_list,
> +			       &uc_mgr->active_verb->value_list,
> +			       &uc_mgr->value_list,
> +			       cdev_in);
> +	return err;
> +}
> +
>  /**
>   * \brief Import master config and execute the default sequence
>   * \param uc_mgr Use case manager
> @@ -455,7 +528,7 @@ static int import_master_config(snd_use_case_mgr_t *uc_mgr)
>  	if (err < 0)
>  		return err;
>  	err = execute_sequence(uc_mgr, &uc_mgr->default_list,
> -			       &uc_mgr->value_list, NULL, NULL);
> +			       &uc_mgr->value_list, NULL, NULL, NULL);
>  	if (err < 0)
>  		uc_error("Unable to execute default sequence");
>  	return err;
> @@ -761,7 +834,7 @@ static int set_verb(snd_use_case_mgr_t *uc_mgr,
>  	err = execute_sequence(uc_mgr, seq,
>  			       &verb->value_list,
>  			       &uc_mgr->value_list,
> -			       NULL);
> +			       NULL, NULL);
>  	if (enable && err >= 0)
>  		uc_mgr->active_verb = verb;
>  	return err;
> @@ -792,7 +865,8 @@ static int set_modifier(snd_use_case_mgr_t *uc_mgr,
>  	err = execute_sequence(uc_mgr, seq,
>  			       &modifier->value_list,
>  			       &uc_mgr->active_verb->value_list,
> -			       &uc_mgr->value_list);
> +			       &uc_mgr->value_list,
> +			       NULL);
>  	if (enable && err >= 0) {
>  		list_add_tail(&modifier->active_list, &uc_mgr->active_modifiers);
>  	} else if (!enable) {
> @@ -826,7 +900,8 @@ static int set_device(snd_use_case_mgr_t *uc_mgr,
>  	err = execute_sequence(uc_mgr, seq,
>  			       &device->value_list,
>  			       &uc_mgr->active_verb->value_list,
> -			       &uc_mgr->value_list);
> +			       &uc_mgr->value_list,
> +			       NULL);
>  	if (enable && err >= 0) {
>  		list_add_tail(&device->active_list, &uc_mgr->active_devices);
>  	} else if (!enable) {
> @@ -953,7 +1028,7 @@ static int dismantle_use_case(snd_use_case_mgr_t *uc_mgr)
>  	uc_mgr->active_verb = NULL;
>  
>  	err = execute_sequence(uc_mgr, &uc_mgr->default_list,
> -			       &uc_mgr->value_list, NULL, NULL);
> +			       &uc_mgr->value_list, NULL, NULL, NULL);
>  	
>  	return err;
>  }
> @@ -969,7 +1044,7 @@ int snd_use_case_mgr_reset(snd_use_case_mgr_t *uc_mgr)
>  
>  	pthread_mutex_lock(&uc_mgr->mutex);
>  	err = execute_sequence(uc_mgr, &uc_mgr->default_list,
> -			       &uc_mgr->value_list, NULL, NULL);
> +			       &uc_mgr->value_list, NULL, NULL, NULL);
>  	INIT_LIST_HEAD(&uc_mgr->active_modifiers);
>  	INIT_LIST_HEAD(&uc_mgr->active_devices);
>  	uc_mgr->active_verb = NULL;
> @@ -1578,6 +1653,7 @@ static int handle_transition_verb(snd_use_case_mgr_t *uc_mgr,
>                          err = execute_sequence(uc_mgr, &trans->transition_list,
>  					       &uc_mgr->active_verb->value_list,
>  					       &uc_mgr->value_list,
> +					       NULL,
>  					       NULL);
>                          if (err >= 0)
>                                  return 1;
> @@ -1689,7 +1765,8 @@ static int switch_device(snd_use_case_mgr_t *uc_mgr,
>                          err = execute_sequence(uc_mgr, &trans->transition_list,
>  					       &xold->value_list,
>  					       &uc_mgr->active_verb->value_list,
> -					       &uc_mgr->value_list);
> +					       &uc_mgr->value_list,
> +					       NULL);
>                          if (err >= 0) {
>                                  list_del(&xold->active_list);
>                                  list_add_tail(&xnew->active_list, &uc_mgr->active_devices);
> @@ -1741,7 +1818,8 @@ static int switch_modifier(snd_use_case_mgr_t *uc_mgr,
>                          err = execute_sequence(uc_mgr, &trans->transition_list,
>  					       &xold->value_list,
>  					       &uc_mgr->active_verb->value_list,
> -					       &uc_mgr->value_list);
> +					       &uc_mgr->value_list,
> +					       NULL);
>                          if (err >= 0) {
>                                  list_del(&xold->active_list);
>                                  list_add_tail(&xnew->active_list, &uc_mgr->active_modifiers);

Re: [RESEND RFC PATCH 0/3] ucm: Add support for component devices

[Liam Girdwood]

On Tue, 2016-11-15 at 16:01 +0800, mengdong.lin@linux.intel.com wrote:
> From: Mengdong Lin <mengdong.lin@linux.intel.com>
> 
> Sound cards are defined by machines. And off-soc codecs and DSPs embedded
> in DSP can be taken as the components and resued by different machines/
> sound cards. This series allows codec and SOC vendors to install UCM
> configuration files for a specific codec or DSP, and sound card verb
> files can just include these conf files of components, and trigger enable/
> disable sequence defined by the component devices.
> 
> Component devices will not be exposed to applications for backward
> compatibility. So audio servers like PulseAudio and CRAS still only see
> the machine devices.

The intention here is that the sound server will load the UCM
configuration based on the sound card name in []. I have patches to
improve this for Intel based machines which tend to all use the platform
name....sending shortly.

Liam

Re: [RFC PATCH 1/3] ucm: Skip component directories when scanning sound card configuration files

[Lin, Mengdong]

> -----Original Message-----
> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
> Sent: Tuesday, November 15, 2016 4:46 PM
 
> On Tue, 2016-11-15 at 16:02 +0800, mengdong.lin@linux.intel.com wrote:
> > From: Mengdong Lin <mengdong.lin@linux.intel.com>
> >
> > Cards are defined by machines. DSPs embedded in SoC and off-soc codecs
> > can be taken as components for machines, and can be reused by
> > different machines/cards. Codec and SoC vendors can define their own
> > UCM config files. If a codec or DSP is used by a machine, the card
> > configuration file can include the conf file of the codec and DSP.
> > Later patches will complete support for this feature.
> >
> > Two new directories will be used to store the UCM configuration files
> > for a specific codec or DSP firmware:
> > - /usr/share/alsa/ucm/dsps ... for DSP embedded in SoC
> > - /usr/share/alsa/ucm/codecs ... for off-soc codecs
> >
> > These two directories will be skipped when UCM manager scans the card
> > directories under /usr/share/alsa/ucm.
> >
> > Signed-off-by: Mengdong Lin <mengdong.lin@linux.intel.com>
> >
> > diff --git a/src/ucm/parser.c b/src/ucm/parser.c index
> > 13f62d7..5fc98a1 100644
> > --- a/src/ucm/parser.c
> > +++ b/src/ucm/parser.c
> > @@ -1259,7 +1259,18 @@ static int filename_filter(const struct dirent
> *dirent)
> >  	return 0;
> >  }
> >
> > -/* scan all cards and comments */
> > +/* scan all cards and comments
> > + *
> > + * Cards are defined by machines. Each card/machine installs its UCM
> > + * configuration files in a subdirectory with the same name as the
> > +sound
> > + * card under /usr/share/alsa/ucm. This function will scan all the
> > +card
> > + * directories.
> > + * Two direcotries, 'codecs' and 'dsps', are skipped in the scanning.
> > +These
> > + * two directories are used to store UCM configurations file for
> > +off-soc
> > + * codecs and DSPs embedded in SoC, which are components of machines.
> > + * Their configuration files can be included by different
> > +machines/cards,
> > + * and alsaconf will import the included files automatically.
> > + */
> >  int uc_mgr_scan_master_configs(const char **_list[])  {
> >  	char filename[MAX_FILE], dfl[MAX_FILE]; @@ -1309,6 +1320,13 @@
> int
> > uc_mgr_scan_master_configs(const char **_list[])
> >  	}
> >
> >  	for (i = 0; i < cnt; i++) {
> > +
> > +		/* Skip the directories for component devices */
> > +		if (!strncmp(namelist[i]->d_name, "codecs", 6))
> > +			continue;
> > +		if (!strncmp(namelist[i]->d_name, "dsps", 4))
> > +			continue;
> 
> Maybe we should have an array of strings that denote directories that
> contain non master components, that way we could easily add other
> directories for components.
> 
> Liam

Yes, an array will be nice. I'll revise the patch.

Thanks for your review!
Mengdong

Re: [RFC PATCH 2/3] ucm: Parse sequence of component devices

[Lin, Mengdong]

> -----Original Message-----
> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
> Sent: Tuesday, November 15, 2016 4:46 PM
 
> > @@ -306,6 +381,30 @@ static int parse_sequence(snd_use_case_mgr_t
> *uc_mgr ATTRIBUTE_UNUSED,
> >  			continue;
> >  		}
> >
> > +		if (strcmp(cmd, "enadev") == 0) {
> > +			/* need to enable a component device */
> > +			curr->type = SEQUENCE_ELEMENT_TYPE_CMPT_SEQ;
> > +			err = parse_component_seq(uc_mgr, n, 1,
> > +						&curr->data.cmpt_seq);
> > +			if (err < 0) {
> > +				uc_error("error: enadev requires a valid
> device!");
> > +				return err;
> > +			}
> > +			continue;
> > +		}
> > +
> > +		if (strcmp(cmd, "disdev") == 0) {
> 
> Do we need to document enadev and disdev anywhere ?
> 
> Liam

Maybe we could document this in the description of function parse_sequence() and parse_verb()?
parse_verb() has an example of EnableSequence and DisableSequence.

Thanks
Mengdong

Re: [RFC PATCH 3/3] ucm: Execute sequence of component devices

[Lin, Mengdong]

> -----Original Message-----
> From: Liam Girdwood [mailto:liam.r.girdwood@linux.intel.com]
> Sent: Tuesday, November 15, 2016 4:50 PM

> > @@ -341,13 +348,22 @@ static int execute_cset(snd_ctl_t *ctl, const char
> *cset, unsigned int type)
> >   * \brief Execute the sequence
> >   * \param uc_mgr Use case manager
> >   * \param seq Sequence
> > + * \param cdev_in input cdev,  parenet's cdev for a component device,
> > + *				or NULL for a machine device.
> >   * \return zero on success, otherwise a negative error code
> > + *
> > + * A machine device's sequence can enable or disable a component device.
> > + * But the enable/disable sequence of a component device does not
> > + define
> > + * cdev, the card device. So when executing a component device's
> > + sequence,
> > + * we need to pass the cdev defined by the sequence of its parent,
> > + the
> > + * machine device. And for machine device, cdev should be set to NULL.
> >   */
> >  static int execute_sequence(snd_use_case_mgr_t *uc_mgr,
> >  			    struct list_head *seq,
> >  			    struct list_head *value_list1,
> >  			    struct list_head *value_list2,
> > -			    struct list_head *value_list3)
> > +			    struct list_head *value_list3,
> > +			    char *cdev_in)
> 
> Could the current cdev be embedded in uc_mgr ?

Yes, it could and there will be less code change. 
Actually I found the machine devices always set the same cdev name in their sequences.

> > @@ -366,7 +382,16 @@ static int execute_sequence(snd_use_case_mgr_t
> *uc_mgr,
> >  		case SEQUENCE_ELEMENT_TYPE_CSET:
> >  		case SEQUENCE_ELEMENT_TYPE_CSET_BIN_FILE:
> >  		case SEQUENCE_ELEMENT_TYPE_CSET_TLV:
> > -			if (cdev == NULL) {
> > +			if (cdev == NULL && cdev_in) {
> > +				/* Sequence of a component device, should
> use
> > +				 * the input cdev defined by sequence of its
> > +				 * parent, the machine device.
> > +				 */
> > +				cdev = strdup(cdev_in);
> > +				if (cdev == NULL)
> > +					goto __fail_nomem;
> > +
> > +			} else if (cdev == NULL) {
> >  				char *playback_ctl = NULL;
> >  				char *capture_ctl = NULL;
> >
> > @@ -405,7 +430,9 @@ static int execute_sequence(snd_use_case_mgr_t
> *uc_mgr,
> >  					free(capture_ctl);
> >  				} else
> >  					cdev = capture_ctl;
> > +
> >  			}
> > +
> 
> Looks like this formatting change was added by mistake ?

Yes, it was added by mistake. I'll remove it.

> > +/* Execute enable or disable sequence of a component device.
> > + *
> > + * For a component device (a codec or embedded DSP), its sequence
> > +doesn't
> > + * specify the sound card device 'cdev', because a component can be
> > +reused
> > + * by different sound cards (machines). So when executing its
> > +sequence, a
> > + * parameter 'cdev_in' is used to pass cdev defined by the sequence
> > +of its
> > + * parent, the machine device.
> > + */
> > +static int execute_component_seq(snd_use_case_mgr_t *uc_mgr,
> > +				 struct component_sequence *cmpt_seq,
> > +				 struct list_head *value_list1,
> > +				 struct list_head *value_list2,
> > +				 struct list_head *value_list3,
> > +				 char *cdev_in)
> > +{
> > +	struct use_case_device *device = cmpt_seq->device;
> > +	struct list_head *seq;
> > +	int err;
> > +
> > +
> > +	if (cmpt_seq->enable)
> > +		seq = &device->enable_list;
> > +	else
> > +		seq = &device->disable_list;
> 
> What happens here if there is only an enable sequence and no disable
> sequence ? i.e. will seq be NULL ?

The device's enable_list or disable_list will be empty, assured by the UCM parser.
And so 'seq' will be an empty list and the following execute_sequence() will exit without doing anything.
> 
> > +
> > +	err = execute_sequence(uc_mgr, seq,
> > +			       &device->value_list,
> > +			       &uc_mgr->active_verb->value_list,
> > +			       &uc_mgr->value_list,
> > +			       cdev_in);
> > +	return err;
> > +}
> > +

Thanks
Mengdong