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

[Ranjani Sridharan <ranjani.sridharan@linux.intel.com>]

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