On Fri, 2022-03-04 at 15:57 +0100, Cezary Rojewski wrote:
> Implement the IPC between Intel audio firmware and kernel driver. The
> IPC allows transmission of requests, handling of responses as well as
> unsolicited (i.e. firmware-generated) notifications.
>
> A subscription mechanism is added to enable different parts of the
> driver to register for specific notifications.
>
> The part of the DSP boot process that involves sending ROM message
> requires an extra step - must be followed by unstall operation of
> MAIN_CORE. All other types of messages do not require such specific
> handling, so separate set of functions is provided for sending these.
>
> Signed-off-by: Amadeusz Sławiński <
> amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> Signed-off-by: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> ---
> sound/soc/intel/avs/Makefile | 2 +-
> sound/soc/intel/avs/avs.h | 100 +++++++++
> sound/soc/intel/avs/ipc.c | 387
> ++++++++++++++++++++++++++++++++
> sound/soc/intel/avs/messages.h | 170 ++++++++++++++
> sound/soc/intel/avs/registers.h | 45 ++++
> 5 files changed, 703 insertions(+), 1 deletion(-)
> create mode 100644 sound/soc/intel/avs/ipc.c
> create mode 100644 sound/soc/intel/avs/messages.h
>
> diff --git a/sound/soc/intel/avs/Makefile
> b/sound/soc/intel/avs/Makefile
> index 5f7976a95fe2..e243806dd38a 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
> +snd-soc-avs-objs := dsp.o ipc.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 d4e6310e4bf7..841b8541b101 100644
> --- a/sound/soc/intel/avs/avs.h
> +++ b/sound/soc/intel/avs/avs.h
> @@ -11,13 +11,27 @@
>
> #include <linux/device.h>
> #include <sound/hda_codec.h>
> +#include "messages.h"
>
> struct avs_dev;
>
> +/*
> + * struct avs_dsp_ops - Platform-specific DSP operations
> + *
> + * @power: Power on or off DSP cores
> + * @reset: Enter or exit reset state on DSP cores
> + * @stall: Stall or run DSP cores
> + * @irq_handler: Top half of IPC servicing
> + * @irq_thread: Bottom half of IPC servicing
> + * @int_control: Enable or disable IPC interrupts
> + */
> struct avs_dsp_ops {
> int (* const power)(struct avs_dev *, u32, bool);
> int (* const reset)(struct avs_dev *, u32, bool);
> int (* const stall)(struct avs_dev *, u32, bool);
> + irqreturn_t (* const irq_handler)(int, void *);
> + irqreturn_t (* const irq_thread)(int, void *);
> + void (* const int_control)(struct avs_dev *, bool);
> };
>
> #define avs_dsp_op(adev, op, ...) \
> @@ -34,6 +48,9 @@ struct avs_spec {
>
> const u32 core_init_mask; /* used during DSP boot */
> const u64 attributes; /* bitmask of AVS_PLATATTR_*
> */
> + const u32 sram_base_offset;
> + const u32 sram_window_size;
> + const u32 rom_status;
> };
>
> /*
> @@ -49,6 +66,9 @@ struct avs_dev {
>
> void __iomem *dsp_ba;
> const struct avs_spec *spec;
> + struct avs_ipc *ipc;
> +
> + struct completion fw_ready;
> };
>
> /* from hda_bus to avs_dev */
> @@ -68,4 +88,84 @@ int avs_dsp_core_stall(struct avs_dev *adev, u32
> core_mask, bool stall);
> int avs_dsp_core_enable(struct avs_dev *adev, u32 core_mask);
> int avs_dsp_core_disable(struct avs_dev *adev, u32 core_mask);
>
> +/* Inter Process Communication */
> +
> +struct avs_ipc_msg {
> + union {
> + u64 header;
> + union avs_global_msg glb;
> + union avs_reply_msg rsp;
> + };
> + void *data;
> + size_t size;
> +};
> +
> +/*
> + * struct avs_ipc - DSP IPC context
> + *
> + * @dev: PCI device
> + * @rx: Reply message cache
> + * @default_timeout_ms: default message timeout in MS
> + * @ready: whether firmware is ready and communication is open
> + * @rx_completed: whether RX for previously sent TX has been
> received
> + * @rx_lock: for serializating manipulation of rx_* fields
> + * @msg_lock: for synchronizing request handling
> + * @done_completion: DONE-part of IPC i.e. ROM and ACKs from FW
> + * @busy_completion: BUSY-part of IPC i.e. receiving responses from
> FW
> + */
> +struct avs_ipc {
> + struct device *dev;
> +
> + struct avs_ipc_msg rx;
> + u32 default_timeout_ms;
> + bool ready;
> +
> + bool rx_completed;
> + spinlock_t rx_lock;
> + struct mutex msg_mutex;
> + struct completion done_completion;
> + struct completion busy_completion;
> +};
> +
> +#define AVS_EIPC EREMOTEIO
> +/*
> + * IPC handlers may return positive value (firmware error code) what
> denotes
> + * successful HOST <-> DSP communication yet failure to process
> specific request.
> + *
> + * Below macro converts returned value to linux kernel error code.
> + * All IPC callers MUST use it as soon as firmware error code is
> consumed.
> + */
> +#define AVS_IPC_RET(ret) \
> + (((ret) <= 0) ? (ret) : -AVS_EIPC)
> +
> +static inline void avs_ipc_err(struct avs_dev *adev, struct
> avs_ipc_msg *tx,
> + const char *name, int error)
> +{
> + /*
> + * If IPC channel is blocked e.g.: due to ongoing recovery,
Do you mean firmware recovery? In which cases do you perform a
recovery?
> + * -EPERM error code is expected and thus it's not an actual
> error.
And what happens in this case? do you retry the IPC after recovery?
> + */
> + if (error == -EPERM)
> + dev_dbg(adev->dev, "%s 0x%08x 0x%08x failed: %d\n",
> name,
> + tx->glb.primary, tx->glb.ext.val, error);
> + else
> + dev_err(adev->dev, "%s 0x%08x 0x%08x failed: %d\n",
> name,
> + tx->glb.primary, tx->glb.ext.val, error);
> +}
> +
> +irqreturn_t avs_dsp_irq_handler(int irq, void *dev_id);
> +irqreturn_t avs_dsp_irq_thread(int irq, void *dev_id);
> +void avs_dsp_process_response(struct avs_dev *adev, u64 header);
> +int avs_dsp_send_msg_timeout(struct avs_dev *adev,
> + struct avs_ipc_msg *request,
> + struct avs_ipc_msg *reply, int timeout);
> +int avs_dsp_send_msg(struct avs_dev *adev,
> + struct avs_ipc_msg *request, struct avs_ipc_msg
> *reply);
> +int avs_dsp_send_rom_msg_timeout(struct avs_dev *adev,
> + struct avs_ipc_msg *request, int
> timeout);
> +int avs_dsp_send_rom_msg(struct avs_dev *adev, struct avs_ipc_msg
> *request);
> +void avs_dsp_interrupt_control(struct avs_dev *adev, bool enable);
> +int avs_ipc_init(struct avs_ipc *ipc, struct device *dev);
> +void avs_ipc_block(struct avs_ipc *ipc);
> +
> #endif /* __SOUND_SOC_INTEL_AVS_H */
> diff --git a/sound/soc/intel/avs/ipc.c b/sound/soc/intel/avs/ipc.c
> new file mode 100644
> index 000000000000..c0722f8b195f
> --- /dev/null
> +++ b/sound/soc/intel/avs/ipc.c
> @@ -0,0 +1,387 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +//
> +// Copyright(c) 2021 Intel Corporation. All rights reserved.
> +//
> +// Authors: Cezary Rojewski <cezary.rojewski@xxxxxxxxx>
> +// Amadeusz Slawinski <amadeuszx.slawinski@xxxxxxxxxxxxxxx>
> +//
> +
> +#include <linux/slab.h>
> +#include <sound/hdaudio_ext.h>
> +#include "avs.h"
> +#include "messages.h"
> +#include "registers.h"
> +
> +#define AVS_IPC_TIMEOUT_MS 300
> +
> +static void avs_dsp_receive_rx(struct avs_dev *adev, u64 header)
> +{
> + struct avs_ipc *ipc = adev->ipc;
> + union avs_reply_msg msg = AVS_MSG(header);
> +
> + ipc->rx.header = header;
> + /* Abort copying payload if request processing was
> unsuccessful. */
This seems misplaced? Why would you called this function is the status
showed an error?
> + if (!msg.status)
> + memcpy_fromio(ipc->rx.data, avs_uplink_addr(adev),
> + ipc->rx.size);
> +}
> +
> +static void avs_dsp_process_notification(struct avs_dev *adev, u64
> header)
> +{
> + struct avs_notify_mod_data mod_data;
> + union avs_notify_msg msg = AVS_MSG(header);
> + size_t data_size = 0;
> + void *data = NULL;
> +
> + /* Ignore spurious notifications until handshake is
> established. */
> + if (!adev->ipc->ready && msg.notify_msg_type !=
> AVS_NOTIFY_FW_READY) {
> + dev_dbg(adev->dev, "FW not ready, skip notification:
> 0x%08x\n",
> + msg.primary);
> + return;
> + }
> +
> + /* Calculate notification payload size. */
> + switch (msg.notify_msg_type) {
> + case AVS_NOTIFY_FW_READY:
> + break;
> +
> + case AVS_NOTIFY_PHRASE_DETECTED:
> + data_size = sizeof(struct avs_notify_voice_data);
> + break;
> +
> + case AVS_NOTIFY_RESOURCE_EVENT:
> + data_size = sizeof(struct avs_notify_res_data);
> + break;
> +
> + case AVS_NOTIFY_MODULE_EVENT:
> + /* To know the total payload size, header needs to be
> read first. */
> + memcpy_fromio(&mod_data, avs_uplink_addr(adev),
> sizeof(mod_data));
> + data_size = sizeof(mod_data) + mod_data.data_size;
> + break;
> +
> + default:
> + dev_info(adev->dev, "unknown notification: 0x%08x\n",
> + msg.primary);
info? should it be a warning?
> + break;
> + }
> +
> + if (data_size) {
> + data = kmalloc(data_size, GFP_KERNEL);
> + if (!data)
> + return;
Should this function be modified to return the error? If it failed
here, all subsequent IPC's rec'd will also fail isnt it?
> +
> + memcpy_fromio(data, avs_uplink_addr(adev), data_size);
> + }
> +
> + /* Perform notification-specific operations. */
> + switch (msg.notify_msg_type) {
> + case AVS_NOTIFY_FW_READY:
> + dev_dbg(adev->dev, "FW READY 0x%08x\n", msg.primary);
> + adev->ipc->ready = true;
> + complete(&adev->fw_ready);
> + break;
> +
> + default:
> + break;
> + }
> +
> + kfree(data);
You alloc memory for "data", copy the data and free it? Where is it
used?
> +}
> +
> +void avs_dsp_process_response(struct avs_dev *adev, u64 header)
> +{
> + struct avs_ipc *ipc = adev->ipc;
> +
> + /*
> + * Response may either be solicited - a reply for a request
> that has
> + * been sent beforehand - or unsolicited (notification).
> + */
> + if (avs_msg_is_reply(header)) {
> + /* Response processing is invoked from IRQ thread. */
> + spin_lock_irq(&ipc->rx_lock);
> + avs_dsp_receive_rx(adev, header);
> + ipc->rx_completed = true;
> + spin_unlock_irq(&ipc->rx_lock);
> + } else {
> + avs_dsp_process_notification(adev, header);
> + }
> +
> + complete(&ipc->busy_completion);
> +}
> +
> +irqreturn_t avs_dsp_irq_handler(int irq, void *dev_id)
> +{
> + struct avs_dev *adev = dev_id;
> + struct avs_ipc *ipc = adev->ipc;
> + u32 adspis, hipc_rsp, hipc_ack;
> + irqreturn_t ret = IRQ_NONE;
> +
> + adspis = snd_hdac_adsp_readl(adev, AVS_ADSP_REG_ADSPIS);
> + if (adspis == UINT_MAX || !(adspis & AVS_ADSP_ADSPIS_IPC))
> + return ret;
> +
> + hipc_ack = snd_hdac_adsp_readl(adev, SKL_ADSP_REG_HIPCIE);
> + hipc_rsp = snd_hdac_adsp_readl(adev, SKL_ADSP_REG_HIPCT);
> +
> + /* DSP acked host's request */
> + if (hipc_ack & SKL_ADSP_HIPCIE_DONE) {
> + /*
> + * As an extra precaution, mask done interrupt. Code
> executed
> + * due to complete() found below does not assume any
> masking.
> + */
> + snd_hdac_adsp_updatel(adev, SKL_ADSP_REG_HIPCCTL,
> + AVS_ADSP_HIPCCTL_DONE, 0);
> +
> + complete(&ipc->done_completion);
> +
> + /* tell DSP it has our attention */
> + snd_hdac_adsp_updatel(adev, SKL_ADSP_REG_HIPCIE,
> + SKL_ADSP_HIPCIE_DONE,
> + SKL_ADSP_HIPCIE_DONE);
> + /* unmask done interrupt */
> + snd_hdac_adsp_updatel(adev, SKL_ADSP_REG_HIPCCTL,
> + AVS_ADSP_HIPCCTL_DONE,
> + AVS_ADSP_HIPCCTL_DONE);
> + ret = IRQ_HANDLED;
> + }
> +
> + /* DSP sent new response to process */
> + if (hipc_rsp & SKL_ADSP_HIPCT_BUSY) {
> + /* mask busy interrupt */
> + snd_hdac_adsp_updatel(adev, SKL_ADSP_REG_HIPCCTL,
> + AVS_ADSP_HIPCCTL_BUSY, 0);
> +
> + ret = IRQ_WAKE_THREAD;
> + }
> +
> + return ret;
> +}
> +
> +irqreturn_t avs_dsp_irq_thread(int irq, void *dev_id)
> +{
> + struct avs_dev *adev = dev_id;
> + union avs_reply_msg msg;
> + u32 hipct, hipcte;
> +
> + hipct = snd_hdac_adsp_readl(adev, SKL_ADSP_REG_HIPCT);
> + hipcte = snd_hdac_adsp_readl(adev, SKL_ADSP_REG_HIPCTE);
> +
> + /* ensure DSP sent new response to process */
> + if (!(hipct & SKL_ADSP_HIPCT_BUSY))
> + return IRQ_NONE;
> +
> + msg.primary = hipct;
> + msg.ext.val = hipcte;
> + avs_dsp_process_response(adev, msg.val);
> +
> + /* tell DSP we accepted its message */
> + snd_hdac_adsp_updatel(adev, SKL_ADSP_REG_HIPCT,
> + SKL_ADSP_HIPCT_BUSY,
> SKL_ADSP_HIPCT_BUSY);
> + /* unmask busy interrupt */
> + snd_hdac_adsp_updatel(adev, SKL_ADSP_REG_HIPCCTL,
> + AVS_ADSP_HIPCCTL_BUSY,
> AVS_ADSP_HIPCCTL_BUSY);
> +
> + return IRQ_HANDLED;
> +}
> +
> +static bool avs_ipc_is_busy(struct avs_ipc *ipc)
> +{
> + struct avs_dev *adev = to_avs_dev(ipc->dev);
> + u32 hipc_rsp;
> +
> + hipc_rsp = snd_hdac_adsp_readl(adev, SKL_ADSP_REG_HIPCT);
> + return hipc_rsp & SKL_ADSP_HIPCT_BUSY;
> +}
> +
> +static int avs_ipc_wait_busy_completion(struct avs_ipc *ipc, int
> timeout)
> +{
> + u32 repeats_left = 128; /* to avoid infinite looping */
> + int ret;
> +
> +again:
> + ret = wait_for_completion_timeout(&ipc->busy_completion,
> + msecs_to_jiffies(timeout));
> +
> + /* DSP could be unresponsive at this point. */
> + if (!ipc->ready)
> + return -EPERM;
> +
> + if (!ret) {
> + if (!avs_ipc_is_busy(ipc))
> + return -ETIMEDOUT;
> + /*
> + * Firmware did its job, either notification or reply
> + * has been received - now wait until it's processed.
> + */
> + wait_for_completion_killable(&ipc->busy_completion);
> + }
> +
> + /* Ongoing notification's bottom-half may cause early wakeup */
> + spin_lock(&ipc->rx_lock);
> + if (!ipc->rx_completed) {
> + if (repeats_left) {
> + /* Reply delayed due to notification. */
> + repeats_left--;
> + reinit_completion(&ipc->busy_completion);
> + spin_unlock(&ipc->rx_lock);
> + goto again;
> + }
> +
> + spin_unlock(&ipc->rx_lock);
> + return -ETIMEDOUT;
> + }
> +
> + spin_unlock(&ipc->rx_lock);
> + return 0;
> +}
> +
> +static void avs_ipc_msg_init(struct avs_ipc *ipc, struct avs_ipc_msg
> *reply)
> +{
> + lockdep_assert_held(&ipc->rx_lock);
> +
> + ipc->rx.header = 0;
> + ipc->rx.size = reply ? reply->size : 0;
> + ipc->rx_completed = false;
> +
> + reinit_completion(&ipc->done_completion);
> + reinit_completion(&ipc->busy_completion);
> +}
> +
> +static void avs_dsp_send_tx(struct avs_dev *adev, struct avs_ipc_msg
> *tx)
send_tx? send and tx both imply the same isnt it? maybe just use one or
the other?
> +{
> + tx->header |= SKL_ADSP_HIPCI_BUSY;
> +
> + if (tx->size)
> + memcpy_toio(avs_downlink_addr(adev), tx->data, tx-
> >size);
> + snd_hdac_adsp_writel(adev, SKL_ADSP_REG_HIPCIE, tx->header >>
> 32);
> + snd_hdac_adsp_writel(adev, SKL_ADSP_REG_HIPCI, tx->header &
> UINT_MAX);
> +}
> +
> +static int avs_dsp_do_send_msg(struct avs_dev *adev, struct
> avs_ipc_msg *request,
> + struct avs_ipc_msg *reply, int timeout)
> +{
> + struct avs_ipc *ipc = adev->ipc;
> + int ret;
> +
> + if (!ipc->ready)
> + return -EPERM;
> +
> + mutex_lock(&ipc->msg_mutex);
> +
> + spin_lock(&ipc->rx_lock);
> + avs_ipc_msg_init(ipc, reply);
> + avs_dsp_send_tx(adev, request);
> + spin_unlock(&ipc->rx_lock);
> +
> + ret = avs_ipc_wait_busy_completion(ipc, timeout);
> + if (ret) {
> + if (ret == -ETIMEDOUT) {
> + dev_crit(adev->dev, "communication severed: %d,
> rebooting dsp..\n",
Where does this reboot happen?
Thanks,
Ranjani
