sof-audio-pci-intel-tng I/O region is too small.

[htot]

On Linux yuna 6.0.0-rc7-edison-acpi-standard #1 SMP PREEMPT_DYNAMIC Sun Sep 25 21:01:02 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux I have:

root@yuna:~# lsmod | grep -i sof
snd_sof_pci_intel_tng    20480  0
snd_sof_pci            24576  1 snd_sof_pci_intel_tng
snd_sof_intel_atom     20480  1 snd_sof_pci_intel_tng
snd_sof               241664  3 snd_sof_intel_atom,snd_sof_pci_intel_tng,snd_sof_pci
snd_sof_utils          20480  1 snd_sof
snd_sof_xtensa_dsp     16384  1 snd_sof_pci_intel_tng
snd_intel_dspcfg       32768  1 snd_sof
snd_soc_acpi           16384  1 snd_sof_intel_atom
...
kernel: sof-audio-pci-intel-tng 0000:00:0d.0: error: I/O region is too small.
kernel: sof-audio-pci-intel-tng 0000:00:0d.0: error: failed to probe DSP -19

I am loading firmware 2.2.1 sof/sof-byt.ri and sof-tplg/sof-byt-nocodec.tplg

[plbossart]

Thanks for the report @htot. The code reporting an issue hasn't changed in a very long time. I am not sure what happened here. Could you recheck if a less-recent kernel has the same issue?

	/* LPE base */
	base = pci_resource_start(pci, desc->resindex_lpe_base) - IRAM_OFFSET;
	size = pci_resource_len(pci, desc->resindex_lpe_base);
	if (size < PCI_BAR_SIZE) {
		dev_err(sdev->dev, "error: I/O region is too small.\n");
		return -ENODEV;
	}

[htot]

root@yuna:~# uname -a
Linux yuna 5.16.0-edison-acpi-standard #1 SMP PREEMPT Sun Jan 9 22:55:34 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux
root@yuna:~# lsmod | grep -i sof
snd_sof_nocodec        16384  0
snd_sof_pci_intel_tng    20480  0
snd_sof_pci            20480  1 snd_sof_pci_intel_tng
snd_sof_intel_atom     20480  1 snd_sof_pci_intel_tng
snd_sof               131072  4 snd_sof_nocodec,snd_sof_intel_atom,snd_sof_pci_intel_tng,snd_sof_pci
snd_sof_xtensa_dsp     16384  1 snd_sof_pci_intel_tng
snd_soc_acpi           16384  1 snd_sof_intel_atom
root@yuna:~# journalctl -b -0 -k | grep -i sof
pps_core: Software ver. 5.3.6 - Copyright 2005-2007 Rodolfo Giometti <[email protected]>
xor: measuring software checksum speed
sof-audio-pci-intel-tng 0000:00:0d.0: warning: No matching ASoC machine driver found
sof-audio-pci-intel-tng 0000:00:0d.0: Using nocodec machine driver
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware info: version 2:2:0-57864
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware: ABI 3:22:1 Kernel ABI 3:18:0
sof-audio-pci-intel-tng 0000:00:0d.0: warn: FW ABI is more recent than kernel
sof-audio-pci-intel-tng 0000:00:0d.0: unknown sof_ext_man header type 3 size 0x30
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware info: version 2:2:0-57864
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware: ABI 3:22:1 Kernel ABI 3:18:0
sof-audio-pci-intel-tng 0000:00:0d.0: warn: FW ABI is more recent than kernel
sof-audio-pci-intel-tng 0000:00:0d.0: Topology: ABI 3:22:1 Kernel ABI 3:18:0
sof-audio-pci-intel-tng 0000:00:0d.0: warn: topology ABI is more recent than kernel
sof-nocodec sof-nocodec: ASoC: Parent card not yet available, widget card binding deferred

Just switching kernel, firmware is a bit too young. But no I/O region error.

[plbossart]

Thanks @htot, this looks like a regression in the last year or so, no idea where to start. I am afraid I won't be able to help much, I have too many things on my plate at the moment, maybe a git bisect could 'quickly' find the issue.

[htot]

Maybe @andy-shev has an idea?

[htot]

The code reporting an issue hasn't changed in a very long time. I am not sure what happened here.

Checking blame this is likely caused by recent patch 5947b272 "ASoC: SOF: Intel: Check the bar size before remapping" landing in v5.19. From LKML it appears the patch was found by fuzzing and did not fix an actual issue.

[htot]

After revert of patch https://github.com/thesofproject/linux/commit/5947b2726beb61fe7911580f239222ec9c4f6967 I get:

root@yuna:~# journalctl -b -0 -k | grep -i sof
sof-audio-pci-intel-tng 0000:00:0d.0: warning: No matching ASoC machine driver found
sof-audio-pci-intel-tng 0000:00:0d.0: Using nocodec machine driver
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware info: version 2:2:0-57864
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware: ABI 3:22:1 Kernel ABI 3:23:0
sof-audio-pci-intel-tng 0000:00:0d.0: unknown sof_ext_man header type 3 size 0x30
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware info: version 2:2:0-57864
sof-audio-pci-intel-tng 0000:00:0d.0: Firmware: ABI 3:22:1 Kernel ABI 3:23:0
sof-audio-pci-intel-tng 0000:00:0d.0: Topology: ABI 3:22:1 Kernel ABI 3:23:0
sof-nocodec sof-nocodec: ASoC: Parent card not yet available, widget card binding deferred

[htot]

I see the problem, @plbossart I hope you will know what to do?

root@yuna:~# lspci -v 
00:0d.0 Multimedia audio controller: Intel Corporation Device 119a (rev 01)
        Flags: 66MHz, user-definable features, ?? devsel, IRQ 29
        Memory at 05e00000 (32-bit, non-prefetchable) [size=2M]
        Memory at ff340000 (32-bit, non-prefetchable) [size=16K]
        Memory at ff344000 (32-bit, non-prefetchable) [size=4K]
        Memory at ff2c0000 (32-bit, non-prefetchable) [size=80K]
        Memory at ff300000 (32-bit, non-prefetchable) [size=160K]
        Capabilities: [b0] Power Management version 3
        Capabilities: [b8] Vendor Specific Information: Len=08 <?>
        Capabilities: [c0] PCI-X non-bridge device
        Capabilities: [100] Vendor Specific Information: ID=0000 Rev=0 Len=024 <?>
        Kernel modules: snd_sof_pci_intel_tng

The driver is asking for index 3 here https://elixir.bootlin.com/linux/latest/source/sound/soc/sof/intel/pci-tng.c#L214

And indeed when I print out

 	if (size < PCI_BAR_SIZE) {
-		dev_err(sdev->dev, "error: I/O region is too small.\n");
+		dev_err(sdev->dev, "error: I/O region is too small for index %i LPE PHY base at 0x%x size 0x%x.\n", desc->resindex_lpe_base, base, size);
 		return -ENODEV;

I get sof-audio-pci-intel-tng 0000:00:0d.0: error: I/O region is too small for index 3 LPE PHY base at 0xff200000 size 0x14000.

So, to me it seems we are trying to remap the wrong memory region, or the right region but with the wrong length.

What to do? Change the index?

[htot]

I believe now index 3 is correct (IRAM start), but we need to change PCI_BAR_SIZE to 80k. Correct?

[andy-shev]

I have no idea if that code was ever been tested by Intel. The resources that PCI provides has never been changed on the Intel Merrifield. If it's a firmware issue, it needs to be worked around, otherwise it's clear software bug. @plbossart Can you shed a light on this?

[andy-shev]

@htot The BAR is 2M, but the rest of the PCI resources are split from that BAR IIUC.

• ALL (remapped by firmware?): Memory at 05e00000 (32-bit, non-prefetchable) [size=2M]
• LPE SHIM: Memory at ff340000 (32-bit, non-prefetchable) [size=16K]
• Mailbox: Memory at ff344000 (32-bit, non-prefetchable) [size=4K]
• IRAM: Memory at ff2c0000 (32-bit, non-prefetchable) [size=80K]
• DRAM: Memory at ff300000 (32-bit, non-prefetchable) [size=160K]

The offsets seem correct in the atom.h, what we need is to fix the probe code for Tangier.

[andy-shev]

The 0xff2xxxxx addresses are reserved up to Audio IPs start with the offset 0xff290000. The pci-tng.c is so broken, that simple fix as proposed by @htot does not seem correct. It needs the entire ->probe() to be reworked. (Actually, why doesn't it use hda_pci_intel_probe() for the main ->probe()?) @plbossart Who can fix that?

[plbossart]

The patch https://github.com/thesofproject/linux/commit/5947b2726beb61fe7911580f239222ec9c4f6967 only did this:

> > -     size = PCI_BAR_SIZE;
> > +     size = pci_resource_len(pci, desc->resindex_lpe_base);

So essentially it removed a valid size of 2M, and introduced a dependency on something else that returns the size of the IRAM region (80K), but still compares it to 2M. Of course that fails 100% of the time. I would err on the side of reverting that patch since it's not adding any value and no one has time to work on this.

@andy-shev the TNG hardware does not have the HDaudio controller so we can't reuse the same code as SKL+. There's a reason why there's an 'hda' prefix :-)

[htot]

We are requesting index 3: .resindex_lpe_base = 3, /* IRAM, but subtract IRAM offset */) And when I print the result, we get index 3 LPE PHY base at 0xff200000 size 0x14000 So we get what we request (IRAM), and it really is 80k.

In the past it seems we requested index 3, set an invalid size of 2M (and caused issues in another driver DMA-API: dwc3 dwc3.0.auto: cacheline tracking EEXIST, overlapping mappings aren't supported). Other then that IIRC I did test with you (using oscilloscope) that SOF seemed to be working.

[plbossart]

the logic is that we request the start of the IRAM, but subtract the IRAM offset to map the ENTIRE BAR.

It makes no sense to request the size of the window based on the IRAM only. That patch 5947b27 was completely wrong IMHO.

[htot]

I see. Could it be that the entire BAR was mapped to 05e00000 to prevent of overlaps?

00:01.0 SD Host controller: .. SD/SDIO/eMMC (rev 01) (prog-if 01)        Region 0: Memory at ff3fc000 (32-bit, non-prefetchable) [size=256]
00:01.2 SD Host controller: .. SD/SDIO/eMMC (rev 01) (prog-if 01)        Region 0: Memory at ff3fa000 (32-bit, non-prefetchable) [size=256]
00:01.3 SD Host controller: .. SD/SDIO/eMMC (rev 01) (prog-if 01)        Region 0: Memory at ff3fb000 (32-bit, non-prefetchable) [size=256]
00:06.0 System peripheral: Intel Corporation Device 1193 (rev 01)        Region 0: Memory at ff2a0000 (32-bit, non-prefetchable) [size=4K]
00:06.1 System peripheral: Intel Corporation Device 1193 (rev 01)        Region 0: Memory at ff2a1000 (32-bit, non-prefetchable) [size=4K]
00:0a.0 Communication controller: Intel Corporation Device 1197 (rev 01) Region 0: Memory at ff3f8000 (32-bit, non-prefetchable) [size=4K]
00:0e.0 System peripheral: Intel Corporation Device 119b (rev 01)        Region 0: Memory at ff298000 (32-bit, non-prefetchable) [size=16K]
00:0e.0 System peripheral: Intel Corporation Device 119b (rev 01)        Region 1: Memory at ff2a2000 (32-bit, non-prefetchable) [size=4K]

[andy-shev]

@plbossart Reverting is not the fix. The entire code assumes wrong start addresses, i.e. mapping of 2M starting at 0xff200000 is incorrect, and I think somebody who knows better what and how should be mapped for SOF should revisit the pci-tng.c. Perhaps changing the size to 80K is the solution, I dunno. But then how to access DMA controllers that are listed in another PCI device on Tangier and not being mapped? I'm really puzzled how it was (if ever) tested.

[andy-shev]

@plbossart Ah, I just read your message before the revert. The BAR I guess should be changed to 0 and then simple pcim_iomap_bar() or equivalent should suffice. As for HDA, I have looked into the implementation of the hda_pci_intel_probe() and I found nothing HDA specific there, maybe I missed something...

[plbossart]

I was suggesting a revert because the logic was flawed, the method to find the address and size are inconsistent and NEVER worked with 5947b27 Doing the right thing means having time and a working setup, I have neither.

[andy-shev]

@plbossart OK, that's what I assumed when said that Intel never tested this. @htot It looks like your setup is closest to the real life, I can mock up a patch that I think should make it correct, but I can't test that right away.

[htot]

I can test although I don't have any codecs here. Last time I tested was using nocodec and recording the stream with oscilloscope. Would that do?

[plbossart]

yes, that's the best way to test. I usually use a stream with 0xaaaa on the left and nothing on the right channel.

[htot]

At the time we did: https://github.com/edison-fw/meta-intel-edison/pull/112#issuecomment-651303135

[plbossart]

@htot @andy-shev it's unlikely that anyone in our team will have time to look into this, my only recommendation at this time is to revert the commit 5947b27 with PR #3913

[andy-shev]

@htot @andy-shev it's unlikely that anyone in our team will have time to look into this, my only recommendation at this time is to revert the commit 5947b27 with PR #3913

Sure, no objections from me. TBH I thought you had proceed already with it, but it seems stuck. Can we move it forward?

[plbossart]

Thanks @andy-shev I'll send this upstream.

[plbossart]

reverted upstream