[PATCH v2] ASoc: tas2781: Playback can work when only RCA binary loading without dsp firmware loading
In only RCA binary loading case, only default dsp program inside the chip will be work.
Fixes: ef3bcde75d06 ("ASoC: tas2781: Add tas2781 driver") Signed-off-by: Shenghao Ding shenghao-ding@ti.com
--- v2: - correct comment. - Add Fixes. - Move header file to the first. v1: - Split out the different logical changes into different patches. - rename tasdevice_dsp_fw_state -> tasdevice_fw_state, the fw are not only dsp fw, but also RCA(Reconfigurable data, such as acoustic data and register setting, etc). - Add TASDEVICE_RCA_FW_OK in tasdevice_fw_state to identify the state that only RCA binary file has been download successfully, but dsp fw is not loaded or loading failure. - Add the this strategy into tasdevice_tuning_switch. - If one side of the if/else has a braces both should in tasdevice_tuning_switch. - Identify whehter both RCA and DSP have been loaded or only RCA has been loaded in tasdevice_fw_ready. - Add check fw load status in tasdevice_startup. - remove ret in tasdevice_startup to make th code neater. --- include/sound/tas2781-dsp.h | 3 ++- sound/soc/codecs/tas2781-fmwlib.c | 14 ++++++++++---- sound/soc/codecs/tas2781-i2c.c | 30 ++++++++++++++++++------------ 3 files changed, 30 insertions(+), 17 deletions(-)
diff --git a/include/sound/tas2781-dsp.h b/include/sound/tas2781-dsp.h index 7fba7ea26a4b..92d68ca5eafb 100644 --- a/include/sound/tas2781-dsp.h +++ b/include/sound/tas2781-dsp.h @@ -117,10 +117,11 @@ struct tasdevice_fw { struct device *dev; };
-enum tasdevice_dsp_fw_state { +enum tasdevice_fw_state { TASDEVICE_DSP_FW_NONE = 0, TASDEVICE_DSP_FW_PENDING, TASDEVICE_DSP_FW_FAIL, + TASDEVICE_RCA_FW_OK, TASDEVICE_DSP_FW_ALL_OK, };
diff --git a/sound/soc/codecs/tas2781-fmwlib.c b/sound/soc/codecs/tas2781-fmwlib.c index 265a8ca25cbb..cfa022ef4a59 100644 --- a/sound/soc/codecs/tas2781-fmwlib.c +++ b/sound/soc/codecs/tas2781-fmwlib.c @@ -2324,13 +2324,18 @@ void tasdevice_tuning_switch(void *context, int state) struct tasdevice_fw *tas_fmw = tas_priv->fmw; int profile_cfg_id = tas_priv->rcabin.profile_cfg_id;
- if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) { - dev_err(tas_priv->dev, "DSP bin file not loaded\n"); + /* + * Only RCA file loaded can still work with default dsp program inside + * the chip? + */ + if (!(tas_priv->fw_state == TASDEVICE_RCA_FW_OK || + tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK)) { + dev_err(tas_priv->dev, "No firmware loaded\n"); return; }
if (state == 0) { - if (tas_priv->cur_prog < tas_fmw->nr_programs) { + if (tas_fmw && tas_priv->cur_prog < tas_fmw->nr_programs) { /*dsp mode or tuning mode*/ profile_cfg_id = tas_priv->rcabin.profile_cfg_id; tasdevice_select_tuningprm_cfg(tas_priv, @@ -2340,9 +2345,10 @@ void tasdevice_tuning_switch(void *context, int state)
tasdevice_select_cfg_blk(tas_priv, profile_cfg_id, TASDEVICE_BIN_BLK_PRE_POWER_UP); - } else + } else { tasdevice_select_cfg_blk(tas_priv, profile_cfg_id, TASDEVICE_BIN_BLK_PRE_SHUTDOWN); + } } EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch, SND_SOC_TAS2781_FMWLIB); diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c index 9350972dfefe..ccb9313ada9b 100644 --- a/sound/soc/codecs/tas2781-i2c.c +++ b/sound/soc/codecs/tas2781-i2c.c @@ -380,23 +380,30 @@ static void tasdevice_fw_ready(const struct firmware *fmw, mutex_lock(&tas_priv->codec_lock);
ret = tasdevice_rca_parser(tas_priv, fmw); - if (ret) + if (ret) { + tasdevice_config_info_remove(tas_priv); goto out; + } tasdevice_create_control(tas_priv);
tasdevice_dsp_remove(tas_priv); tasdevice_calbin_remove(tas_priv); - tas_priv->fw_state = TASDEVICE_DSP_FW_PENDING; + tas_priv->fw_state = TASDEVICE_RCA_FW_OK; scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", tas_priv->dev_name); + ret = tasdevice_dsp_parser(tas_priv); if (ret) { dev_err(tas_priv->dev, "dspfw load %s error\n", tas_priv->coef_binaryname); - tas_priv->fw_state = TASDEVICE_DSP_FW_FAIL; goto out; } - tasdevice_dsp_create_ctrls(tas_priv); + + ret = tasdevice_dsp_create_ctrls(tas_priv); + if (ret) { + dev_err(tas_priv->dev, "dsp controls error\n"); + goto out; + }
tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;
@@ -417,9 +424,8 @@ static void tasdevice_fw_ready(const struct firmware *fmw, tasdevice_prmg_load(tas_priv, 0); tas_priv->cur_prog = 0; out: - if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) { - /*If DSP FW fail, kcontrol won't be created */ - tasdevice_config_info_remove(tas_priv); + if (tas_priv->fw_state == TASDEVICE_RCA_FW_OK) { + /*If DSP FW fail, DSP kcontrol won't be created */ tasdevice_dsp_remove(tas_priv); } mutex_unlock(&tas_priv->codec_lock); @@ -466,14 +472,14 @@ static int tasdevice_startup(struct snd_pcm_substream *substream, { struct snd_soc_component *codec = dai->component; struct tasdevice_priv *tas_priv = snd_soc_component_get_drvdata(codec); - int ret = 0;
- if (tas_priv->fw_state != TASDEVICE_DSP_FW_ALL_OK) { - dev_err(tas_priv->dev, "DSP bin file not loaded\n"); - ret = -EINVAL; + if (!(tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK || + tas_priv->fw_state == TASDEVICE_RCA_FW_OK)) { + dev_err(tas_priv->dev, "Bin file not loaded\n"); + return -EINVAL; }
- return ret; + return 0; }
static int tasdevice_hw_params(struct snd_pcm_substream *substream,
On 5/24/24 20:47, Shenghao Ding wrote:
In only RCA binary loading case, only default dsp program inside the chip will be work.
What does 'RCA' stand for?
Also clarify the commit title without double negatives, e.g.
"Enable RCA-based playback without DSP firmware download"
- if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
dev_err(tas_priv->dev, "DSP bin file not loaded\n");
- /*
* Only RCA file loaded can still work with default dsp program inside
* the chip?
reword the commit and remove question mark.
*/
if (!(tas_priv->fw_state == TASDEVICE_RCA_FW_OK ||
tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK)) {
dev_err(tas_priv->dev, "No firmware loaded\n");
return; }
if (state == 0) {
if (tas_priv->cur_prog < tas_fmw->nr_programs) {
if (tas_fmw && tas_priv->cur_prog < tas_fmw->nr_programs) { /*dsp mode or tuning mode*/
spaces in comments
profile_cfg_id = tas_priv->rcabin.profile_cfg_id; tasdevice_select_tuningprm_cfg(tas_priv,
@@ -2340,9 +2345,10 @@ void tasdevice_tuning_switch(void *context, int state)
tasdevice_select_cfg_blk(tas_priv, profile_cfg_id, TASDEVICE_BIN_BLK_PRE_POWER_UP);
- } else
- } else { tasdevice_select_cfg_blk(tas_priv, profile_cfg_id, TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
- }
} EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch, SND_SOC_TAS2781_FMWLIB); diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c index 9350972dfefe..ccb9313ada9b 100644 --- a/sound/soc/codecs/tas2781-i2c.c +++ b/sound/soc/codecs/tas2781-i2c.c @@ -380,23 +380,30 @@ static void tasdevice_fw_ready(const struct firmware *fmw, mutex_lock(&tas_priv->codec_lock);
ret = tasdevice_rca_parser(tas_priv, fmw);
- if (ret)
if (ret) {
tasdevice_config_info_remove(tas_priv);
goto out;
} tasdevice_create_control(tas_priv);
tasdevice_dsp_remove(tas_priv); tasdevice_calbin_remove(tas_priv);
- tas_priv->fw_state = TASDEVICE_DSP_FW_PENDING;
- tas_priv->fw_state = TASDEVICE_RCA_FW_OK; scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", tas_priv->dev_name);
- ret = tasdevice_dsp_parser(tas_priv); if (ret) { dev_err(tas_priv->dev, "dspfw load %s error\n", tas_priv->coef_binaryname);
goto out; }tas_priv->fw_state = TASDEVICE_DSP_FW_FAIL;
- tasdevice_dsp_create_ctrls(tas_priv);
- ret = tasdevice_dsp_create_ctrls(tas_priv);
- if (ret) {
dev_err(tas_priv->dev, "dsp controls error\n");
goto out;
- }
this seems unrelated to the boot process? Move to a different patch?
tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;
@@ -417,9 +424,8 @@ static void tasdevice_fw_ready(const struct firmware *fmw, tasdevice_prmg_load(tas_priv, 0); tas_priv->cur_prog = 0; out:
- if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
/*If DSP FW fail, kcontrol won't be created */
tasdevice_config_info_remove(tas_priv);
- if (tas_priv->fw_state == TASDEVICE_RCA_FW_OK) {
/*If DSP FW fail, DSP kcontrol won't be created */
It looks like you're no longer using PENDING and FAIL states? The state machine is becoming really hard to follow.
tasdevice_dsp_remove(tas_priv);
} mutex_unlock(&tas_priv->codec_lock); @@ -466,14 +472,14 @@ static int tasdevice_startup(struct snd_pcm_substream *substream, { struct snd_soc_component *codec = dai->component; struct tasdevice_priv *tas_priv = snd_soc_component_get_drvdata(codec);
int ret = 0;
if (tas_priv->fw_state != TASDEVICE_DSP_FW_ALL_OK) {
dev_err(tas_priv->dev, "DSP bin file not loaded\n");
ret = -EINVAL;
- if (!(tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK ||
tas_priv->fw_state == TASDEVICE_RCA_FW_OK)) {
dev_err(tas_priv->dev, "Bin file not loaded\n");
}return -EINVAL;
- return ret;
- return 0;
}
static int tasdevice_hw_params(struct snd_pcm_substream *substream,
Thanks for your comments.
-----Original Message----- From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Sent: Monday, May 27, 2024 9:44 PM To: Ding, Shenghao shenghao-ding@ti.com; broonie@kernel.org Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; perex@perex.cz; 13916275206@139.com; alsa-devel@alsa-project.org; Salazar, Ivan i-salazar@ti.com; linux-kernel@vger.kernel.org; Chadha, Jasjot Singh j-chadha@ti.com; liam.r.girdwood@intel.com; Yue, Jaden jaden-yue@ti.com; yung-chuan.liao@linux.intel.com; Rao, Dipa dipa@ti.com; Lu, Kevin kevin-lu@ti.com; yuhsuan@google.com; tiwai@suse.de; Xu, Baojun baojun.xu@ti.com; soyer@irl.hu; Baojun.Xu@fpt.com; judyhsiao@google.com; Navada Kanyana, Mukund navada@ti.com; cujomalainey@google.com; Kutty, Aanya aanya@ti.com; Mahmud, Nayeem nayeem.mahmud@ti.com Subject: [EXTERNAL] Re: [PATCH v2] ASoc: tas2781: Playback can work when only RCA binary loading without dsp firmware loading
On 5/24/24 20:47, Shenghao Ding wrote:
In only RCA binary loading case, only default dsp program inside the chip will be work.
What does 'RCA' stand for?
Reconfigurable architecture binary file
Also clarify the commit title without double negatives, e.g.
"Enable RCA-based playback without DSP firmware download"
- if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
dev_err(tas_priv->dev, "DSP bin file not loaded\n");
- /*
* Only RCA file loaded can still work with default dsp program inside
* the chip?
reword the commit and remove question mark.
accept
*/
if (!(tas_priv->fw_state == TASDEVICE_RCA_FW_OK ||
tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK)) {
dev_err(tas_priv->dev, "No firmware loaded\n");
return; }
if (state == 0) {
if (tas_priv->cur_prog < tas_fmw->nr_programs) {
if (tas_fmw && tas_priv->cur_prog < tas_fmw->nr_programs)
{
/*dsp mode or tuning mode*/
spaces in comments
accept
profile_cfg_id = tas_priv->rcabin.profile_cfg_id; tasdevice_select_tuningprm_cfg(tas_priv,
@@ -2340,9 +2345,10 @@ void tasdevice_tuning_switch(void *context, int state)
tasdevice_select_cfg_blk(tas_priv, profile_cfg_id, TASDEVICE_BIN_BLK_PRE_POWER_UP);
- } else
- } else { tasdevice_select_cfg_blk(tas_priv, profile_cfg_id, TASDEVICE_BIN_BLK_PRE_SHUTDOWN);
- }
} EXPORT_SYMBOL_NS_GPL(tasdevice_tuning_switch, SND_SOC_TAS2781_FMWLIB); diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c index 9350972dfefe..ccb9313ada9b 100644 --- a/sound/soc/codecs/tas2781-i2c.c +++ b/sound/soc/codecs/tas2781-i2c.c @@ -380,23 +380,30 @@ static void tasdevice_fw_ready(const struct
firmware *fmw,
mutex_lock(&tas_priv->codec_lock);
ret = tasdevice_rca_parser(tas_priv, fmw);
- if (ret)
if (ret) {
tasdevice_config_info_remove(tas_priv);
goto out;
} tasdevice_create_control(tas_priv);
tasdevice_dsp_remove(tas_priv); tasdevice_calbin_remove(tas_priv);
- tas_priv->fw_state = TASDEVICE_DSP_FW_PENDING;
- tas_priv->fw_state = TASDEVICE_RCA_FW_OK; scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", tas_priv->dev_name);
- ret = tasdevice_dsp_parser(tas_priv); if (ret) { dev_err(tas_priv->dev, "dspfw load %s error\n", tas_priv->coef_binaryname);
goto out; }tas_priv->fw_state = TASDEVICE_DSP_FW_FAIL;
- tasdevice_dsp_create_ctrls(tas_priv);
- ret = tasdevice_dsp_create_ctrls(tas_priv);
- if (ret) {
dev_err(tas_priv->dev, "dsp controls error\n");
goto out;
- }
this seems unrelated to the boot process? Move to a different patch?
It is a part of boot process, if no dsp-related kcontrol created, the dsp resource will be freed.
tas_priv->fw_state = TASDEVICE_DSP_FW_ALL_OK;
@@ -417,9 +424,8 @@ static void tasdevice_fw_ready(const struct
firmware *fmw,
tasdevice_prmg_load(tas_priv, 0); tas_priv->cur_prog = 0; out:
- if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
/*If DSP FW fail, kcontrol won't be created */
tasdevice_config_info_remove(tas_priv);
- if (tas_priv->fw_state == TASDEVICE_RCA_FW_OK) {
/*If DSP FW fail, DSP kcontrol won't be created */
It looks like you're no longer using PENDING and FAIL states? The state machine is becoming really hard to follow.
Remove unused states.
tasdevice_dsp_remove(tas_priv);
} mutex_unlock(&tas_priv->codec_lock); @@ -466,14 +472,14 @@ static int tasdevice_startup(struct snd_pcm_substream *substream, { struct snd_soc_component *codec = dai->component; struct tasdevice_priv *tas_priv =
snd_soc_component_get_drvdata(codec);
int ret = 0;
if (tas_priv->fw_state != TASDEVICE_DSP_FW_ALL_OK) {
dev_err(tas_priv->dev, "DSP bin file not loaded\n");
ret = -EINVAL;
- if (!(tas_priv->fw_state == TASDEVICE_DSP_FW_ALL_OK ||
tas_priv->fw_state == TASDEVICE_RCA_FW_OK)) {
dev_err(tas_priv->dev, "Bin file not loaded\n");
}return -EINVAL;
- return ret;
- return 0;
}
static int tasdevice_hw_params(struct snd_pcm_substream *substream,
-----Original Message----- From: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com Sent: Monday, May 27, 2024 9:44 PM To: Ding, Shenghao shenghao-ding@ti.com; broonie@kernel.org Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; perex@perex.cz; 13916275206@139.com; alsa-devel@alsa-project.org; Salazar, Ivan i-salazar@ti.com; linux-kernel@vger.kernel.org; Chadha, Jasjot Singh j-chadha@ti.com; liam.r.girdwood@intel.com; Yue, Jaden jaden-yue@ti.com; yung-chuan.liao@linux.intel.com; Rao, Dipa dipa@ti.com; Lu, Kevin kevin-lu@ti.com; yuhsuan@google.com; tiwai@suse.de; Xu, Baojun baojun.xu@ti.com; soyer@irl.hu; Baojun.Xu@fpt.com; judyhsiao@google.com; Navada Kanyana, Mukund navada@ti.com; cujomalainey@google.com; Kutty, Aanya aanya@ti.com; Mahmud, Nayeem nayeem.mahmud@ti.com Subject: [EXTERNAL] Re: [PATCH v2] ASoc: tas2781: Playback can work when only RCA binary loading without dsp firmware loading
On 5/24/24 20: 47, Shenghao Ding wrote: > In only RCA binary loading case, only default dsp program inside the > chip will be work. What does 'RCA' stand for? Also clarify the commit title without double negatives, e. g. "Enable RCA-based ZjQcmQRYFpfptBannerStart This message was sent from outside of Texas Instruments. Do not click links or open attachments unless you recognize the source of this email and know the content is safe. If you wish to report this message to IT Security, please forward the message as an attachment to phishing@list.ti.com
ZjQcmQRYFpfptBannerEnd
On 5/24/24 20:47, Shenghao Ding wrote:
In only RCA binary loading case, only default dsp program inside the chip will be work.
......................................................................
- if (tas_priv->fw_state == TASDEVICE_DSP_FW_FAIL) {
/*If DSP FW fail, kcontrol won't be created */
tasdevice_config_info_remove(tas_priv);
- if (tas_priv->fw_state == TASDEVICE_RCA_FW_OK) {
/*If DSP FW fail, DSP kcontrol won't be created */
It looks like you're no longer using PENDING and FAIL states? The state machine is becoming really hard to follow.
PENDING and FAIL states have been used in HDA-based tas2563/tas2781 driver
tasdevice_dsp_remove(tas_priv);
} mutex_unlock(&tas_priv->codec_lock); @@ -466,14 +472,14 @@ static int tasdevice_startup(struct snd_pcm_substream *substream, { struct snd_soc_component *codec = dai->component; struct tasdevice_priv *tas_priv =
..................................................................................
participants (3)
-
Ding, Shenghao
-
Pierre-Louis Bossart
-
Shenghao Ding