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);
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,