Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ASoC: SOF: Intel/ipc4: Add support for two staged library loading #4521

Merged
merged 4 commits into from
Aug 22, 2023
Merged

ASoC: SOF: Intel/ipc4: Add support for two staged library loading #4521

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
fd73e9a
ASoC: SOF: ipc4: Convert status code 2 and 15 to -EOPNOTSUPP
ujfalusi Aug 4, 2023
eeeeeb1
ASoC: SOF: Intel: hda: Add definition for SDxFIFOS.FIFOS mask
ujfalusi Jul 27, 2023
ee0afd5
ASoC: SOF: ipc4: Add new message type: SOF_IPC4_GLB_LOAD_LIBRARY_PREPARE
ujfalusi Aug 14, 2023
b0a7d2b
ASoC: SOF: Intel: hda-loader: Add support for split library loading
ujfalusi Jul 14, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions include/sound/sof/ipc4/header.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,12 +106,19 @@ enum sof_ipc4_global_msg {
SOF_IPC4_GLB_SAVE_PIPELINE,
SOF_IPC4_GLB_RESTORE_PIPELINE,

/* Loads library (using Code Load or HD/A Host Output DMA) */
/*
* library loading
*
* Loads library (using Code Load or HD/A Host Output DMA)
*/
SOF_IPC4_GLB_LOAD_LIBRARY,
/*
plbossart marked this conversation as resolved.
Show resolved Hide resolved
* Prepare the host DMA channel for library loading, must be followed by
* a SOF_IPC4_GLB_LOAD_LIBRARY message as the library loading step
*/
SOF_IPC4_GLB_LOAD_LIBRARY_PREPARE,
plbossart marked this conversation as resolved.
Show resolved Hide resolved

/* 25: RESERVED - do not use */

SOF_IPC4_GLB_INTERNAL_MESSAGE = 26,
SOF_IPC4_GLB_INTERNAL_MESSAGE,

/* Notification (FW to SW driver) */
SOF_IPC4_GLB_NOTIFICATION,
Expand Down
42 changes: 40 additions & 2 deletions sound/soc/sof/intel/hda-loader.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,20 +545,58 @@ int hda_dsp_ipc4_load_library(struct snd_sof_dev *sdev,

memcpy(dmab.area, stripped_firmware.data, stripped_firmware.size);

/*
* 1st stage: SOF_IPC4_GLB_LOAD_LIBRARY_PREPARE
* Message includes the dma_id to be prepared for the library loading.
* If the firmware does not have support for the message, we will
* receive -EOPNOTSUPP. In this case we will use single step library
* loading and proceed to send the LOAD_LIBRARY message.
*/
msg.primary = hext_stream->hstream.stream_tag - 1;
msg.primary |= SOF_IPC4_MSG_TYPE_SET(SOF_IPC4_GLB_LOAD_LIBRARY);
msg.primary |= SOF_IPC4_MSG_TYPE_SET(SOF_IPC4_GLB_LOAD_LIBRARY_PREPARE);
msg.primary |= SOF_IPC4_MSG_DIR(SOF_IPC4_MSG_REQUEST);
msg.primary |= SOF_IPC4_MSG_TARGET(SOF_IPC4_FW_GEN_MSG);
msg.primary |= SOF_IPC4_GLB_LOAD_LIBRARY_LIB_ID(fw_lib->id);
ret = sof_ipc_tx_message_no_reply(sdev->ipc, &msg, 0);
if (!ret) {
int sd_offset = SOF_STREAM_SD_OFFSET(&hext_stream->hstream);
unsigned int status;

/*
* Make sure that the FIFOS value is not 0 in SDxFIFOS register
* which indicates that the firmware set the GEN bit and we can
* continue to start the DMA
*/
ret = snd_sof_dsp_read_poll_timeout(sdev, HDA_DSP_HDA_BAR,
sd_offset + SOF_HDA_ADSP_REG_SD_FIFOSIZE,
status,
status & SOF_HDA_SD_FIFOSIZE_FIFOS_MASK,
HDA_DSP_REG_POLL_INTERVAL_US,
HDA_DSP_BASEFW_TIMEOUT_US);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ujfalusi is this just a paranoia check or is it really required to check the FIFO value?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @ujfalusi, if we want this in 6.6 we don't have much time left

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is paranoia check, but I feel more comfortable doing this, The firmware will set the GEN bit before it sends the reply.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, the FIFO value check? It is paranoia and also to make sure that we only set the RUN bit after the GEN was set. It should be, but I do feel that this a good practice and provides additional point if the loading fails (like the firmware have bug and not set the GEN bit and returns with success)

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ping @ujfalusi, if we want this in 6.6 we don't have much time left

I missed the comment, sorry.


if (ret < 0)
dev_warn(sdev->dev,
"%s: timeout waiting for FIFOS\n", __func__);
} else if (ret != -EOPNOTSUPP) {
goto cleanup;
}

ret = cl_trigger(sdev, hext_stream, SNDRV_PCM_TRIGGER_START);
if (ret < 0) {
dev_err(sdev->dev, "%s: DMA trigger start failed\n", __func__);
goto cleanup;
}

/*
* 2nd stage: LOAD_LIBRARY
* Message includes the dma_id and the lib_id, the dma_id must be
* identical to the one sent via LOAD_LIBRARY_PREPARE
*/
msg.primary &= ~SOF_IPC4_MSG_TYPE_MASK;
msg.primary |= SOF_IPC4_MSG_TYPE_SET(SOF_IPC4_GLB_LOAD_LIBRARY);
msg.primary |= SOF_IPC4_GLB_LOAD_LIBRARY_LIB_ID(fw_lib->id);
ret = sof_ipc_tx_message_no_reply(sdev->ipc, &msg, 0);

/* Stop the DMA channel */
ret1 = cl_trigger(sdev, hext_stream, SNDRV_PCM_TRIGGER_STOP);
if (ret1 < 0) {
dev_err(sdev->dev, "%s: DMA trigger stop failed\n", __func__);
Expand Down
2 changes: 1 addition & 1 deletion sound/soc/sof/intel/hda-stream.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,7 +668,7 @@ int hda_dsp_stream_hw_params(struct snd_sof_dev *sdev,
snd_sof_dsp_read(sdev, HDA_DSP_HDA_BAR,
sd_offset +
SOF_HDA_ADSP_REG_SD_FIFOSIZE);
hstream->fifo_size &= 0xffff;
hstream->fifo_size &= SOF_HDA_SD_FIFOSIZE_FIFOS_MASK;
hstream->fifo_size += 1;
} else {
hstream->fifo_size = 0;
Expand Down
3 changes: 3 additions & 0 deletions sound/soc/sof/intel/hda.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,9 @@
#define SOF_HDA_ADSP_REG_SD_BDLPU 0x1C
#define SOF_HDA_ADSP_SD_ENTRY_SIZE 0x20

/* SDxFIFOS FIFOS */
#define SOF_HDA_SD_FIFOSIZE_FIFOS_MASK GENMASK(15, 0)

/* CL: Software Position Based FIFO Capability Registers */
#define SOF_DSP_REG_CL_SPBFIFO \
(SOF_HDA_ADSP_LOADER_BASE + 0x20)
Expand Down
5 changes: 5 additions & 0 deletions sound/soc/sof/ipc4.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,10 @@ static int sof_ipc4_check_reply_status(struct snd_sof_dev *sdev, u32 status)

to_errno:
switch (status) {
case 2:
case 15:
ret = -EOPNOTSUPP;
break;
plbossart marked this conversation as resolved.
Show resolved Hide resolved
case 8:
case 11:
case 105 ... 109:
Expand Down Expand Up @@ -153,6 +157,7 @@ static const char * const ipc4_dbg_glb_msg_type[] = {
DBG_IPC4_MSG_TYPE_ENTRY(GLB_SAVE_PIPELINE),
DBG_IPC4_MSG_TYPE_ENTRY(GLB_RESTORE_PIPELINE),
DBG_IPC4_MSG_TYPE_ENTRY(GLB_LOAD_LIBRARY),
DBG_IPC4_MSG_TYPE_ENTRY(GLB_LOAD_LIBRARY_PREPARE),
DBG_IPC4_MSG_TYPE_ENTRY(GLB_INTERNAL_MESSAGE),
DBG_IPC4_MSG_TYPE_ENTRY(GLB_NOTIFICATION),
};
Expand Down