From d34069364597a0df3db02719e50b47cc89a616bf Mon Sep 17 00:00:00 2001 From: Peter Ujfalusi Date: Wed, 18 Sep 2024 13:53:32 +0300 Subject: [PATCH] ASoC: SOF: Intel: hda: Support for preserved DMA buffer for firmware download 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 buffer 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 keep_firmware_dma_buffer module parameter the persistent firmware download DMA buffer can be disabled to fall back to on demand allocation. Signed-off-by: Peter Ujfalusi --- sound/soc/sof/intel/hda-loader.c | 79 +++++++++++++++++++++----------- sound/soc/sof/intel/hda.c | 3 ++ sound/soc/sof/intel/hda.h | 12 ++++- 3 files changed, 66 insertions(+), 28 deletions(-) diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 9d8ebb7c6a1060..f0dfcf1d047431 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -26,6 +26,11 @@ #include "../sof-priv.h" #include "hda.h" +static bool hda_keep_fw_dma_buffer = true; +module_param_named(keep_firmware_dma_buffer, hda_keep_fw_dma_buffer, bool, 0444); +MODULE_PARM_DESC(keep_firmware_dma_buffer, "Allocate firmware loader DMA buffer once " + "(default = Y, use N to force buffer re-allocation)"); + static void hda_ssp_set_cbp_cfp(struct snd_sof_dev *sdev) { struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; @@ -43,9 +48,10 @@ static void hda_ssp_set_cbp_cfp(struct snd_sof_dev *sdev) } } -struct hdac_ext_stream *hda_cl_prepare(struct device *dev, unsigned int format, - unsigned int size, struct snd_dma_buffer *dmab, - int direction, bool is_iccmax) +struct hdac_ext_stream* +hda_cl_prepare(struct device *dev, unsigned int format, unsigned int size, + struct snd_dma_buffer *dmab, bool persistent_buffer, int direction, + bool is_iccmax) { struct snd_sof_dev *sdev = dev_get_drvdata(dev); struct hdac_ext_stream *hext_stream; @@ -61,11 +67,19 @@ struct hdac_ext_stream *hda_cl_prepare(struct device *dev, unsigned int format, hstream = &hext_stream->hstream; hstream->substream = NULL; - /* allocate DMA buffer */ - ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, dev, size, dmab); - if (ret < 0) { - dev_err(sdev->dev, "error: memory alloc failed: %d\n", ret); - goto out_put; + /* + * Allocate DMA buffer if it is temporary or if the buffer is intended + * to be persistent but not yet allocated. + * We cannot rely solely on !dmab->area as caller might use a struct on + * stack without clearing it to 0. + */ + if (!persistent_buffer || (persistent_buffer && !dmab->area)) { + ret = snd_dma_alloc_pages(SNDRV_DMA_TYPE_DEV_SG, dev, size, dmab); + if (ret < 0) { + dev_err(sdev->dev, "%s: memory alloc failed: %d\n", + __func__, ret); + goto out_put; + } } hstream->period_bytes = 0;/* initialize period_bytes */ @@ -91,6 +105,9 @@ struct hdac_ext_stream *hda_cl_prepare(struct device *dev, unsigned int format, out_free: snd_dma_free_pages(dmab); + dmab->area = NULL; + hstream->bufsize = 0; + hstream->format_val = 0; out_put: hda_dsp_stream_put(sdev, direction, hstream->stream_tag); return ERR_PTR(ret); @@ -255,7 +272,7 @@ int hda_cl_trigger(struct device *dev, struct hdac_ext_stream *hext_stream, int EXPORT_SYMBOL_NS(hda_cl_trigger, SND_SOC_SOF_INTEL_HDA_COMMON); int hda_cl_cleanup(struct device *dev, struct snd_dma_buffer *dmab, - struct hdac_ext_stream *hext_stream) + bool persistent_buffer, struct hdac_ext_stream *hext_stream) { struct snd_sof_dev *sdev = dev_get_drvdata(dev); struct hdac_stream *hstream = &hext_stream->hstream; @@ -279,10 +296,13 @@ int hda_cl_cleanup(struct device *dev, struct snd_dma_buffer *dmab, sd_offset + SOF_HDA_ADSP_REG_SD_BDLPU, 0); snd_sof_dsp_write(sdev, HDA_DSP_HDA_BAR, sd_offset, 0); - snd_dma_free_pages(dmab); - dmab->area = NULL; - hstream->bufsize = 0; - hstream->format_val = 0; + + if (!persistent_buffer) { + snd_dma_free_pages(dmab); + dmab->area = NULL; + hstream->bufsize = 0; + hstream->format_val = 0; + } return ret; } @@ -354,7 +374,8 @@ int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev) * the data, so use a buffer of PAGE_SIZE for receiving. */ iccmax_stream = hda_cl_prepare(sdev->dev, HDA_CL_STREAM_FORMAT, PAGE_SIZE, - &dmab_bdl, SNDRV_PCM_STREAM_CAPTURE, true); + &dmab_bdl, false, SNDRV_PCM_STREAM_CAPTURE, + true); if (IS_ERR(iccmax_stream)) { dev_err(sdev->dev, "error: dma prepare for ICCMAX stream failed\n"); return PTR_ERR(iccmax_stream); @@ -366,7 +387,7 @@ int hda_dsp_cl_boot_firmware_iccmax(struct snd_sof_dev *sdev) * Perform iccmax stream cleanup. This should be done even if firmware loading fails. * If the cleanup also fails, we return the initial error */ - ret1 = hda_cl_cleanup(sdev->dev, &dmab_bdl, iccmax_stream); + ret1 = hda_cl_cleanup(sdev->dev, &dmab_bdl, false, iccmax_stream); if (ret1 < 0) { dev_err(sdev->dev, "error: ICCMAX stream cleanup failed\n"); @@ -408,7 +429,7 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) const struct sof_intel_dsp_desc *chip_info; struct hdac_ext_stream *hext_stream; struct firmware stripped_firmware; - struct snd_dma_buffer dmab; + bool memcpy_needed = true; int ret, ret1, i; if (hda->imrboot_supported && !sdev->first_boot && !hda->skip_imr_boot) { @@ -432,23 +453,28 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) return -EINVAL; } - stripped_firmware.data = sdev->basefw.fw->data + sdev->basefw.payload_offset; - stripped_firmware.size = sdev->basefw.fw->size - sdev->basefw.payload_offset; - /* init for booting wait */ init_waitqueue_head(&sdev->boot_wait); + /* If the DMA buffer is preserved, the memcpy only needs to be done once */ + if (hda_keep_fw_dma_buffer && hda->fw_dmab.area) + memcpy_needed = false; + /* prepare DMA for code loader stream */ + stripped_firmware.size = sdev->basefw.fw->size - sdev->basefw.payload_offset; hext_stream = hda_cl_prepare(sdev->dev, HDA_CL_STREAM_FORMAT, stripped_firmware.size, - &dmab, SNDRV_PCM_STREAM_PLAYBACK, false); + &hda->fw_dmab, hda_keep_fw_dma_buffer, + SNDRV_PCM_STREAM_PLAYBACK, false); if (IS_ERR(hext_stream)) { dev_err(sdev->dev, "error: dma prepare for fw loading failed\n"); return PTR_ERR(hext_stream); } - memcpy(dmab.area, stripped_firmware.data, - stripped_firmware.size); + if (memcpy_needed) { + stripped_firmware.data = sdev->basefw.fw->data + sdev->basefw.payload_offset; + memcpy(hda->fw_dmab.area, stripped_firmware.data, stripped_firmware.size); + } /* try ROM init a few times before giving up */ for (i = 0; i < HDA_FW_BOOT_ATTEMPTS; i++) { @@ -514,7 +540,8 @@ int hda_dsp_cl_boot_firmware(struct snd_sof_dev *sdev) * This should be done even if firmware loading fails. * If the cleanup also fails, we return the initial error */ - ret1 = hda_cl_cleanup(sdev->dev, &dmab, hext_stream); + ret1 = hda_cl_cleanup(sdev->dev, &hda->fw_dmab, hda_keep_fw_dma_buffer, + hext_stream); if (ret1 < 0) { dev_err(sdev->dev, "error: Code loader DSP cleanup failed\n"); @@ -558,8 +585,8 @@ int hda_dsp_ipc4_load_library(struct snd_sof_dev *sdev, /* prepare DMA for code loader stream */ hext_stream = hda_cl_prepare(sdev->dev, HDA_CL_STREAM_FORMAT, - stripped_firmware.size, - &dmab, SNDRV_PCM_STREAM_PLAYBACK, false); + stripped_firmware.size, &dmab, false, + SNDRV_PCM_STREAM_PLAYBACK, false); if (IS_ERR(hext_stream)) { dev_err(sdev->dev, "%s: DMA prepare failed\n", __func__); return PTR_ERR(hext_stream); @@ -628,7 +655,7 @@ int hda_dsp_ipc4_load_library(struct snd_sof_dev *sdev, cleanup: /* clean up even in case of error and return the first error */ - ret1 = hda_cl_cleanup(sdev->dev, &dmab, hext_stream); + ret1 = hda_cl_cleanup(sdev->dev, &dmab, false, hext_stream); if (ret1 < 0) { dev_err(sdev->dev, "%s: Code loader DSP cleanup failed\n", __func__); diff --git a/sound/soc/sof/intel/hda.c b/sound/soc/sof/intel/hda.c index e4cb4ffc727048..46ad9e8d3709ce 100644 --- a/sound/soc/sof/intel/hda.c +++ b/sound/soc/sof/intel/hda.c @@ -936,6 +936,9 @@ void hda_dsp_remove(struct snd_sof_dev *sdev) /* disable DSP */ hda_dsp_ctrl_ppcap_enable(sdev, false); + if (hda->fw_dmab.area) + snd_dma_free_pages(&hda->fw_dmab); + skip_disable_dsp: free_irq(sdev->ipc_irq, sdev); if (sdev->msi_enabled) diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index b74a472435b5d2..dc5b384fb794ee 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -495,6 +495,13 @@ struct sof_intel_hda_dev { int boot_iteration; + /* + * DMA buffer for base firmware download. By default the buffer is + * allocated once and kept through the lifetime of the driver. + * See module parameter: keep_firmware_dma_buffer + */ + struct snd_dma_buffer fw_dmab; + struct hda_bus hbus; /* hw config */ @@ -714,11 +721,12 @@ int hda_cl_copy_fw(struct snd_sof_dev *sdev, struct hdac_ext_stream *hext_stream struct hdac_ext_stream *hda_cl_prepare(struct device *dev, unsigned int format, unsigned int size, struct snd_dma_buffer *dmab, - int direction, bool is_iccmax); + bool persistent_buffer, int direction, + bool is_iccmax); int hda_cl_trigger(struct device *dev, struct hdac_ext_stream *hext_stream, int cmd); int hda_cl_cleanup(struct device *dev, struct snd_dma_buffer *dmab, - struct hdac_ext_stream *hext_stream); + bool persistent_buffer, struct hdac_ext_stream *hext_stream); int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag, bool imr_boot); #define HDA_CL_STREAM_FORMAT 0x40