Re: [PATCH v3 14/17] ASoC: Intel: avs: General code loading flow

On Fri, 2022-03-04 at 15:57 +0100, Cezary Rojewski wrote:

Code loading is a complex procedure and requires combined effort of DMA and IPCs. With IPCs already in place, lay out ground for specific DMA transfer operations.

Signed-off-by: Amadeusz Sławiński < amadeuszx.slawinski@linux.intel.com> Signed-off-by: Cezary Rojewski cezary.rojewski@intel.com

---

sound/soc/intel/avs/Makefile | 2 +- sound/soc/intel/avs/avs.h | 18 +++ sound/soc/intel/avs/core.c | 62 +++++++++ sound/soc/intel/avs/dsp.c | 26 ++++ sound/soc/intel/avs/loader.c | 237 ++++++++++++++++++++++++++++++++ sound/soc/intel/avs/registers.h | 6 + 6 files changed, 350 insertions(+), 1 deletion(-) create mode 100644 sound/soc/intel/avs/core.c create mode 100644 sound/soc/intel/avs/loader.c

diff --git a/sound/soc/intel/avs/Makefile b/sound/soc/intel/avs/Makefile index d9f92c5f5407..d9c793160612 100644 --- a/sound/soc/intel/avs/Makefile +++ b/sound/soc/intel/avs/Makefile @@ -1,5 +1,5 @@ # SPDX-License-Identifier: GPL-2.0-only

-snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o +snd-soc-avs-objs := dsp.o ipc.o messages.o utils.o core.o loader.o

obj-$(CONFIG_SND_SOC_INTEL_AVS) += snd-soc-avs.o diff --git a/sound/soc/intel/avs/avs.h b/sound/soc/intel/avs/avs.h index 0034c075fa64..2527d6170417 100644 --- a/sound/soc/intel/avs/avs.h +++ b/sound/soc/intel/avs/avs.h @@ -10,8 +10,11 @@ #define __SOUND_SOC_INTEL_AVS_H

#include <linux/device.h> +#include <linux/firmware.h> #include <sound/hda_codec.h> +#include <sound/hda_register.h> #include "messages.h" +#include "registers.h"

struct avs_dev;

@@ -32,6 +35,10 @@ struct avs_dsp_ops { irqreturn_t (* const irq_handler)(int, void *); irqreturn_t (* const irq_thread)(int, void *); void (* const int_control)(struct avs_dev *, bool);

• int (* const load_basefw)(struct avs_dev *, struct firmware *);
• int (* const load_lib)(struct avs_dev *, struct firmware *,

u32);

• int (* const transfer_mods)(struct avs_dev *, bool,

• 		    struct avs_module_entry *, u32);
  

};

#define avs_dsp_op(adev, op, ...) \ @@ -45,6 +52,7 @@ struct avs_spec { const char *name;

const struct avs_dsp_ops *const dsp_ops;

• struct avs_fw_version min_fw_version; /* anything below is

rejected */

const u32 core_init_mask; /* used during DSP boot */ const u64 attributes; /* bitmask of AVS_PLATATTR_* */ @@ -90,6 +98,7 @@ struct avs_dev { struct ida ppl_ida; struct list_head fw_list; int *core_refs;

• char **lib_names;

  struct completion fw_ready;

}; @@ -215,4 +224,13 @@ int avs_dsp_create_pipeline(struct avs_dev *adev, u16 req_size, u8 priority, bool lp, u16 attributes, u8 *instance_id); int avs_dsp_delete_pipeline(struct avs_dev *adev, u8 instance_id);

+/* Firmware loading */

+void avs_hda_clock_gating_enable(struct avs_dev *adev, bool enable); +void avs_hda_power_gating_enable(struct avs_dev *adev, bool enable); +void avs_hda_l1sen_enable(struct avs_dev *adev, bool enable);

+int avs_dsp_boot_firmware(struct avs_dev *adev, bool purge); +int avs_dsp_first_boot_firmware(struct avs_dev *adev);

#endif /* __SOUND_SOC_INTEL_AVS_H */ diff --git a/sound/soc/intel/avs/core.c b/sound/soc/intel/avs/core.c new file mode 100644 index 000000000000..117b31ef9cd0 --- /dev/null +++ b/sound/soc/intel/avs/core.c @@ -0,0 +1,62 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// Copyright(c) 2021 Intel Corporation. All rights reserved. +// +// Authors: Cezary Rojewski cezary.rojewski@intel.com +// Amadeusz Slawinski amadeuszx.slawinski@linux.intel.com +// +// Special thanks to: +// Krzysztof Hejmowski krzysztof.hejmowski@intel.com +// Michal Sienkiewicz michal.sienkiewicz@intel.com +// Filip Proborszcz +// +// for sharing Intel AudioDSP expertise and helping shape the very +// foundation of this driver +//

+#include <linux/pci.h> +#include <sound/hdaudio.h> +#include "avs.h"

+static void +avs_hda_update_config_dword(struct hdac_bus *bus, u32 reg, u32 mask, u32 value) +{

• struct pci_dev *pci = to_pci_dev(bus->dev);
• u32 data;
• pci_read_config_dword(pci, reg, &data);
• data &= ~mask;
• data |= (value & mask);
• pci_write_config_dword(pci, reg, data);

+}

+void avs_hda_power_gating_enable(struct avs_dev *adev, bool enable) +{

• u32 value;
• value = enable ? 0 : AZX_PGCTL_LSRMD_MASK;
• avs_hda_update_config_dword(&adev->base.core, AZX_PCIREG_PGCTL,

• 		    AZX_PGCTL_LSRMD_MASK, value);
  

+}

+static void avs_hdac_clock_gating_enable(struct hdac_bus *bus, bool enable) +{

• u32 value;
• value = enable ? AZX_CGCTL_MISCBDCGE_MASK : 0;
• avs_hda_update_config_dword(bus, AZX_PCIREG_CGCTL,

• 		    AZX_CGCTL_MISCBDCGE_MASK, value);
  

+}

+void avs_hda_clock_gating_enable(struct avs_dev *adev, bool enable) +{

• avs_hdac_clock_gating_enable(&adev->base.core, enable);

+}

+void avs_hda_l1sen_enable(struct avs_dev *adev, bool enable) +{

• u32 value;
• value = enable ? AZX_VS_EM2_L1SEN : 0;
• snd_hdac_chip_updatel(&adev->base.core, VS_EM2,

AZX_VS_EM2_L1SEN, value); +} diff --git a/sound/soc/intel/avs/dsp.c b/sound/soc/intel/avs/dsp.c index 5994d64d2468..a434e9918c51 100644 --- a/sound/soc/intel/avs/dsp.c +++ b/sound/soc/intel/avs/dsp.c @@ -198,6 +198,7 @@ int avs_dsp_init_module(struct avs_dev *adev, u16 module_id, u8 ppl_instance_id, u16 *instance_id) { struct avs_module_entry mentry;

• bool was_loaded = false; int ret, id;

  id = avs_module_id_alloc(adev, module_id);

@@ -212,6 +213,16 @@ int avs_dsp_init_module(struct avs_dev *adev, u16 module_id, u8 ppl_instance_id, if (ret) goto err_mod_entry;

• /* Load code into memory if this is the first instance. */
• if (!id && !avs_module_entry_is_loaded(&mentry)) {

• ret = avs_dsp_op(adev, transfer_mods, true, &mentry,
  

1);

• if (ret) {
  

• 	dev_err(adev->dev, "load modules failed: %d\n",
  

ret);

• 	goto err_mod_entry;
  

• }
  

• was_loaded = true;
  

• }
• ret = avs_ipc_init_instance(adev, module_id, id,

ppl_instance_id, core_id, domain, param, param_size); if (ret) { @@ -223,6 +234,8 @@ int avs_dsp_init_module(struct avs_dev *adev, u16 module_id, u8 ppl_instance_id, return 0;

err_ipc:

• if (was_loaded)

• avs_dsp_op(adev, transfer_mods, false, &mentry, 1);
  

  avs_dsp_put_core(adev, core_id);

err_mod_entry: avs_module_id_free(adev, module_id, id); @@ -232,12 +245,25 @@ int avs_dsp_init_module(struct avs_dev *adev, u16 module_id, u8 ppl_instance_id, void avs_dsp_delete_module(struct avs_dev *adev, u16 module_id, u16 instance_id, u8 ppl_instance_id, u8 core_id) {

• struct avs_module_entry mentry;
• int ret;
• /* Modules not owned by any pipeline need to be freed

explicitly. */ if (ppl_instance_id == INVALID_PIPELINE_ID) avs_ipc_delete_instance(adev, module_id, instance_id);

avs_module_id_free(adev, module_id, instance_id);

• ret = avs_get_module_id_entry(adev, module_id, &mentry);
• /* Unload occupied memory if this was the last instance. */
• if (!ret && mentry.type.load_type ==

AVS_MODULE_LOAD_TYPE_LOADABLE) {

• if (avs_is_module_ida_empty(adev, module_id)) {
  

• 	ret = avs_dsp_op(adev, transfer_mods, false,
  

&mentry, 1);

• 	if (ret)
  

• 		dev_err(adev->dev, "unload modules
  

failed: %d\n", ret);

• }
  

• }
• avs_dsp_put_core(adev, core_id);

}

diff --git a/sound/soc/intel/avs/loader.c b/sound/soc/intel/avs/loader.c new file mode 100644 index 000000000000..47e1f9a21e43 --- /dev/null +++ b/sound/soc/intel/avs/loader.c @@ -0,0 +1,237 @@ +// SPDX-License-Identifier: GPL-2.0-only +// +// Copyright(c) 2021 Intel Corporation. All rights reserved. +// +// Authors: Cezary Rojewski cezary.rojewski@intel.com +// Amadeusz Slawinski amadeuszx.slawinski@linux.intel.com +//

+#include <linux/firmware.h> +#include <linux/module.h> +#include <linux/slab.h> +#include "avs.h" +#include "messages.h" +#include "registers.h"

+#define AVS_FW_INIT_TIMEOUT_MS 3000

+#define AVS_ROOT_DIR "intel/avs" +#define AVS_BASEFW_FILENAME "dsp_basefw.bin" +#define AVS_EXT_MANIFEST_MAGIC 0x31454124 +#define SKL_MANIFEST_MAGIC 0x00000006 +#define SKL_ADSPFW_OFFSET 0x284

+/* Occasionally, engineering (release candidate) firmware is provided for testing. */ +static bool debug_ignore_fw_version; +module_param_named(ignore_fw_version, debug_ignore_fw_version, bool, 0444); +MODULE_PARM_DESC(ignore_fw_version, "Verify FW version 0=yes (default), 1=no");

+#define AVS_LIB_NAME_SIZE 8

+struct avs_fw_manifest {

• u32 id;
• u32 len;
• char name[AVS_LIB_NAME_SIZE];
• u32 preload_page_count;
• u32 img_flags;
• u32 feature_mask;
• struct avs_fw_version version;

+} __packed;

+struct avs_fw_ext_manifest {

• u32 id;
• u32 len;
• u16 version_major;
• u16 version_minor;
• u32 entries;

+} __packed;

+static int avs_fw_ext_manifest_strip(struct firmware *fw) +{

• struct avs_fw_ext_manifest *man;
• if (fw->size < sizeof(*man))

• return -EINVAL;
  

• man = (struct avs_fw_ext_manifest *)fw->data;
• if (man->id == AVS_EXT_MANIFEST_MAGIC) {

• fw->data += man->len;
  

• fw->size -= man->len;
  

• }
• return 0;

+}

+static int avs_fw_manifest_offset(struct firmware *fw) +{

• /* Header type found in first DWORD of fw binary. */
• u32 magic = *(u32 *)fw->data;
• switch (magic) {
• case SKL_MANIFEST_MAGIC:

• return SKL_ADSPFW_OFFSET;
  

• default:

• return -EINVAL;
  

• }

+}

+static int avs_fw_manifest_strip_verify(struct avs_dev *adev, struct firmware *fw,

• 			const struct avs_fw_version
  

*min) +{

• struct avs_fw_manifest *man;
• int offset, ret;
• ret = avs_fw_ext_manifest_strip(fw);
• if (ret)

• return ret;
  

• offset = avs_fw_manifest_offset(fw);
• if (offset < 0)

• return offset;
  

• if (fw->size < offset + sizeof(*man))

• return -EINVAL;
  

• if (!min)

• return 0;
  

• man = (struct avs_fw_manifest *)(fw->data + offset);
• if (man->version.major != min->major ||

•    man->version.minor != min->minor ||
  

•    man->version.hotfix != min->hotfix ||
  

•    man->version.build < min->build) {
  

Isnt this check a bit too strict? Isnt a check major enough?

• dev_warn(adev->dev, "bad FW version %d.%d.%d.%d,
  

expected %d.%d.%d.%d or newer\n",

• 	 man->version.major, man->version.minor,
  

• 	 man->version.hotfix, man->version.build,
  

• 	 min->major, min->minor, min->hotfix, min-
  

...

build);

• if (!debug_ignore_fw_version)
  

• 	return -EINVAL;
  

• }
• return 0;

+}

+static int avs_dsp_load_basefw(struct avs_dev *adev) +{

• const struct avs_fw_version *min_req;
• const struct avs_spec *const spec = adev->spec;
• const struct firmware *fw;
• struct firmware stripped_fw;
• char *filename;
• int ret;
• filename = kasprintf(GFP_KERNEL, "%s/%s/%s", AVS_ROOT_DIR,

spec->name,

• 	     AVS_BASEFW_FILENAME);
  

• if (!filename)

• return -ENOMEM;
  

• ret = avs_request_firmware(adev, &fw, filename);
• kfree(filename);
• if (ret < 0) {

• dev_err(adev->dev, "request firmware failed: %d\n",
  

ret);

• return ret;
  

• }
• stripped_fw = *fw;
• min_req = &adev->spec->min_fw_version;
• ret = avs_fw_manifest_strip_verify(adev, &stripped_fw,

min_req);

• if (ret < 0) {

• dev_err(adev->dev, "invalid firmware data: %d\n", ret);
  

• goto release_fw;
  

• }
• ret = avs_dsp_op(adev, load_basefw, &stripped_fw);
• if (ret < 0) {

• dev_err(adev->dev, "basefw load failed: %d\n", ret);
  

• goto release_fw;
  

• }
• ret = wait_for_completion_timeout(&adev->fw_ready,

• 			  msecs_to_jiffies(AVS_FW_INIT_
  

TIMEOUT_MS));

• if (!ret) {

• dev_err(adev->dev, "firmware ready timeout\n");
  

• avs_dsp_core_disable(adev, AVS_MAIN_CORE_MASK);
  

• ret = -ETIMEDOUT;
  

• goto release_fw;
  

• }
• return 0;

+release_fw:

• avs_release_last_firmware(adev);
• return ret;

+}

+int avs_dsp_boot_firmware(struct avs_dev *adev, bool purge) +{

• int ret, i;
• /* Full boot, clear cached data except for basefw (slot 0). */

Does this mean IMR restore is only available for base FW and not for module libraries? Do I understand this correctly?

• for (i = 1; i < adev->fw_cfg.max_libs_count; i++)

• memset(adev->lib_names[i], 0, AVS_LIB_NAME_SIZE);
  

• avs_hda_clock_gating_enable(adev, false);
• avs_hda_l1sen_enable(adev, false);
• ret = avs_dsp_load_basefw(adev);
• avs_hda_l1sen_enable(adev, true);
• avs_hda_clock_gating_enable(adev, true);
• if (ret < 0)

• return ret;
  

• /* With all code loaded, refresh module information. */
• ret = avs_module_info_init(adev, true);

It is not clear if this required only after first boot or after a suspend/resume as well. Thanks,Ranjani