[PATCH v7] ASoC: tas2783-sdw: add firmware download status check

[PATCH v7] ASoC: tas2783-sdw: add firmware download status check

[Baojun Xu]

Currently, firmware download during system resume from suspend introduces
significant wake-up latency. However, since PRAM content in the AMP
persists across resets and is not erased, this step can be optimized.
By verifying that the firmware version read from registers matches the
expected value, we can confirm that memory content was retained.
Consequently, the PRAM download can be skipped, significantly reducing
resume latency.

Signed-off-by: Baojun Xu <baojun.xu@ti.com>
---
v7:
 - Update the description about this patch.
 - Restore PRAM and YRAM in tas2783_sdca_mbq_size to avoid register access
   error, it will cause download failed without this.
 - Remove register check for firmware download.
v6:
- Exclude PRAM and YRAM from the MBQ register group.
- Revert changes to usleep_range().
- Replace uint with unsigned int for kernel coding style compliance.
- Remove AMP reset logic from the driver remove path.
v5:
- Add fw_version field for PRAM status check.
- Adjust register range from pages 1–127, 253 to pages 0–1, 253.
- Include PRAM and YRAM ranges in tas2783_sdca_mbq_size.
- Set max_register to the end of PRAM.
- Add PRAM status check before firmware download to skip reloading if PRAM
  content is retained.
- Add a retry mechanism after download failure to handle register write
  issues on initial power-up.
- Call regcache_drop_region() after firmware download to ensure correct
  firmware version reading.
- Read firmware version after download for subsequent comparison.
- Remove the "separate two monos to stereo" workaround as it causes missing
  audio on the right channel; stereo configuration should be handled by the
  DisCo table.
- Add AMP reset in the driver remove path.
- Update PRAM_ADDR_END from 0x7f to 0x80 in tas2783.h.
- Add address definition for the firmware version register in tas2783.h.
v4:
 - Since first_hw_init is only required for download reduction, remove it
   when this feature is not enabled.
v3:
 - Updated description about memory page download reduce.
 - Removed the logic that skips memory page downloads.
v2:
 - Update register address from 0x07 to 7 and keep the line within the
   80-character limit.‌
 - ‌Remove stray/unnecessary changes.‌
 - ‌Change variable type to unsigned int to comply with API requirements.‌
 - ‌Replace usleep_range() with fsleep().‌
 - ‌Set idle_bias_on to 0 to enable low-power mode.
 - Reactivate the AMP after resume.
---
 sound/soc/codecs/tas2783-sdw.c | 70 ++++++++++++++++++++--------------
 sound/soc/codecs/tas2783.h     |  4 +-
 2 files changed, 44 insertions(+), 30 deletions(-)

diff --git a/sound/soc/codecs/tas2783-sdw.c b/sound/soc/codecs/tas2783-sdw.c
index 85ab3fd83c7b..a57230e01f75 100644
--- a/sound/soc/codecs/tas2783-sdw.c
+++ b/sound/soc/codecs/tas2783-sdw.c
@@ -97,6 +97,7 @@ struct tas2783_prv {
 	u8 rca_binaryname[64];
 	u8 dev_name[32];
 	bool hw_init;
+	unsigned int fw_version;
 	/* wq for firmware download */
 	wait_queue_head_t fw_wait;
 	bool fw_dl_task_done;
@@ -315,8 +316,10 @@ static int tas2783_sdca_mbq_size(struct device *dev, u32 reg)
 	case 0x300 ... 0x340: /* Data port 3. */
 	case 0x400 ... 0x440: /* Data port 4. */
 	case 0x500 ... 0x540: /* Data port 5. */
-	case 0x800000 ... 0x803fff: /* Page 0 ~ 127. */
-	case 0x807e80 ... 0x807eff: /* Page 253. */
+	case TASDEV_REG_SDW(0, 0, 0) ... TASDEV_REG_SDW(0x00, 0x01, 0x80):
+	case TASDEV_REG_SDW(0, 0xfd, 0) ... TASDEV_REG_SDW(0, 0xfd, 0x80):
+	case PRAM_ADDR_START ... PRAM_ADDR_END:
+	case YRAM_ADDR_START ... YRAM_ADDR_END:
 	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_UDMPU23,
 			  TAS2783_SDCA_CTL_UDMPU_CLUSTER, 0):
 	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_FU21, TAS2783_SDCA_CTL_FU_MUTE,
@@ -517,7 +520,7 @@ static const struct regmap_config tas_regmap = {
 	.volatile_reg = tas2783_volatile_register,
 	.reg_defaults = tas2783_reg_default,
 	.num_reg_defaults = ARRAY_SIZE(tas2783_reg_default),
-	.max_register = 0x41008000 + TASDEV_REG_SDW(0xa1, 0x60, 0x7f),
+	.max_register = 0x41000000 + PRAM_ADDR_END,
 	.cache_type = REGCACHE_MAPLE,
 	.use_single_read = true,
 	.use_single_write = true,
@@ -745,6 +748,7 @@ static void tas2783_fw_ready(const struct firmware *fmw, void *context)
 	const u8 *buf = NULL;
 	s32  img_sz, ret = 0, cur_file = 0;
 	s32 offset = 0;
+	u32 val[4], fw_version;
 
 	struct tas_fw_hdr *hdr __free(kfree) = kzalloc_obj(*hdr);
 	struct tas_fw_file *file __free(kfree) = kzalloc_obj(*file);
@@ -786,6 +790,11 @@ static void tas2783_fw_ready(const struct firmware *fmw, void *context)
 	}
 
 	mutex_lock(&tas_dev->pde_lock);
+	ret = regmap_bulk_read(tas_dev->regmap, TAS2783_FW_VERSION, &val, 4);
+	fw_version = (val[0] << 24) | (val[1] << 16) | (val[2] << 8) | val[3];
+	dev_dbg(tas_dev->dev, "Get Firmware version: %08x == %08x?, err=%d",
+		fw_version, tas_dev->fw_version, ret);
+
 	while (offset < (img_sz - FW_FL_HDR)) {
 		offset += tas_fw_get_next_file(&buf[offset], file);
 		dev_dbg(tas_dev->dev,
@@ -794,6 +803,13 @@ static void tas2783_fw_ready(const struct firmware *fmw, void *context)
 			file->version, file->length,
 			file->dest_addr, file->fw_data);
 
+		if (tas_dev->fw_version == fw_version &&
+		    file->dest_addr >= PRAM_ADDR_START &&
+		    (file->dest_addr + file->length) <= PRAM_ADDR_END) {
+			cur_file++;
+			dev_dbg(tas_dev->dev, "Ignore PRAM block");
+			continue;
+		}
 		ret = sdw_nwrite_no_pm(tas_dev->sdw_peripheral,
 				       file->dest_addr,
 				       file->length,
@@ -801,17 +817,34 @@ static void tas2783_fw_ready(const struct firmware *fmw, void *context)
 		if (ret < 0) {
 			dev_err(tas_dev->dev,
 				"FW download failed: %d", ret);
-			break;
+			/*
+			 * We do retry here for some special case of download
+			 * failed after Power-On.
+			 */
+			ret = sdw_nwrite_no_pm(tas_dev->sdw_peripheral,
+					       file->dest_addr,
+					       file->length,
+					       file->fw_data);
+			if (ret < 0) {
+				dev_err(tas_dev->dev,
+					"FW download failed again: %d", ret);
+				break;
+			}
 		}
 		cur_file++;
 	}
 	mutex_unlock(&tas_dev->pde_lock);
+	regcache_drop_region(tas_dev->regmap, 0, UINT_MAX);
 
 	if (cur_file == 0) {
 		dev_err(tas_dev->dev, "fw with no files");
 		ret = -EINVAL;
 	} else {
 		tas2783_update_calibdata(tas_dev);
+		ret = regmap_bulk_read(tas_dev->regmap, TAS2783_FW_VERSION,
+					&val, 4);
+		tas_dev->fw_version = (val[0] << 24) | (val[1] << 16) |
+				       (val[2] << 8) | val[3];
 	}
 
 out:
@@ -962,30 +995,6 @@ static s32 tas_sdw_hw_params(struct snd_pcm_substream *substream,
 	snd_sdw_params_to_config(substream, params,
 				 &stream_config, &port_config);
 
-	/*
-	 * The two mono amps each render one channel of the stereo stream:
-	 * snd_sdw_params_to_config() hands every codec the full mask for
-	 * playback, which leaves the pair in mirror mode and one channel
-	 * unreproduced.  Claim a single channel instead, keyed off the
-	 * machine-assigned component prefix rather than the SoundWire
-	 * address, which is board-specific: soc_sdw_ti_amp.c names the amps
-	 * tas2783-1..4.
-	 *
-	 * Which side an amp then renders does not follow from the bit that
-	 * is set - sdw_compute_slave_ports() advances the payload offset by
-	 * the popcount of ch_mask and never looks at which bit it is - but
-	 * from the amp's position in the codec order of the DAI link, which
-	 * on these boards matches the prefix numbering.
-	 */
-	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK &&
-	    params_channels(params) == 2 && component->name_prefix) {
-		const char *idx_str = strrchr(component->name_prefix, '-');
-		unsigned long idx;
-
-		if (idx_str && !kstrtoul(idx_str + 1, 10, &idx) && idx)
-			port_config.ch_mask = (idx & 1) ? BIT(0) : BIT(1);
-	}
-
 	/* port 1 for playback */
 	if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK)
 		port_config.num = 1;
@@ -1074,7 +1083,7 @@ static const struct snd_soc_component_driver soc_codec_driver_tasdevice = {
 	.num_dapm_widgets = ARRAY_SIZE(tas_dapm_widgets),
 	.dapm_routes = tas_audio_map,
 	.num_dapm_routes = ARRAY_SIZE(tas_audio_map),
-	.idle_bias_on = 1,
+	.idle_bias_on = 0,
 	.endianness = 1,
 };
 
@@ -1234,6 +1243,8 @@ static s32 tas_io_init(struct device *dev, struct sdw_slave *slave)
 			ret = regmap_multi_reg_write(tas_dev->regmap, tas2783_init_seq,
 						     ARRAY_SIZE(tas2783_init_seq));
 
+		/* Re-active AMP after resume. */
+		regmap_write(tas_dev->regmap, TASDEV_REG_SDW(0, 0, 2), 0);
 		if (ret)
 			dev_err(tas_dev->dev,
 				"init writes failed, err=%d", ret);
@@ -1413,6 +1424,7 @@ static s32 tas_sdw_probe(struct sdw_slave *peripheral,
 	tas_dev->dev = dev;
 	tas_dev->sdw_peripheral = peripheral;
 	tas_dev->hw_init = false;
+	tas_dev->fw_version = 0;
 	mutex_init(&tas_dev->calib_lock);
 	mutex_init(&tas_dev->pde_lock);
 
diff --git a/sound/soc/codecs/tas2783.h b/sound/soc/codecs/tas2783.h
index d5996c73526c..2f034617e356 100644
--- a/sound/soc/codecs/tas2783.h
+++ b/sound/soc/codecs/tas2783.h
@@ -35,10 +35,12 @@
 #define TAS2783_AMP_LEVEL_MASK	GENMASK(5, 1)
 
 #define PRAM_ADDR_START		TASDEV_REG_SDW(0x8c, 0x01, 0x8)
-#define PRAM_ADDR_END		TASDEV_REG_SDW(0x8c, 0xff, 0x7f)
+#define PRAM_ADDR_END		TASDEV_REG_SDW(0x8c, 0xff, 0x80)
 #define YRAM_ADDR_START		TASDEV_REG_SDW(0x00, 0x02, 0x8)
 #define YRAM_ADDR_END		TASDEV_REG_SDW(0x00, 0x37, 0x7f)
 
+#define TAS2783_FW_VERSION	TASDEV_REG_SDW(0x00, 0x20, 0x3c)
+
 /* Calibration data */
 #define TAS2783_CAL_R0		TASDEV_REG_SDW(0, 0x16, 0x4C)
 #define TAS2783_CAL_INVR0	TASDEV_REG_SDW(0, 0x16, 0x5C)
-- 
2.25.1

Re: [PATCH v7] ASoC: tas2783-sdw: add firmware download status check

[Mark Brown]

On Mon, 21 Sep 2026 19:19:30 +0800, Baojun Xu wrote:
> ASoC: tas2783-sdw: add firmware download status check

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-7.4

Thanks!

[1/1] ASoC: tas2783-sdw: add firmware download status check
      https://git.kernel.org/broonie/sound/c/f42fbd560cd8

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

Re: [PATCH v7] ASoC: tas2783-sdw: add firmware download status check

[Lee Mann]

Hi Baojun,

This patch, now in v7.4-rc1 as f42fbd560cd8, drops the stereo split
from tas2783_sdw_hw_params() (v5 changelog: "stereo configuration
should be handled by the DisCo table"). On the ASUS ProArt PX13
(HN7306EA, AMD ACP, 2x TAS2783 on SoundWire link 1) that table is
never applied, so the split was what gave stereo there.

The ASUS ACPI declares the SDCA function type without a DisCo constant.
Both amps log "function type only supported as DisCo constant",
find_sdca_function() bails out and the per-amp init table is never
written. ClusterIndex (SDW_SDCA_CTL(1, PPU21, 0x10, 0)) then stays at
0x01 from tas2783_init_seq on both amps.

Tested on 7.3.1 by ear with speaker-test -c2 -t wav, by swapping only
snd-soc-tas2783-sdw:

  tas2783-sdw.c from v7.3 (+ a local volume-restore patch, no
  channel changes)             : Front Left left, Front Right right
  tas2783-sdw.c from v7.4-rc1  : Front Left from the left speaker,
                                 Front Right not heard at all, right
                                 speaker silent

The v7.4-rc1 driver probes cleanly (no tas2783 errors in the kernel
log, both amps Attached), so with this patch the PX13's right speaker
no longer works on a stock v7.4 kernel.

I agree the split is wrong where the DisCo init table is applied. I
saw that myself with the SDCA fallback from SOF PR #5904 (which makes
the PX13 table apply: 0x01 on amp 0x8, 0x04 on amp 0xb): with the
split, amp 0xb went silent; with the full mask, stereo. So the two
cases need different handling. Two options I can see:

 - keep the split only when no SmartAmp function data was parsed
   (sa_func_data absent), or
 - land a fallback that lets the ASUS table be parsed (PR #5904 does
   this for mfg 0x0102) before 7.4 final.

I'm happy to test either on the PX13.

#regzbot introduced: f42fbd560cd8
#regzbot title: ASoC: tas2783-sdw: right channel lost on ASUS ProArt PX13 after stereo split removal

Thanks,
Lee Mann

Re: [PATCH v7] ASoC: tas2783-sdw: add firmware download status check

[Lee Mann]

Follow-up with one more data point, which supports the approach in
this patch.

Same PX13, kernel 7.3.3, with two modules swapped:

  - snd-soc-tas2783-sdw: unmodified v7.4-rc1 tas2783-sdw.c/tas2783.h
  - snd-soc-sdca: v7.3.3 plus only the sdca_functions.c fallback from
    SOF PR #5904 (function type for mfg 0x0102 / part 0x0000)

Kernel log:

  acpi device:26: function type not a DisCo constant, using fallback for known peripheral
  acpi device:26: SDCA function SmartAmp (type 1) at 0x1
  acpi device:28: function type not a DisCo constant, using fallback for known peripheral
  acpi device:28: SDCA function SmartAmp (type 1) at 0x1
  slave-tas2783 sdw:0:1:0102:0000:01:8: Found Smart Amp function at index 0
  slave-tas2783 sdw:0:1:0102:0000:01:b: Found Smart Amp function at index 0

speaker-test -c2 -t wav: Front Left from the left speaker, Front Right
from the right speaker. No "Defer on undeferrable control" warnings
(with the same fallback on the v7.3-rc6 driver I saw ~1600 of them
during playback).

So on the PX13 the v7.4 driver works as intended once the ASUS SDCA
function is parsed. The regression is only that the parse fails on
this firmware. Getting the PR #5904 fallback (or an equivalent) into
v7.4 would fix it without bringing back the split. I can test a
posted version.

https://github.com/thesofproject/linux/pull/5904

Thanks,
Lee Mann