[PATCH 0/8] Changes to SOF kcontrol data set/get ops
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.
Peter Ujfalusi (7): ASoC: SOF: ipc: Rename send parameter in snd_sof_ipc_set_get_comp_data() ASoC: SOF: Drop ipc_cmd parameter for snd_sof_ipc_set_get_comp_data() ASoC: SOF: topology: Set control_data->cmd alongside scontrol->cmd ASoC: SOF: Drop ctrl_cmd parameter for snd_sof_ipc_set_get_comp_data() ASoC: SOF: sof-audio: Drop the `cmd` member from struct snd_sof_control ASoC: SOF: control: Do not handle control notification with component type ASoC: SOF: Drop ctrl_type parameter for snd_sof_ipc_set_get_comp_data()
Ranjani Sridharan (1): ASoC: SOF: topology: read back control data from DSP
sound/soc/sof/control.c | 61 +++++++++++---------------------------- sound/soc/sof/ipc.c | 49 +++++++++++++++---------------- sound/soc/sof/sof-audio.c | 33 ++++++++++----------- sound/soc/sof/sof-audio.h | 7 +---- sound/soc/sof/topology.c | 10 +++---- 5 files changed, 62 insertions(+), 98 deletions(-)
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
Rename the send parameter to set in snd_sof_ipc_set_get_comp_data() and sof_set_get_large_ctrl_data() to be more aligned with the function name.
No functional change.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/ipc.c | 17 ++++++++--------- sound/soc/sof/sof-audio.h | 3 +-- 2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 6771b444065d..670d780241a3 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -745,7 +745,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 +760,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 +789,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 +801,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; @@ -819,8 +819,7 @@ static int sof_set_get_large_ctrl_data(struct snd_sof_dev *sdev, 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) + enum sof_ipc_ctrl_cmd ctrl_cmd, bool set) { struct snd_soc_component *scomp = scontrol->scomp; struct sof_ipc_ctrl_data *cdata = scontrol->control_data; @@ -858,7 +857,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,7 +869,7 @@ 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; }
@@ -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", diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index e419e7082c28..1c1d68e220d5 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -242,8 +242,7 @@ static inline void snd_sof_compr_init_elapsed_work(struct work_struct *work) { } 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); + enum sof_ipc_ctrl_cmd ctrl_cmd, bool set);
/* DAI link fixup */ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params);
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
The correct ipc_cmd can be selected based on the `ctrl_cmd` and the `set` parameters: if the ctrl_cmd is SOF_CTRL_CMD_BINARY then SOF_IPC_COMP_*_DATA otherwise SOF_IPC_COMP_*_VALUE.
The SET or GET direction can be selected with the use of `set` parameter.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/control.c | 15 ++------------- sound/soc/sof/ipc.c | 8 +++++++- sound/soc/sof/sof-audio.c | 6 ++---- sound/soc/sof/sof-audio.h | 1 - 4 files changed, 11 insertions(+), 19 deletions(-)
diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c index bb1dfe4f6d40..299ee466625e 100644 --- a/sound/soc/sof/control.c +++ b/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,18 +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, + ret = snd_sof_ipc_set_get_comp_data(scontrol, SOF_CTRL_TYPE_VALUE_CHAN_GET, scontrol->cmd, false); if (ret < 0) { @@ -143,7 +137,6 @@ 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); @@ -216,7 +209,6 @@ 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); @@ -265,7 +257,6 @@ 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); @@ -343,7 +334,6 @@ 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); @@ -423,7 +413,6 @@ 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); @@ -463,7 +452,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, + ret = snd_sof_ipc_set_get_comp_data(scontrol, SOF_CTRL_TYPE_DATA_GET, scontrol->cmd, false); if (ret < 0) goto out; diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 670d780241a3..bcfe7edee05e 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -817,7 +817,6 @@ 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 set) { @@ -830,6 +829,7 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol, 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) { @@ -873,6 +873,12 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol, return err; }
+ /* Select the IPC cmd based on the ctrl_cmd and the direction */ + if (ctrl_cmd == SOF_CTRL_CMD_BINARY) + ipc_cmd = set ? SOF_IPC_COMP_SET_DATA : SOF_IPC_COMP_GET_DATA; + else + ipc_cmd = set ? SOF_IPC_COMP_SET_VALUE : SOF_IPC_COMP_GET_VALUE; + cdata->rhdr.hdr.cmd = SOF_IPC_GLB_COMP_MSG | ipc_cmd; cdata->cmd = ctrl_cmd; cdata->type = ctrl_type; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 58a62bfb16ab..dacc0122c3b4 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -14,7 +14,7 @@
static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control *scontrol) { - int ipc_cmd, ctrl_type; + enum sof_ipc_ctrl_type ctrl_type; int ret;
/* reset readback offset for scontrol */ @@ -25,18 +25,16 @@ static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control * 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, ctrl_type, scontrol->cmd, true); if (ret < 0) dev_err(sdev->dev, "error: failed kcontrol value set for widget: %d\n", scontrol->comp_id); diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 1c1d68e220d5..f4316cd742a7 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -240,7 +240,6 @@ 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 set);
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
Set the scontrol->control_data->cmd early to the same as scontrol->cmd.
This is a preparatory patch to remove the ctrl_cmd parameter for the snd_sof_ipc_set_get_comp_data() function.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/topology.c | 4 ++++ 1 file changed, 4 insertions(+)
diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index b3ad3a604918..c440e1c53ca5 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1091,10 +1091,12 @@ 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 = scontrol->cmd; goto skip; }
scontrol->cmd = SOF_CTRL_CMD_VOLUME; + scontrol->control_data->cmd = scontrol->cmd;
/* extract tlv data */ if (!kc->tlv.p || get_tlv_data(kc->tlv.p, tlv) < 0) { @@ -1166,6 +1168,7 @@ static int sof_control_load_enum(struct snd_soc_component *scomp, 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 = scontrol->cmd;
dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d comp_id %d\n", scontrol->comp_id, scontrol->num_channels, scontrol->comp_id); @@ -1212,6 +1215,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 = scontrol->cmd; scontrol->control_data->index = kc->index;
dev_dbg(scomp->dev, "tplg: load kcontrol index %d chans %d\n",
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
The scontrol->control_data->cmd has been configured during initialization to the correct sof_ipc_ctrl_cmd.
No need to pass duplicated information, let's use the already available one via scontrol.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/control.c | 26 +++++++------------------- sound/soc/sof/ipc.c | 6 ++---- sound/soc/sof/sof-audio.c | 2 +- sound/soc/sof/sof-audio.h | 3 +-- 4 files changed, 11 insertions(+), 26 deletions(-)
diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c index 299ee466625e..23a916ea93f8 100644 --- a/sound/soc/sof/control.c +++ b/sound/soc/sof/control.c @@ -84,8 +84,7 @@ static void snd_sof_refresh_control(struct snd_sof_control *scontrol) /* refresh the component data from DSP */ scontrol->comp_data_dirty = false; ret = snd_sof_ipc_set_get_comp_data(scontrol, - SOF_CTRL_TYPE_VALUE_CHAN_GET, - scontrol->cmd, false); + SOF_CTRL_TYPE_VALUE_CHAN_GET, 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 */ @@ -137,9 +136,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_CTRL_TYPE_VALUE_CHAN_SET, - SOF_CTRL_CMD_VOLUME, - true); + SOF_CTRL_TYPE_VALUE_CHAN_SET, true); return change; }
@@ -209,9 +206,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_CTRL_TYPE_VALUE_CHAN_SET, - SOF_CTRL_CMD_SWITCH, - true); + SOF_CTRL_TYPE_VALUE_CHAN_SET, true);
return change; } @@ -257,9 +252,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_CTRL_TYPE_VALUE_CHAN_SET, - SOF_CTRL_CMD_ENUM, - true); + SOF_CTRL_TYPE_VALUE_CHAN_SET, true);
return change; } @@ -334,9 +327,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_CTRL_TYPE_DATA_SET, - scontrol->cmd, - true); + SOF_CTRL_TYPE_DATA_SET, true);
return 0; } @@ -413,9 +404,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_CTRL_TYPE_DATA_SET, - scontrol->cmd, - true); + SOF_CTRL_TYPE_DATA_SET, true);
return 0; } @@ -452,8 +441,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_CTRL_TYPE_DATA_GET, - scontrol->cmd, false); + ret = snd_sof_ipc_set_get_comp_data(scontrol, SOF_CTRL_TYPE_DATA_GET, false); if (ret < 0) goto out;
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index bcfe7edee05e..69c8a9964960 100644 --- a/sound/soc/sof/ipc.c +++ b/sound/soc/sof/ipc.c @@ -817,8 +817,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, - enum sof_ipc_ctrl_type ctrl_type, - enum sof_ipc_ctrl_cmd ctrl_cmd, bool set) + enum sof_ipc_ctrl_type ctrl_type, bool set) { struct snd_soc_component *scomp = scontrol->scomp; struct sof_ipc_ctrl_data *cdata = scontrol->control_data; @@ -874,13 +873,12 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol, }
/* Select the IPC cmd based on the ctrl_cmd and the direction */ - if (ctrl_cmd == SOF_CTRL_CMD_BINARY) + if (cdata->cmd == SOF_CTRL_CMD_BINARY) ipc_cmd = set ? SOF_IPC_COMP_SET_DATA : SOF_IPC_COMP_GET_DATA; else ipc_cmd = set ? SOF_IPC_COMP_SET_VALUE : SOF_IPC_COMP_GET_VALUE;
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; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index dacc0122c3b4..269eca26eab9 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -34,7 +34,7 @@ static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control * return 0; }
- ret = snd_sof_ipc_set_get_comp_data(scontrol, ctrl_type, scontrol->cmd, true); + ret = snd_sof_ipc_set_get_comp_data(scontrol, ctrl_type, true); if (ret < 0) dev_err(sdev->dev, "error: failed kcontrol value set for widget: %d\n", scontrol->comp_id); diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index f4316cd742a7..5bcc842e4792 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -240,8 +240,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, - enum sof_ipc_ctrl_type ctrl_type, - enum sof_ipc_ctrl_cmd ctrl_cmd, bool set); + enum sof_ipc_ctrl_type ctrl_type, bool set);
/* DAI link fixup */ int sof_pcm_dai_link_fixup(struct snd_soc_pcm_runtime *rtd, struct snd_pcm_hw_params *params);
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
There is no need to use two variables to store and check the same information, the scontrol->cmd is the same as scontrol->control_data->cmd.
Drop the former one and when it is needed, access the cmd from the control_data.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/control.c | 6 +++--- sound/soc/sof/sof-audio.c | 2 +- sound/soc/sof/sof-audio.h | 1 - sound/soc/sof/topology.c | 14 +++++--------- 4 files changed, 9 insertions(+), 14 deletions(-)
diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c index 23a916ea93f8..9297b29d65cd 100644 --- a/sound/soc/sof/control.c +++ b/sound/soc/sof/control.c @@ -372,7 +372,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); @@ -462,7 +462,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; @@ -522,7 +522,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; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 269eca26eab9..4530c6ed34e0 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -21,7 +21,7 @@ static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control * scontrol->readback_offset = 0;
/* notify DSP of kcontrol values */ - switch (scontrol->cmd) { + switch (scontrol->control_data->cmd) { case SOF_CTRL_CMD_VOLUME: case SOF_CTRL_CMD_ENUM: case SOF_CTRL_CMD_SWITCH: diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 5bcc842e4792..84a8ebe3b1c3 100644 --- a/sound/soc/sof/sof-audio.h +++ b/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 */ diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index c440e1c53ca5..ec59baf32699 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -1090,13 +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 = scontrol->cmd; + scontrol->control_data->cmd = SOF_CTRL_CMD_SWITCH; goto skip; }
- scontrol->cmd = SOF_CTRL_CMD_VOLUME; - scontrol->control_data->cmd = scontrol->cmd; + scontrol->control_data->cmd = SOF_CTRL_CMD_VOLUME;
/* extract tlv data */ if (!kc->tlv.p || get_tlv_data(kc->tlv.p, tlv) < 0) { @@ -1167,8 +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 = scontrol->cmd; + 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); @@ -1214,8 +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 = scontrol->cmd; + 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", @@ -2080,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:
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
The component type is not used in firmware nor in the kernel currently and it is not even clear how it should be handled.
Do not even try to handle it to avoid errors.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/control.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c index 9297b29d65cd..dac0b630b6a0 100644 --- a/sound/soc/sof/control.c +++ b/sound/soc/sof/control.c @@ -577,6 +577,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) { @@ -643,11 +650,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);
From: Peter Ujfalusi peter.ujfalusi@linux.intel.com
The SOF_CTRL_TYPE_VALUE_COMP_* type is not used by the firmware nor in the kernel side. It is also not clear what action should be taken for such type.
With this in mind: The correct ipc_cmd can be selected based on the `ctrl_cmd` and the `set` parameters: if the ctrl_cmd is SOF_CTRL_CMD_BINARY then SOF_CTRL_TYPE_DATA_* otherwise SOF_CTRL_TYPE_VALUE_CHAN_*.
The SET or GET direction can be selected with the use of `set` parameter.
Signed-off-by: Peter Ujfalusi peter.ujfalusi@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Bard Liao yung-chuan.liao@linux.intel.com Reviewed-by: Kai Vehmanen kai.vehmanen@linux.intel.com Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com --- sound/soc/sof/control.c | 20 +++++++------------- sound/soc/sof/ipc.c | 30 +++++++++++++----------------- sound/soc/sof/sof-audio.c | 17 +---------------- sound/soc/sof/sof-audio.h | 3 +-- 4 files changed, 22 insertions(+), 48 deletions(-)
diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c index dac0b630b6a0..ef61936dad59 100644 --- a/sound/soc/sof/control.c +++ b/sound/soc/sof/control.c @@ -83,8 +83,7 @@ static void snd_sof_refresh_control(struct snd_sof_control *scontrol)
/* refresh the component data from DSP */ scontrol->comp_data_dirty = false; - ret = snd_sof_ipc_set_get_comp_data(scontrol, - SOF_CTRL_TYPE_VALUE_CHAN_GET, 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 */ @@ -135,8 +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_CTRL_TYPE_VALUE_CHAN_SET, true); + snd_sof_ipc_set_get_comp_data(scontrol, true); return change; }
@@ -205,8 +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_CTRL_TYPE_VALUE_CHAN_SET, true); + snd_sof_ipc_set_get_comp_data(scontrol, true);
return change; } @@ -251,8 +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_CTRL_TYPE_VALUE_CHAN_SET, true); + snd_sof_ipc_set_get_comp_data(scontrol, true);
return change; } @@ -326,8 +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_CTRL_TYPE_DATA_SET, true); + snd_sof_ipc_set_get_comp_data(scontrol, true);
return 0; } @@ -403,8 +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_CTRL_TYPE_DATA_SET, true); + snd_sof_ipc_set_get_comp_data(scontrol, true);
return 0; } @@ -441,7 +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_CTRL_TYPE_DATA_GET, false); + ret = snd_sof_ipc_set_get_comp_data(scontrol, false); if (ret < 0) goto out;
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c index 69c8a9964960..8a1eacc7ec5f 100644 --- a/sound/soc/sof/ipc.c +++ b/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; @@ -816,8 +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, - enum sof_ipc_ctrl_type ctrl_type, bool set) +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; @@ -825,6 +819,7 @@ 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; @@ -872,11 +867,19 @@ int snd_sof_ipc_set_get_comp_data(struct snd_sof_control *scontrol, return err; }
- /* Select the IPC cmd based on the ctrl_cmd and the direction */ - if (cdata->cmd == SOF_CTRL_CMD_BINARY) + /* + * 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; - else + 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->type = ctrl_type; @@ -892,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; diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 4530c6ed34e0..735fbc5fe1bd 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -14,27 +14,12 @@
static int sof_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_control *scontrol) { - enum sof_ipc_ctrl_type ctrl_type; int ret;
/* reset readback offset for scontrol */ scontrol->readback_offset = 0;
- /* notify DSP of kcontrol values */ - switch (scontrol->control_data->cmd) { - case SOF_CTRL_CMD_VOLUME: - case SOF_CTRL_CMD_ENUM: - case SOF_CTRL_CMD_SWITCH: - ctrl_type = SOF_CTRL_TYPE_VALUE_CHAN_SET; - break; - case SOF_CTRL_CMD_BINARY: - ctrl_type = SOF_CTRL_TYPE_DATA_SET; - break; - default: - return 0; - } - - ret = snd_sof_ipc_set_get_comp_data(scontrol, ctrl_type, 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); diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h index 84a8ebe3b1c3..f3009e6b91a1 100644 --- a/sound/soc/sof/sof-audio.h +++ b/sound/soc/sof/sof-audio.h @@ -238,8 +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, - enum sof_ipc_ctrl_type ctrl_type, bool set); +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);
Read back the control data from the DSP to initialize the control data size to match that of the data in the DSP. This is particularly useful for volatile read-only kcontrols in static pipelines.
Signed-off-by: Ranjani Sridharan ranjani.sridharan@linux.intel.com Reviewed-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Reviewed-by: Péter Ujfalusi peter.ujfalusi@linux.intel.com --- sound/soc/sof/sof-audio.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+)
diff --git a/sound/soc/sof/sof-audio.c b/sound/soc/sof/sof-audio.c index 735fbc5fe1bd..91e3fa5a7350 100644 --- a/sound/soc/sof/sof-audio.c +++ b/sound/soc/sof/sof-audio.c @@ -59,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;
On Wed, 15 Dec 2021 10:03:56 -0800, Ranjani Sridharan wrote:
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.
Peter Ujfalusi (7): ASoC: SOF: ipc: Rename send parameter in snd_sof_ipc_set_get_comp_data() ASoC: SOF: Drop ipc_cmd parameter for snd_sof_ipc_set_get_comp_data() ASoC: SOF: topology: Set control_data->cmd alongside scontrol->cmd ASoC: SOF: Drop ctrl_cmd parameter for snd_sof_ipc_set_get_comp_data() ASoC: SOF: sof-audio: Drop the `cmd` member from struct snd_sof_control ASoC: SOF: control: Do not handle control notification with component type ASoC: SOF: Drop ctrl_type parameter for snd_sof_ipc_set_get_comp_data()
[...]
Applied to
https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next
Thanks!
[1/8] ASoC: SOF: ipc: Rename send parameter in snd_sof_ipc_set_get_comp_data() commit: 9d562fdcd52b1bb1a13cd5078ffc06dd3eff3aef [2/8] ASoC: SOF: Drop ipc_cmd parameter for snd_sof_ipc_set_get_comp_data() commit: d4a06c4334aed1fe76ae2b7aaae6ee8b72f30a8e [3/8] ASoC: SOF: topology: Set control_data->cmd alongside scontrol->cmd commit: 8af783723f41d3b3d4f7f8452f190405e7059472 [4/8] ASoC: SOF: Drop ctrl_cmd parameter for snd_sof_ipc_set_get_comp_data() commit: 9182f3c40b52ebd91d4796d96186ba10b720b4ba [5/8] ASoC: SOF: sof-audio: Drop the `cmd` member from struct snd_sof_control commit: dd2fef982ff75fbae618cc274fda09bd40582acd [6/8] ASoC: SOF: control: Do not handle control notification with component type commit: 68be4f0ed40cce833cb313871c52878025e40596 [7/8] ASoC: SOF: Drop ctrl_type parameter for snd_sof_ipc_set_get_comp_data() commit: 47d7328f8cda15e60422c8ca36d067c4deb19b7e [8/8] ASoC: SOF: topology: read back control data from DSP commit: fc5adc2bb13a6988df7ce377320f381add236002
All being well this means that it will be integrated into the linux-next tree (usually sometime in the next 24 hours) and sent to Linus during the next merge window (or sooner if it is a bug fix), however if problems are discovered then the patch may be dropped or reverted.
You may get further e-mails resulting from automated or manual testing and review of the tree, please engage with people reporting problems and send followup patches addressing any issues that are reported if needed.
If any updates are required or you are submitting further changes they should be sent as incremental updates against current git, existing patches will not be replaced.
Please add any relevant lists and maintainers to the CCs when replying to this mail.
Thanks, Mark
participants (2)
-
Mark Brown -
Ranjani Sridharan