2024-05-31 05:28:14

by Shenghao Ding

[permalink] [raw]
Subject: [PATCH v4] ASoc: tas2781: Enable RCA-based playback without DSP firmware download

In only RCA (Reconfigurable Architecture) binary case, no DSP program will
be working inside tas2563/tas2781, that is dsp-bypass mode, do not support
speaker protection, and audio acoustic algorithms in this mode.

Fixes: ef3bcde75d06 ("ASoC: tas2781: Add tas2781 driver")
Signed-off-by: Shenghao Ding <[email protected]>

---
v4:
- add a description of what the state machine looks like, and why
FW_PENDING/FAIL remain but are not used.
- Remove TASDEVICE_DSP_FW_NONE because of unused.
- Remove stray change.
- Fix broken indentation.
v3:
- Add description on RCA is Reconfigurable Architecture.
- Add the description on enabling
- Reword the commit
- Remove question mark in the comments.
- Add spaces in comments.
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 the code neater.
---
include/sound/tas2781-dsp.h | 11 ++++++++--
sound/soc/codecs/tas2781-fmwlib.c | 18 +++++++++++-----
sound/soc/codecs/tas2781-i2c.c | 34 +++++++++++++++++++------------
3 files changed, 43 insertions(+), 20 deletions(-)

diff --git a/include/sound/tas2781-dsp.h b/include/sound/tas2781-dsp.h
index 7fba7ea26a4b..3cda9da14f6d 100644
--- a/include/sound/tas2781-dsp.h
+++ b/include/sound/tas2781-dsp.h
@@ -117,10 +117,17 @@ struct tasdevice_fw {
struct device *dev;
};

-enum tasdevice_dsp_fw_state {
- TASDEVICE_DSP_FW_NONE = 0,
+enum tasdevice_fw_state {
+ /* Driver in startup mode, not load any firmware. */
TASDEVICE_DSP_FW_PENDING,
+ /* DSP firmware in the system, but parsing error. */
TASDEVICE_DSP_FW_FAIL,
+ /*
+ * Only RCA (Reconfigurable Architecture) firmware load
+ * successfully.
+ */
+ TASDEVICE_RCA_FW_OK,
+ /* Both RCA and DSP firmware load successfully. */
TASDEVICE_DSP_FW_ALL_OK,
};

diff --git a/sound/soc/codecs/tas2781-fmwlib.c b/sound/soc/codecs/tas2781-fmwlib.c
index 265a8ca25cbb..838d29fead96 100644
--- a/sound/soc/codecs/tas2781-fmwlib.c
+++ b/sound/soc/codecs/tas2781-fmwlib.c
@@ -2324,14 +2324,21 @@ 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-based Playback can still work with no dsp program running
+ * inside the chip.
+ */
+ switch (tas_priv->fw_state) {
+ case TASDEVICE_RCA_FW_OK:
+ case TASDEVICE_DSP_FW_ALL_OK:
+ break;
+ default:
return;
}

if (state == 0) {
- if (tas_priv->cur_prog < tas_fmw->nr_programs) {
- /*dsp mode or tuning mode*/
+ 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,
tas_priv->cur_prog, tas_priv->cur_conf,
@@ -2340,9 +2347,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..9c3c89cb36de 100644
--- a/sound/soc/codecs/tas2781-i2c.c
+++ b/sound/soc/codecs/tas2781-i2c.c
@@ -380,23 +380,32 @@ 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);
+
+ /*
+ * If no dsp-related kcontrol created, the dsp resource will be freed.
+ */
+ 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 +426,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 +474,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;
+ switch (tas_priv->fw_state) {
+ case TASDEVICE_RCA_FW_OK:
+ case TASDEVICE_DSP_FW_ALL_OK:
+ return 0;
+ default:
+ return -EINVAL;
}
-
- return ret;
}

static int tasdevice_hw_params(struct snd_pcm_substream *substream,
--
2.34.1



2024-05-31 14:49:01

by Pierre-Louis Bossart

[permalink] [raw]
Subject: Re: [PATCH v4] ASoc: tas2781: Enable RCA-based playback without DSP firmware download


> -enum tasdevice_dsp_fw_state {
> - TASDEVICE_DSP_FW_NONE = 0,
> +enum tasdevice_fw_state {
> + /* Driver in startup mode, not load any firmware. */
> TASDEVICE_DSP_FW_PENDING,
> + /* DSP firmware in the system, but parsing error. */
> TASDEVICE_DSP_FW_FAIL,
> + /*
> + * Only RCA (Reconfigurable Architecture) firmware load
> + * successfully.
> + */
> + TASDEVICE_RCA_FW_OK,
> + /* Both RCA and DSP firmware load successfully. */
> TASDEVICE_DSP_FW_ALL_OK,

I appreciate the effort to document the states, but for the RCA cases we
can have two 'success' states?

> - 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);
> +
> + /*
> + * If no dsp-related kcontrol created, the dsp resource will be freed.
> + */
> + 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;

from this code, it seems that the RCA case goes from RCA_FW_OK to
TASDEVICE_DSP_FW_ALL_OK, so there's a difference between the two states, no?


> @@ -466,14 +474,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;
> + switch (tas_priv->fw_state) {
> + case TASDEVICE_RCA_FW_OK:
> + case TASDEVICE_DSP_FW_ALL_OK:
> + return 0;
> + default:
> + return -EINVAL;

maybe keep the error logs?