TAS2783A: add initial driver #5510

niranjanhyti · 2025-08-13T13:35:43Z

No description provided.

sofci · 2025-08-13T13:37:03Z

Can one of the admins verify this patch?

reply test this please to run this test once

bardliao · 2025-08-14T03:04:44Z

sound/soc/codecs/tas2783-sdw.c

+	struct sdw_stream_runtime *sdw_stream;
+};
+
+static const struct reg_default tas2783_reg_defaults[] = {


This is more like reg_sequence instead of reg_default because you write the registers in tas2783_initialize_device.

bardliao · 2025-08-14T03:05:49Z

sound/soc/codecs/tas2783-sdw.c

+	.reg_bits = 32,
+	.val_bits = 8,
+	.readable_reg = tas2783_readable_register,
+	.volatile_reg = tas2783_volatile_register,


I believe that .reg_defaults is needed.

Earlier I used .reg_default alone without manual initialization function tas2783_initialize_device. But the device did not function as expected. So I added manual initialization to duplicate the .reg_defaults behavior and dropped .reg_defaults . Is the .reg_default suppose to do the initialization to the device as well or it this behavior is because of my programming error ?

The .reg_default should be the default register value of the chip. It is the initial value of the regmap cache and in theory, the .reg_default should include all the readable registers except the volatile registers. And, no, .reg_defaults will not do the initialization to the device, you have to do it by yourself.

bardliao · 2025-08-14T03:11:33Z

sound/soc/codecs/tas2783-sdw.c

+	for (i = 0; i < ARRAY_SIZE(tas2783_reg_defaults); i++) {
+		ret = sdw_write_no_pm(tas_dev->sdw_peripheral,
+				      tas2783_reg_defaults[i].reg,
+				      tas2783_reg_defaults[i].def);


You could use regmap_multi_reg_write

bardliao · 2025-08-14T03:17:37Z

sound/soc/codecs/tas2783-sdw.c

+
+	val = tas_clamp(ucontrol->value.integer.value[0],
+			mc->max, mc->invert);
+	ret = sdw_write_no_pm(tas_dev->sdw_peripheral, mc->reg, val);


I would recommend to use regmap_write if possible. Note that access the register directly will not update the regmap cache.

Thanks for the information. I believe this applies for the entire driver code. Any general guideline on when it is safe to use the sdw_write/read* functions ?

I think the only scenario to use sdw_write/read is if the register needs to be accessed before the regmap is registered. Or you want to access the register silently. I.e. Without touch the regmap cache.

bardliao · 2025-08-14T03:29:26Z

sound/soc/codecs/tas2783-sdw.c

+		tas_dev->hw_init = true;
+	}
+
+	tas2783_initialize_device(tas_dev);


Do we still need initialize the device if firmware request failed?

bardliao · 2025-08-14T03:42:12Z

sound/soc/sdw_utils/soc_sdw_utils.c

+				.direction = {true, true},
+				.dai_name = "tas2783-codec",
+				.dai_type = SOC_SDW_DAI_TYPE_AMP,
+				.dailink = {SOC_SDW_AMP_OUT_DAI_ID, SOC_SDW_AMP_IN_DAI_ID},


Do we need to add the machine driver controls and a .rtd_init to give user a way to switch on/off the speakers? Like

.rtd_init = asoc_sdw_rt_amp_spk_rtd_init, .controls = generic_spk_controls, .num_controls = ARRAY_SIZE(generic_spk_controls), .widgets = generic_spk_widgets, .num_widgets = ARRAY_SIZE(generic_spk_widgets),

bardliao · 2025-08-14T03:43:21Z

sound/soc/intel/common/soc-acpi-intel-mtl-match.c

+		.links = tas2783_link0,
+		.drv_name = "sof_sdw",
+		.sof_tplg_filename = "sof-mtl-tas2783.tplg",
+	},


The new item should be added after the mockup devices.

bardliao · 2025-08-14T03:43:53Z

test this please

bardliao · 2025-08-21T07:26:13Z

sound/soc/codecs/tas2783-sdw.c

+	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_MFPU26, 0x08, 0):
+	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_SAPU29, 0x05, 0):
+	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PPU21, 0x06, 0):
+	case SDW_SDCA_CTL(1, TAS2783_SDCA_ENT_PPU26, 0x06, 0):


In theory, all readable registers except the volatile registers need a default value. So I would expect that the registers above will list in tas2783_reg_default[].

bardliao · 2025-08-21T07:32:44Z

sound/soc/codecs/tas2783-sdw.c

+				      u8 *data, u32 size)
+{
+	u32 ts, spk_count, size_calculated;
+	u32  crc_calculated, crc_read, i;


nitpick: redundant space between u32 and crc_calculated.

bardliao · 2025-08-21T07:41:20Z

sound/soc/codecs/tas2783-sdw.c

+	u32 bit;
+	unsigned long addr;
+	struct sdw_dpn_prop *dpn;
+


You will need to call sdw_slave_read_prop(slave); if you need to read some properties from the ACPI table.

bardliao · 2025-08-21T07:48:33Z

sound/soc/sdw_utils/soc_sdw_utils.c

+				.controls = maxim_controls,
+				.num_controls = ARRAY_SIZE(maxim_controls),
+				.widgets = maxim_widgets,
+				.num_widgets = ARRAY_SIZE(maxim_widgets),


I would move the commits before ASoc: tas2783A: add machine driver changes. So that we can add the above in the ASoc: tas2783A: add machine driver changes commit.

bardliao · 2025-08-21T07:58:36Z

sound/soc/sdw_utils/soc_sdw_ti_amp.c

+		if (!strncmp(prefix, "tas2783-1", strlen("tas2783-1"))) {
+			strncpy(speaker, "Left Spk", strlen("Left Spk") + 1);
+		} else if (!strncmp(prefix, "tas2783-2", strlen("tas2783-2"))) {
+			strncpy(speaker, "Right Spk", strlen("Right Spk") + 1);


Personally, I prefer using a single route like what asoc_sdw_rt_amp_spk_rtd_init does and generic_spk_widgets[]. You may have more than 2 amps in the future.

I found that we will have one switch for each amp with this implementation. So I thought it will be helpful for turning off individual channels.
With the suggested method similar to asoc_sdw_rt_amp_spk_rtd_init, there will be only one 'Speaker Switch' mixer control.

Yes, if you need to control left and right separately, you need to create 2 controls.

bardliao · 2025-08-21T08:02:18Z

sound/soc/sdw_utils/soc_sdw_utils.c

 				.dailink = {SOC_SDW_AMP_OUT_DAI_ID, SOC_SDW_AMP_IN_DAI_ID},
+				.init = asoc_sdw_ti_amp_init,
+				.rtd_init = asoc_sdw_ti_spk_rtd_init,
+				.controls = maxim_controls,


Maybe a more generic name is needed if you want to reuse the maxim controls/widgets. TI amp uses Maxim controls/widgets is confusing.

TAS2783 is mono digital input class-D Smart Amplifier based on MIPI Alliance Soundwire interface. The driver supports loading the algorithm coefficients for one or more tas2783 chips. Signed-off-by: Niranjan H Y <[email protected]> --- v3: - update default register list - re-order entiry id in header files and update duplicate entry - indentation issue fixes v2: - add default registers - refactor device initilisation with reg_sequence - use regmap_read/write APIs for device register access for some register access - initialize the device only on successful firmware download - clear latch before power up and log error - error handling and retry during power up - rename the volume mixer control for readability - modify the widgets for machine driver support checkpatch issue fix

Signed-off-by: Niranjan H Y <[email protected]> --- v3: - re-ordered the commit to appear before machine driver changes

Add tas2783-codec for codec_info Signed-off-by: Niranjan H Y <[email protected]> -- v3: - generic names for widgets and controls for 2 channels support.

acpi match changes to support tas2783a on mtl on link 0 Signed-off-by: Niranjan H Y <[email protected]> --- v3: - no changes v2: - move ACPI entries for tas2783 after mockup

bardliao · 2025-08-27T11:08:00Z

Thanks @niranjanhyti It looks good to me. Please send the patches to the mailing list. Feel free to add my Reviewed-by: Bard Liao <[email protected]> tag.

niranjanhyti requested review from lgirdwood, plbossart, ranj063, kv2019i, dbaluta, bardliao and ujfalusi as code owners August 13, 2025 13:35

bardliao reviewed Aug 14, 2025

View reviewed changes

niranjanhyti force-pushed the for-tas2783a branch 2 times, most recently from 2d2255b to 8b9a614 Compare August 20, 2025 02:55

bardliao reviewed Aug 21, 2025

View reviewed changes

niranjanhyti added 4 commits August 25, 2025 20:40

ASoc: tas2783A: machine driver amp utility for TI devices

a0671f6

Signed-off-by: Niranjan H Y <[email protected]> --- v3: - re-ordered the commit to appear before machine driver changes

ASoc: tas2783A: add machine driver changes

25238be

Add tas2783-codec for codec_info Signed-off-by: Niranjan H Y <[email protected]> -- v3: - generic names for widgets and controls for 2 channels support.

tas2783A: Add acpi match changes for Intel MTL

bb22b43

acpi match changes to support tas2783a on mtl on link 0 Signed-off-by: Niranjan H Y <[email protected]> --- v3: - no changes v2: - move ACPI entries for tas2783 after mockup

niranjanhyti force-pushed the for-tas2783a branch from 8b9a614 to bb22b43 Compare August 25, 2025 15:16

bardliao approved these changes Aug 27, 2025

View reviewed changes

TAS2783A: add initial driver #5510

Are you sure you want to change the base?

TAS2783A: add initial driver #5510

Uh oh!

Conversation

niranjanhyti commented Aug 13, 2025

Uh oh!

sofci commented Aug 13, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bardliao Aug 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bardliao commented Aug 14, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bardliao commented Aug 27, 2025

Uh oh!

Uh oh!

bardliao Aug 14, 2025 •

edited

Loading