ASoC: Changes to SOF kcontrol data set/get ops

Merge series from Ranjani Sridharan <ranjani.sridharan@linux.intel.com>:

This set of patches deals with modifications to the signature of kcontrol
get/set data functions to make them more intuitive. The last patch deals
with initializing the binary control data size after boot up.

sound/soc/sof/control.c

@@ -69,7 +69,6 @@ static void snd_sof_refresh_control(struct snd_sof_control *scontrol)
 {
 	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
 	struct snd_soc_component *scomp = scontrol->scomp;
-	u32 ipc_cmd;
 	int ret;
 
 	if (!scontrol->comp_data_dirty)
@@ -78,20 +77,13 @@ static void snd_sof_refresh_control(struct snd_sof_control *scontrol)
 	if (!pm_runtime_active(scomp->dev))
 		return;
 
-	if (scontrol->cmd == SOF_CTRL_CMD_BINARY)
-		ipc_cmd = SOF_IPC_COMP_GET_DATA;
-	else
-		ipc_cmd = SOF_IPC_COMP_GET_VALUE;
-
 	/* set the ABI header values */
 	cdata->data->magic = SOF_ABI_MAGIC;
 	cdata->data->abi = SOF_ABI_VERSION;
 
 	/* refresh the component data from DSP */
 	scontrol->comp_data_dirty = false;
-	ret = snd_sof_ipc_set_get_comp_data(scontrol, ipc_cmd,
-					    SOF_CTRL_TYPE_VALUE_CHAN_GET,
-					    scontrol->cmd, false);
+	ret = snd_sof_ipc_set_get_comp_data(scontrol, false);
 	if (ret < 0) {
 		dev_err(scomp->dev, "error: failed to get control data: %d\n", ret);
 		/* Set the flag to re-try next time to get the data */
@@ -142,11 +134,7 @@ int snd_sof_volume_put(struct snd_kcontrol *kcontrol,
 
 	/* notify DSP of mixer updates */
 	if (pm_runtime_active(scomp->dev))
-		snd_sof_ipc_set_get_comp_data(scontrol,
-					      SOF_IPC_COMP_SET_VALUE,
-					      SOF_CTRL_TYPE_VALUE_CHAN_SET,
-					      SOF_CTRL_CMD_VOLUME,
-					      true);
+		snd_sof_ipc_set_get_comp_data(scontrol, true);
 	return change;
 }
 
@@ -215,11 +203,7 @@ int snd_sof_switch_put(struct snd_kcontrol *kcontrol,
 
 	/* notify DSP of mixer updates */
 	if (pm_runtime_active(scomp->dev))
-		snd_sof_ipc_set_get_comp_data(scontrol,
-					      SOF_IPC_COMP_SET_VALUE,
-					      SOF_CTRL_TYPE_VALUE_CHAN_SET,
-					      SOF_CTRL_CMD_SWITCH,
-					      true);
+		snd_sof_ipc_set_get_comp_data(scontrol, true);
 
 	return change;
 }
@@ -264,11 +248,7 @@ int snd_sof_enum_put(struct snd_kcontrol *kcontrol,
 
 	/* notify DSP of enum updates */
 	if (pm_runtime_active(scomp->dev))
-		snd_sof_ipc_set_get_comp_data(scontrol,
-					      SOF_IPC_COMP_SET_VALUE,
-					      SOF_CTRL_TYPE_VALUE_CHAN_SET,
-					      SOF_CTRL_CMD_ENUM,
-					      true);
+		snd_sof_ipc_set_get_comp_data(scontrol, true);
 
 	return change;
 }
@@ -342,11 +322,7 @@ int snd_sof_bytes_put(struct snd_kcontrol *kcontrol,
 
 	/* notify DSP of byte control updates */
 	if (pm_runtime_active(scomp->dev))
-		snd_sof_ipc_set_get_comp_data(scontrol,
-					      SOF_IPC_COMP_SET_DATA,
-					      SOF_CTRL_TYPE_DATA_SET,
-					      scontrol->cmd,
-					      true);
+		snd_sof_ipc_set_get_comp_data(scontrol, true);
 
 	return 0;
 }
@@ -391,7 +367,7 @@ int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
 	}
 
 	/* Check that header id matches the command */
-	if (header.numid != scontrol->cmd) {
+	if (header.numid != cdata->cmd) {
 		dev_err_ratelimited(scomp->dev,
 				    "error: incorrect numid %d\n",
 				    header.numid);
@@ -422,11 +398,7 @@ int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
 
 	/* notify DSP of byte control updates */
 	if (pm_runtime_active(scomp->dev))
-		snd_sof_ipc_set_get_comp_data(scontrol,
-					      SOF_IPC_COMP_SET_DATA,
-					      SOF_CTRL_TYPE_DATA_SET,
-					      scontrol->cmd,
-					      true);
+		snd_sof_ipc_set_get_comp_data(scontrol, true);
 
 	return 0;
 }
@@ -463,8 +435,7 @@ int snd_sof_bytes_ext_volatile_get(struct snd_kcontrol *kcontrol, unsigned int _
 	cdata->data->magic = SOF_ABI_MAGIC;
 	cdata->data->abi = SOF_ABI_VERSION;
 	/* get all the component data from DSP */
-	ret = snd_sof_ipc_set_get_comp_data(scontrol, SOF_IPC_COMP_GET_DATA, SOF_CTRL_TYPE_DATA_GET,
-					    scontrol->cmd, false);
+	ret = snd_sof_ipc_set_get_comp_data(scontrol, false);
 	if (ret < 0)
 		goto out;
 
@@ -485,7 +456,7 @@ int snd_sof_bytes_ext_volatile_get(struct snd_kcontrol *kcontrol, unsigned int _
 		goto out;
 	}
 
-	header.numid = scontrol->cmd;
+	header.numid = cdata->cmd;
 	header.length = data_size;
 	if (copy_to_user(tlvd, &header, sizeof(struct snd_ctl_tlv))) {
 		ret = -EFAULT;
@@ -545,7 +516,7 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
 	if (data_size > size)
 		return -ENOSPC;
 
-	header.numid = scontrol->cmd;
+	header.numid = cdata->cmd;
 	header.length = data_size;
 	if (copy_to_user(tlvd, &header, sizeof(struct snd_ctl_tlv)))
 		return -EFAULT;
@@ -600,6 +571,13 @@ void snd_sof_control_notify(struct snd_sof_dev *sdev,
 	bool found = false;
 	int i, type;
 
+	if (cdata->type == SOF_CTRL_TYPE_VALUE_COMP_GET ||
+	    cdata->type == SOF_CTRL_TYPE_VALUE_COMP_SET) {
+		dev_err(sdev->dev,
+			"Component data is not supported in control notification\n");
+		return;
+	}
+
 	/* Find the swidget first */
 	list_for_each_entry(swidget, &sdev->widget_list, list) {
 		if (swidget->comp_id == cdata->comp_id) {
@@ -666,11 +644,6 @@ void snd_sof_control_notify(struct snd_sof_dev *sdev,
 		expected_size += cdata->num_elems *
 				 sizeof(struct sof_ipc_ctrl_value_chan);
 		break;
-	case SOF_CTRL_TYPE_VALUE_COMP_GET:
-	case SOF_CTRL_TYPE_VALUE_COMP_SET:
-		expected_size += cdata->num_elems *
-				 sizeof(struct sof_ipc_ctrl_value_comp);
-		break;
 	case SOF_CTRL_TYPE_DATA_GET:
 	case SOF_CTRL_TYPE_DATA_SET:
 		expected_size += cdata->num_elems + sizeof(struct sof_abi_hdr);

sound/soc/sof/ipc.c

@@ -721,11 +721,6 @@ static int sof_get_ctrl_copy_params(enum sof_ipc_ctrl_type ctrl_type,
 		sparams->src = (u8 *)src->chanv;
 		sparams->dst = (u8 *)dst->chanv;
 		break;
-	case SOF_CTRL_TYPE_VALUE_COMP_GET:
-	case SOF_CTRL_TYPE_VALUE_COMP_SET:
-		sparams->src = (u8 *)src->compv;
-		sparams->dst = (u8 *)dst->compv;
-		break;
 	case SOF_CTRL_TYPE_DATA_GET:
 	case SOF_CTRL_TYPE_DATA_SET:
 		sparams->src = (u8 *)src->data->data;
@@ -745,7 +740,7 @@ static int sof_get_ctrl_copy_params(enum sof_ipc_ctrl_type ctrl_type,
 static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev,
 				       struct sof_ipc_ctrl_data *cdata,
 				       struct sof_ipc_ctrl_data_params *sparams,
-				       bool send)
+				       bool set)
 {
 	struct sof_ipc_ctrl_data *partdata;
 	size_t send_bytes;
@@ -760,7 +755,7 @@ static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev,
 	if (!partdata)
 		return -ENOMEM;
 
-	if (send)
+	if (set)
 		err = sof_get_ctrl_copy_params(cdata->type, cdata, partdata,
 					       sparams);
 	else
@@ -789,7 +784,7 @@ static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev,
 		msg_bytes -= send_bytes;
 		partdata->elems_remaining = msg_bytes;
 
-		if (send)
+		if (set)
 			memcpy(sparams->dst, sparams->src + offset, send_bytes);
 
 		err = sof_ipc_tx_message_unlocked(sdev->ipc,
@@ -801,7 +796,7 @@ static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev,
 		if (err < 0)
 			break;
 
-		if (!send)
+		if (!set)
 			memcpy(sparams->dst + offset, sparams->src, send_bytes);
 
 		offset += pl_size;
@@ -816,11 +811,7 @@ static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev,
 /*
  * IPC get()/set() for kcontrols.
  */
-int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
-				  u32 ipc_cmd,
-				  enum sof_ipc_ctrl_type ctrl_type,
-				  enum sof_ipc_ctrl_cmd ctrl_cmd,
-				  bool send)
+int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol, bool set)
 {
 	struct snd_soc_component *scomp = scontrol->scomp;
 	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
@@ -828,9 +819,11 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 	struct sof_ipc_fw_ready *ready = &sdev->fw_ready;
 	struct sof_ipc_fw_version *v = &ready->version;
 	struct sof_ipc_ctrl_data_params sparams;
+	enum sof_ipc_ctrl_type ctrl_type;
 	struct snd_sof_widget *swidget;
 	bool widget_found = false;
 	size_t send_bytes;
+	u32 ipc_cmd;
 	int err;
 
 	list_for_each_entry(swidget, &sdev->widget_list, list) {
@@ -858,7 +851,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 		/* write/read value header via mmaped region */
 		send_bytes = sizeof(struct sof_ipc_ctrl_value_chan) *
 		cdata->num_elems;
-		if (send)
+		if (set)
 			err = snd_sof_dsp_block_write(sdev, SOF_FW_BLK_TYPE_IRAM,
 						      scontrol->readback_offset,
 						      cdata->chanv, send_bytes);
@@ -870,12 +863,25 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 
 		if (err)
 			dev_err_once(sdev->dev, "error: %s TYPE_IRAM failed\n",
-				     send ? "write to" :  "read from");
+				     set ? "write to" :  "read from");
 		return err;
 	}
 
+	/*
+	 * Select the IPC cmd and the ctrl_type based on the ctrl_cmd and the
+	 * direction
+	 * Note: SOF_CTRL_TYPE_VALUE_COMP_* is not used and supported currently
+	 *	 for ctrl_type
+	 */
+	if (cdata->cmd == SOF_CTRL_CMD_BINARY) {
+		ipc_cmd = set ? SOF_IPC_COMP_SET_DATA : SOF_IPC_COMP_GET_DATA;
+		ctrl_type = set ? SOF_CTRL_TYPE_DATA_SET : SOF_CTRL_TYPE_DATA_GET;
+	} else {
+		ipc_cmd = set ? SOF_IPC_COMP_SET_VALUE : SOF_IPC_COMP_GET_VALUE;
+		ctrl_type = set ? SOF_CTRL_TYPE_VALUE_CHAN_SET : SOF_CTRL_TYPE_VALUE_CHAN_GET;
+	}
+
 	cdata->rhdr.hdr.cmd = SOF_IPC_GLB_COMP_MSG | ipc_cmd;
-	cdata->cmd = ctrl_cmd;
 	cdata->type = ctrl_type;
 	cdata->comp_id = scontrol->comp_id;
 	cdata->msg_index = 0;
@@ -889,13 +895,6 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 		sparams.hdr_bytes = sizeof(struct sof_ipc_ctrl_data);
 		sparams.elems = scontrol->num_channels;
 		break;
-	case SOF_CTRL_TYPE_VALUE_COMP_GET:
-	case SOF_CTRL_TYPE_VALUE_COMP_SET:
-		sparams.msg_bytes = scontrol->num_channels *
-			sizeof(struct sof_ipc_ctrl_value_comp);
-		sparams.hdr_bytes = sizeof(struct sof_ipc_ctrl_data);
-		sparams.elems = scontrol->num_channels;
-		break;
 	case SOF_CTRL_TYPE_DATA_GET:
 	case SOF_CTRL_TYPE_DATA_SET:
 		sparams.msg_bytes = cdata->data->size;
@@ -934,7 +933,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
 		return -EINVAL;
 	}
 
-	err = sof_set_get_large_ctrl_data(sdev, cdata, &sparams, send);
+	err = sof_set_get_large_ctrl_data(sdev, cdata, &sparams, set);
 
 	if (err < 0)
 		dev_err(sdev->dev, "error: set/get large ctrl ipc comp %d\n",

sound/soc/sof/sof-audio.c

@@ -14,29 +14,12 @@
 
 static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control *scontrol)
 {
-	int ipc_cmd, ctrl_type;
 	int ret;
 
 	/* reset readback offset for scontrol */
 	scontrol->readback_offset = 0;
 
-	/* notify DSP of kcontrol values */
-	switch (scontrol->cmd) {
-	case SOF_CTRL_CMD_VOLUME:
-	case SOF_CTRL_CMD_ENUM:
-	case SOF_CTRL_CMD_SWITCH:
-		ipc_cmd = SOF_IPC_COMP_SET_VALUE;
-		ctrl_type = SOF_CTRL_TYPE_VALUE_CHAN_SET;
-		break;
-	case SOF_CTRL_CMD_BINARY:
-		ipc_cmd = SOF_IPC_COMP_SET_DATA;
-		ctrl_type = SOF_CTRL_TYPE_DATA_SET;
-		break;
-	default:
-		return 0;
-	}
-
-	ret = snd_sof_ipc_set_get_comp_data(scontrol, ipc_cmd, ctrl_type, scontrol->cmd, true);
+	ret = snd_sof_ipc_set_get_comp_data(scontrol, true);
 	if (ret < 0)
 		dev_err(sdev->dev, "error: failed kcontrol value set for widget: %d\n",
 			scontrol->comp_id);
@@ -76,12 +59,26 @@ static int sof_widget_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_wi
 	/* set up all controls for the widget */
 	list_for_each_entry(scontrol, &sdev->kcontrol_list, list)
 		if (scontrol->comp_id == swidget->comp_id) {
+			/* set kcontrol data in DSP */
 			ret = sof_kcontrol_setup(sdev, scontrol);
 			if (ret < 0) {
 				dev_err(sdev->dev, "error: fail to set up kcontrols for widget %s\n",
 					swidget->widget->name);
 				return ret;
 			}
+
+			/*
+			 * Read back the data from the DSP for static widgets. This is particularly
+			 * useful for binary kcontrols associated with static pipeline widgets to
+			 * initialize the data size to match that in the DSP.
+			 */
+			if (swidget->dynamic_pipeline_widget)
+				continue;
+
+			ret = snd_sof_ipc_set_get_comp_data(scontrol, false);
+			if (ret < 0)
+				dev_warn(sdev->dev, "Failed kcontrol get for control in widget %s\n",
+					 swidget->widget->name);
 		}
 
 	return 0;

sound/soc/sof/sof-audio.h

@@ -74,7 +74,6 @@ struct snd_sof_control {
 	u32 readback_offset; /* offset to mmapped data if used */
 	struct sof_ipc_ctrl_data *control_data;
 	u32 size;	/* cdata size */
-	enum sof_ipc_ctrl_cmd cmd;
 	u32 *volume_table; /* volume table computed from tlv data*/
 
 	struct list_head list;	/* list in sdev control list */
@@ -239,11 +238,7 @@ static inline void snd_sof_compr_init_elapsed_work(struct work_struct *work) { }
 /*
  * Mixer IPC
  */
-int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol,
-				  u32 ipc_cmd,
-				  enum sof_ipc_ctrl_type ctrl_type,
-				  enum sof_ipc_ctrl_cmd ctrl_cmd,
-				  bool send);
+int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol, bool set);
 
 /* DAI link fixup */
 int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params);

sound/soc/sof/topology.c

@@ -1090,11 +1090,11 @@ static int sof_control_load_volume(struct snd_soc_component *scomp,
 
 	/* set cmd for mixer control */
 	if (le32_to_cpu(mc->max) == 1) {
-		scontrol->cmd = SOF_CTRL_CMD_SWITCH;
+		scontrol->control_data->cmd = SOF_CTRL_CMD_SWITCH;
 		goto skip;
 	}
 
-	scontrol->cmd = SOF_CTRL_CMD_VOLUME;
+	scontrol->control_data->cmd = SOF_CTRL_CMD_VOLUME;
 
 	/* extract tlv data */
 	if (!kc->tlv.p || get_tlv_data(kc->tlv.p, tlv) < 0) {
@@ -1165,7 +1165,7 @@ static int sof_control_load_enum(struct snd_soc_component *scomp,
 	scontrol->comp_id = sdev->next_comp_id;
 	scontrol->num_channels = le32_to_cpu(ec->num_channels);
 	scontrol->control_data->index = kc->index;
-	scontrol->cmd = SOF_CTRL_CMD_ENUM;
+	scontrol->control_data->cmd = SOF_CTRL_CMD_ENUM;
 
 	dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d comp_id %d\n",
 		scontrol->comp_id, scontrol->num_channels, scontrol->comp_id);
@@ -1211,7 +1211,7 @@ static int sof_control_load_bytes(struct snd_soc_component *scomp,
 	}
 
 	scontrol->comp_id = sdev->next_comp_id;
-	scontrol->cmd = SOF_CTRL_CMD_BINARY;
+	scontrol->control_data->cmd = SOF_CTRL_CMD_BINARY;
 	scontrol->control_data->index = kc->index;
 
 	dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d\n",
@@ -2076,7 +2076,7 @@ static int sof_get_control_data(struct snd_soc_component *scomp,
 		*size += wdata[i].pdata->size;
 
 		/* get data type */
-		switch (wdata[i].control->cmd) {
+		switch (wdata[i].control->control_data->cmd) {
 		case SOF_CTRL_CMD_VOLUME:
 		case SOF_CTRL_CMD_ENUM:
 		case SOF_CTRL_CMD_SWITCH: