| Message ID | 20240629101112.628-1-shenghao-ding@ti.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 55f0a1fa6ea4e339c797e9a3292ca0caa4ab3885 |
| Headers | show |
| Series | [v1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files | expand |
On Sat, Jun 29, 2024 at 06:11:10PM +0800, Shenghao Ding wrote: > tas_priv->fw_state = TASDEVICE_RCA_FW_OK; > - scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", > - tas_priv->dev_name); > + if (tas_priv->name_prefix) > + scnprintf(tas_priv->rca_binaryname, 64, "%s-%s_coef.bin", > + tas_priv->name_prefix, tas_priv->dev_name); > + else > + scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", > + tas_priv->dev_name); I'll apply this but I do wonder if it's worth falling back to trying to load the unprefixed name if we fail to load the prefixed one.
Hi Brown Thanks for your comment. > -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Monday, July 1, 2024 8:23 PM > To: Ding, Shenghao <shenghao-ding@ti.com> > Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; > perex@perex.cz; pierre-louis.bossart@linux.intel.com; > 13916275206@139.com; zhourui@huaqin.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>; yuhsuan@google.com; Lo, Henry <henry.lo@ti.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>; > savyasanchi.shukla@netradyne.com; flaviopr@microsoft.com; Ji, Jesse > <jesse-ji@ti.com>; darren.ye@mediatek.com > Subject: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as the > prefix name of DSP firmwares and calibrated data files > > On Sat, Jun 29, 2024 at 06:11:10PM +0800, Shenghao Ding wrote: > > > tas_priv->fw_state = TASDEVICE_RCA_FW_OK; > > - scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", > > - tas_priv->dev_name); > > + if (tas_priv->name_prefix) > > + scnprintf(tas_priv->rca_binaryname, 64, "%s-%s_coef.bin", > > + tas_priv->name_prefix, tas_priv->dev_name); > > + else > > + scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", > > + tas_priv->dev_name); > > I'll apply this but I do wonder if it's worth falling back to trying to load the > unprefixed name if we fail to load the prefixed one. If fail to load dsp firmware, the driver won't load unprefixed name firmware, but switch tas2563/tas2781 to bypass-dsp mode automatically. In this mode, smartamp become simple amp. These day, I met a case from one of my customers, they put 2 pieces of tas2563, and 2 pieces of tas2781 in the same i2c bus. In order to identify tas2563 and tas2781, I think name_prefix is a good solution for this case. Looking forward to your comment. Thanks.
On Sat, 29 Jun 2024 18:11:10 +0800, Shenghao Ding wrote: > Add name_prefix as the prefix name of DSP firmwares > and calibrated data files which stored speaker > calibrated impedance. > > Applied to https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next Thanks! [1/1] ASoc: tas2781: Add name_prefix as the prefix name of DSP firmwares and calibrated data files commit: 55f0a1fa6ea4e339c797e9a3292ca0caa4ab3885 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
On Mon, Jul 01, 2024 at 02:00:13PM +0000, Ding, Shenghao wrote: > > I'll apply this but I do wonder if it's worth falling back to trying to load the > > unprefixed name if we fail to load the prefixed one. > If fail to load dsp firmware, the driver won't load unprefixed name firmware, > but switch tas2563/tas2781 to bypass-dsp mode automatically. > In this mode, smartamp become simple amp. > These day, I met a case from one of my customers, they put 2 pieces of tas2563, > and 2 pieces of tas2781 in the same i2c bus. In order to identify tas2563 and > tas2781, I think name_prefix is a good solution for this case. > Looking forward to your comment. Thanks. Yes, the name_prefix is a good idea and probably people want things specifically tuned for the DSP - I was thinking about error handling or upgrade cases where wrong calibration might work better. Bypass mode means the device will still function at least.
Hi Brown Thanks for your comment. Feedback is inline. > -----Original Message----- > From: Mark Brown <broonie@kernel.org> > Sent: Tuesday, July 2, 2024 1:06 AM > To: Ding, Shenghao <shenghao-ding@ti.com> > Cc: andriy.shevchenko@linux.intel.com; lgirdwood@gmail.com; > perex@perex.cz; pierre-louis.bossart@linux.intel.com; > 13916275206@139.com; zhourui@huaqin.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>; yuhsuan@google.com; Lo, Henry <henry.lo@ti.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>; > savyasanchi.shukla@netradyne.com; flaviopr@microsoft.com; Ji, Jesse > <jesse-ji@ti.com>; darren.ye@mediatek.com > Subject: Re: [EXTERNAL] Re: [PATCH v1] ASoc: tas2781: Add name_prefix as > the prefix name of DSP firmwares and calibrated data files > > On Mon, Jul 01, 2024 at 02:00:13PM +0000, Ding, Shenghao wrote: > > > > I'll apply this but I do wonder if it's worth falling back to trying > > > to load the unprefixed name if we fail to load the prefixed one. > > > If fail to load dsp firmware, the driver won't load unprefixed name > > firmware, but switch tas2563/tas2781 to bypass-dsp mode automatically. > > In this mode, smartamp become simple amp. > > These day, I met a case from one of my customers, they put 2 pieces of > > tas2563, and 2 pieces of tas2781 in the same i2c bus. In order to > > identify tas2563 and tas2781, I think name_prefix is a good solution for this > case. > > Looking forward to your comment. Thanks. > > Yes, the name_prefix is a good idea and probably people want things > specifically tuned for the DSP - I was thinking about error handling or > upgrade cases where wrong calibration might work better. Bypass mode > means the device will still function at least. In bypass mode, tas2563/tas2781 can still work without speaker protection or audio acoustic function. In case of only dsp firnmware loading without calibrated data, tas2563/tas2781 still can work with default calibrated data and default audio acoustic parameter.
diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c index 4d1a0d836e77..cc765d45c6b5 100644 --- a/sound/soc/codecs/tas2781-i2c.c +++ b/sound/soc/codecs/tas2781-i2c.c @@ -394,8 +394,12 @@ static void tasdevice_fw_ready(const struct firmware *fmw, * failing to load DSP firmware is NOT an error. */ tas_priv->fw_state = TASDEVICE_RCA_FW_OK; - scnprintf(tas_priv->coef_binaryname, 64, "%s_coef.bin", - tas_priv->dev_name); + if (tas_priv->name_prefix) + scnprintf(tas_priv->rca_binaryname, 64, "%s-%s_coef.bin", + tas_priv->name_prefix, tas_priv->dev_name); + else + 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", @@ -418,8 +422,15 @@ static void tasdevice_fw_ready(const struct firmware *fmw, * calibrated data inside algo. */ for (i = 0; i < tas_priv->ndev; i++) { - scnprintf(tas_priv->cal_binaryname[i], 64, "%s_cal_0x%02x.bin", - tas_priv->dev_name, tas_priv->tasdevice[i].dev_addr); + if (tas_priv->name_prefix) + scnprintf(tas_priv->cal_binaryname[i], 64, + "%s-%s_cal_0x%02x.bin", tas_priv->name_prefix, + tas_priv->dev_name, + tas_priv->tasdevice[i].dev_addr); + else + scnprintf(tas_priv->cal_binaryname[i], 64, + "%s_cal_0x%02x.bin", tas_priv->dev_name, + tas_priv->tasdevice[i].dev_addr); ret = tas2781_load_calibration(tas_priv, tas_priv->cal_binaryname[i], i); if (ret != 0)
Add name_prefix as the prefix name of DSP firmwares and calibrated data files which stored speaker calibrated impedance. Signed-off-by: Shenghao Ding <shenghao-ding@ti.com> --- sound/soc/codecs/tas2781-i2c.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)