Re: [PATCH v1] ASoC: tas2781: Put three different calibrated data solution into the same data structure

[Matthew Schwartz]

On 2/2/26 2:27 AM, Shenghao Ding wrote:
> TAS2781 driver supports three solutions of calibrated data. The first is
> from the driver itself: driver reads the calibrated files directly during
> probe; The second is from user space: during init of audio hal, the audio
> hal will pass the calibrated data via kcontrol interface. Driver will
> store this data in "struct calidata" for use. The third is from UEFI,
> mainly used in hda device. These three solutions save the calibrated data
> into different data structures. It is time to put them together into
> "struct calidata" for use.

Hello,

I tested this on my ROG Xbox Ally X by reverting b7e26c8bdae7 ("ALSA: hda/tas2781: Skip UEFI calibration on ASUS ROG Xbox Ally X") and applying only this patch on top of 6.19-rc8. 

The same speaker issues reported in [1] persist, where one or both speakers drop out intermittently. I leave on vacation soon until February 24th but I will be available for further testing after that.

Thanks,
Matt

[1]: https://lore.kernel.org/all/0ba100d0-9b6f-4a3b-bffa-61abe1b46cd5@xxxxxxxxx/

> 
> Signed-off-by: Shenghao Ding <shenghao-ding@xxxxxx>
> 
> ---
> v1:
>  - Drop is_user_space_calidata
>  - Update the year of tas2781_hda.c, tas2781.h, tas2781-fmwlib.c,
>    tas2781-i2c.c and tas2781_hda_i2c.c
>  - Set to an invalid value before the calibrated data loading
>  - Drop load_calib_data()
>  - Add calbin_conversion() and call it after loading calibin file
> ---
>  include/sound/tas2781.h                       |   3 +-
>  sound/hda/codecs/side-codecs/tas2781_hda.c    |   9 +-
>  .../hda/codecs/side-codecs/tas2781_hda_i2c.c  |  13 --
>  sound/soc/codecs/tas2781-fmwlib.c             | 138 ++++++++++++++----
>  sound/soc/codecs/tas2781-i2c.c                |  11 +-
>  5 files changed, 121 insertions(+), 53 deletions(-)
> 
> diff --git a/include/sound/tas2781.h b/include/sound/tas2781.h
> index 9d3c54cb8223..7c03bdc951bb 100644
> --- a/include/sound/tas2781.h
> +++ b/include/sound/tas2781.h
> @@ -2,7 +2,7 @@
>  //
>  // ALSA SoC Texas Instruments TAS2563/TAS2781 Audio Smart Amplifier
>  //
> -// Copyright (C) 2022 - 2025 Texas Instruments Incorporated
> +// Copyright (C) 2022 - 2026 Texas Instruments Incorporated
>  // https://www.ti.com
>  //
>  // The TAS2563/TAS2781 driver implements a flexible and configurable
> @@ -233,7 +233,6 @@ struct tasdevice_priv {
>  	bool playback_started;
>  	bool isacpi;
>  	bool isspi;
> -	bool is_user_space_calidata;
>  	unsigned int global_addr;
>  
>  	int (*fw_parse_variable_header)(struct tasdevice_priv *tas_priv,
> diff --git a/sound/hda/codecs/side-codecs/tas2781_hda.c b/sound/hda/codecs/side-codecs/tas2781_hda.c
> index 96e6d82dc69e..30a1bae51663 100644
> --- a/sound/hda/codecs/side-codecs/tas2781_hda.c
> +++ b/sound/hda/codecs/side-codecs/tas2781_hda.c
> @@ -2,7 +2,7 @@
>  //
>  // TAS2781 HDA Shared Lib for I2C&SPI driver
>  //
> -// Copyright 2025 Texas Instruments, Inc.
> +// Copyright 2025 - 2026 Texas Instruments, Inc.
>  //
>  // Author: Shenghao Ding <shenghao-ding@xxxxxx>
>  
> @@ -159,7 +159,6 @@ static void tas2781_apply_calib(struct tasdevice_priv *p)
>  		r->tlimit_reg = cali_reg[4];
>  	}
>  
> -	p->is_user_space_calidata = true;
>  	cali_data->total_sz = p->ndev * (cali_data->cali_dat_sz_per_dev + 1);
>  }
>  
> @@ -216,6 +215,12 @@ int tas2781_save_calibration(struct tas2781_hda *hda)
>  				status = -ENOMEM;
>  				continue;
>  			}
> +			/*
> +			 * Set to an invalid value before the calibrated data
> +			 * is stored into it, for the default value is 0, which
> +			 * means the first device.
> +			 */
> +			data[0] = 0xff;
>  			/* Get variable contents into buffer */
>  			status = efi.get_variable(efi_name[i], &efi_guid,
>  				&attr, &cali_data->total_sz, data);
> diff --git a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c
> index 49c80babb500..74c3cf1e45e1 100644
> --- a/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c
> +++ b/sound/hda/codecs/side-codecs/tas2781_hda_i2c.c
> @@ -393,19 +393,6 @@ static int tas2563_save_calibration(struct tas2781_hda *h)
>  	r->pow_reg = TAS2563_CAL_POWER;
>  	r->tlimit_reg = TAS2563_CAL_TLIM;
>  
> -	/*
> -	 * TAS2781_FMWLIB supports two solutions of calibrated data. One is
> -	 * from the driver itself: driver reads the calibrated files directly
> -	 * during probe; The other from user space: during init of audio hal,
> -	 * the audio hal will pass the calibrated data via kcontrol interface.
> -	 * Driver will store this data in "struct calidata" for use. For hda
> -	 * device, calibrated data are usunally saved into UEFI. So Hda side
> -	 * codec driver use the mixture of these two solutions, driver reads
> -	 * the data from UEFI, then store this data in "struct calidata" for
> -	 * use.
> -	 */
> -	p->is_user_space_calidata = true;
> -
>  	return 0;
>  }
>  
> diff --git a/sound/soc/codecs/tas2781-fmwlib.c b/sound/soc/codecs/tas2781-fmwlib.c
> index 78fd0a5dc6f2..0e084c3a162d 100644
> --- a/sound/soc/codecs/tas2781-fmwlib.c
> +++ b/sound/soc/codecs/tas2781-fmwlib.c
> @@ -2,7 +2,7 @@
>  //
>  // tas2781-fmwlib.c -- TASDEVICE firmware support
>  //
> -// Copyright 2023 - 2025 Texas Instruments, Inc.
> +// Copyright 2023 - 2026 Texas Instruments, Inc.
>  //
>  // Author: Shenghao Ding <shenghao-ding@xxxxxx>
>  // Author: Baojun Xu <baojun.xu@xxxxxx>
> @@ -80,6 +80,14 @@
>  #define POST_SOFTWARE_RESET_DEVICE_C			0x47
>  #define POST_SOFTWARE_RESET_DEVICE_D			0x48
>  
> +#define COPY_CAL_DATA(i) \
> +	do { \
> +		calbin_data[i + 1] = data[7]; \
> +		calbin_data[i + 2] = data[8]; \
> +		calbin_data[i + 3] = data[9]; \
> +		calbin_data[i + 4] = data[10]; \
> +	} while (0)
> +
>  struct tas_crc {
>  	unsigned char offset;
>  	unsigned char len;
> @@ -1952,23 +1960,6 @@ static int dspfw_default_callback(struct tasdevice_priv *tas_priv,
>  	return rc;
>  }
>  
> -static int load_calib_data(struct tasdevice_priv *tas_priv,
> -	struct tasdevice_data *dev_data)
> -{
> -	struct tasdev_blk *block;
> -	unsigned int i;
> -	int ret = 0;
> -
> -	for (i = 0; i < dev_data->nr_blk; i++) {
> -		block = &(dev_data->dev_blks[i]);
> -		ret = tasdevice_load_block(tas_priv, block);
> -		if (ret < 0)
> -			break;
> -	}
> -
> -	return ret;
> -}
> -
>  static int fw_parse_header(struct tasdevice_priv *tas_priv,
>  	struct tasdevice_fw *tas_fmw, const struct firmware *fmw, int offset)
>  {
> @@ -2029,6 +2020,103 @@ static int fw_parse_variable_hdr_cal(struct tasdevice_priv *tas_priv,
>  	return offset;
>  }
>  
> +static inline int check_cal_bin_data(struct device *dev,
> +	const unsigned char *data, const char *name)
> +{
> +	if (data[2] != 0x85 || data[1] != 4) {
> +		dev_err(dev, "Invalid cal bin file in %s\n", name);
> +		return -1;
> +	}
> +	return 0;
> +}
> +
> +static void calbin_conversion(struct tasdevice_priv *priv,
> +	struct tasdevice_fw *tas_fmw)
> +{
> +	struct calidata *cali_data = &priv->cali_data;
> +	unsigned char *calbin_data = cali_data->data;
> +	struct cali_reg *p = &cali_data->cali_reg_array;
> +	struct tasdevice_calibration *calibration;
> +	struct tasdevice_data *img_data;
> +	struct tasdev_blk *blk;
> +	unsigned char *data;
> +	int chn, k;
> +
> +	if (cali_data->total_sz != priv->ndev *
> +		(cali_data->cali_dat_sz_per_dev + 1)) {
> +		dev_err(priv->dev, "%s: cali_data size err\n",
> +			__func__);
> +		return;
> +	}
> +	calibration = &(tas_fmw->calibrations[0]);
> +	img_data = &(calibration->dev_data);
> +
> +	if (img_data->nr_blk != 1) {
> +		dev_err(priv->dev, "%s: Invalid nr_blk, wrong cal bin\n",
> +			__func__);
> +		return;
> +	}
> +
> +	blk = &(img_data->dev_blks[0]);
> +	if (blk->nr_cmds != 15) {
> +		dev_err(priv->dev, "%s: Invalid nr_cmds, wrong cal bin\n",
> +			__func__);
> +		return;
> +	}
> +
> +	switch (blk->type) {
> +	case COEFF_DEVICE_A:
> +		chn = 0;
> +		break;
> +	case COEFF_DEVICE_B:
> +		chn = 1;
> +		break;
> +	case COEFF_DEVICE_C:
> +		chn = 2;
> +		break;
> +	case COEFF_DEVICE_D:
> +		chn = 3;
> +		break;
> +	default:
> +		dev_err(priv->dev, "%s: Other Type = 0x%02x\n",
> +			__func__, blk->type);
> +		return;
> +	}
> +	k = chn * (cali_data->cali_dat_sz_per_dev + 1);
> +
> +	data = blk->data;
> +	if (check_cal_bin_data(priv->dev, data, "r0_reg") < 0)
> +		return;
> +	p->r0_reg = TASDEVICE_REG(data[4], data[5], data[6]);
> +	COPY_CAL_DATA(k);
> +
> +	data = blk->data + 12;
> +	if (check_cal_bin_data(priv->dev, data, "r0_low_reg") < 0)
> +		return;
> +	p->r0_low_reg = TASDEVICE_REG(data[4], data[5], data[6]);
> +	COPY_CAL_DATA(k + 4);
> +
> +	data = blk->data + 24;
> +	if (check_cal_bin_data(priv->dev, data, "invr0_reg") < 0)
> +		return;
> +	p->invr0_reg = TASDEVICE_REG(data[4], data[5], data[6]);
> +	COPY_CAL_DATA(k + 8);
> +
> +	data = blk->data + 36;
> +	if (check_cal_bin_data(priv->dev, data, "pow_reg") < 0)
> +		return;
> +	p->pow_reg = TASDEVICE_REG(data[4], data[5], data[6]);
> +	COPY_CAL_DATA(k + 12);
> +
> +	data = blk->data + 48;
> +	if (check_cal_bin_data(priv->dev, data, "tlimit_reg") < 0)
> +		return;
> +	p->tlimit_reg = TASDEVICE_REG(data[4], data[5], data[6]);
> +	COPY_CAL_DATA(k + 16);
> +
> +	calbin_data[k] = chn;
> +}
> +
>  /* When calibrated data parsing error occurs, DSP can still work with default
>   * calibrated data, memory resource related to calibrated data will be
>   * released in the tasdevice_codec_remove.
> @@ -2086,6 +2174,7 @@ static int fw_parse_calibration_data(struct tasdevice_priv *tas_priv,
>  			goto out;
>  	}
>  
> +	calbin_conversion(tas_priv, tas_fmw);
>  out:
>  	return offset;
>  }
> @@ -2371,25 +2460,12 @@ static int tasdevice_load_data(struct tasdevice_priv *tas_priv,
>  
>  static void tasdev_load_calibrated_data(struct tasdevice_priv *priv, int i)
>  {
> -	struct tasdevice_fw *cal_fmw = priv->tasdevice[i].cali_data_fmw;
>  	struct calidata *cali_data = &priv->cali_data;
>  	struct cali_reg *p = &cali_data->cali_reg_array;
>  	unsigned char *data = cali_data->data;
> -	struct tasdevice_calibration *cal;
>  	int k = i * (cali_data->cali_dat_sz_per_dev + 1);
>  	int rc;
>  
> -	/* Load the calibrated data from cal bin file */
> -	if (!priv->is_user_space_calidata && cal_fmw) {
> -		cal = cal_fmw->calibrations;
> -
> -		if (cal)
> -			load_calib_data(priv, &cal->dev_data);
> -		return;
> -	}
> -	if (!priv->is_user_space_calidata)
> -		return;
> -	/* load calibrated data from user space */
>  	if (data[k] != i) {
>  		dev_err(priv->dev, "%s: no cal-data for dev %d from usr-spc\n",
>  			__func__, i);
> diff --git a/sound/soc/codecs/tas2781-i2c.c b/sound/soc/codecs/tas2781-i2c.c
> index d1c76ab0144d..41b89fcc69c3 100644
> --- a/sound/soc/codecs/tas2781-i2c.c
> +++ b/sound/soc/codecs/tas2781-i2c.c
> @@ -2,7 +2,7 @@
>  //
>  // ALSA SoC Texas Instruments TAS2563/TAS2781 Audio Smart Amplifier
>  //
> -// Copyright (C) 2022 - 2025 Texas Instruments Incorporated
> +// Copyright (C) 2022 - 2026 Texas Instruments Incorporated
>  // https://www.ti.com
>  //
>  // The TAS2563/TAS2781 driver implements a flexible and configurable
> @@ -255,8 +255,6 @@ static int tasdev_cali_data_get(struct snd_kcontrol *kcontrol,
>  	int rc;
>  
>  	guard(mutex)(&priv->codec_lock);
> -	if (!priv->is_user_space_calidata)
> -		return -1;
>  
>  	if (!p->r0_reg)
>  		return -1;
> @@ -654,7 +652,6 @@ static int tasdev_cali_data_put(struct snd_kcontrol *kcontrol,
>  		}
>  	}
>  	i += 2;
> -	priv->is_user_space_calidata = true;
>  
>  	if (priv->dspbin_typ == TASDEV_BASIC) {
>  		p->r0_reg = TASDEVICE_REG(src[i], src[i + 1], src[i + 2]);
> @@ -1444,7 +1441,11 @@ static int tasdevice_create_cali_ctrls(struct tasdevice_priv *priv)
>  		GFP_KERNEL);
>  	if (!cali_data->data)
>  		return -ENOMEM;
> -
> +	/*
> +	 * Set to an invalid value before the calibrated data is stored into
> +	 * it, for the default value is 0, which means the first device.
> +	 */
> +	cali_data->data[0] = 0xff;
>  	if (priv->chip_id == TAS2781) {
>  		struct soc_bytes_ext *ext_cali_start;
>  		char *cali_start_name;