Skip to contentSkip to conversation

Add support for component devices

RFC PATCH Superseded [email protected] 3 participants 11 messages Nov 15, 2016 – Nov 16, 2016

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

[email protected]

author
#0 lore raw

cover Add support for component devices

From: Mengdong Lin <[email protected]>

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(-)
Signature · 1 line
2.7.4
author
#1 lore raw

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

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/parser.c +19 -1
index 13f62d7..5fc98a1 100644
@@ -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;
Signature · 1 line
2.7.4
author
#4 lore raw

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

In reply to [email protected]

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/parser.c +101 -1
index 5fc98a1..fe1dfbc 100644
@@ -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);
src/ucm/ucm_local.h +11 -0
index b89de2a..3bfdd67 100644
@@ -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;
src/ucm/utils.c +1 -0
index 45307b0..0fba85a 100644
@@ -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);
Signature · 1 line
2.7.4
author
#7 lore raw

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

In reply to [email protected]

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/main.c +88 -10
index 8cc9208..a852cf4 100644
@@ -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);
Signature · 1 line
2.7.4
#2 lore raw

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

In reply to [email protected]
Quoted 44 lines · On Tue, 2016-11-15 at 16:02 +0800, [email protected] wrote:

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/parser.c +12 -1
index 13f62d7..5fc98a1 100644
@@ -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];

src/ucm/parser.c int uc_mgr_scan_master_configs(const char **_list[]) · line 1328

Quoted 9 lines
src/ucm/parser.c +6 -0
@@ -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

#5 lore raw

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

In reply to [email protected]
Quoted 29 lines · On Tue, 2016-11-15 at 16:02 +0800, [email protected] wrote:

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/parser.c +3 -0
@@ -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;

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, · line 396

Quoted 10 lines
src/ucm/parser.c +10 -0
+			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

#8 lore raw

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

In reply to [email protected]
Quoted 44 lines · On Tue, 2016-11-15 at 16:02 +0800, [email protected] wrote:

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/main.c +13 -0
index 8cc9208..a852cf4 100644
@@ -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,

src/ucm/main.c static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type) · line 366

Quoted 10 lines
src/ucm/main.c +4 -1
+ * 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 ?

Quoted 22 lines
src/ucm/main.c +10 -1
 {
 	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;

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr, · line 435

Quoted 6 lines
src/ucm/main.c +2 -0
@@ -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 ?

Quoted 41 lines
src/ucm/main.c +27 -0
 			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)

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr, · line 508

Quoted 10 lines
src/ucm/main.c +10 -0
+{
+	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 ?

Quoted 96 lines
src/ucm/main.c +22 -8
+
+	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);
#10 lore raw

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

In reply to [email protected]
Quoted 9 lines · On Tue, 2016-11-15 at 16:01 +0800, [email protected] wrote:

From: Mengdong Lin <[email protected]>

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.

Quoted 3 lines

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

#3 lore raw

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

In reply to Liam Girdwood
Quoted 69 lines

-----Original Message-----
From: Liam Girdwood [mailto:[email protected]]
Sent: Tuesday, November 15, 2016 4:46 PM

src/ucm/parser.c static int filename_filter(const struct dirent

Quoted 24 lines · On Tue, 2016-11-15 at 16:02 +0800, [email protected] wrote:

From: Mengdong Lin <[email protected]>

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 <[email protected]>
src/ucm/parser.c index renamed from src/ucm/parser.c +0 -0

13f62d7..5fc98a1 100644

src/ucm/parser.c +0 -0
@@ -1259,7 +1259,18 @@ static int filename_filter(const struct dirent

*dirent)

Quoted 13 lines
src/ucm/parser.c +9 -1
 	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.

src/ucm/parser.c static int filename_filter(const struct dirent

Quoted 10 lines
src/ucm/parser.c +8 -0
+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

src/ucm/parser.c int uc_mgr_scan_master_configs(const char **_list[]) · line 1328

Quoted 10 lines

uc_mgr_scan_master_configs(const char **_list[])

src/ucm/parser.c +6 -0
 	}
 
 	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.

Quoted 1 line

Liam

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

Thanks for your review!
Mengdong

#6 lore raw

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

In reply to Liam Girdwood
Quoted 26 lines

-----Original Message-----
From: Liam Girdwood [mailto:[email protected]]
Sent: Tuesday, November 15, 2016 4:46 PM

Review static int parse_sequence(snd_use_case_mgr_t

Quoted 1 line
(quoted diff) +0 -0
@@ -306,6 +381,30 @@ static int parse_sequence(snd_use_case_mgr_t

*uc_mgr ATTRIBUTE_UNUSED,

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED,

Quoted 10 lines
src/ucm/parser.c +7 -0
 			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!");

src/ucm/parser.c static int parse_sequence(snd_use_case_mgr_t *uc_mgr ATTRIBUTE_UNUSED, · line 396

Quoted 6 lines
src/ucm/parser.c +6 -0
+				return err;
+			}
+			continue;
+		}
+
+		if (strcmp(cmd, "disdev") == 0) {

Do we need to document enadev and disdev anywhere ?

Quoted 1 line

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

#9 lore raw

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

In reply to Liam Girdwood
Quoted 23 lines

-----Original Message-----
From: Liam Girdwood [mailto:[email protected]]
Sent: Tuesday, November 15, 2016 4:50 PM

Review static int execute_cset(snd_ctl_t *ctl, const char

Quoted 1 line
(quoted diff) +0 -0
@@ -341,13 +348,22 @@ static int execute_cset(snd_ctl_t *ctl, const char

*cset, unsigned int type)

Quoted 17 lines
src/ucm/main.c +11 -0
  * \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,
Quoted 8 lines

src/ucm/main.c static int execute_cset(snd_ctl_t *ctl, const char *cset, unsigned int type) · line 366

Quoted 6 lines
src/ucm/main.c +2 -1
 			    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.

Quoted 22 lines

Review static int execute_sequence(snd_use_case_mgr_t

Quoted 1 line
(quoted diff) +0 -0
@@ -366,7 +382,16 @@ static int execute_sequence(snd_use_case_mgr_t

*uc_mgr,

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr,

Quoted 6 lines
src/ucm/main.c +2 -1
 		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

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t

Quoted 12 lines
src/ucm/main.c +8 -0
+				 * 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,

Quoted 8 lines

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr, · line 435

Quoted 6 lines
src/ucm/main.c +2 -0
 					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.

Quoted 23 lines
Quoted 23 lines
src/ucm/main.c +23 -0
+/* 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;
+
Quoted 8 lines

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr, · line 508

Quoted 5 lines
src/ucm/main.c +5 -0
+
+	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.

Quoted 10 lines

src/ucm/main.c static int execute_sequence(snd_use_case_mgr_t *uc_mgr, · line 517

Quoted 10 lines
src/ucm/main.c +9 -0
+
+	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