From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from out-171.mta0.migadu.com (out-171.mta0.migadu.com [91.218.175.171]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4F98034677F for ; Mon, 2 Feb 2026 22:16:58 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=91.218.175.171 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770070620; cv=none; b=EmJ+4bBWdtt8noe68xJ9pprvILssxIu96RAKDrL+RHAhD1iH6xbFq06E05lZWc7AFibyXdXsMe/3JDadM2vdHKbHo9qrO1Uu37NwDt3ub1IYADa5+LQ6yGM4Mbpu6SYljQllhjtpBF+h+C5EfUHxnYlI5gYXxLJz1nEICQsVSWg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1770070620; c=relaxed/simple; bh=zHzYBjmfo/90ECS0y2lSqhw2NboTIgMrqnALAYkc9Nw=; h=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From: In-Reply-To:Content-Type; b=GKlnDcn18Jkd6nlOsowYeC/k9tl0ifDzMvdDu7rWw2o664+9Z4o5OO30DOzZT7XvcT8QLVqm2ZT5lh3NKs4ipWEYc/cWmI93+xICUJkEWZzu2lMBH+fZ6I3XAA9SH9sxB4538jW5dCxGBSycE349BRECElrYuKf4YTm3vQIDh58= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev; spf=pass smtp.mailfrom=linux.dev; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b=HK6eWers; arc=none smtp.client-ip=91.218.175.171 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.dev Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.dev Authentication-Results: smtp.subspace.kernel.org; dkim=pass (1024-bit key) header.d=linux.dev header.i=@linux.dev header.b="HK6eWers" Message-ID: DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linux.dev; s=key1; t=1770070616; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=ibRkWKAW76yeoP4vUGyCDE6Y6XiEGus+CWMEKGbnXYI=; b=HK6eWersIcbdM+Z++m+AAOYLA0cpvisKevM0Udp8j5878jzXzhvimi4g3BJyxtM79R2Ko/ 6x7583h19e4JGiiN1Hgd9vQMf+TKuzeKx33fNlQ1vhDdypBJx9REuRH+shwc2KRQzQWKWf kHaAt64/2gcHMzYYyYs25M9wxKcy5Yo= Date: Mon, 2 Feb 2026 14:16:50 -0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Subject: Re: [PATCH v1] ASoC: tas2781: Put three different calibrated data solution into the same data structure To: Shenghao Ding , tiwai@suse.de Cc: broonie@kernel.org, andriy.shevchenko@linux.intel.com, linux-kernel@vger.kernel.org, baojun.xu@ti.com, Baojun.Xu@fpt.com, 13564923607@139.com, 13916275206@139.com, lkml@antheas.dev References: <20260202102757.532-1-shenghao-ding@ti.com> Content-Language: en-US X-Report-Abuse: Please report any abuse attempt to abuse@migadu.com and include these headers. From: Matthew Schwartz In-Reply-To: <20260202102757.532-1-shenghao-ding@ti.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Migadu-Flow: FLOW_OUT 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@linux.dev/ > > Signed-off-by: Shenghao Ding > > --- > 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 > > @@ -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 > // Author: Baojun Xu > @@ -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;