Message-ID: 
 <CAGwozwH6A-NNCioS7J_EiTbxv2Y0B3YFXuyeWU_RMsW2Z+YsRA@mail.gmail.com>
Date: Sat, 3 Jan 2026 01:13:03 +0200
From: Antheas Kapenekakis <lkml@...heas.dev>
To: Matthew Schwartz <matthew.schwartz@...ux.dev>
Cc: Baojun Xu <baojun.xu@...com>, Shenghao Ding <shenghao-ding@...com>,
 linux-sound@...r.kernel.org,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, tiwai@...e.de
Subject: Re: [BUG] hda/tas2781: ASUS ROG Xbox Ally X audio issues with default
 firmware

On Fri, 2 Jan 2026 at 21:17, Matthew Schwartz
<matthew.schwartz@...ux.dev> wrote:
>
> On 1/2/26 9:12 AM, Antheas Kapenekakis wrote:
> > On Tue, 30 Dec 2025 at 22:44, Matthew Schwartz
> > <matthew.schwartz@...ux.dev> wrote:
> >>
> >>
> >>
> >>> On Dec 8, 2025, at 8:00 PM, Matthew Schwartz <matthew.schwartz@...ux.dev> wrote:
> >>>
> >> - snip
> >>> 2.52.0
> >>
> >> After reading the TI E2E forums, it seems these calibration tuning configurations are only meant to be used during a calibration process.
> >
> > A source would be good here, a link or two
>
> Sorry about that, here is where I read about the differences between the two configs:
> https://e2e.ti.com/support/audio-group/audio/f/audio-forum/1558310/tas2563-what-is-the-difference-between-tuning-and-calibration-configuration-in-exported-smartamp-binary

This link describes "calibration" configurations that are used in the
calibration procedure. It is not clear to me that it refers to the
calibration parameters exported by UEFI or in the configuration itself
to be used alongside a configuration. I would tend toward this being
irrelevant.

> It's about a different amplifier model, but I assume the same applies to tas2781 given the naming structure is the same for the configurations.
>
> >
> >> Instead, something else I found that works is not overriding the firmware file calibration data with the UEFI calibration data:

I misunderstood what you meant by this before. I thought you meant
that the firmware overrode the UEFI data, not the other way around.
Surely, using the dummy data in the firmware file is better than using
incorrect data from UEFI. However, the manufacturer calibrated data
from the factory floor for each specific unit is in UEFI, so that is
what should be used.

> > I did not look into the source code, do you have any reference in the
> > ACPI TAS code that r0_buf is prefilled with UEFI data?
>
> From what I understand, I think the current flow goes like this:
>
> 1. During driver init, tas2781_save_calibration() reads UEFI calibration data into the cali_data memory buffer and sets is_user_space_calidata=true: https://github.com/torvalds/linux/blob/master/sound/hda/codecs/side-codecs/tas2781_hda.c#L162-L173
>
> 2. When switching to a DSP config, tasdevice_select_tuningprm_cfg() calls tasdevice_load_data() which writes the firmware configuration, including any calibration values embedded in that config: https://github.com/torvalds/linux/blob/master/sound/soc/codecs/tas2781-fmwlib.c#L2510
>
> 3. Immediately after, tasdev_load_calibrated_data() writes the UEFI calibration data from step 1, overwriting the values just set in step 2: https://github.com/torvalds/linux/blob/9b043680446067358913edc2e9dd71bf8ffae208/sound/soc/codecs/tas2781-fmwlib.c#L2392-L2428 + https://github.com/torvalds/linux/blob/master/sound/soc/codecs/tas2781-fmwlib.c#L2519
>
> I confirmed this this by inserting some debug logs around tasdev_load_calibrated_data:

I went through the code. It loads the UEFI data, sets
is_user_space_calidata=1, then if the data is available it loads it.
This is correct.

To me this seems like the calibration data for amp 1 is written to
both amp 1 and amp 2, and for your firmware this breaks amp2.

In tasdev_load_calibrated_data(), an i index is provided, but
cali_data is nested under priv. So only one calibration set is
supported. This means that amp 2 gets amp 1 calibrations.

Perhaps there is a "SmartAmpCalibrationData2" that should be read
instead for amp 2 instead, can you dump the EFI variables and check?

In that case, a minor refactor to move cali_data and
is_user_space_calidata from priv to priv->tasdevice[i] and then use
the proper efi var would fix this.

Antheas

> [    3.908963] tas2781-hda i2c-TXNW2781:00-tas2781-hda.0: tas2781_apply_calib: dspbin_typ=2, ndev=2, Setting is_user_space_calidata=true
> <snip>
>
> where it loads the UEFI calibration over top of the firmware calibration.
>
> >
> > It could be that there is a specific priority, where UEFI data is
> > supposed to be loaded after the firmware code, replacing the
> > calibration data from the file, or that is what is done in Windows.
> > But here it is done the other way. In that case, it might be more
> > appropriate to set a dummy var such as bool uefi_calib that becomes 1
> > when loading calibration from UEFI, and skip loading from the fw file
> > if available.
> >
> > But links, etc. Here, this would affect all TAS devices too, so it is
> > more major.
>
> Yes, was really hoping to get TI's feedback before potentially sending it out, as it would be a major change and the documentation on this is scarce. I could also be misunderstanding the calibration data load flow, but this is just from poking at this issue from every angle I can think of.
>
> Happy new year,
> Matt
>
> >
> > Happy new year,
> > Antheas
> >
> >> diff --git a/sound/soc/codecs/tas2781-fmwlib.c b/sound/soc/codecs/tas2781-fmwlib.c
> >> index 78fd0a5dc6f2..1e768e6187da 100644
> >> --- a/sound/soc/codecs/tas2781-fmwlib.c
> >> +++ b/sound/soc/codecs/tas2781-fmwlib.c
> >> @@ -2377,6 +2377,7 @@ static void tasdev_load_calibrated_data(struct tasdevice_priv *priv, int i)
> >>         unsigned char *data = cali_data->data;
> >>         struct tasdevice_calibration *cal;
> >>         int k = i * (cali_data->cali_dat_sz_per_dev + 1);
> >> +       unsigned char r0_buf[4];
> >>         int rc;
> >>
> >>         /* Load the calibrated data from cal bin file */
> >> @@ -2389,6 +2390,20 @@ static void tasdev_load_calibrated_data(struct tasdevice_priv *priv, int i)
> >>         }
> >>         if (!priv->is_user_space_calidata)
> >>                 return;
> >> +
> >> +       /*
> >> +        * Check if the DSP config already set the calibration registers.
> >> +        * Some tuning configs contain their own calibration data which should
> >> +        * not be overwritten by user space calibration data.
> >> +        */
> >> +       rc = tasdevice_dev_bulk_read(priv, i, p->r0_reg, r0_buf, 4);
> >> +       if (rc >= 0 && (r0_buf[0] | r0_buf[1] | r0_buf[2] | r0_buf[3])) {
> >> +               dev_dbg(priv->dev,
> >> +                       "%s: dev %d r0_reg already set by config, skipping calibration\n",
> >> +                       __func__, i);
> >> +               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",
> >>
> >> Is this more suitable to be upstreamed?
> >>
> >> Thanks,
> >> Matt
> >> <snip>
> >>
> >
>
>

Powered by blists - more mailing lists