ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format #4690

ujfalusi · 2023-11-07T14:11:25Z

When looking up DMIC blob from the NHLT table and the format is 32 bits,
ignore the vbps matching for 32 bps for DMIC since some NHLT table have
the vbps as 24, some have it as 32.
The DMIC hardware supports only one type of 32 bit sample size, which is
24 bit sampling on the MSB side and bits[1:0] is used for indicating the
channel number.

ranj063 · 2023-11-07T16:06:10Z

sound/hda/intel-nhlt.c

+			/*
+			 * The DMIC bps=32 configuration is described in some
+			 * table with vbps=32 (the DMIC hardware is vbps=24)
+			 */


@ujfalusi this patch feels a bit odd. May I suggest a different approach in the SOF driver by looking up both 32/32 and 24/32 when the bit-depth is 32 instead?

@ranj063, I had that implementation first, but it felt out of place and hackish. The reason why I have moved it here is that it is the NHLT table itself which is incorrect and it is better to handle this in the NHLT code rather then in upper layer.

no, the NHLT is not incorrect. It's really that vbs is irrelevant and should be ignored.

OK, so rewording the comments and the commit message would suffice?
But then if the user asks for 32/32 then we need to look for 32/24 also, right? For DMIC, that is.
The NHLT core can do this:
if DMIC and bps == 32, set the vbps to 24 first, try to find the blob, if not found, look for it using vbps = 32.
Or pass vbps = 0 to nhlt_get_specific_cfg() and if vbps is 0, do not try to match it, only look for bps match.

pass vbps = 0 to nhlt_get_specific_cfg() and if vbps is 0, do not try to match it, only look for bps match.

that would be my preference. vbps is a wildcard - ignored.

I'm ok with this as long as we can limit this to DMIC in the caller (i.e. pass 0 to use a wildcard).

ujfalusi · 2023-11-08T07:33:27Z

Changes since v1:

Drop the SOF patch
Ignore vbps when looking for DMIC blob with 32bps

sound/hda/intel-nhlt.c

kv2019i

Code-wise ok. I have no favorites whether this should be on SOF or in intel-nhlt.c. I guess depends on wheter AVS et al assume this to be handled by intel-nhlt.c or not.

ujfalusi · 2023-11-09T13:18:14Z

Changes since v2:

Only ignore vbps in case of DMIC with bps==32, use a local flag to not change the handling of vbps from caller in other cases.

ranj063

a couple of typos to fix before upstreaming

ranj063 · 2023-11-10T16:34:39Z

sound/hda/intel-nhlt.c

+		 * table have the vbps as 24, some have it as 32.
+		 * The DMIC hardware supports only one type of 32 bit sample
+		 * size, which is 24 bit sampling on the MSB side and bits[1:0]
+		 * is used for indicating the channel number.


a couple of typos: some NHLT tables .... bits[1:0] are used....

we should also say that the 32 bit case is harmless, otherwise the whole point of ignoring vbs could be seen as a regression or a miss.

ujfalusi · 2023-11-13T10:25:46Z

Changes since v2:

Reword the comment
Make the ignoring vbps more strict and only do it when vbps is either 32 or 24

sound/hda/intel-nhlt.c

ujfalusi · 2023-11-13T15:15:45Z

Changes since v3:

Fix the stale comment in nhlt_get_specific_cfg()

ranj063 · 2023-11-13T16:27:47Z

sound/hda/intel-nhlt.c

@@ -255,8 +255,12 @@ nhlt_get_specific_cfg(struct device *dev, struct nhlt_fmt *fmt, u8 num_ch,
 		dev_dbg(dev, "Endpoint format: ch=%d fmt=%d/%d rate=%d\n",
 			wfmt->channels, _vbps, _bps, wfmt->samples_per_sec);

+		/*
+		 * When looking for exact match of configuration ignore the vbps
+		 * from NHTL table when ignore_vbps is true


Typo NHTL 🙂

ujfalusi · 2023-11-14T11:05:00Z

Changers since v4:

Fixed another typo in a comment (NHTL -> NHLT)

ujfalusi · 2023-11-16T13:39:32Z

SOFCI TEST

ranj063 · 2023-11-16T16:49:28Z

SOFCI TEST

When looking up DMIC blob from the NHLT table and the format is 32 bits, ignore the vbps matching for 32 bps for DMIC since some NHLT table have the vbps as 24, some have it as 32. The DMIC hardware supports only one type of 32 bit sample size, which is 24 bit sampling on the MSB side and bits[1:0] is used for indicating the channel number. Signed-off-by: Peter Ujfalusi <[email protected]>

ujfalusi · 2023-11-20T06:52:19Z

Sorry, force pushed to kick a new CI run. The recent failures cannot be caused by this PR - they were not present in earlier versions and there were no change in functionality...

ujfalusi requested a review from singalsu November 7, 2023 14:11

ujfalusi requested review from lgirdwood, plbossart, ranj063, kv2019i, dbaluta, bardliao, vijendarmukunda, kuanhsuncheng and yaochunhung as code owners November 7, 2023 14:11

ujfalusi linked an issue Nov 7, 2023 that may be closed by this pull request

[BUG][MTL] 24bit pch-dmic capture fails due to missing config in NHLT #4689

Closed

ranj063 reviewed Nov 7, 2023

View reviewed changes

ujfalusi force-pushed the peter/sof/pr/32bit_dmic_blob_01 branch from 9205b48 to f294891 Compare November 8, 2023 07:30

ujfalusi changed the title ~~ALSA/ASoC: SOF: nhlt dmic blob lookup fix for 32bit format~~ ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format Nov 8, 2023

plbossart reviewed Nov 8, 2023

View reviewed changes

sound/hda/intel-nhlt.c Show resolved Hide resolved

kv2019i previously approved these changes Nov 8, 2023

View reviewed changes

ujfalusi dismissed kv2019i’s stale review via bcd24c4 November 9, 2023 11:37

ujfalusi force-pushed the peter/sof/pr/32bit_dmic_blob_01 branch from f294891 to bcd24c4 Compare November 9, 2023 11:37

kv2019i previously approved these changes Nov 10, 2023

View reviewed changes

ranj063 previously approved these changes Nov 10, 2023

View reviewed changes

ujfalusi dismissed stale reviews from ranj063 and kv2019i via bb43bdd November 13, 2023 10:23

ujfalusi force-pushed the peter/sof/pr/32bit_dmic_blob_01 branch from bcd24c4 to bb43bdd Compare November 13, 2023 10:23

plbossart reviewed Nov 13, 2023

View reviewed changes

sound/hda/intel-nhlt.c Outdated Show resolved Hide resolved

ujfalusi force-pushed the peter/sof/pr/32bit_dmic_blob_01 branch from bb43bdd to c315e56 Compare November 13, 2023 15:15

plbossart previously approved these changes Nov 13, 2023

View reviewed changes

ranj063 reviewed Nov 13, 2023

View reviewed changes

ranj063 previously approved these changes Nov 13, 2023

View reviewed changes

ujfalusi dismissed stale reviews from ranj063 and plbossart via 00a77ff November 14, 2023 11:04

ujfalusi force-pushed the peter/sof/pr/32bit_dmic_blob_01 branch from c315e56 to 00a77ff Compare November 14, 2023 11:04

kv2019i approved these changes Nov 14, 2023

View reviewed changes

plbossart approved these changes Nov 14, 2023

View reviewed changes

plbossart requested a review from ranj063 November 14, 2023 15:56

ranj063 approved these changes Nov 14, 2023

View reviewed changes

ujfalusi force-pushed the peter/sof/pr/32bit_dmic_blob_01 branch from 00a77ff to 07cfd5b Compare November 20, 2023 06:50

plbossart merged commit ff10cf7 into thesofproject:topic/sof-dev Nov 20, 2023
9 checks passed

ujfalusi deleted the peter/sof/pr/32bit_dmic_blob_01 branch November 30, 2023 06:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format #4690

ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format #4690

ujfalusi commented Nov 7, 2023 •

edited

Loading

ranj063 Nov 7, 2023

ujfalusi Nov 7, 2023

plbossart Nov 7, 2023

ujfalusi Nov 7, 2023

plbossart Nov 7, 2023

ranj063 Nov 7, 2023

kv2019i Nov 7, 2023

ujfalusi commented Nov 8, 2023

kv2019i left a comment

ujfalusi commented Nov 9, 2023

ranj063 left a comment

ranj063 Nov 10, 2023

plbossart Nov 10, 2023

ujfalusi Nov 13, 2023

ujfalusi commented Nov 13, 2023

ujfalusi commented Nov 13, 2023

ranj063 Nov 13, 2023

ujfalusi commented Nov 14, 2023

ujfalusi commented Nov 16, 2023

ranj063 commented Nov 16, 2023

ujfalusi commented Nov 20, 2023

ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format #4690

ALSA: hda: intel-nhlt: Ignore vbps when looking for DMIC 32 bps format #4690

Conversation

ujfalusi commented Nov 7, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi commented Nov 8, 2023

kv2019i left a comment

Choose a reason for hiding this comment

ujfalusi commented Nov 9, 2023

ranj063 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ujfalusi commented Nov 13, 2023

ujfalusi commented Nov 13, 2023

Choose a reason for hiding this comment

ujfalusi commented Nov 14, 2023

ujfalusi commented Nov 16, 2023

ranj063 commented Nov 16, 2023

ujfalusi commented Nov 20, 2023

ujfalusi commented Nov 7, 2023 •

edited

Loading