ALSA: hda/tas2781: Skip invalid TAS2563 UEFI calibration data

Hi,

Georg Wenzel experienced a regession with TAS2563 amplifiers. His laptop lacks proper calibration data, and due to a bug, the driver failed to apply them in the v6.16.x. That's why they worked.

The amplifiers operate even without calibration data, so the calibration setup can be skipped if the data is invalid.

The first patch simplifies error paths in the tas2563_save_calibration() function.

The second patch skips the zero UEFI calibration data as it is not necessary to apply.

The previous patch for this problem
3bc4fc135ec7 ("ALSA: hda/tas2781: Enable bass speakers support") should be removed/reverted.

Gergo Koteles (2):
ALSA: hda/tas2781: Assign total_sz at the end of the function
ALSA: hda/tas2781: Skip invalid TAS2563 UEFI calibration data

 .../hda/codecs/side-codecs/tas2781_hda_i2c.c  | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

2.56.0

In the tas2563_save_calibration() function, cd->total_sz is assigned in several error paths.

Move the assignment to the end of the function.

---
Changes in v2:
- Zero total_sz at the beginning of the function as it may run multiple times (Sashiko)

---
 sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

index 5e5f46c9fb6b..cff8fab8133a 100644
@@ -335,10 +335,11 @@ static int tas2563_save_calibration(struct tas2781_hda *h)
 	}
 
 	cd->cali_dat_sz_per_dev = TAS2563_CAL_DATA_SIZE * TASDEV_CALIB_N;
+	cd->total_sz = 0;
 
 	/* extra byte for each device is the device number */
-	cd->total_sz = (cd->cali_dat_sz_per_dev + 1) * p->ndev;
-	data = cd->data = devm_kzalloc(p->dev, cd->total_sz,
+	data = cd->data = devm_kzalloc(p->dev,
+		(cd->cali_dat_sz_per_dev + 1) * p->ndev,
 		GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;

@@ -352,7 +353,6 @@ static int tas2563_save_calibration(struct tas2781_hda *h)
 			if (ret < 0 || ret >= sizeof(var8) - 1) {
 				dev_err(p->dev, "%s: Read %s failed\n",
 					__func__, var8);
-				cd->total_sz = 0;
 				return -EINVAL;
 			}
 			/*

@@ -370,7 +370,6 @@ static int tas2563_save_calibration(struct tas2781_hda *h)
 				dev_warn(p->dev,
 					"Dev %d: Caldat[%d] read failed %ld\n",
 					i, j, status);
-				cd->total_sz = 0;
 				return -EINVAL;
 			}
 			bedata = cpu_to_be32(*(uint32_t *)&data[offset]);

@@ -379,12 +378,7 @@ static int tas2563_save_calibration(struct tas2781_hda *h)
 		}
 	}
 
-	if (cd->total_sz != offset) {
-		dev_err(p->dev, "%s: tot_size(%lu) and offset(%u) mismatch\n",
-			__func__, cd->total_sz, offset);
-		cd->total_sz = 0;
-		return -EINVAL;
-	}
+	cd->total_sz = offset;
 
 	r->r0_reg = TAS2563_CAL_R0;
 	r->invr0_reg = TAS2563_CAL_INVR0;

2.56.0

Some Lenovo 14ARB7 laptops do not have proper calibration data.

Skip these zero calibration values and fall back to the calibration data provided by the DSP firmware.

---
Changes is v2:
- none

---
 sound/hda/codecs/side-codecs/tas2781_hda_i2c.c | 5 +++++
 1 file changed, 5 insertions(+)

index cff8fab8133a..44855f593daf 100644
@@ -372,6 +372,11 @@ static int tas2563_save_calibration(struct tas2781_hda *h)
 					i, j, status);
 				return -EINVAL;
 			}
+			if (*(uint32_t *)&data[offset] == 0) {
+				dev_warn(p->dev,
+					 "Dev %d: Caldat[%d] is zero\n", i, j);
+				return -EINVAL;
+			}
 			bedata = cpu_to_be32(*(uint32_t *)&data[offset]);
 			memcpy(&data[offset], &bedata, sizeof(bedata));
 			offset += TAS2563_CAL_DATA_SIZE;

2.56.0

Hi Gergo,

thanks for extending this to the zero-calibration case.

I tested the v2 series on my Lenovo Yoga 7 14ARB7 (82QF). Here the UEFI calibration variables are missing altogether, so tas2563_save_calibration() bails out at "Dev 0: Caldat[0] read failed" before the new zero check is reached -- the case my earlier patch was about. With both patches applied on top of v7.2.9, the warning is logged as expected, no calibration data is applied, and all four speakers, including the woofers, play normally after a reboot.

Thanks,
Philipp

Applied with the revert of the previous fix to for-next branch now.

thanks,

Takashi