ASoC: SOF: Intel: hda: Add support for persistent Code Loader DMA buffers

[ujfalusi]

Hi,

every now and then we see the DMA buffer allocation failure coming back [1]. While the issue is not originating from SOF stack, it does pops up from the logs.

It has been reported that the DMA memory allocation for firmware download
can fail after extended period of uptime on systems with relatively small
amount of RAM when the system memory becomes fragmented.

The issue primarily happens when the system is waking up from system
suspend, swap might not be available and the MM system cannot move things
around to allow for successful allocation.

If the IMR boot is not supported then for each DSP boot we would need to
allocate the DMA buffer for firmware transfer, which can increase the
chances of the issue to be hit.

This patch adds support for allocating the DMA buffers once at first boot
time and keep it until the system is shut down, rebooted or the drivers
re-loaded and makes this as the default operation.

With persistent_cl_buffer module parameter the persistent Code Loader
DMA buffer can be disabled to fall back to on demand allocation.

[1] #5161

[ujfalusi]

Changes since v1:

• single patch
• move the module parameter to hda-loader.c and use it directly
• the firmware download DMA buffer is by default persistent
• add explanatory comments

[ujfalusi]

Changes since v2:

• Use persistent_buffer as parameter names in functions

[marc-hb]

I very painfully found this error buried deep down in the Jenkins logs. See internal issue 607 for more details.

It's failing only the NOCODEC build, the regular build is fine.

Yet Jenkins thinks the build was fine, keeps going and that's why things get super confusing.

Considering Jenkins is basically orphaned, you want to add the NODEC build to Github Actions. It shouldn't be hard, I can review if you want.

  CC      net/ethtool/rss.o
  CC      drivers/pnp/support.o
  CC      drivers/acpi/acpica/rsdumpinfo.o
/srv/home/jenkins/workspace/linux_config_build/linux/sound/soc/sof/intel/hda.c: In function ‘hda_dsp_pre_fw_run’:
/srv/home/jenkins/workspace/linux_config_build/linux/sound/soc/sof/intel/hda.c:404:21: error: ‘hda_keep_fw_dma_buffer’ undeclared (first use in this function)
  404 |                 if (hda_keep_fw_dma_buffer == 0 || hda_keep_fw_dma_buffer == 1)
      |                     ^~~~~~~~~~~~~~~~~~~~~~
/srv/home/jenkins/workspace/linux_config_build/linux/sound/soc/sof/intel/hda.c:404:21: note: each undeclared identifier is reported only once for each function it appears in
make[10]: *** [/srv/home/jenkins/workspace/linux_config_build/linux/scripts/Makefile.build:244: sound/soc/sof/intel/hda.o] Error 1
make[9]: *** [/srv/home/jenkins/workspace/linux_config_build/linux/scripts/Makefile.build:485: sound/soc/sof/intel] Error 2
make[9]: *** Waiting for unfinished jobs....
  AR      drivers/amba/built-in.a
  CC      drivers/pnp/interface.o
  CC      drivers/acpi/acpica/rsinfo.o

[ujfalusi]

SOFCI TEST

[marc-hb]

Considering Jenkins is basically orphaned, you want to add the NODEC build to Github Actions. It shouldn't be hard, I can review if you want.

BTW you should really use a matrix to avoid the massive copy/paste/diverge in .github/workflows/. Simple matrix example in: thesofproject/sof@425a5b5

[marc-hb]

SOFCI TEST

[ujfalusi]

Changes since v3:

• Preserve the iccmax DMA buffer if it is used by the platform
• With this update the whole base firmware handling is covered, IPC4 library DMA buffer caching will be added as a followup series as it needs to touch top-level IPC4 headers and structures - only Intel is using struct snd_dma_buffer for download afaik.

[ujfalusi]

Changes since v4:

• cover library loading as well with persistent code loader buffer
• single cl_dmab buffer to be used by basefw and library loading alike.

[plbossart]

I don't fully remember the iccmax details but would it be possible to use the same trick and use the same buffer as for regular firmware?

[ujfalusi]

I rather not mix the two, while iccmax afaik is not running a capture, the buffer needs to be valid before we set up the code loader DMA.
I think it would result rather confusing code to follow what is going on.

[ranj063]

where is this module parameter defined?

[ujfalusi]

good catch, I renamed the parameter at the last time and missed this comment.

[ranj063]

this comment is not easy to follow. What are we trying to say here? Also, if I understand the patch correctly, we're trying to propose to only keep the code loader DMA buffer allocation be pesistent isnt it? Why not then use the temp dmab in the iccmax and the library cases so that we never use the same buffer in those cases?

[ujfalusi]

We are using the cl_dmab for basefw and library.
For iccmax I rather not use the same buffer by principle, iccmax is capture stream, code loader is playback for one.
We allocate iccmax stream first (with the buffer) and then the buffer for code loader, for iccmax we use one page, which rarely enough for the firmware, so we would need to free and allocate a new buffer, thus re-configure iccmax, but this will bring up other issues.

If we overlook that we would need to allocate by-directional DMA memory and absolutely be certain that iccmax is not capturing data (which it should not afaik) then we still need quite a big refactoring to do just to flip over the allocation flow for basefw.

For me the use the same buffer for playback and capture which sounds off limits.

[plbossart]

Agree with Peter, the iccmax thing is a work-around only needed on a small set of platforms, it's best to keep a different buffer for iccmax capture and avoid mixing different types of uses.

[ujfalusi]

Changes since v5:

• comments updated (module parameter rename and clarification)